From a63cdefd649e004741a3e441cec9a10488772d3c Mon Sep 17 00:00:00 2001 From: Garrett Delfosse Date: Tue, 5 Dec 2023 15:18:08 +0000 Subject: [PATCH 1/7] fix: use template daus over unique users --- coderd/templates.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/coderd/templates.go b/coderd/templates.go index 227e5934af257..3e2803e4dcd3b 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -831,7 +831,13 @@ func (api *API) convertTemplate( template database.Template, ) codersdk.Template { templateAccessControl := (*(api.Options.AccessControlStore.Load())).GetTemplateAccessControl(template) - activeCount, _ := api.metricsCache.TemplateUniqueUsers(template.ID) + daus := 0 + _, resp, ok := api.metricsCache.TemplateDAUs(template.ID, 0) + if ok { + // this is safe because we currently only support one timezone offset: 0 + // see metricscache.templateTimezoneOffsets + daus = resp.Entries[0].Amount + } buildTimeStats := api.metricsCache.TemplateBuildTimeStats(template.ID) @@ -849,7 +855,7 @@ func (api *API) convertTemplate( DisplayName: template.DisplayName, Provisioner: codersdk.ProvisionerType(template.Provisioner), ActiveVersionID: template.ActiveVersionID, - ActiveUserCount: activeCount, + ActiveUserCount: daus, BuildTimeStats: buildTimeStats, Description: template.Description, Icon: template.Icon, From 81c2be131396aec107a3cd44dac92a1d9d7a7392 Mon Sep 17 00:00:00 2001 From: Garrett Delfosse Date: Wed, 6 Dec 2023 14:47:27 +0000 Subject: [PATCH 2/7] add template workspace owners to metrics cache --- coderd/database/dbauthz/dbauthz.go | 4 +++ coderd/database/dbmem/dbmem.go | 4 +++ coderd/database/dbmetrics/dbmetrics.go | 7 ++++++ coderd/database/dbmock/dbmock.go | 15 +++++++++++ coderd/database/querier.go | 1 + coderd/database/queries.sql.go | 35 ++++++++++++++++++++++++++ coderd/database/queries/workspaces.sql | 6 +++++ coderd/metricscache/metricscache.go | 32 ++++++++++++++++++++++- coderd/templates.go | 10 +++----- 9 files changed, 107 insertions(+), 7 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 1ba76a6c7431c..dcc81d31aecab 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -2049,6 +2049,10 @@ func (q *querier) GetWorkspaceResourcesCreatedAfter(ctx context.Context, created return q.db.GetWorkspaceResourcesCreatedAfter(ctx, createdAt) } +func (q *querier) GetWorkspaceUniqueOwnerCountByTemplateIDs(ctx context.Context, templateIds []uuid.UUID) ([]database.GetWorkspaceUniqueOwnerCountByTemplateIDsRow, error) { + panic("not implemented") +} + func (q *querier) GetWorkspaces(ctx context.Context, arg database.GetWorkspacesParams) ([]database.GetWorkspacesRow, error) { prep, err := prepareSQLFilter(ctx, q.auth, rbac.ActionRead, rbac.ResourceWorkspace.Type) if err != nil { diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 4cae64776d4d9..77696f4ec79bb 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -4462,6 +4462,10 @@ func (q *FakeQuerier) GetWorkspaceResourcesCreatedAfter(_ context.Context, after return resources, nil } +func (q *FakeQuerier) GetWorkspaceUniqueOwnerCountByTemplateIDs(ctx context.Context, templateIds []uuid.UUID) ([]database.GetWorkspaceUniqueOwnerCountByTemplateIDsRow, error) { + panic("not implemented") +} + func (q *FakeQuerier) GetWorkspaces(ctx context.Context, arg database.GetWorkspacesParams) ([]database.GetWorkspacesRow, error) { if err := validateDatabaseType(arg); err != nil { return nil, err diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go index e7ecf99596e4b..af296c78bea80 100644 --- a/coderd/database/dbmetrics/dbmetrics.go +++ b/coderd/database/dbmetrics/dbmetrics.go @@ -1222,6 +1222,13 @@ func (m metricsStore) GetWorkspaceResourcesCreatedAfter(ctx context.Context, cre return resources, err } +func (m metricsStore) GetWorkspaceUniqueOwnerCountByTemplateIDs(ctx context.Context, templateIds []uuid.UUID) ([]database.GetWorkspaceUniqueOwnerCountByTemplateIDsRow, error) { + start := time.Now() + r0, r1 := m.s.GetWorkspaceUniqueOwnerCountByTemplateIDs(ctx, templateIds) + m.queryLatencies.WithLabelValues("GetWorkspaceUniqueOwnerCountByTemplateIDs").Observe(time.Since(start).Seconds()) + return r0, r1 +} + func (m metricsStore) GetWorkspaces(ctx context.Context, arg database.GetWorkspacesParams) ([]database.GetWorkspacesRow, error) { start := time.Now() workspaces, err := m.s.GetWorkspaces(ctx, arg) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index e2abf1f91eaff..a52057c795069 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -2554,6 +2554,21 @@ func (mr *MockStoreMockRecorder) GetWorkspaceResourcesCreatedAfter(arg0, arg1 in return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetWorkspaceResourcesCreatedAfter", reflect.TypeOf((*MockStore)(nil).GetWorkspaceResourcesCreatedAfter), arg0, arg1) } +// GetWorkspaceUniqueOwnerCountByTemplateIDs mocks base method. +func (m *MockStore) GetWorkspaceUniqueOwnerCountByTemplateIDs(arg0 context.Context, arg1 []uuid.UUID) ([]database.GetWorkspaceUniqueOwnerCountByTemplateIDsRow, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetWorkspaceUniqueOwnerCountByTemplateIDs", arg0, arg1) + ret0, _ := ret[0].([]database.GetWorkspaceUniqueOwnerCountByTemplateIDsRow) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetWorkspaceUniqueOwnerCountByTemplateIDs indicates an expected call of GetWorkspaceUniqueOwnerCountByTemplateIDs. +func (mr *MockStoreMockRecorder) GetWorkspaceUniqueOwnerCountByTemplateIDs(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetWorkspaceUniqueOwnerCountByTemplateIDs", reflect.TypeOf((*MockStore)(nil).GetWorkspaceUniqueOwnerCountByTemplateIDs), arg0, arg1) +} + // GetWorkspaces mocks base method. func (m *MockStore) GetWorkspaces(arg0 context.Context, arg1 database.GetWorkspacesParams) ([]database.GetWorkspacesRow, error) { m.ctrl.T.Helper() diff --git a/coderd/database/querier.go b/coderd/database/querier.go index a0115fb22bd1c..4558094577576 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -252,6 +252,7 @@ type sqlcQuerier interface { GetWorkspaceResourcesByJobID(ctx context.Context, jobID uuid.UUID) ([]WorkspaceResource, error) GetWorkspaceResourcesByJobIDs(ctx context.Context, ids []uuid.UUID) ([]WorkspaceResource, error) GetWorkspaceResourcesCreatedAfter(ctx context.Context, createdAt time.Time) ([]WorkspaceResource, error) + GetWorkspaceUniqueOwnerCountByTemplateIDs(ctx context.Context, templateIds []uuid.UUID) ([]GetWorkspaceUniqueOwnerCountByTemplateIDsRow, error) GetWorkspaces(ctx context.Context, arg GetWorkspacesParams) ([]GetWorkspacesRow, error) GetWorkspacesEligibleForTransition(ctx context.Context, now time.Time) ([]Workspace, error) InsertAPIKey(ctx context.Context, arg InsertAPIKeyParams) (APIKey, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index a99a1cefc9884..3f7f20e8e4054 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -10720,6 +10720,41 @@ func (q *sqlQuerier) GetWorkspaceByWorkspaceAppID(ctx context.Context, workspace return i, err } +const getWorkspaceUniqueOwnerCountByTemplateIDs = `-- name: GetWorkspaceUniqueOwnerCountByTemplateIDs :many +SELECT template_id, COUNT(DISTINCT owner_id) AS unique_owners_sum +FROM workspaces +WHERE template_id = ANY($1 :: uuid[]) +GROUP BY template_id +` + +type GetWorkspaceUniqueOwnerCountByTemplateIDsRow struct { + TemplateID uuid.UUID `db:"template_id" json:"template_id"` + UniqueOwnersSum int64 `db:"unique_owners_sum" json:"unique_owners_sum"` +} + +func (q *sqlQuerier) GetWorkspaceUniqueOwnerCountByTemplateIDs(ctx context.Context, templateIds []uuid.UUID) ([]GetWorkspaceUniqueOwnerCountByTemplateIDsRow, error) { + rows, err := q.db.QueryContext(ctx, getWorkspaceUniqueOwnerCountByTemplateIDs, pq.Array(templateIds)) + if err != nil { + return nil, err + } + defer rows.Close() + var items []GetWorkspaceUniqueOwnerCountByTemplateIDsRow + for rows.Next() { + var i GetWorkspaceUniqueOwnerCountByTemplateIDsRow + if err := rows.Scan(&i.TemplateID, &i.UniqueOwnersSum); 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, workspaces.dormant_at, workspaces.deleting_at, workspaces.automatic_updates, diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index ac3a1fd86c11b..97e8a21bbf040 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -287,6 +287,12 @@ WHERE AND LOWER("name") = LOWER(@name) ORDER BY created_at DESC; +-- name: GetWorkspaceUniqueOwnerCountByTemplateIDs :many +SELECT template_id, COUNT(DISTINCT owner_id) AS unique_owners_sum +FROM workspaces +WHERE template_id = ANY(@template_ids :: uuid[]) +GROUP BY template_id; + -- name: InsertWorkspace :one INSERT INTO workspaces ( diff --git a/coderd/metricscache/metricscache.go b/coderd/metricscache/metricscache.go index dc5f7dbb8c45b..8c541bcbf45f6 100644 --- a/coderd/metricscache/metricscache.go +++ b/coderd/metricscache/metricscache.go @@ -52,6 +52,7 @@ type Cache struct { deploymentDAUResponses atomic.Pointer[map[int]codersdk.DAUsResponse] templateDAUResponses atomic.Pointer[map[int]map[uuid.UUID]codersdk.DAUsResponse] templateUniqueUsers atomic.Pointer[map[uuid.UUID]int] + templateWorkspaceOwners atomic.Pointer[map[uuid.UUID]int] templateAverageBuildTime atomic.Pointer[map[uuid.UUID]database.GetTemplateAverageBuildTimeRow] deploymentStatsResponse atomic.Pointer[codersdk.DeploymentStats] @@ -206,6 +207,7 @@ func (c *Cache) refreshTemplateDAUs(ctx context.Context) error { var ( templateDAUs = make(map[int]map[uuid.UUID]codersdk.DAUsResponse, len(templates)) templateUniqueUsers = make(map[uuid.UUID]int) + templateWorkspaceOwners = make(map[uuid.UUID]int) templateAverageBuildTimes = make(map[uuid.UUID]database.GetTemplateAverageBuildTimeRow) ) @@ -214,7 +216,9 @@ func (c *Cache) refreshTemplateDAUs(ctx context.Context) error { return xerrors.Errorf("deployment daus: %w", err) } - for _, template := range templates { + ids := make([]uuid.UUID, len(templates)) + for i, template := range templates { + ids[i] = template.ID for _, tzOffset := range templateTimezoneOffsets { rows, err := c.database.GetTemplateDAUs(ctx, database.GetTemplateDAUsParams{ TemplateID: template.ID, @@ -249,6 +253,17 @@ func (c *Cache) refreshTemplateDAUs(ctx context.Context) error { } templateAverageBuildTimes[template.ID] = templateAvgBuildTime } + + owners, err := c.database.GetWorkspaceUniqueOwnerCountByTemplateIDs(ctx, ids) + if err != nil { + return xerrors.Errorf("get workspace unique owner count by template ids: %w", err) + } + + for _, owner := range owners { + templateWorkspaceOwners[owner.TemplateID] = int(owner.UniqueOwnersSum) + } + + c.templateWorkspaceOwners.Store(&templateWorkspaceOwners) c.templateDAUResponses.Store(&templateDAUs) c.templateUniqueUsers.Store(&templateUniqueUsers) c.templateAverageBuildTime.Store(&templateAverageBuildTimes) @@ -469,6 +484,21 @@ func (c *Cache) TemplateBuildTimeStats(id uuid.UUID) codersdk.TemplateBuildTimeS } } +func (c *Cache) TemplateWorkspaceOwners(id uuid.UUID) (int, bool) { + m := c.templateWorkspaceOwners.Load() + if m == nil { + // Data loading. + return -1, false + } + + resp, ok := (*m)[id] + if !ok { + // Probably no data. + return -1, false + } + return resp, true +} + func (c *Cache) DeploymentStats() (codersdk.DeploymentStats, bool) { deploymentStats := c.deploymentStatsResponse.Load() if deploymentStats == nil { diff --git a/coderd/templates.go b/coderd/templates.go index 3e2803e4dcd3b..bfc9324c4524f 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -831,12 +831,10 @@ func (api *API) convertTemplate( template database.Template, ) codersdk.Template { templateAccessControl := (*(api.Options.AccessControlStore.Load())).GetTemplateAccessControl(template) - daus := 0 - _, resp, ok := api.metricsCache.TemplateDAUs(template.ID, 0) + owners := 0 + o, ok := api.metricsCache.TemplateWorkspaceOwners(template.ID) if ok { - // this is safe because we currently only support one timezone offset: 0 - // see metricscache.templateTimezoneOffsets - daus = resp.Entries[0].Amount + owners = o } buildTimeStats := api.metricsCache.TemplateBuildTimeStats(template.ID) @@ -855,7 +853,7 @@ func (api *API) convertTemplate( DisplayName: template.DisplayName, Provisioner: codersdk.ProvisionerType(template.Provisioner), ActiveVersionID: template.ActiveVersionID, - ActiveUserCount: daus, + ActiveUserCount: owners, BuildTimeStats: buildTimeStats, Description: template.Description, Icon: template.Icon, From 8169c9eb9371a70af71543d30ed97ec3e92090ff Mon Sep 17 00:00:00 2001 From: Garrett Delfosse Date: Wed, 6 Dec 2023 15:06:25 +0000 Subject: [PATCH 3/7] do dbmem --- coderd/database/dbauthz/dbauthz.go | 5 ++++- coderd/database/dbmem/dbmem.go | 27 +++++++++++++++++++++++++-- coderd/templates.go | 1 + 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index dcc81d31aecab..4148f96256000 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -2050,7 +2050,10 @@ func (q *querier) GetWorkspaceResourcesCreatedAfter(ctx context.Context, created } func (q *querier) GetWorkspaceUniqueOwnerCountByTemplateIDs(ctx context.Context, templateIds []uuid.UUID) ([]database.GetWorkspaceUniqueOwnerCountByTemplateIDsRow, error) { - panic("not implemented") + if err := q.authorizeContext(ctx, rbac.ActionRead, rbac.ResourceSystem); err != nil { + return nil, err + } + return q.db.GetWorkspaceUniqueOwnerCountByTemplateIDs(ctx, templateIds) } func (q *querier) GetWorkspaces(ctx context.Context, arg database.GetWorkspacesParams) ([]database.GetWorkspacesRow, error) { diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 77696f4ec79bb..b8edd736c6c2e 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -4462,8 +4462,31 @@ func (q *FakeQuerier) GetWorkspaceResourcesCreatedAfter(_ context.Context, after return resources, nil } -func (q *FakeQuerier) GetWorkspaceUniqueOwnerCountByTemplateIDs(ctx context.Context, templateIds []uuid.UUID) ([]database.GetWorkspaceUniqueOwnerCountByTemplateIDsRow, error) { - panic("not implemented") +func (q *FakeQuerier) GetWorkspaceUniqueOwnerCountByTemplateIDs(_ context.Context, templateIds []uuid.UUID) ([]database.GetWorkspaceUniqueOwnerCountByTemplateIDsRow, error) { + q.mutex.RLock() + defer q.mutex.RUnlock() + + workspaceOwners := make(map[uuid.UUID]map[uuid.UUID]struct{}) + for _, workspace := range q.workspaces { + if !slices.Contains(templateIds, workspace.TemplateID) { + continue + } + _, ok := workspaceOwners[workspace.TemplateID] + if !ok { + workspaceOwners[workspace.TemplateID] = make(map[uuid.UUID]struct{}) + } + workspaceOwners[workspace.TemplateID][workspace.OwnerID] = struct{}{} + } + resp := make([]database.GetWorkspaceUniqueOwnerCountByTemplateIDsRow, 0) + for _, templateID := range templateIds { + count := len(workspaceOwners[templateID]) + resp = append(resp, database.GetWorkspaceUniqueOwnerCountByTemplateIDsRow{ + TemplateID: templateID, + UniqueOwnersSum: int64(count), + }) + } + + return resp, nil } func (q *FakeQuerier) GetWorkspaces(ctx context.Context, arg database.GetWorkspacesParams) ([]database.GetWorkspacesRow, error) { diff --git a/coderd/templates.go b/coderd/templates.go index bfc9324c4524f..9ac8c2d85d5aa 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -831,6 +831,7 @@ func (api *API) convertTemplate( template database.Template, ) codersdk.Template { templateAccessControl := (*(api.Options.AccessControlStore.Load())).GetTemplateAccessControl(template) + owners := 0 o, ok := api.metricsCache.TemplateWorkspaceOwners(template.ID) if ok { From 359100136ae7bcf2c254db3d06c1c93842a174be Mon Sep 17 00:00:00 2001 From: Garrett Delfosse Date: Wed, 6 Dec 2023 15:38:57 +0000 Subject: [PATCH 4/7] add tests --- coderd/database/dbmem/dbmem.go | 3 ++ coderd/database/queries.sql.go | 2 +- coderd/database/queries/workspaces.sql | 2 +- coderd/metricscache/metricscache_test.go | 62 ++++++++++++++++++++++++ 4 files changed, 67 insertions(+), 2 deletions(-) diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index b8edd736c6c2e..6f1097cac5478 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -4468,6 +4468,9 @@ func (q *FakeQuerier) GetWorkspaceUniqueOwnerCountByTemplateIDs(_ context.Contex workspaceOwners := make(map[uuid.UUID]map[uuid.UUID]struct{}) for _, workspace := range q.workspaces { + if workspace.Deleted { + continue + } if !slices.Contains(templateIds, workspace.TemplateID) { continue } diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 3f7f20e8e4054..4d0b4d5c60384 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -10723,7 +10723,7 @@ func (q *sqlQuerier) GetWorkspaceByWorkspaceAppID(ctx context.Context, workspace const getWorkspaceUniqueOwnerCountByTemplateIDs = `-- name: GetWorkspaceUniqueOwnerCountByTemplateIDs :many SELECT template_id, COUNT(DISTINCT owner_id) AS unique_owners_sum FROM workspaces -WHERE template_id = ANY($1 :: uuid[]) +WHERE template_id = ANY($1 :: uuid[]) AND deleted = false GROUP BY template_id ` diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index 97e8a21bbf040..a290629cd349d 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -290,7 +290,7 @@ ORDER BY created_at DESC; -- name: GetWorkspaceUniqueOwnerCountByTemplateIDs :many SELECT template_id, COUNT(DISTINCT owner_id) AS unique_owners_sum FROM workspaces -WHERE template_id = ANY(@template_ids :: uuid[]) +WHERE template_id = ANY(@template_ids :: uuid[]) AND deleted = false GROUP BY template_id; -- name: InsertWorkspace :one diff --git a/coderd/metricscache/metricscache_test.go b/coderd/metricscache/metricscache_test.go index 996b9940d0056..9fce29420da69 100644 --- a/coderd/metricscache/metricscache_test.go +++ b/coderd/metricscache/metricscache_test.go @@ -254,6 +254,68 @@ func TestCache_TemplateUsers(t *testing.T) { } } +func TestCache_TemplateWorkspaceOwners(t *testing.T) { + t.Parallel() + var () + + var ( + db = dbmem.New() + cache = metricscache.New(db, slogtest.Make(t, nil), metricscache.Intervals{ + TemplateDAUs: testutil.IntervalFast, + }) + ) + + defer cache.Close() + + user1 := dbgen.User(t, db, database.User{}) + user2 := dbgen.User(t, db, database.User{}) + template := dbgen.Template(t, db, database.Template{ + Provisioner: database.ProvisionerTypeEcho, + }) + require.Eventuallyf(t, func() bool { + count, ok := cache.TemplateWorkspaceOwners(template.ID) + return ok && count == 0 + }, testutil.WaitShort, testutil.IntervalMedium, + "TemplateWorkspaceOwners never populated 0 owners", + ) + + dbgen.Workspace(t, db, database.Workspace{ + TemplateID: template.ID, + OwnerID: user1.ID, + }) + + require.Eventuallyf(t, func() bool { + count, _ := cache.TemplateWorkspaceOwners(template.ID) + return count == 1 + }, testutil.WaitShort, testutil.IntervalMedium, + "TemplateWorkspaceOwners never populated 1 owner", + ) + + workspace2 := dbgen.Workspace(t, db, database.Workspace{ + TemplateID: template.ID, + OwnerID: user2.ID, + }) + + require.Eventuallyf(t, func() bool { + count, _ := cache.TemplateWorkspaceOwners(template.ID) + return count == 2 + }, testutil.WaitShort, testutil.IntervalMedium, + "TemplateWorkspaceOwners never populated 2 owners", + ) + + db.UpdateWorkspaceDeletedByID(context.Background(), database.UpdateWorkspaceDeletedByIDParams{ + ID: workspace2.ID, + Deleted: true, + }) + + require.Eventuallyf(t, func() bool { + count, _ := cache.TemplateWorkspaceOwners(template.ID) + return count == 1 + }, testutil.WaitShort, testutil.IntervalMedium, + "TemplateWorkspaceOwners never populated 1 owner after delete", + ) +} + func clockTime(t time.Time, hour, minute, sec int) time.Time { return time.Date(t.Year(), t.Month(), t.Day(), hour, minute, sec, t.Nanosecond(), t.Location()) } From 53d788a34c21d5b275ce1f98d6deaea30596bfaf Mon Sep 17 00:00:00 2001 From: Garrett Delfosse Date: Wed, 6 Dec 2023 15:40:56 +0000 Subject: [PATCH 5/7] test unique --- coderd/metricscache/metricscache_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/coderd/metricscache/metricscache_test.go b/coderd/metricscache/metricscache_test.go index 9fce29420da69..5e226ae68f176 100644 --- a/coderd/metricscache/metricscache_test.go +++ b/coderd/metricscache/metricscache_test.go @@ -303,6 +303,12 @@ func TestCache_TemplateWorkspaceOwners(t *testing.T) { "TemplateWorkspaceOwners never populated 2 owners", ) + // 3rd workspace should not be counted since we have the same owner as workspace2. + dbgen.Workspace(t, db, database.Workspace{ + TemplateID: template.ID, + OwnerID: user1.ID, + }) + db.UpdateWorkspaceDeletedByID(context.Background(), database.UpdateWorkspaceDeletedByIDParams{ ID: workspace2.ID, Deleted: true, From 7a8829334ec8a60fa67a48ca7b510b24d8f35594 Mon Sep 17 00:00:00 2001 From: Garrett Delfosse Date: Thu, 7 Dec 2023 15:42:09 +0000 Subject: [PATCH 6/7] pr comments --- coderd/database/queries/workspaces.sql | 9 ++++++--- coderd/metricscache/metricscache.go | 6 +++--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/coderd/database/queries/workspaces.sql b/coderd/database/queries/workspaces.sql index a290629cd349d..54869a11b8638 100644 --- a/coderd/database/queries/workspaces.sql +++ b/coderd/database/queries/workspaces.sql @@ -288,9 +288,12 @@ WHERE ORDER BY created_at DESC; -- name: GetWorkspaceUniqueOwnerCountByTemplateIDs :many -SELECT template_id, COUNT(DISTINCT owner_id) AS unique_owners_sum -FROM workspaces -WHERE template_id = ANY(@template_ids :: uuid[]) AND deleted = false +SELECT + template_id, COUNT(DISTINCT owner_id) AS unique_owners_sum +FROM + workspaces +WHERE + template_id = ANY(@template_ids :: uuid[]) AND deleted = false GROUP BY template_id; -- name: InsertWorkspace :one diff --git a/coderd/metricscache/metricscache.go b/coderd/metricscache/metricscache.go index 8c541bcbf45f6..94623a8c2b648 100644 --- a/coderd/metricscache/metricscache.go +++ b/coderd/metricscache/metricscache.go @@ -216,9 +216,9 @@ func (c *Cache) refreshTemplateDAUs(ctx context.Context) error { return xerrors.Errorf("deployment daus: %w", err) } - ids := make([]uuid.UUID, len(templates)) - for i, template := range templates { - ids[i] = template.ID + ids := make([]uuid.UUID, 0, len(templates)) + for _, template := range templates { + ids = append(ids, template.ID) for _, tzOffset := range templateTimezoneOffsets { rows, err := c.database.GetTemplateDAUs(ctx, database.GetTemplateDAUsParams{ TemplateID: template.ID, From 4a64826dba8fe2d2b7b3e91e58697492dc48b353 Mon Sep 17 00:00:00 2001 From: Garrett Delfosse Date: Thu, 7 Dec 2023 15:43:03 +0000 Subject: [PATCH 7/7] make gen --- coderd/database/queries.sql.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 4d0b4d5c60384..d95c8e5f35087 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -10721,9 +10721,12 @@ func (q *sqlQuerier) GetWorkspaceByWorkspaceAppID(ctx context.Context, workspace } const getWorkspaceUniqueOwnerCountByTemplateIDs = `-- name: GetWorkspaceUniqueOwnerCountByTemplateIDs :many -SELECT template_id, COUNT(DISTINCT owner_id) AS unique_owners_sum -FROM workspaces -WHERE template_id = ANY($1 :: uuid[]) AND deleted = false +SELECT + template_id, COUNT(DISTINCT owner_id) AS unique_owners_sum +FROM + workspaces +WHERE + template_id = ANY($1 :: uuid[]) AND deleted = false GROUP BY template_id `