Skip to content

Commit fad3f23

Browse files
dannykoppingdannykopping
committed
Correct queries; running prebuilds only needs to return current preset ID
Signed-off-by: Danny Kopping <danny@coder.com>
1 parent e9b56d9 commit fad3f23

File tree

3 files changed

+106
-110
lines changed

3 files changed

+106
-110
lines changed

‎coderd/database/queries.sql.go

+56-56
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/prebuilds.sql

+49-53
Original file line numberDiff line numberDiff line change
@@ -1,74 +1,70 @@
11
-- name: GetRunningPrebuilds :many
22
SELECT p.id AS workspace_id,
3-
p.name AS workspace_name,
4-
p.template_id,
5-
b.template_version_id,
6-
tvp_curr.id AS current_preset_id,
7-
tvp_desired.id AS desired_preset_id,
8-
-- TODO: just because a prebuild is in a ready state doesn't mean it's eligible; if the prebuild is due to be
9-
-- deleted to reconcile state then it MUST NOT be eligible for claiming. We'll need some kind of lock here.
10-
CASE
11-
WHEN p.lifecycle_state = 'ready'::workspace_agent_lifecycle_state THEN TRUE
12-
ELSE FALSE END AS ready,
13-
p.created_at
3+
p.name AS workspace_name,
4+
p.template_id,
5+
b.template_version_id,
6+
tvp_curr.id AS current_preset_id,
7+
-- TODO: just because a prebuild is in a ready state doesn't mean it's eligible; if the prebuild is due to be
8+
-- deleted to reconcile state then it MUST NOT be eligible for claiming. We'll need some kind of lock here.
9+
CASE
10+
WHEN p.lifecycle_state = 'ready'::workspace_agent_lifecycle_state THEN TRUE
11+
ELSE FALSE END AS ready,
12+
p.created_at
1413
FROM workspace_prebuilds p
15-
INNER JOIN workspace_latest_build b ON b.workspace_id = p.id
16-
INNER JOIN provisioner_jobs pj ON b.job_id = pj.id
17-
INNER JOIN templates t ON p.template_id = t.id
18-
LEFT JOIN template_version_presets tvp_curr
19-
ON tvp_curr.id = p.current_preset_id -- See https://github.com/coder/internal/issues/398.
20-
LEFT JOIN template_version_presets tvp_desired
21-
ON tvp_desired.template_version_id = t.active_version_id
14+
INNER JOIN workspace_latest_build b ON b.workspace_id = p.id
15+
INNER JOIN provisioner_jobs pj ON b.job_id = pj.id
16+
INNER JOIN templates t ON p.template_id = t.id
17+
LEFT JOIN template_version_presets tvp_curr
18+
ON tvp_curr.id = p.current_preset_id -- See https://github.com/coder/internal/issues/398.
2219
WHERE (b.transition = 'start'::workspace_transition
23-
-- if a deletion job fails, the workspace will still be running
24-
OR pj.job_status IN ('failed'::provisioner_job_status, 'canceled'::provisioner_job_status,
25-
'unknown'::provisioner_job_status))
26-
AND (tvp_curr.name = tvp_desired.name
27-
OR tvp_desired.id IS NULL);
20+
-- Jobs that are not in terminal states.
21+
OR pj.job_status IN ('failed'::provisioner_job_status, 'canceled'::provisioner_job_status,
22+
'unknown'::provisioner_job_status));
2823

2924
-- name: GetTemplatePresetsWithPrebuilds :many
3025
SELECT t.id AS template_id,
31-
tv.id AS template_version_id,
32-
tv.id = t.active_version_id AS using_active_version,
33-
tvpp.preset_id,
34-
tvp.name,
35-
tvpp.desired_instances AS desired_instances,
36-
t.deleted,
37-
t.deprecated != '' AS deprecated
26+
tv.id AS template_version_id,
27+
tv.id = t.active_version_id AS using_active_version,
28+
tvpp.preset_id,
29+
tvp.name,
30+
tvpp.desired_instances AS desired_instances,
31+
t.deleted,
32+
t.deprecated != '' AS deprecated
3833
FROM templates t
39-
INNER JOIN template_versions tv ON tv.template_id = t.id
40-
INNER JOIN template_version_presets tvp ON tvp.template_version_id = tv.id
41-
INNER JOIN template_version_preset_prebuilds tvpp ON tvpp.preset_id = tvp.id
34+
INNER JOIN template_versions tv ON tv.template_id = t.id
35+
INNER JOIN template_version_presets tvp ON tvp.template_version_id = tv.id
36+
INNER JOIN template_version_preset_prebuilds tvpp ON tvpp.preset_id = tvp.id
4237
WHERE (t.id = sqlc.narg('template_id')::uuid OR sqlc.narg('template_id') IS NULL);
4338

4439
-- name: GetPrebuildsInProgress :many
45-
SELECT wpb.template_version_id, wpb.transition, COUNT(wpb.transition) AS count
40+
SELECT t.id AS template_id, wpb.template_version_id, wpb.transition, COUNT(wpb.transition) AS count
4641
FROM workspace_latest_build wlb
47-
INNER JOIN provisioner_jobs pj ON wlb.job_id = pj.id
48-
INNER JOIN workspace_prebuild_builds wpb ON wpb.id = wlb.id
49-
WHERE pj.job_status NOT IN
50-
('succeeded'::provisioner_job_status, 'canceled'::provisioner_job_status,
51-
'failed'::provisioner_job_status)
52-
GROUP BY wpb.template_version_id, wpb.transition;
42+
INNER JOIN provisioner_jobs pj ON wlb.job_id = pj.id
43+
INNER JOIN workspace_prebuild_builds wpb ON wpb.id = wlb.id
44+
INNER JOIN templates t ON t.active_version_id = wlb.template_version_id
45+
WHERE pj.job_status NOT IN -- Jobs that are not in terminal states.
46+
('succeeded'::provisioner_job_status, 'canceled'::provisioner_job_status,
47+
'failed'::provisioner_job_status)
48+
GROUP BY t.id, wpb.template_version_id, wpb.transition;
5349

5450
-- name: ClaimPrebuild :one
5551
-- TODO: rewrite to use named CTE instead?
5652
UPDATE workspaces w
5753
SET owner_id = @new_user_id::uuid,
58-
name = @new_name::text,
59-
updated_at = NOW()
54+
name = @new_name::text,
55+
updated_at = NOW()
6056
WHERE w.id IN (SELECT p.id
61-
FROM workspace_prebuilds p
62-
INNER JOIN workspace_latest_build b ON b.workspace_id = p.id
63-
INNER JOIN provisioner_jobs pj ON b.job_id = pj.id
64-
INNER JOIN templates t ON p.template_id = t.id
65-
WHERE (b.transition = 'start'::workspace_transition
66-
AND pj.job_status IN ('succeeded'::provisioner_job_status))
67-
AND b.template_version_id = t.active_version_id
68-
AND b.template_version_preset_id = @preset_id::uuid
69-
AND p.lifecycle_state = 'ready'::workspace_agent_lifecycle_state
70-
ORDER BY random()
71-
LIMIT 1 FOR UPDATE OF p SKIP LOCKED)
57+
FROM workspace_prebuilds p
58+
INNER JOIN workspace_latest_build b ON b.workspace_id = p.id
59+
INNER JOIN provisioner_jobs pj ON b.job_id = pj.id
60+
INNER JOIN templates t ON p.template_id = t.id
61+
WHERE (b.transition = 'start'::workspace_transition
62+
AND pj.job_status IN ('succeeded'::provisioner_job_status))
63+
AND b.template_version_id = t.active_version_id
64+
AND b.template_version_preset_id = @preset_id::uuid
65+
AND p.lifecycle_state = 'ready'::workspace_agent_lifecycle_state
66+
ORDER BY random()
67+
LIMIT 1 FOR UPDATE OF p SKIP LOCKED)
7268
RETURNING w.id, w.name;
7369

7470
-- name: InsertPresetPrebuild :one

‎enterprise/coderd/prebuilds/controller.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,7 @@ func (c *Controller) determineState(ctx context.Context, store database.Store, i
210210
err := store.InTx(func(db database.Store) error {
211211
start := time.Now()
212212

213-
// TODO: give up after some time waiting on this?
213+
// TODO: per-template ID lock?
214214
err := db.AcquireLock(ctx, database.LockIDDeterminePrebuildsState)
215215
if err != nil {
216216
return xerrors.Errorf("failed to acquire state determination lock: %w", err)

0 commit comments

Comments
 (0)