mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
perf(coderd/database): limit GetLatestWorkspaceAppStatusByAppID to 1 row (#20917)
## 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 🏄🏻♂️
This commit is contained in:
@@ -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) {
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user