Skip to content

Commit 068dbd6

Browse files
mafredrimafredri
committed
fix(coderd/prometheusmetrics): no deleted wsbuilds to reduce db load
This change removes the `GetLatestWorkspaceBuilds` query which includes all workspaces for all time (including deleted). This allows us to also stop using `GetProvisionerJobsByIDs` for said builds as the job status is included in `GetWorkspaces` called separately. Fixes coder/internal#717
1 parent 408e19f commit 068dbd6

File tree

8 files changed

+93
-149
lines changed

8 files changed

+93
-149
lines changed

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2157,17 +2157,6 @@ func (q *querier) GetLatestWorkspaceBuildByWorkspaceID(ctx context.Context, work
21572157
return q.db.GetLatestWorkspaceBuildByWorkspaceID(ctx, workspaceID)
21582158
}
21592159

2160-
func (q *querier) GetLatestWorkspaceBuilds(ctx context.Context) ([]database.WorkspaceBuild, error) {
2161-
// This function is a system function until we implement a join for workspace builds.
2162-
// This is because we need to query for all related workspaces to the returned builds.
2163-
// This is a very inefficient method of fetching the latest workspace builds.
2164-
// We should just join the rbac properties.
2165-
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil {
2166-
return nil, err
2167-
}
2168-
return q.db.GetLatestWorkspaceBuilds(ctx)
2169-
}
2170-
21712160
func (q *querier) GetLatestWorkspaceBuildsByWorkspaceIDs(ctx context.Context, ids []uuid.UUID) ([]database.WorkspaceBuild, error) {
21722161
// This function is a system function until we implement a join for workspace builds.
21732162
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil {

‎coderd/database/dbmetrics/querymetrics.go

Lines changed: 0 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/dbmock/dbmock.go

Lines changed: 0 additions & 15 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/querier.go

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries.sql.go

Lines changed: 0 additions & 59 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/workspacebuilds.sql

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -91,20 +91,6 @@ JOIN
9191
workspace_build_with_user AS wb
9292
ON m.workspace_id = wb.workspace_id AND m.max_build_number = wb.build_number;
9393

94-
-- name: GetLatestWorkspaceBuilds :many
95-
SELECT wb.*
96-
FROM (
97-
SELECT
98-
workspace_id, MAX(build_number) as max_build_number
99-
FROM
100-
workspace_build_with_user AS workspace_builds
101-
GROUP BY
102-
workspace_id
103-
) m
104-
JOIN
105-
workspace_build_with_user AS wb
106-
ON m.workspace_id = wb.workspace_id AND m.max_build_number = wb.build_number;
107-
10894
-- name: InsertWorkspaceBuild :exec
10995
INSERT INTO
11096
workspace_builds (

‎coderd/prometheusmetrics/prometheusmetrics.go

Lines changed: 19 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -168,59 +168,37 @@ func Workspaces(ctx context.Context, logger slog.Logger, registerer prometheus.R
168168
ctx, cancelFunc := context.WithCancel(ctx)
169169
done := make(chan struct{})
170170

171-
updateWorkspaceTotals := func() {
172-
builds, err := db.GetLatestWorkspaceBuilds(ctx)
173-
if err != nil {
174-
if errors.Is(err, sql.ErrNoRows) {
175-
// clear all series if there are no database entries
176-
workspaceLatestBuildTotals.Reset()
177-
} else {
178-
logger.Warn(ctx, "failed to load latest workspace builds", slog.Error(err))
179-
}
180-
return
181-
}
182-
jobIDs := make([]uuid.UUID, 0, len(builds))
183-
for _, build := range builds {
184-
jobIDs = append(jobIDs, build.JobID)
185-
}
186-
jobs, err := db.GetProvisionerJobsByIDs(ctx, jobIDs)
187-
if err != nil {
188-
ids := make([]string, 0, len(jobIDs))
189-
for _, id := range jobIDs {
190-
ids = append(ids, id.String())
191-
}
192-
193-
logger.Warn(ctx, "failed to load provisioner jobs", slog.F("ids", ids), slog.Error(err))
194-
return
195-
}
196-
197-
workspaceLatestBuildTotals.Reset()
198-
for _, job := range jobs {
199-
status := codersdk.ProvisionerJobStatus(job.JobStatus)
200-
workspaceLatestBuildTotals.WithLabelValues(string(status)).Add(1)
201-
// TODO: deprecated: remove in the future
202-
workspaceLatestBuildTotalsDeprecated.WithLabelValues(string(status)).Add(1)
203-
}
204-
}
205-
206-
updateWorkspaceStatuses := func() {
171+
updateWorkspaceMetrics := func() {
207172
ws, err := db.GetWorkspaces(ctx, database.GetWorkspacesParams{
208173
Deleted: false,
209174
WithSummary: false,
210175
})
211176
if err != nil {
212177
if errors.Is(err, sql.ErrNoRows) {
213-
// clear all series if there are no database entries
178+
workspaceLatestBuildTotals.Reset()
214179
workspaceLatestBuildStatuses.Reset()
180+
} else {
181+
logger.Warn(ctx, "failed to load active workspaces for metrics", slog.Error(err))
215182
}
216-
217-
logger.Warn(ctx, "failed to load active workspaces", slog.Error(err))
218183
return
219184
}
220185

186+
workspaceLatestBuildTotals.Reset()
221187
workspaceLatestBuildStatuses.Reset()
188+
222189
for _, w := range ws {
223-
workspaceLatestBuildStatuses.WithLabelValues(string(w.LatestBuildStatus), w.TemplateName, w.TemplateVersionName.String, w.OwnerUsername, string(w.LatestBuildTransition)).Add(1)
190+
status := string(w.LatestBuildStatus)
191+
workspaceLatestBuildTotals.WithLabelValues(status).Add(1)
192+
// TODO: deprecated: remove in the future
193+
workspaceLatestBuildTotalsDeprecated.WithLabelValues(status).Add(1)
194+
195+
workspaceLatestBuildStatuses.WithLabelValues(
196+
status,
197+
w.TemplateName,
198+
w.TemplateVersionName.String,
199+
w.OwnerUsername,
200+
string(w.LatestBuildTransition),
201+
).Add(1)
224202
}
225203
}
226204

@@ -230,8 +208,7 @@ func Workspaces(ctx context.Context, logger slog.Logger, registerer prometheus.R
230208
doTick := func() {
231209
defer ticker.Reset(duration)
232210

233-
updateWorkspaceTotals()
234-
updateWorkspaceStatuses()
211+
updateWorkspaceMetrics()
235212
}
236213

237214
go func() {

‎coderd/prometheusmetrics/prometheusmetrics_test.go

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,32 @@ func TestWorkspaceLatestBuildTotals(t *testing.T) {
247247
codersdk.ProvisionerJobSucceeded: 3,
248248
codersdk.ProvisionerJobRunning: 1,
249249
},
250+
}, {
251+
Name: "MultipleWithDeleted",
252+
Database: func() database.Store {
253+
db, _ := dbtestutil.NewDB(t)
254+
u := dbgen.User(t, db, database.User{})
255+
org := dbgen.Organization(t, db, database.Organization{})
256+
insertCanceled(t, db, u, org)
257+
insertFailed(t, db, u, org)
258+
insertSuccess(t, db, u, org)
259+
insertRunning(t, db, u, org)
260+
261+
// Verify that deleted workspaces/builds are NOT counted in metrics.
262+
n, err := cryptorand.Intn(5)
263+
require.NoError(t, err)
264+
for range 1 + n {
265+
insertDeleted(t, db, u, org)
266+
}
267+
return db
268+
},
269+
Total: 4, // Only non-deleted workspaces should be counted
270+
Status: map[codersdk.ProvisionerJobStatus]int{
271+
codersdk.ProvisionerJobCanceled: 1,
272+
codersdk.ProvisionerJobFailed: 1,
273+
codersdk.ProvisionerJobSucceeded: 1,
274+
codersdk.ProvisionerJobRunning: 1,
275+
},
250276
}} {
251277
t.Run(tc.Name, func(t *testing.T) {
252278
t.Parallel()
@@ -323,6 +349,33 @@ func TestWorkspaceLatestBuildStatuses(t *testing.T) {
323349
codersdk.ProvisionerJobSucceeded: 3,
324350
codersdk.ProvisionerJobRunning: 1,
325351
},
352+
}, {
353+
Name: "MultipleWithDeleted",
354+
Database: func() database.Store {
355+
db, _ := dbtestutil.NewDB(t)
356+
u := dbgen.User(t, db, database.User{})
357+
org := dbgen.Organization(t, db, database.Organization{})
358+
insertTemplates(t, db, u, org)
359+
insertCanceled(t, db, u, org)
360+
insertFailed(t, db, u, org)
361+
insertSuccess(t, db, u, org)
362+
insertRunning(t, db, u, org)
363+
364+
// Verify that deleted workspaces/builds are NOT counted in metrics.
365+
n, err := cryptorand.Intn(5)
366+
require.NoError(t, err)
367+
for range 1 + n {
368+
insertDeleted(t, db, u, org)
369+
}
370+
return db
371+
},
372+
ExpectedWorkspaces: 4, // Only non-deleted workspaces should be counted
373+
ExpectedStatuses: map[codersdk.ProvisionerJobStatus]int{
374+
codersdk.ProvisionerJobCanceled: 1,
375+
codersdk.ProvisionerJobFailed: 1,
376+
codersdk.ProvisionerJobSucceeded: 1,
377+
codersdk.ProvisionerJobRunning: 1,
378+
},
326379
}} {
327380
t.Run(tc.Name, func(t *testing.T) {
328381
t.Parallel()
@@ -907,3 +960,24 @@ func insertSuccess(t *testing.T, db database.Store, u database.User, org databas
907960
})
908961
require.NoError(t, err)
909962
}
963+
964+
func insertDeleted(t *testing.T, db database.Store, u database.User, org database.Organization) {
965+
job := insertRunning(t, db, u, org)
966+
err := db.UpdateProvisionerJobWithCompleteByID(context.Background(), database.UpdateProvisionerJobWithCompleteByIDParams{
967+
ID: job.ID,
968+
CompletedAt: sql.NullTime{
969+
Time: dbtime.Now(),
970+
Valid: true,
971+
},
972+
})
973+
require.NoError(t, err)
974+
975+
build, err := db.GetWorkspaceBuildByJobID(context.Background(), job.ID)
976+
require.NoError(t, err)
977+
978+
err = db.UpdateWorkspaceDeletedByID(context.Background(), database.UpdateWorkspaceDeletedByIDParams{
979+
ID: build.WorkspaceID,
980+
Deleted: true,
981+
})
982+
require.NoError(t, err)
983+
}

0 commit comments

Comments
 (0)