8000 refactor: add comments + improve tests · coder/coder@fc32154 · GitHub
[go: up one dir, main page]
Skip to content

Commit fc32154

Browse files
evgeniy-scherbinaevgeniy-scherbina
committed
refactor: add comments + improve tests
1 parent 3cc74fb commit fc32154

File tree

4 files changed

+108
-27
lines changed

4 files changed

+108
-27
lines changed

‎coderd/database/querier.go

Lines changed: 11 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/querier_test.go

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3935,6 +3935,57 @@ func TestGetPresetsBackoff(t *testing.T) {
39353935
require.Equal(t, int32(1), backoff.NumFailed)
39363936
}
39373937
})
3938+
3939+
t.Run("3 job failed out of 5 - backoff", func(t *testing.T) {
3940+
t.Parallel()
3941+
3942+
db, _ := dbtestutil.NewDB(t)
3943+
ctx := testutil.Context(t, testutil.WaitShort)
3944+
dbgen.Organization(t, db, database.Organization{
3945+
ID: orgID,
3946+
})
3947+
dbgen.User(t, db, database.User{
3948+
ID: userID,
3949+
})
3950+
lookbackPeriod := time.Hour
3951+
3952+
tmpl1 := createTemplate(db)
3953+
tmpl1V1 := createTmplVersion(db, tmpl1, tmpl1.ActiveVersionID, &tmplVersionOpts{
3954+
DesiredInstances: 3,
3955+
})
3956+
createWorkspaceBuild(db, tmpl1, tmpl1V1, &workspaceBuildOpts{
3957+
successfulJob: false,
3958+
createdAt: now.Add(-lookbackPeriod - time.Minute), // earlier than lookback period - skipped
3959+
})
3960+
createWorkspaceBuild(db, tmpl1, tmpl1V1, &workspaceBuildOpts{
3961+
successfulJob: false,
3962+
createdAt: now.Add(-4 * time.Minute), // within lookback period - counted as failed job
3963+
})
3964+
createWorkspaceBuild(db, tmpl1, tmpl1V1, &workspaceBuildOpts{
3965+
successfulJob: false,
3966+
createdAt: now.Add(-3 * time.Minute), // within lookback period - counted as failed job
3967+
})
3968+
createWorkspaceBuild(db, tmpl1, tmpl1V1, &workspaceBuildOpts{
3969+
successfulJob: true,
3970+
createdAt: now.Add(-2 * time.Minute),
3971+
})
3972+
createWorkspaceBuild(db, tmpl1, tmpl1V1, &workspaceBuildOpts{
3973+
successfulJob: true,
3974+
createdAt: now.Add(-1 * time.Minute),
3975+
})
3976+
3977+
backoffs, err := db.GetPresetsBackoff(ctx, now.Add(-lookbackPeriod))
3978+
require.NoError(t, err)
3979+
3980+
require.Len(t, backoffs, 1)
3981+
{
3982+
backoff := backoffs[0]
3983+
require.Equal(t, backoff.TemplateVersionID, tmpl1.ActiveVersionID)
3984+
require.Equal(t, backoff.PresetID, tmpl1V1.preset.ID)
3985+
require.Equal(t, database.ProvisionerJobStatusFailed, backoff.LatestBuildStatus)
3986+
require.Equal(t, int32(2), backoff.NumFailed)
3987+
}
3988+
})
39383989
}
39393990

39403991
func requireUsersMatch(t testing.TB, expected []database.User, found []database.GetUsersRow, msg string) {

‎coderd/database/queries.sql.go

Lines changed: 23 additions & 13 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/prebuilds.sql

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,17 @@ GROUP BY t.id, wpb.template_version_id, wpb.transition;
5151
-- For each group, the query checks the last N jobs, where N equals the number of desired instances for the corresponding preset.
5252
-- If at least one of the last N jobs has failed, we should backoff on the corresponding template version ID.
5353
-- Query returns a list of template version IDs for which we should backoff.
54-
-- Only template versions with configured presets are considered.
54+
-- Only active template versions with configured presets are considered.
55+
--
56+
-- NOTE:
57+
-- We back off on the template version ID if at least one of the N latest workspace builds has failed.
58+
-- However, we also return the number of failed workspace builds that occurred during the lookback period.
59+
--
60+
-- In other words:
61+
-- - To **decide whether to back off**, we look at the N most recent builds (regardless of when they happened).
62+
-- - To **calculate the number of failed builds**, we consider all builds within the defined lookback period.
63+
--
64+
-- The number of failed builds is used downstream to determine the backoff duration.
5565
WITH filtered_builds AS (
5666
-- Only select builds which are for prebuild creations
5767
SELECT wlb.*, tvp.id AS preset_id, pj.job_status, tvpp.desired_instances
@@ -62,8 +72,8 @@ WITH filtered_builds AS (
6272
JOIN templates t ON tv.template_id = t.id AND t.active_version_id = tv.id
6373
JOIN template_version_preset_prebuilds tvpp ON tvpp.preset_id = tvp.id
6474
WHERE wlb.transition = 'start'::workspace_transition),
65-
latest_builds AS (
66-
-- Select only the latest build per template_version AND preset
75+
time_sorted_builds AS (
76+
-- Group builds by template version, then sort each group by created_at.
6777
SELECT fb.*,
6878
ROW_NUMBER() OVER (PARTITION BY fb.template_version_preset_id ORDER BY fb.created_at DESC) as rn
6979
FROM filtered_builds fb),
@@ -74,16 +84,16 @@ WITH filtered_builds AS (
7484
WHERE job_status = 'failed'::provisioner_job_status
7585
AND created_at >= @lookback::timestamptz
7686
GROUP BY preset_id)
77-
SELECT lb.template_version_id,
78-
lb.preset_id,
79-
MAX(lb.job_status)::provisioner_job_status AS latest_build_status,
80-
MAX(COALESCE(fc.num_failed, 0))::int AS num_failed,
81-
MAX(lb.created_at)::timestamptz AS last_build_at
82-
FROM latest_builds lb
83-
LEFT JOIN failed_count fc ON fc.preset_id = lb.preset_id
84-
WHERE lb.rn <= lb.desired_instances -- Fetch the last N builds, where N is the number of desired instances; if any fail, we backoff
85-
AND lb.job_status = 'failed'::provisioner_job_status
86-
GROUP BY lb.template_version_id, lb.preset_id, lb.job_status;
87+
SELECT tsb.template_version_id,
88+
tsb.preset_id,
89+
tsb.job_status::provisioner_job_status AS latest_build_status,
90+
COALESCE(fc.num_failed, 0)::int AS num_failed,
91+
tsb.created_at::timestamptz AS last_build_at
92+
FROM time_sorted_builds tsb
93+
LEFT JOIN failed_count fc ON fc.preset_id = tsb.preset_id
94+
WHERE tsb.rn <= tsb.desired_instances -- Fetch the last N builds, where N is the number of desired instances; if any fail, we backoff
95+
AND tsb.job_status = 'failed'::provisioner_job_status
96+
GROUP BY tsb.template_version_id, tsb.preset_id, tsb.job_status, tsb.created_at, fc.num_failed;
8797

8898
-- name: ClaimPrebuild :one
8999
UPDATE workspaces w

0 commit comments

Comments
 (0)
0