From 37fc6646adf553251d3d46ef6c056b17f4f74d97 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 25 Nov 2025 16:56:42 +0200 Subject: [PATCH] perf(coderd/database): limit `GetLatestWorkspaceAppStatusByAppID` to 1 row (#20917) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description This PR fixes an issue where `GetLatestWorkspaceAppStatusesByAppID` returned an unbounded number of rows for a given app ID, which could cause performance issues for noisy or long-running AI tasks. ## Impact This change reduces database query overhead for workspace app status updates, particularly for busy AI tasks that update their status frequently. Previously, fetching the latest status would return all historical statuses, now it returns only the most recent one. Fixes #20862 --- 🤖 This change was written by Claude Sonnet 4.5 Thinking using [mux](https://github.com/coder/mux) and reviewed by a human 🏄🏻‍♂️ --- coderd/database/dbauthz/dbauthz.go | 6 +-- coderd/database/dbauthz/dbauthz_test.go | 4 +- coderd/database/dbmetrics/querymetrics.go | 6 +-- coderd/database/dbmock/dbmock.go | 14 +++---- coderd/database/querier.go | 2 +- coderd/database/queries.sql.go | 47 ++++++++--------------- coderd/database/queries/workspaceapps.sql | 5 ++- coderd/workspaceagents.go | 17 ++++---- 8 files changed, 45 insertions(+), 56 deletions(-) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 5edc286613..287f289451 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -2426,11 +2426,11 @@ func (q *querier) GetLatestCryptoKeyByFeature(ctx context.Context, feature datab return q.db.GetLatestCryptoKeyByFeature(ctx, feature) } -func (q *querier) GetLatestWorkspaceAppStatusesByAppID(ctx context.Context, appID uuid.UUID) ([]database.WorkspaceAppStatus, error) { +func (q *querier) GetLatestWorkspaceAppStatusByAppID(ctx context.Context, appID uuid.UUID) (database.WorkspaceAppStatus, error) { if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil { - return nil, err + return database.WorkspaceAppStatus{}, err } - return q.db.GetLatestWorkspaceAppStatusesByAppID(ctx, appID) + return q.db.GetLatestWorkspaceAppStatusByAppID(ctx, appID) } func (q *querier) GetLatestWorkspaceAppStatusesByWorkspaceIDs(ctx context.Context, ids []uuid.UUID) ([]database.WorkspaceAppStatus, error) { diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 3304155af8..d14429a105 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -2864,9 +2864,9 @@ func (s *MethodTestSuite) TestSystemFunctions() { dbm.EXPECT().UpdateUserLinkedID(gomock.Any(), arg).Return(l, nil).AnyTimes() check.Args(arg).Asserts(rbac.ResourceSystem, policy.ActionUpdate).Returns(l) })) - s.Run("GetLatestWorkspaceAppStatusesByAppID", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) { + s.Run("GetLatestWorkspaceAppStatusByAppID", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) { appID := uuid.New() - dbm.EXPECT().GetLatestWorkspaceAppStatusesByAppID(gomock.Any(), appID).Return([]database.WorkspaceAppStatus{}, nil).AnyTimes() + dbm.EXPECT().GetLatestWorkspaceAppStatusByAppID(gomock.Any(), appID).Return(database.WorkspaceAppStatus{}, nil).AnyTimes() check.Args(appID).Asserts(rbac.ResourceSystem, policy.ActionRead) })) s.Run("GetLatestWorkspaceAppStatusesByWorkspaceIDs", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) { diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index 8b3190ee17..89269b295c 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -1033,10 +1033,10 @@ func (m queryMetricsStore) GetLatestCryptoKeyByFeature(ctx context.Context, feat return r0, r1 } -func (m queryMetricsStore) GetLatestWorkspaceAppStatusesByAppID(ctx context.Context, appID uuid.UUID) ([]database.WorkspaceAppStatus, error) { +func (m queryMetricsStore) GetLatestWorkspaceAppStatusByAppID(ctx context.Context, appID uuid.UUID) (database.WorkspaceAppStatus, error) { start := time.Now() - r0, r1 := m.s.GetLatestWorkspaceAppStatusesByAppID(ctx, appID) - m.queryLatencies.WithLabelValues("GetLatestWorkspaceAppStatusesByAppID").Observe(time.Since(start).Seconds()) + r0, r1 := m.s.GetLatestWorkspaceAppStatusByAppID(ctx, appID) + m.queryLatencies.WithLabelValues("GetLatestWorkspaceAppStatusByAppID").Observe(time.Since(start).Seconds()) return r0, r1 } diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index 4970a53701..ce50bf87d2 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -2172,19 +2172,19 @@ func (mr *MockStoreMockRecorder) GetLatestCryptoKeyByFeature(ctx, feature any) * return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLatestCryptoKeyByFeature", reflect.TypeOf((*MockStore)(nil).GetLatestCryptoKeyByFeature), ctx, feature) } -// GetLatestWorkspaceAppStatusesByAppID mocks base method. -func (m *MockStore) GetLatestWorkspaceAppStatusesByAppID(ctx context.Context, appID uuid.UUID) ([]database.WorkspaceAppStatus, error) { +// GetLatestWorkspaceAppStatusByAppID mocks base method. +func (m *MockStore) GetLatestWorkspaceAppStatusByAppID(ctx context.Context, appID uuid.UUID) (database.WorkspaceAppStatus, error) { m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "GetLatestWorkspaceAppStatusesByAppID", ctx, appID) - ret0, _ := ret[0].([]database.WorkspaceAppStatus) + ret := m.ctrl.Call(m, "GetLatestWorkspaceAppStatusByAppID", ctx, appID) + ret0, _ := ret[0].(database.WorkspaceAppStatus) ret1, _ := ret[1].(error) return ret0, ret1 } -// GetLatestWorkspaceAppStatusesByAppID indicates an expected call of GetLatestWorkspaceAppStatusesByAppID. -func (mr *MockStoreMockRecorder) GetLatestWorkspaceAppStatusesByAppID(ctx, appID any) *gomock.Call { +// GetLatestWorkspaceAppStatusByAppID indicates an expected call of GetLatestWorkspaceAppStatusByAppID. +func (mr *MockStoreMockRecorder) GetLatestWorkspaceAppStatusByAppID(ctx, appID any) *gomock.Call { mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLatestWorkspaceAppStatusesByAppID", reflect.TypeOf((*MockStore)(nil).GetLatestWorkspaceAppStatusesByAppID), ctx, appID) + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetLatestWorkspaceAppStatusByAppID", reflect.TypeOf((*MockStore)(nil).GetLatestWorkspaceAppStatusByAppID), ctx, appID) } // GetLatestWorkspaceAppStatusesByWorkspaceIDs mocks base method. diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 0f8570127b..5c249f3bdf 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -238,7 +238,7 @@ type sqlcQuerier interface { GetInboxNotificationsByUserID(ctx context.Context, arg GetInboxNotificationsByUserIDParams) ([]InboxNotification, error) GetLastUpdateCheck(ctx context.Context) (string, error) GetLatestCryptoKeyByFeature(ctx context.Context, feature CryptoKeyFeature) (CryptoKey, error) - GetLatestWorkspaceAppStatusesByAppID(ctx context.Context, appID uuid.UUID) ([]WorkspaceAppStatus, error) + GetLatestWorkspaceAppStatusByAppID(ctx context.Context, appID uuid.UUID) (WorkspaceAppStatus, error) GetLatestWorkspaceAppStatusesByWorkspaceIDs(ctx context.Context, ids []uuid.UUID) ([]WorkspaceAppStatus, error) GetLatestWorkspaceBuildByWorkspaceID(ctx context.Context, workspaceID uuid.UUID) (WorkspaceBuild, error) GetLatestWorkspaceBuildsByWorkspaceIDs(ctx context.Context, ids []uuid.UUID) ([]WorkspaceBuild, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index d592a0f4b5..d99155ba99 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -20041,43 +20041,28 @@ func (q *sqlQuerier) UpsertWorkspaceAppAuditSession(ctx context.Context, arg Ups return new_or_stale, err } -const getLatestWorkspaceAppStatusesByAppID = `-- name: GetLatestWorkspaceAppStatusesByAppID :many +const getLatestWorkspaceAppStatusByAppID = `-- name: GetLatestWorkspaceAppStatusByAppID :one SELECT id, created_at, agent_id, app_id, workspace_id, state, message, uri FROM workspace_app_statuses WHERE app_id = $1::uuid ORDER BY created_at DESC, id DESC +LIMIT 1 ` -func (q *sqlQuerier) GetLatestWorkspaceAppStatusesByAppID(ctx context.Context, appID uuid.UUID) ([]WorkspaceAppStatus, error) { - rows, err := q.db.QueryContext(ctx, getLatestWorkspaceAppStatusesByAppID, appID) - if err != nil { - return nil, err - } - defer rows.Close() - var items []WorkspaceAppStatus - for rows.Next() { - var i WorkspaceAppStatus - if err := rows.Scan( - &i.ID, - &i.CreatedAt, - &i.AgentID, - &i.AppID, - &i.WorkspaceID, - &i.State, - &i.Message, - &i.Uri, - ); 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 +func (q *sqlQuerier) GetLatestWorkspaceAppStatusByAppID(ctx context.Context, appID uuid.UUID) (WorkspaceAppStatus, error) { + row := q.db.QueryRowContext(ctx, getLatestWorkspaceAppStatusByAppID, appID) + var i WorkspaceAppStatus + err := row.Scan( + &i.ID, + &i.CreatedAt, + &i.AgentID, + &i.AppID, + &i.WorkspaceID, + &i.State, + &i.Message, + &i.Uri, + ) + return i, err } const getLatestWorkspaceAppStatusesByWorkspaceIDs = `-- name: GetLatestWorkspaceAppStatusesByWorkspaceIDs :many diff --git a/coderd/database/queries/workspaceapps.sql b/coderd/database/queries/workspaceapps.sql index d76e789f19..b3694fb275 100644 --- a/coderd/database/queries/workspaceapps.sql +++ b/coderd/database/queries/workspaceapps.sql @@ -73,11 +73,12 @@ RETURNING *; -- name: GetWorkspaceAppStatusesByAppIDs :many SELECT * FROM workspace_app_statuses WHERE app_id = ANY(@ids :: uuid [ ]); --- name: GetLatestWorkspaceAppStatusesByAppID :many +-- name: GetLatestWorkspaceAppStatusByAppID :one SELECT * FROM workspace_app_statuses WHERE app_id = @app_id::uuid -ORDER BY created_at DESC, id DESC; +ORDER BY created_at DESC, id DESC +LIMIT 1; -- name: GetLatestWorkspaceAppStatusesByWorkspaceIDs :many SELECT DISTINCT ON (workspace_id) diff --git a/coderd/workspaceagents.go b/coderd/workspaceagents.go index eced6ff6d3..d3cca07066 100644 --- a/coderd/workspaceagents.go +++ b/coderd/workspaceagents.go @@ -388,16 +388,17 @@ func (api *API) patchWorkspaceAgentAppStatus(rw http.ResponseWriter, r *http.Req // Treat the message as untrusted input. cleaned := strutil.UISanitize(req.Message) - // Get the latest statuses for the workspace app to detect no-op updates + // Get the latest status for the workspace app to detect no-op updates // nolint:gocritic // This is a system restricted operation. - latestAppStatus, err := api.Database.GetLatestWorkspaceAppStatusesByAppID(dbauthz.AsSystemRestricted(ctx), app.ID) - if err != nil { + latestAppStatus, err := api.Database.GetLatestWorkspaceAppStatusByAppID(dbauthz.AsSystemRestricted(ctx), app.ID) + if err != nil && !errors.Is(err, sql.ErrNoRows) { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Failed to get latest workspace app statuses.", + Message: "Failed to get latest workspace app status.", Detail: err.Error(), }) return } + // If no rows found, latestAppStatus will be a zero-value struct (ID == uuid.Nil) // nolint:gocritic // This is a system restricted operation. _, err = api.Database.InsertWorkspaceAppStatus(dbauthz.AsSystemRestricted(ctx), database.InsertWorkspaceAppStatusParams{ @@ -442,7 +443,7 @@ func (api *API) patchWorkspaceAgentAppStatus(rw http.ResponseWriter, r *http.Req func (api *API) enqueueAITaskStateNotification( ctx context.Context, appID uuid.UUID, - latestAppStatus []database.WorkspaceAppStatus, + latestAppStatus database.WorkspaceAppStatus, newAppStatus codersdk.WorkspaceAppStatusState, workspace database.Workspace, agent database.WorkspaceAgent, @@ -492,14 +493,16 @@ func (api *API) enqueueAITaskStateNotification( } // Skip if the latest persisted state equals the new state (no new transition) - if len(latestAppStatus) > 0 && latestAppStatus[0].State == database.WorkspaceAppStatusState(newAppStatus) { + // Note: uuid.Nil check is valid here. If no previous status exists, + // GetLatestWorkspaceAppStatusByAppID returns sql.ErrNoRows and we get a zero-value struct. + if latestAppStatus.ID != uuid.Nil && latestAppStatus.State == database.WorkspaceAppStatusState(newAppStatus) { return } // Skip the initial "Working" notification when task first starts. // This is obvious to the user since they just created the task. // We still notify on first "Idle" status and all subsequent transitions. - if len(latestAppStatus) == 0 && newAppStatus == codersdk.WorkspaceAppStatusStateWorking { + if latestAppStatus.ID == uuid.Nil && newAppStatus == codersdk.WorkspaceAppStatusStateWorking { return }