Skip to content

Commit dc3519e

Browse files
ammarioammario
authored
Support all transitions in build progress bar (#4575)
* Use null types instead of -1 for simplicity * Fix pgcrypto bug in migration 59 * Add stories * Fix visual stutter
1 parent ee2c29d commit dc3519e

File tree

19 files changed

+309
-124
lines changed

19 files changed

+309
-124
lines changed

‎coderd/database/databasefake/databasefake.go

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -235,33 +235,50 @@ func (q *fakeQuerier) GetTemplateDAUs(_ context.Context, templateID uuid.UUID) (
235235
return rs, nil
236236
}
237237

238-
func (q *fakeQuerier) GetTemplateAverageBuildTime(ctx context.Context, arg database.GetTemplateAverageBuildTimeParams) (float64, error) {
239-
var times []float64
238+
func (q *fakeQuerier) GetTemplateAverageBuildTime(ctx context.Context, arg database.GetTemplateAverageBuildTimeParams) (database.GetTemplateAverageBuildTimeRow, error) {
239+
var emptyRow database.GetTemplateAverageBuildTimeRow
240+
var (
241+
startTimes []float64
242+
stopTimes []float64
243+
deleteTimes []float64
244+
)
240245
for _, wb := range q.workspaceBuilds {
241-
if wb.Transition != database.WorkspaceTransitionStart {
242-
continue
243-
}
244246
version, err := q.GetTemplateVersionByID(ctx, wb.TemplateVersionID)
245247
if err != nil {
246-
return -1, err
248+
return emptyRow, err
247249
}
248250
if version.TemplateID != arg.TemplateID {
249251
continue
250252
}
251253

252254
job, err := q.GetProvisionerJobByID(ctx, wb.JobID)
253255
if err != nil {
254-
return -1, err
256+
return emptyRow, err
255257
}
256258
if job.CompletedAt.Valid {
257-
times = append(times, job.CompletedAt.Time.Sub(job.StartedAt.Time).Seconds())
259+
took := job.CompletedAt.Time.Sub(job.StartedAt.Time).Seconds()
260+
if wb.Transition == database.WorkspaceTransitionStart {
261+
startTimes = append(startTimes, took)
262+
} else if wb.Transition == database.WorkspaceTransitionStop {
263+
stopTimes = append(stopTimes, took)
264+
} else if wb.Transition == database.WorkspaceTransitionDelete {
265+
deleteTimes = append(deleteTimes, took)
266+
}
258267
}
259268
}
260-
sort.Float64s(times)
261-
if len(times) == 0 {
262-
return -1, nil
269+
270+
tryMedian := func(fs []float64) float64 {
271+
if len(fs) == 0 {
272+
return -1
273+
}
274+
sort.Float64s(fs)
275+
return fs[len(fs)/2]
263276
}
264-
return times[len(times)/2], nil
277+
var row database.GetTemplateAverageBuildTimeRow
278+
row.DeleteMedian = tryMedian(deleteTimes)
279+
row.StopMedian = tryMedian(stopTimes)
280+
row.StartMedian = tryMedian(startTimes)
281+
return row, nil
265282
}
266283

267284
func (q *fakeQuerier) ParameterValue(_ context.Context, id uuid.UUID) (database.ParameterValue, error) {

‎coderd/database/dump.sql

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

‎coderd/database/migrations/000059_file_id.up.sql

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44
-- template to be able to push and read files used for template
55
-- versions they create.
66
-- Prior to this collisions on file.hash were not an issue
7-
-- since users who could push files could also read all files.
8-
--
7+
-- since users who could push files could also read all files.
8+
--
99
-- This migration also adds a 'files.id' column as the primary
1010
-- key. As a side effect the provisioner_jobs must now reference
1111
-- the files.id column since the 'hash' column is now ambiguous.
@@ -14,10 +14,13 @@ BEGIN;
1414
-- Drop the primary key on hash.
1515
ALTER TABLE files DROP CONSTRAINT files_pkey;
1616

17+
-- This extension is required by gen_random_uuid
18+
CREATE EXTENSION IF NOT EXISTS pgcrypto;
19+
1720
-- Add an 'id' column and designate it the primary key.
18-
ALTER TABLE files ADD COLUMN
21+
ALTER TABLE files ADD COLUMN
1922
id uuid NOT NULL PRIMARY KEY DEFAULT gen_random_uuid ();
20-
23+
2124
-- Update the constraint to include the user who created it.
2225
ALTER TABLE files ADD UNIQUE(hash, created_by);
2326

‎coderd/database/querier.go

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

‎coderd/database/queries.sql.go

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

‎coderd/database/queries/templates.sql

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,8 @@ RETURNING
109109
-- name: GetTemplateAverageBuildTime :one
110110
WITH build_times AS (
111111
SELECT
112-
EXTRACT(EPOCH FROM (pj.completed_at - pj.started_at))::FLOAT AS exec_time_sec
112+
EXTRACT(EPOCH FROM (pj.completed_at - pj.started_at))::FLOAT AS exec_time_sec,
113+
workspace_builds.transition
113114
FROM
114115
workspace_builds
115116
JOIN template_versions ON
@@ -118,14 +119,18 @@ JOIN provisioner_jobs pj ON
118119
workspace_builds.job_id = pj.id
119120
WHERE
120121
template_versions.template_id = @template_id AND
121-
(workspace_builds.transition = 'start') AND
122122
(pj.completed_at IS NOT NULL) AND (pj.started_at IS NOT NULL) AND
123123
(pj.started_at > @start_time) AND
124124
(pj.canceled_at IS NULL) AND
125125
((pj.error IS NULL) OR (pj.error = ''))
126126
ORDER BY
127127
workspace_builds.created_at DESC
128128
)
129-
SELECT coalesce((PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec)), -1)::FLOAT
129+
SELECT
130+
-- Postgres offers no clear way to DRY this short of a function or other
131+
-- complexities.
132+
coalesce((PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'start')), -1)::FLOAT AS start_median,
133+
coalesce((PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'stop')), -1)::FLOAT AS stop_median,
134+
coalesce((PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'delete')), -1)::FLOAT AS delete_median
130135
FROM build_times
131136
;

‎coderd/metricscache/metricscache.go

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ type Cache struct {
2929

3030
templateDAUResponses atomic.Pointer[map[uuid.UUID]codersdk.TemplateDAUsResponse]
3131
templateUniqueUsers atomic.Pointer[map[uuid.UUID]int]
32-
templateAverageBuildTime atomic.Pointer[map[uuid.UUID]time.Duration]
32+
templateAverageBuildTime atomic.Pointer[map[uuid.UUID]database.GetTemplateAverageBuildTimeRow]
3333

3434
done chan struct{}
3535
cancel func()
@@ -130,9 +130,9 @@ func (c *Cache) refresh(ctx context.Context) error {
130130
}
131131

132132
var (
133-
templateDAUs = make(map[uuid.UUID]codersdk.TemplateDAUsResponse, len(templates))
134-
templateUniqueUsers = make(map[uuid.UUID]int)
135-
templateAverageBuildTimeSec = make(map[uuid.UUID]time.Duration)
133+
templateDAUs = make(map[uuid.UUID]codersdk.TemplateDAUsResponse, len(templates))
134+
templateUniqueUsers = make(map[uuid.UUID]int)
135+
templateAverageBuildTimes = make(map[uuid.UUID]database.GetTemplateAverageBuildTimeRow)
136136
)
137137
for _, template := range templates {
138138
rows, err := c.database.GetTemplateDAUs(ctx, template.ID)
@@ -141,6 +141,7 @@ func (c *Cache) refresh(ctx context.Context) error {
141141
}
142142
templateDAUs[template.ID] = convertDAUResponse(rows)
143143
templateUniqueUsers[template.ID] = countUniqueUsers(rows)
144+
144145
templateAvgBuildTime, err := c.database.GetTemplateAverageBuildTime(ctx, database.GetTemplateAverageBuildTimeParams{
145146
TemplateID: uuid.NullUUID{
146147
UUID: template.ID,
@@ -151,14 +152,15 @@ func (c *Cache) refresh(ctx context.Context) error {
151152
Valid: true,
152153
},
153154
})
155+
154156
if err != nil {
155157
return err
156158
}
157-
templateAverageBuildTimeSec[template.ID] = time.Duration(float64(time.Second) * templateAvgBuildTime)
159+
templateAverageBuildTimes[template.ID] = templateAvgBuildTime
158160
}
159161
c.templateDAUResponses.Store(&templateDAUs)
160162
c.templateUniqueUsers.Store(&templateUniqueUsers)
161-
c.templateAverageBuildTime.Store(&templateAverageBuildTimeSec)
163+
c.templateAverageBuildTime.Store(&templateAverageBuildTimes)
162164

163165
return nil
164166
}
@@ -239,17 +241,32 @@ func (c *Cache) TemplateUniqueUsers(id uuid.UUID) (int, bool) {
239241
return resp, true
240242
}
241243

242-
func (c *Cache) TemplateAverageBuildTime(id uuid.UUID) (time.Duration, bool) {
244+
func (c *Cache) TemplateBuildTimeStats(id uuid.UUID) codersdk.TemplateBuildTimeStats {
245+
var unknown codersdk.TemplateBuildTimeStats
246+
243247
m := c.templateAverageBuildTime.Load()
244248
if m == nil {
245249
// Data loading.
246-
return -1, false
250+
return unknown
247251
}
248252

249253
resp, ok := (*m)[id]
250-
if !ok || resp <= 0 {
254+
if !ok {
251255
// No data or not enough builds.
252-
return -1, false
256+
return unknown
257+
}
258+
259+
convertMedian := func(m float64) *int64 {
260+
if m <= 0 {
261+
return nil
262+
}
263+
i := int64(m * 1000)
264+
return &i
265+
}
266+
267+
return codersdk.TemplateBuildTimeStats{
268+
StartMillis: convertMedian(resp.StartMedian),
269+
StopMillis: convertMedian(resp.StopMedian),
270+
DeleteMillis: convertMedian(resp.DeleteMedian),
253271
}
254-
return resp, true
255272
}

0 commit comments

Comments
 (0)