selfhosted
/
coder
mirror of https://github.com/coder/coder.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

chore: remove unused `workspace_owner_count` field (#5958) 

This added unnecessary database load, because it's not used!
This commit is contained in:
Kyle Carberry 2023-02-02 11:59:43 -06:00 committed by GitHub
parent 92c5be971c
commit be00e2541c
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 23 additions and 242 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
coderd/apidoc/docs.go generated
View File

@ -7345,9 +7345,6 @@ const docTemplate = `{
"updated_at": {
"type": "string",
"format": "date-time"
},
"workspace_owner_count": {
"type": "integer"
}
}
},

3
coderd/apidoc/swagger.json generated
View File

@ -6602,9 +6602,6 @@
"updated_at": {
"type": "string",
"format": "date-time"
},
"workspace_owner_count": {
"type": "integer"
}
}
},

35
coderd/database/databasefake/databasefake.go
View File

@ -1272,41 +1272,6 @@ func (q *fakeQuerier) GetWorkspaceAppsByAgentIDs(_ context.Context, ids []uuid.U
return apps, nil
}
func (q *fakeQuerier) GetWorkspaceOwnerCountsByTemplateIDs(_ context.Context, templateIDs []uuid.UUID) ([]database.GetWorkspaceOwnerCountsByTemplateIDsRow, error) {
q.mutex.RLock()
defer q.mutex.RUnlock()
counts := map[uuid.UUID]map[uuid.UUID]struct{}{}
for _, templateID := range templateIDs {
counts[templateID] = map[uuid.UUID]struct{}{}
for _, workspace := range q.workspaces {
if workspace.TemplateID != templateID {
continue
}
if workspace.Deleted {
continue
}
countByOwnerID, ok := counts[templateID]
if !ok {
countByOwnerID = map[uuid.UUID]struct{}{}
}
countByOwnerID[workspace.OwnerID] = struct{}{}
counts[templateID] = countByOwnerID
}
}
res := make([]database.GetWorkspaceOwnerCountsByTemplateIDsRow, 0)
for key, value := range counts {
res = append(res, database.GetWorkspaceOwnerCountsByTemplateIDsRow{
TemplateID: key,
Count: int64(len(value)),
})
}
if len(res) == 0 {
return nil, sql.ErrNoRows
}
return res, nil
}
func (q *fakeQuerier) GetWorkspaceBuildByID(_ context.Context, id uuid.UUID) (database.WorkspaceBuild, error) {
q.mutex.RLock()
defer q.mutex.RUnlock()

1
coderd/database/querier.go
View File

@ -121,7 +121,6 @@ type sqlcQuerier interface {
GetWorkspaceByAgentID(ctx context.Context, agentID uuid.UUID) (Workspace, error)
GetWorkspaceByID(ctx context.Context, id uuid.UUID) (Workspace, error)
GetWorkspaceByOwnerIDAndName(ctx context.Context, arg GetWorkspaceByOwnerIDAndNameParams) (Workspace, error)
GetWorkspaceOwnerCountsByTemplateIDs(ctx context.Context, ids []uuid.UUID) ([]GetWorkspaceOwnerCountsByTemplateIDsRow, error)
GetWorkspaceResourceByID(ctx context.Context, id uuid.UUID) (WorkspaceResource, error)
GetWorkspaceResourceMetadataByResourceIDs(ctx context.Context, ids []uuid.UUID) ([]WorkspaceResourceMetadatum, error)
GetWorkspaceResourceMetadataCreatedAfter(ctx context.Context, createdAt time.Time) ([]WorkspaceResourceMetadatum, error)

42
coderd/database/queries.sql.go
View File

@ -6477,48 +6477,6 @@ func (q *sqlQuerier) GetWorkspaceByOwnerIDAndName(ctx context.Context, arg GetWo
return i, err
}
const getWorkspaceOwnerCountsByTemplateIDs = `-- name: GetWorkspaceOwnerCountsByTemplateIDs :many
SELECT
template_id,
COUNT(DISTINCT owner_id)
FROM
workspaces
WHERE
template_id = ANY($1 :: uuid [ ])
-- Ignore deleted workspaces
AND deleted != true
GROUP BY
template_id
`
type GetWorkspaceOwnerCountsByTemplateIDsRow struct {
TemplateID uuid.UUID `db:"template_id" json:"template_id"`
Count int64 `db:"count" json:"count"`
}
func (q *sqlQuerier) GetWorkspaceOwnerCountsByTemplateIDs(ctx context.Context, ids []uuid.UUID) ([]GetWorkspaceOwnerCountsByTemplateIDsRow, error) {
rows, err := q.db.QueryContext(ctx, getWorkspaceOwnerCountsByTemplateIDs, pq.Array(ids))
if err != nil {
return nil, err
}
defer rows.Close()
var items []GetWorkspaceOwnerCountsByTemplateIDsRow
for rows.Next() {
var i GetWorkspaceOwnerCountsByTemplateIDsRow
if err := rows.Scan(&i.TemplateID, &i.Count); err != nil {
return nil, err
}
items = append(items, i)
}
if err := rows.Close(); err != nil {
return nil, err
}
if err := rows.Err(); err != nil {
return nil, err
}
return items, nil
}
const getWorkspaces = `-- name: GetWorkspaces :many
SELECT
workspaces.id, workspaces.created_at, workspaces.updated_at, workspaces.owner_id, workspaces.organization_id, workspaces.template_id, workspaces.deleted, workspaces.name, workspaces.autostart_schedule, workspaces.ttl, workspaces.last_used_at, COUNT(*) OVER () as count

13
coderd/database/queries/workspaces.sql
View File

@ -223,19 +223,6 @@ WHERE
AND LOWER("name") = LOWER(@name)
ORDER BY created_at DESC;
-- name: GetWorkspaceOwnerCountsByTemplateIDs :many
SELECT
template_id,
COUNT(DISTINCT owner_id)
FROM
workspaces
WHERE
template_id = ANY(@ids :: uuid [ ])
-- Ignore deleted workspaces
AND deleted != true
GROUP BY
template_id;
-- name: InsertWorkspace :one
INSERT INTO
workspaces (

94
coderd/templates.go
View File

@ -42,23 +42,6 @@ func (api *API) template(rw http.ResponseWriter, r *http.Request) {
return
}
workspaceCounts, err := api.Database.GetWorkspaceOwnerCountsByTemplateIDs(ctx, []uuid.UUID{template.ID})
if errors.Is(err, sql.ErrNoRows) {
err = nil
}
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error fetching workspace count.",
Detail: err.Error(),
})
return
}
count := uint32(0)
if len(workspaceCounts) > 0 {
count = uint32(workspaceCounts[0].Count)
}
createdByNameMap, err := getCreatedByNamesByTemplateIDs(ctx, api.Database, []database.Template{template})
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
@ -68,7 +51,7 @@ func (api *API) template(rw http.ResponseWriter, r *http.Request) {
return
}
httpapi.Write(ctx, rw, http.StatusOK, api.convertTemplate(template, count, createdByNameMap[template.ID.String()]))
httpapi.Write(ctx, rw, http.StatusOK, api.convertTemplate(template, createdByNameMap[template.ID.String()]))
}
// @Summary Delete template by ID
@ -313,7 +296,7 @@ func (api *API) postTemplateByOrganization(rw http.ResponseWriter, r *http.Reque
return xerrors.Errorf("get creator name: %w", err)
}
template = api.convertTemplate(dbTemplate, 0, createdByNameMap[dbTemplate.ID.String()])
template = api.convertTemplate(dbTemplate, createdByNameMap[dbTemplate.ID.String()])
return nil
}, nil)
if err != nil {
@ -369,23 +352,6 @@ func (api *API) templatesByOrganization(rw http.ResponseWriter, r *http.Request)
return
}
templateIDs := make([]uuid.UUID, 0, len(templates))
for _, template := range templates {
templateIDs = append(templateIDs, template.ID)
}
workspaceCounts, err := api.Database.GetWorkspaceOwnerCountsByTemplateIDs(ctx, templateIDs)
if errors.Is(err, sql.ErrNoRows) {
err = nil
}
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error fetching workspace counts.",
Detail: err.Error(),
})
return
}
createdByNameMap, err := getCreatedByNamesByTemplateIDs(ctx, api.Database, templates)
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
@ -395,7 +361,7 @@ func (api *API) templatesByOrganization(rw http.ResponseWriter, r *http.Request)
return
}
httpapi.Write(ctx, rw, http.StatusOK, api.convertTemplates(templates, workspaceCounts, createdByNameMap))
httpapi.Write(ctx, rw, http.StatusOK, api.convertTemplates(templates, createdByNameMap))
}
// @Summary Get templates by organization and template name
@ -433,23 +399,6 @@ func (api *API) templateByOrganizationAndName(rw http.ResponseWriter, r *http.Re
return
}
workspaceCounts, err := api.Database.GetWorkspaceOwnerCountsByTemplateIDs(ctx, []uuid.UUID{template.ID})
if errors.Is(err, sql.ErrNoRows) {
err = nil
}
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
Message: "Internal error fetching workspace counts.",
Detail: err.Error(),
})
return
}
count := uint32(0)
if len(workspaceCounts) > 0 {
count = uint32(workspaceCounts[0].Count)
}
createdByNameMap, err := getCreatedByNamesByTemplateIDs(ctx, api.Database, []database.Template{template})
if err != nil {
httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{
@ -459,7 +408,7 @@ func (api *API) templateByOrganizationAndName(rw http.ResponseWriter, r *http.Re
return
}
httpapi.Write(ctx, rw, http.StatusOK, api.convertTemplate(template, count, createdByNameMap[template.ID.String()]))
httpapi.Write(ctx, rw, http.StatusOK, api.convertTemplate(template, createdByNameMap[template.ID.String()]))
}
// @Summary Update template metadata by ID
@ -508,22 +457,8 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) {
return
}
count := uint32(0)
var updated database.Template
err := api.Database.InTx(func(tx database.Store) error {
// Fetch workspace counts
workspaceCounts, err := tx.GetWorkspaceOwnerCountsByTemplateIDs(ctx, []uuid.UUID{template.ID})
if xerrors.Is(err, sql.ErrNoRows) {
err = nil
}
if err != nil {
return err
}
if len(workspaceCounts) > 0 {
count = uint32(workspaceCounts[0].Count)
}
if req.Name == template.Name &&
req.Description == template.Description &&
req.DisplayName == template.DisplayName &&
@ -550,6 +485,7 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) {
desc = template.Description
}
var err error
updated, err = tx.UpdateTemplateMetaByID(ctx, database.UpdateTemplateMetaByIDParams{
ID: template.ID,
UpdatedAt: database.Now(),
@ -587,7 +523,7 @@ func (api *API) patchTemplateMeta(rw http.ResponseWriter, r *http.Request) {
return
}
httpapi.Write(ctx, rw, http.StatusOK, api.convertTemplate(updated, count, createdByNameMap[updated.ID.String()]))
httpapi.Write(ctx, rw, http.StatusOK, api.convertTemplate(updated, createdByNameMap[updated.ID.String()]))
}
// @Summary Get template DAUs by ID
@ -659,22 +595,11 @@ func getCreatedByNamesByTemplateIDs(ctx context.Context, db database.Store, temp
return creators, nil
}
func (api *API) convertTemplates(templates []database.Template, workspaceCounts []database.GetWorkspaceOwnerCountsByTemplateIDsRow, createdByNameMap map[string]string) []codersdk.Template {
func (api *API) convertTemplates(templates []database.Template, createdByNameMap map[string]string) []codersdk.Template {
apiTemplates := make([]codersdk.Template, 0, len(templates))
for _, template := range templates {
found := false
for _, workspaceCount := range workspaceCounts {
if workspaceCount.TemplateID.String() != template.ID.String() {
continue
}
apiTemplates = append(apiTemplates, api.convertTemplate(template, uint32(workspaceCount.Count), createdByNameMap[template.ID.String()]))
found = true
break
}
if !found {
apiTemplates = append(apiTemplates, api.convertTemplate(template, uint32(0), createdByNameMap[template.ID.String()]))
}
apiTemplates = append(apiTemplates, api.convertTemplate(template, createdByNameMap[template.ID.String()]))
}
// Sort templates by ActiveUserCount DESC
@ -686,7 +611,7 @@ func (api *API) convertTemplates(templates []database.Template, workspaceCounts
}
func (api *API) convertTemplate(
template database.Template, workspaceOwnerCount uint32, createdByName string,
template database.Template, createdByName string,
) codersdk.Template {
activeCount, _ := api.metricsCache.TemplateUniqueUsers(template.ID)
@ -701,7 +626,6 @@ func (api *API) convertTemplate(
DisplayName: template.DisplayName,
Provisioner: codersdk.ProvisionerType(template.Provisioner),
ActiveVersionID: template.ActiveVersionID,
WorkspaceOwnerCount: workspaceOwnerCount,
ActiveUserCount: activeCount,
BuildTimeStats: buildTimeStats,
Description: template.Description,

35
coderd/templates_test.go
View File

@ -15,7 +15,6 @@ import (
"github.com/coder/coder/coderd/audit"
"github.com/coder/coder/coderd/coderdtest"
"github.com/coder/coder/coderd/database"
"github.com/coder/coder/coderd/rbac"
"github.com/coder/coder/coderd/util/ptr"
"github.com/coder/coder/codersdk"
"github.com/coder/coder/codersdk/agentsdk"
@ -40,40 +39,6 @@ func TestTemplate(t *testing.T) {
_, err := client.Template(ctx, template.ID)
require.NoError(t, err)
})
t.Run("WorkspaceCount", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
user := coderdtest.CreateFirstUser(t, client)
member := coderdtest.CreateAnotherUser(t, client, user.OrganizationID, rbac.RoleOwner())
memberWithDeleted := coderdtest.CreateAnotherUser(t, client, user.OrganizationID, rbac.RoleOwner())
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
// Create 3 workspaces with 3 users. 2 workspaces exist, 1 is deleted
workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID)
coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
memberWorkspace := coderdtest.CreateWorkspace(t, member, user.OrganizationID, template.ID)
coderdtest.AwaitWorkspaceBuildJob(t, member, memberWorkspace.LatestBuild.ID)
deletedWorkspace := coderdtest.CreateWorkspace(t, memberWithDeleted, user.OrganizationID, template.ID)
coderdtest.AwaitWorkspaceBuildJob(t, client, deletedWorkspace.LatestBuild.ID)
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()
build, err := client.CreateWorkspaceBuild(ctx, deletedWorkspace.ID, codersdk.CreateWorkspaceBuildRequest{
Transition: codersdk.WorkspaceTransitionDelete,
})
require.NoError(t, err)
coderdtest.AwaitWorkspaceBuildJob(t, client, build.ID)
template, err = client.Template(ctx, template.ID)
require.NoError(t, err)
require.Equal(t, 2, int(template.WorkspaceOwnerCount), "workspace count")
})
}
func TestPostTemplateByOrganization(t *testing.T) {

17
codersdk/templates.go
View File

@ -14,15 +14,14 @@ import (
// Template is the JSON representation of a Coder template. This type matches the
// database object for now, but is abstracted for ease of change later on.
type Template struct {
ID uuid.UUID `json:"id" format:"uuid"`
CreatedAt time.Time `json:"created_at" format:"date-time"`
UpdatedAt time.Time `json:"updated_at" format:"date-time"`
OrganizationID uuid.UUID `json:"organization_id" format:"uuid"`
Name string `json:"name"`
DisplayName string `json:"display_name"`
Provisioner ProvisionerType `json:"provisioner" enums:"terraform"`
ActiveVersionID uuid.UUID `json:"active_version_id" format:"uuid"`
WorkspaceOwnerCount uint32 `json:"workspace_owner_count"`
ID uuid.UUID `json:"id" format:"uuid"`
CreatedAt time.Time `json:"created_at" format:"date-time"`
UpdatedAt time.Time `json:"updated_at" format:"date-time"`
OrganizationID uuid.UUID `json:"organization_id" format:"uuid"`
Name string `json:"name"`
DisplayName string `json:"display_name"`
Provisioner ProvisionerType `json:"provisioner" enums:"terraform"`
ActiveVersionID uuid.UUID `json:"active_version_id" format:"uuid"`
// ActiveUserCount is set to -1 when loading.
ActiveUserCount int `json:"active_user_count"`
BuildTimeStats TemplateBuildTimeStats `json:"build_time_stats"`

4
docs/api/schemas.md
View File

@ -4240,8 +4240,7 @@ Parameter represents a set value for the scope.
"name": "string",
"organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6",
"provisioner": "terraform",
"updated_at": "2019-08-24T14:15:22Z",
"workspace_owner_count": 0
"updated_at": "2019-08-24T14:15:22Z"
}
```
@ -4265,7 +4264,6 @@ Parameter represents a set value for the scope.
| `organization_id` | string | false | | |
| `provisioner` | string | false | | |
| `updated_at` | string | false | | |
| `workspace_owner_count` | integer | false | | |
#### Enumerated Values

16
docs/api/templates.md
View File

@ -121,8 +121,7 @@ curl -X GET http://coder-server:8080/api/v2/organizations/{organization}/templat
"name": "string",
"organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6",
"provisioner": "terraform",
"updated_at": "2019-08-24T14:15:22Z",
"workspace_owner_count": 0
"updated_at": "2019-08-24T14:15:22Z"
}
]
```
@ -159,7 +158,6 @@ Status Code **200**
| `» organization_id` | string(uuid) | false | | |
| `» provisioner` | string | false | | |
| `» updated_at` | string(date-time) | false | | |
| `» workspace_owner_count` | integer | false | | |
#### Enumerated Values
@ -243,8 +241,7 @@ curl -X POST http://coder-server:8080/api/v2/organizations/{organization}/templa
"name": "string",
"organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6",
"provisioner": "terraform",
"updated_at": "2019-08-24T14:15:22Z",
"workspace_owner_count": 0
"updated_at": "2019-08-24T14:15:22Z"
}
```
@ -366,8 +363,7 @@ curl -X GET http://coder-server:8080/api/v2/organizations/{organization}/templat
"name": "string",
"organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6",
"provisioner": "terraform",
"updated_at": "2019-08-24T14:15:22Z",
"workspace_owner_count": 0
"updated_at": "2019-08-24T14:15:22Z"
}
```
@ -677,8 +673,7 @@ curl -X GET http://coder-server:8080/api/v2/templates/{template} \
"name": "string",
"organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6",
"provisioner": "terraform",
"updated_at": "2019-08-24T14:15:22Z",
"workspace_owner_count": 0
"updated_at": "2019-08-24T14:15:22Z"
}
```
@ -783,8 +778,7 @@ curl -X PATCH http://coder-server:8080/api/v2/templates/{template} \
"name": "string",
"organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6",
"provisioner": "terraform",
"updated_at": "2019-08-24T14:15:22Z",
"workspace_owner_count": 0
"updated_at": "2019-08-24T14:15:22Z"
}
```

1
site/src/api/typesGenerated.ts
View File

@ -685,7 +685,6 @@ export interface Template {
readonly display_name: string
readonly provisioner: ProvisionerType
readonly active_version_id: string
readonly workspace_owner_count: number
readonly active_user_count: number
readonly build_time_stats: TemplateBuildTimeStats
readonly description: string

1
site/src/testHelpers/entities.ts
View File

@ -247,7 +247,6 @@ export const MockTemplate: TypesGen.Template = {
display_name: "Test Template",
provisioner: MockProvisioner.provisioners[0],
active_version_id: MockTemplateVersion.id,
workspace_owner_count: 2,
active_user_count: 1,
build_time_stats: {
start: {