Skip to content

fix: prevent activity bump for prebuilt workspaces #19263

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion coderd/database/queries.sql.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 5 additions & 1 deletion coderd/database/queries/activitybump.sql
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you check the EXPLAIN output before and after?

Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,11 @@ WITH latest AS (
ON workspaces.id = workspace_builds.workspace_id
JOIN templates
ON templates.id = workspaces.template_id
WHERE workspace_builds.workspace_id = @workspace_id::uuid
WHERE
workspace_builds.workspace_id = @workspace_id::uuid
-- Prebuilt workspaces (identified by having the prebuilds system user as owner_id)
-- are managed by the reconciliation loop and not subject to activity bumping
AND workspaces.owner_id != 'c42fdf75-3097-471c-8c33-fb52454d81c0'::UUID
ORDER BY workspace_builds.build_number DESC
LIMIT 1
)
Expand Down
51 changes: 27 additions & 24 deletions coderd/workspacestats/reporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,33 +149,36 @@ func (r *Reporter) ReportAgentStats(ctx context.Context, now time.Time, workspac
return nil
}

// check next autostart
var nextAutostart time.Time
if workspace.AutostartSchedule.String != "" {
templateSchedule, err := (*(r.opts.TemplateScheduleStore.Load())).Get(ctx, r.opts.Database, workspace.TemplateID)
// If the template schedule fails to load, just default to bumping
// without the next transition and log it.
switch {
case err == nil:
next, allowed := schedule.NextAutostart(now, workspace.AutostartSchedule.String, templateSchedule)
if allowed {
nextAutostart = next
// Prebuilds are not subject to activity-based deadline bumps
if !workspace.IsPrebuild() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion: invert the logic and continue / return early

// check next autostart
var nextAutostart time.Time
if workspace.AutostartSchedule.String != "" {
templateSchedule, err := (*(r.opts.TemplateScheduleStore.Load())).Get(ctx, r.opts.Database, workspace.TemplateID)
// If the template schedule fails to load, just default to bumping
// without the next transition and log it.
switch {
case err == nil:
next, allowed := schedule.NextAutostart(now, workspace.AutostartSchedule.String, templateSchedule)
if allowed {
nextAutostart = next
}
case database.IsQueryCanceledError(err):
r.opts.Logger.Debug(ctx, "query canceled while loading template schedule",
slog.F("workspace_id", workspace.ID),
slog.F("template_id", workspace.TemplateID))
default:
r.opts.Logger.Error(ctx, "failed to load template schedule bumping activity, defaulting to bumping by 60min",
slog.F("workspace_id", workspace.ID),
slog.F("template_id", workspace.TemplateID),
slog.Error(err),
)
}
case database.IsQueryCanceledError(err):
r.opts.Logger.Debug(ctx, "query canceled while loading template schedule",
slog.F("workspace_id", workspace.ID),
slog.F("template_id", workspace.TemplateID))
default:
r.opts.Logger.Error(ctx, "failed to load template schedule bumping activity, defaulting to bumping by 60min",
slog.F("workspace_id", workspace.ID),
slog.F("template_id", workspace.TemplateID),
slog.Error(err),
)
}
}

// bump workspace activity
ActivityBumpWorkspace(ctx, r.opts.Logger.Named("activity_bump"), r.opts.Database, workspace.ID, nextAutostart)
// bump workspace activity
ActivityBumpWorkspace(ctx, r.opts.Logger.Named("activity_bump"), r.opts.Database, workspace.ID, nextAutostart)
}

// bump workspace last_used_at
r.opts.UsageTracker.Add(workspace.ID)
Expand Down
Loading