diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index ae3a3a036a..e6cdb83068 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -5406,20 +5406,20 @@ func (q *sqlQuerier) GetParameterSchemasByJobID(ctx context.Context, jobID uuid. const claimPrebuild = `-- name: ClaimPrebuild :one UPDATE workspaces w SET owner_id = $1::uuid, - name = $2::text, - updated_at = NOW() + name = $2::text, + updated_at = NOW() WHERE w.id IN (SELECT p.id - FROM workspace_prebuilds p - INNER JOIN workspace_latest_build b ON b.workspace_id = p.id - INNER JOIN provisioner_jobs pj ON b.job_id = pj.id - INNER JOIN templates t ON p.template_id = t.id - WHERE (b.transition = 'start'::workspace_transition - AND pj.job_status IN ('succeeded'::provisioner_job_status)) - AND b.template_version_id = t.active_version_id - AND b.template_version_preset_id = $3::uuid - AND p.lifecycle_state = 'ready'::workspace_agent_lifecycle_state - ORDER BY random() - LIMIT 1 FOR UPDATE OF p SKIP LOCKED) + FROM workspace_prebuilds p + INNER JOIN workspace_latest_build b ON b.workspace_id = p.id + INNER JOIN provisioner_jobs pj ON b.job_id = pj.id + INNER JOIN templates t ON p.template_id = t.id + WHERE (b.transition = 'start'::workspace_transition + AND pj.job_status IN ('succeeded'::provisioner_job_status)) + AND b.template_version_id = t.active_version_id + AND b.template_version_preset_id = $3::uuid + AND p.lifecycle_state = 'ready'::workspace_agent_lifecycle_state + ORDER BY random() + LIMIT 1 FOR UPDATE OF p SKIP LOCKED) RETURNING w.id, w.name ` @@ -5443,17 +5443,19 @@ func (q *sqlQuerier) ClaimPrebuild(ctx context.Context, arg ClaimPrebuildParams) } const getPrebuildsInProgress = `-- name: GetPrebuildsInProgress :many -SELECT wpb.template_version_id, wpb.transition, COUNT(wpb.transition) AS count +SELECT t.id AS template_id, wpb.template_version_id, wpb.transition, COUNT(wpb.transition) AS count FROM workspace_latest_build wlb - INNER JOIN provisioner_jobs pj ON wlb.job_id = pj.id - INNER JOIN workspace_prebuild_builds wpb ON wpb.id = wlb.id -WHERE pj.job_status NOT IN - ('succeeded'::provisioner_job_status, 'canceled'::provisioner_job_status, - 'failed'::provisioner_job_status) -GROUP BY wpb.template_version_id, wpb.transition + INNER JOIN provisioner_jobs pj ON wlb.job_id = pj.id + INNER JOIN workspace_prebuild_builds wpb ON wpb.id = wlb.id + INNER JOIN templates t ON t.active_version_id = wlb.template_version_id +WHERE pj.job_status NOT IN -- Jobs that are not in terminal states. + ('succeeded'::provisioner_job_status, 'canceled'::provisioner_job_status, + 'failed'::provisioner_job_status) +GROUP BY t.id, wpb.template_version_id, wpb.transition ` type GetPrebuildsInProgressRow struct { + TemplateID uuid.UUID `db:"template_id" json:"template_id"` TemplateVersionID uuid.UUID `db:"template_version_id" json:"template_version_id"` Transition WorkspaceTransition `db:"transition" json:"transition"` Count int64 `db:"count" json:"count"` @@ -5468,7 +5470,12 @@ func (q *sqlQuerier) GetPrebuildsInProgress(ctx context.Context) ([]GetPrebuilds var items []GetPrebuildsInProgressRow for rows.Next() { var i GetPrebuildsInProgressRow - if err := rows.Scan(&i.TemplateVersionID, &i.Transition, &i.Count); err != nil { + if err := rows.Scan( + &i.TemplateID, + &i.TemplateVersionID, + &i.Transition, + &i.Count, + ); err != nil { return nil, err } items = append(items, i) @@ -5484,31 +5491,26 @@ func (q *sqlQuerier) GetPrebuildsInProgress(ctx context.Context) ([]GetPrebuilds const getRunningPrebuilds = `-- name: GetRunningPrebuilds :many SELECT p.id AS workspace_id, - p.name AS workspace_name, - p.template_id, - b.template_version_id, - tvp_curr.id AS current_preset_id, - tvp_desired.id AS desired_preset_id, - -- TODO: just because a prebuild is in a ready state doesn't mean it's eligible; if the prebuild is due to be - -- deleted to reconcile state then it MUST NOT be eligible for claiming. We'll need some kind of lock here. - CASE - WHEN p.lifecycle_state = 'ready'::workspace_agent_lifecycle_state THEN TRUE - ELSE FALSE END AS ready, - p.created_at + p.name AS workspace_name, + p.template_id, + b.template_version_id, + tvp_curr.id AS current_preset_id, + -- TODO: just because a prebuild is in a ready state doesn't mean it's eligible; if the prebuild is due to be + -- deleted to reconcile state then it MUST NOT be eligible for claiming. We'll need some kind of lock here. + CASE + WHEN p.lifecycle_state = 'ready'::workspace_agent_lifecycle_state THEN TRUE + ELSE FALSE END AS ready, + p.created_at FROM workspace_prebuilds p - INNER JOIN workspace_latest_build b ON b.workspace_id = p.id - INNER JOIN provisioner_jobs pj ON b.job_id = pj.id - INNER JOIN templates t ON p.template_id = t.id - LEFT JOIN template_version_presets tvp_curr - ON tvp_curr.id = p.current_preset_id -- See https://github.com/coder/internal/issues/398. - LEFT JOIN template_version_presets tvp_desired - ON tvp_desired.template_version_id = t.active_version_id + INNER JOIN workspace_latest_build b ON b.workspace_id = p.id + INNER JOIN provisioner_jobs pj ON b.job_id = pj.id + INNER JOIN templates t ON p.template_id = t.id + LEFT JOIN template_version_presets tvp_curr + ON tvp_curr.id = p.current_preset_id -- See https://github.com/coder/internal/issues/398. WHERE (b.transition = 'start'::workspace_transition - -- if a deletion job fails, the workspace will still be running - OR pj.job_status IN ('failed'::provisioner_job_status, 'canceled'::provisioner_job_status, - 'unknown'::provisioner_job_status)) - AND (tvp_curr.name = tvp_desired.name - OR tvp_desired.id IS NULL) + -- Jobs that are not in terminal states. + OR pj.job_status IN ('failed'::provisioner_job_status, 'canceled'::provisioner_job_status, + 'unknown'::provisioner_job_status)) ` type GetRunningPrebuildsRow struct { @@ -5517,7 +5519,6 @@ type GetRunningPrebuildsRow struct { TemplateID uuid.UUID `db:"template_id" json:"template_id"` TemplateVersionID uuid.UUID `db:"template_version_id" json:"template_version_id"` CurrentPresetID uuid.NullUUID `db:"current_preset_id" json:"current_preset_id"` - DesiredPresetID uuid.NullUUID `db:"desired_preset_id" json:"desired_preset_id"` Ready bool `db:"ready" json:"ready"` CreatedAt time.Time `db:"created_at" json:"created_at"` } @@ -5537,7 +5538,6 @@ func (q *sqlQuerier) GetRunningPrebuilds(ctx context.Context) ([]GetRunningPrebu &i.TemplateID, &i.TemplateVersionID, &i.CurrentPresetID, - &i.DesiredPresetID, &i.Ready, &i.CreatedAt, ); err != nil { @@ -5556,17 +5556,17 @@ func (q *sqlQuerier) GetRunningPrebuilds(ctx context.Context) ([]GetRunningPrebu const getTemplatePresetsWithPrebuilds = `-- name: GetTemplatePresetsWithPrebuilds :many SELECT t.id AS template_id, - tv.id AS template_version_id, - tv.id = t.active_version_id AS using_active_version, - tvpp.preset_id, - tvp.name, - tvpp.desired_instances AS desired_instances, - t.deleted, - t.deprecated != '' AS deprecated + tv.id AS template_version_id, + tv.id = t.active_version_id AS using_active_version, + tvpp.preset_id, + tvp.name, + tvpp.desired_instances AS desired_instances, + t.deleted, + t.deprecated != '' AS deprecated FROM templates t - INNER JOIN template_versions tv ON tv.template_id = t.id - INNER JOIN template_version_presets tvp ON tvp.template_version_id = tv.id - INNER JOIN template_version_preset_prebuilds tvpp ON tvpp.preset_id = tvp.id + INNER JOIN template_versions tv ON tv.template_id = t.id + INNER JOIN template_version_presets tvp ON tvp.template_version_id = tv.id + INNER JOIN template_version_preset_prebuilds tvpp ON tvpp.preset_id = tvp.id WHERE (t.id = $1::uuid OR $1 IS NULL) ` diff --git a/coderd/database/queries/prebuilds.sql b/coderd/database/queries/prebuilds.sql index 45b57a042b..f760b094f3 100644 --- a/coderd/database/queries/prebuilds.sql +++ b/coderd/database/queries/prebuilds.sql @@ -1,74 +1,70 @@ -- name: GetRunningPrebuilds :many SELECT p.id AS workspace_id, - p.name AS workspace_name, - p.template_id, - b.template_version_id, - tvp_curr.id AS current_preset_id, - tvp_desired.id AS desired_preset_id, - -- TODO: just because a prebuild is in a ready state doesn't mean it's eligible; if the prebuild is due to be - -- deleted to reconcile state then it MUST NOT be eligible for claiming. We'll need some kind of lock here. - CASE - WHEN p.lifecycle_state = 'ready'::workspace_agent_lifecycle_state THEN TRUE - ELSE FALSE END AS ready, - p.created_at + p.name AS workspace_name, + p.template_id, + b.template_version_id, + tvp_curr.id AS current_preset_id, + -- TODO: just because a prebuild is in a ready state doesn't mean it's eligible; if the prebuild is due to be + -- deleted to reconcile state then it MUST NOT be eligible for claiming. We'll need some kind of lock here. + CASE + WHEN p.lifecycle_state = 'ready'::workspace_agent_lifecycle_state THEN TRUE + ELSE FALSE END AS ready, + p.created_at FROM workspace_prebuilds p - INNER JOIN workspace_latest_build b ON b.workspace_id = p.id - INNER JOIN provisioner_jobs pj ON b.job_id = pj.id - INNER JOIN templates t ON p.template_id = t.id - LEFT JOIN template_version_presets tvp_curr - ON tvp_curr.id = p.current_preset_id -- See https://github.com/coder/internal/issues/398. - LEFT JOIN template_version_presets tvp_desired - ON tvp_desired.template_version_id = t.active_version_id + INNER JOIN workspace_latest_build b ON b.workspace_id = p.id + INNER JOIN provisioner_jobs pj ON b.job_id = pj.id + INNER JOIN templates t ON p.template_id = t.id + LEFT JOIN template_version_presets tvp_curr + ON tvp_curr.id = p.current_preset_id -- See https://github.com/coder/internal/issues/398. WHERE (b.transition = 'start'::workspace_transition - -- if a deletion job fails, the workspace will still be running - OR pj.job_status IN ('failed'::provisioner_job_status, 'canceled'::provisioner_job_status, - 'unknown'::provisioner_job_status)) - AND (tvp_curr.name = tvp_desired.name - OR tvp_desired.id IS NULL); + -- Jobs that are not in terminal states. + OR pj.job_status IN ('failed'::provisioner_job_status, 'canceled'::provisioner_job_status, + 'unknown'::provisioner_job_status)); -- name: GetTemplatePresetsWithPrebuilds :many SELECT t.id AS template_id, - tv.id AS template_version_id, - tv.id = t.active_version_id AS using_active_version, - tvpp.preset_id, - tvp.name, - tvpp.desired_instances AS desired_instances, - t.deleted, - t.deprecated != '' AS deprecated + tv.id AS template_version_id, + tv.id = t.active_version_id AS using_active_version, + tvpp.preset_id, + tvp.name, + tvpp.desired_instances AS desired_instances, + t.deleted, + t.deprecated != '' AS deprecated FROM templates t - INNER JOIN template_versions tv ON tv.template_id = t.id - INNER JOIN template_version_presets tvp ON tvp.template_version_id = tv.id - INNER JOIN template_version_preset_prebuilds tvpp ON tvpp.preset_id = tvp.id + INNER JOIN template_versions tv ON tv.template_id = t.id + INNER JOIN template_version_presets tvp ON tvp.template_version_id = tv.id + INNER JOIN template_version_preset_prebuilds tvpp ON tvpp.preset_id = tvp.id WHERE (t.id = sqlc.narg('template_id')::uuid OR sqlc.narg('template_id') IS NULL); -- name: GetPrebuildsInProgress :many -SELECT wpb.template_version_id, wpb.transition, COUNT(wpb.transition) AS count +SELECT t.id AS template_id, wpb.template_version_id, wpb.transition, COUNT(wpb.transition) AS count FROM workspace_latest_build wlb - INNER JOIN provisioner_jobs pj ON wlb.job_id = pj.id - INNER JOIN workspace_prebuild_builds wpb ON wpb.id = wlb.id -WHERE pj.job_status NOT IN - ('succeeded'::provisioner_job_status, 'canceled'::provisioner_job_status, - 'failed'::provisioner_job_status) -GROUP BY wpb.template_version_id, wpb.transition; + INNER JOIN provisioner_jobs pj ON wlb.job_id = pj.id + INNER JOIN workspace_prebuild_builds wpb ON wpb.id = wlb.id + INNER JOIN templates t ON t.active_version_id = wlb.template_version_id +WHERE pj.job_status NOT IN -- Jobs that are not in terminal states. + ('succeeded'::provisioner_job_status, 'canceled'::provisioner_job_status, + 'failed'::provisioner_job_status) +GROUP BY t.id, wpb.template_version_id, wpb.transition; -- name: ClaimPrebuild :one -- TODO: rewrite to use named CTE instead? UPDATE workspaces w SET owner_id = @new_user_id::uuid, - name = @new_name::text, - updated_at = NOW() + name = @new_name::text, + updated_at = NOW() WHERE w.id IN (SELECT p.id - FROM workspace_prebuilds p - INNER JOIN workspace_latest_build b ON b.workspace_id = p.id - INNER JOIN provisioner_jobs pj ON b.job_id = pj.id - INNER JOIN templates t ON p.template_id = t.id - WHERE (b.transition = 'start'::workspace_transition - AND pj.job_status IN ('succeeded'::provisioner_job_status)) - AND b.template_version_id = t.active_version_id - AND b.template_version_preset_id = @preset_id::uuid - AND p.lifecycle_state = 'ready'::workspace_agent_lifecycle_state - ORDER BY random() - LIMIT 1 FOR UPDATE OF p SKIP LOCKED) + FROM workspace_prebuilds p + INNER JOIN workspace_latest_build b ON b.workspace_id = p.id + INNER JOIN provisioner_jobs pj ON b.job_id = pj.id + INNER JOIN templates t ON p.template_id = t.id + WHERE (b.transition = 'start'::workspace_transition + AND pj.job_status IN ('succeeded'::provisioner_job_status)) + AND b.template_version_id = t.active_version_id + AND b.template_version_preset_id = @preset_id::uuid + AND p.lifecycle_state = 'ready'::workspace_agent_lifecycle_state + ORDER BY random() + LIMIT 1 FOR UPDATE OF p SKIP LOCKED) RETURNING w.id, w.name; -- name: InsertPresetPrebuild :one diff --git a/enterprise/coderd/prebuilds/controller.go b/enterprise/coderd/prebuilds/controller.go index 620227b2ba..de4e02508c 100644 --- a/enterprise/coderd/prebuilds/controller.go +++ b/enterprise/coderd/prebuilds/controller.go @@ -210,7 +210,7 @@ func (c *Controller) determineState(ctx context.Context, store database.Store, i err := store.InTx(func(db database.Store) error { start := time.Now() - // TODO: give up after some time waiting on this? + // TODO: per-template ID lock? err := db.AcquireLock(ctx, database.LockIDDeterminePrebuildsState) if err != nil { return xerrors.Errorf("failed to acquire state determination lock: %w", err)