Skip to content

Commit ac521f0

Browse files
ammariopull[bot]
ammario
authored and
pull[bot]
committed
site: support high build time variation in progress bar (#4941)
1 parent b6b39ff commit ac521f0

File tree

15 files changed

+187
-120
lines changed

15 files changed

+187
-120
lines changed

‎cli/loadtest_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
)
2727

2828
func TestLoadTest(t *testing.T) {
29+
t.Skipf("This test is flakey. See https://github.com/coder/coder/issues/4942")
2930
t.Parallel()
3031

3132
t.Run("PlaceboFromStdin", func(t *testing.T) {

‎coderd/database/databasefake/databasefake.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -300,17 +300,18 @@ func (q *fakeQuerier) GetTemplateAverageBuildTime(ctx context.Context, arg datab
300300
}
301301
}
302302

303-
tryMedian := func(fs []float64) float64 {
303+
tryPercentile := func(fs []float64, p float64) float64 {
304304
if len(fs) == 0 {
305305
return -1
306306
}
307307
sort.Float64s(fs)
308-
return fs[len(fs)/2]
308+
return fs[int(float64(len(fs))*p/100)]
309309
}
310+
310311
var row database.GetTemplateAverageBuildTimeRow
311-
row.DeleteMedian = tryMedian(deleteTimes)
312-
row.StopMedian = tryMedian(stopTimes)
313-
row.StartMedian = tryMedian(startTimes)
312+
row.Delete50, row.Delete95 = tryPercentile(deleteTimes, 50), tryPercentile(deleteTimes, 95)
313+
row.Stop50, row.Stop95 = tryPercentile(stopTimes, 50), tryPercentile(stopTimes, 95)
314+
row.Start50, row.Start95 = tryPercentile(startTimes, 50), tryPercentile(startTimes, 95)
314315
return row, nil
315316
}
316317

‎coderd/database/queries.sql.go

Lines changed: 20 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: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -142,8 +142,11 @@ ORDER BY
142142
SELECT
143143
-- Postgres offers no clear way to DRY this short of a function or other
144144
-- complexities.
145-
coalesce((PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'start')), -1)::FLOAT AS start_median,
146-
coalesce((PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'stop')), -1)::FLOAT AS stop_median,
147-
coalesce((PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'delete')), -1)::FLOAT AS delete_median
145+
coalesce((PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'start')), -1)::FLOAT AS start_50,
146+
coalesce((PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'stop')), -1)::FLOAT AS stop_50,
147+
coalesce((PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'delete')), -1)::FLOAT AS delete_50,
148+
coalesce((PERCENTILE_DISC(0.95) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'start')), -1)::FLOAT AS start_95,
149+
coalesce((PERCENTILE_DISC(0.95) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'stop')), -1)::FLOAT AS stop_95,
150+
coalesce((PERCENTILE_DISC(0.95) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'delete')), -1)::FLOAT AS delete_95
148151
FROM build_times
149152
;

‎coderd/metricscache/metricscache.go

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -242,7 +242,11 @@ func (c *Cache) TemplateUniqueUsers(id uuid.UUID) (int, bool) {
242242
}
243243

244244
func (c *Cache) TemplateBuildTimeStats(id uuid.UUID) codersdk.TemplateBuildTimeStats {
245-
var unknown codersdk.TemplateBuildTimeStats
245+
unknown := codersdk.TemplateBuildTimeStats{
246+
codersdk.WorkspaceTransitionStart: {},
247+
codersdk.WorkspaceTransitionStop: {},
248+
codersdk.WorkspaceTransitionDelete: {},
249+
}
246250

247251
m := c.templateAverageBuildTime.Load()
248252
if m == nil {
@@ -256,7 +260,7 @@ func (c *Cache) TemplateBuildTimeStats(id uuid.UUID) codersdk.TemplateBuildTimeS
256260
return unknown
257261
}
258262

259-
convertMedian := func(m float64) *int64 {
263+
convertMillis := func(m float64) *int64 {
260264
if m <= 0 {
261265
return nil
262266
}
@@ -265,8 +269,17 @@ func (c *Cache) TemplateBuildTimeStats(id uuid.UUID) codersdk.TemplateBuildTimeS
265269
}
266270

267271
return codersdk.TemplateBuildTimeStats{
268-
StartMillis: convertMedian(resp.StartMedian),
269-
StopMillis: convertMedian(resp.StopMedian),
270-
DeleteMillis: convertMedian(resp.DeleteMedian),
272+
codersdk.WorkspaceTransitionStart: {
273+
P50: convertMillis(resp.Start50),
274+
P95: convertMillis(resp.Start95),
275+
},
276+
codersdk.WorkspaceTransitionStop: {
277+
P50: convertMillis(resp.Stop50),
278+
P95: convertMillis(resp.Stop95),
279+
},
280+
codersdk.WorkspaceTransitionDelete: {
281+
P50: convertMillis(resp.Delete50),
282+
P95: convertMillis(resp.Delete95),
283+
},
271284
}
272285
}

‎coderd/metricscache/metricscache_test.go

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"time"
88

99
"github.com/google/uuid"
10-
"github.com/stretchr/testify/assert"
1110
"github.com/stretchr/testify/require"
1211

1312
"cdr.dev/slog/sloggers/slogtest"
@@ -204,6 +203,12 @@ func clockTime(t time.Time, hour, minute, sec int) time.Time {
204203
return time.Date(t.Year(), t.Month(), t.Day(), hour, minute, sec, t.Nanosecond(), t.Location())
205204
}
206205

206+
func requireBuildTimeStatsEmpty(t *testing.T, stats codersdk.TemplateBuildTimeStats) {
207+
require.Empty(t, stats[codersdk.WorkspaceTransitionStart])
208+
require.Empty(t, stats[codersdk.WorkspaceTransitionStop])
209+
require.Empty(t, stats[codersdk.WorkspaceTransitionDelete])
210+
}
211+
207212
func TestCache_BuildTime(t *testing.T) {
208213
t.Parallel()
209214

@@ -296,7 +301,7 @@ func TestCache_BuildTime(t *testing.T) {
296301
require.NoError(t, err)
297302

298303
gotStats := cache.TemplateBuildTimeStats(template.ID)
299-
require.Empty(t, gotStats, "should not have loaded yet")
304+
requireBuildTimeStatsEmpty(t, gotStats)
300305

301306
for _, row := range tt.args.rows {
302307
_, err := db.InsertProvisionerJob(ctx, database.InsertProvisionerJobParams{
@@ -328,36 +333,32 @@ func TestCache_BuildTime(t *testing.T) {
328333
}
329334

330335
if tt.want.loads {
336+
wantTransition := codersdk.WorkspaceTransition(tt.args.transition)
331337
require.Eventuallyf(t, func() bool {
332338
stats := cache.TemplateBuildTimeStats(template.ID)
333-
return stats != codersdk.TemplateBuildTimeStats{}
339+
return stats[wantTransition] != codersdk.TransitionStats{}
334340
}, testutil.WaitLong, testutil.IntervalMedium,
335341
"BuildTime never populated",
336342
)
337343

338344
gotStats = cache.TemplateBuildTimeStats(template.ID)
339-
340-
if tt.args.transition == database.WorkspaceTransitionDelete {
341-
require.Nil(t, gotStats.StopMillis)
342-
require.Nil(t, gotStats.StartMillis)
343-
require.Equal(t, tt.want.buildTimeMs, *gotStats.DeleteMillis)
344-
}
345-
if tt.args.transition == database.WorkspaceTransitionStart {
346-
require.Nil(t, gotStats.StopMillis)
347-
require.Nil(t, gotStats.DeleteMillis)
348-
require.Equal(t, tt.want.buildTimeMs, *gotStats.StartMillis)
349-
}
350-
if tt.args.transition == database.WorkspaceTransitionStop {
351-
require.Nil(t, gotStats.StartMillis)
352-
require.Nil(t, gotStats.DeleteMillis)
353-
require.Equal(t, tt.want.buildTimeMs, *gotStats.StopMillis)
345+
for transition, stats := range gotStats {
346+
if transition == wantTransition {
347+
require.Equal(t, tt.want.buildTimeMs, *stats.P50)
348+
} else {
349+
require.Empty(
350+
t, stats, "%v", transition,
351+
)
352+
}
354353
}
355354
} else {
355+
var stats codersdk.TemplateBuildTimeStats
356356
require.Never(t, func() bool {
357-
stats := cache.TemplateBuildTimeStats(template.ID)
358-
return !assert.Empty(t, stats)
357+
stats = cache.TemplateBuildTimeStats(template.ID)
358+
requireBuildTimeStatsEmpty(t, stats)
359+
return t.Failed()
359360
}, testutil.WaitShort/2, testutil.IntervalMedium,
360-
"BuildTimeStats populated",
361+
"BuildTimeStats populated", stats,
361362
)
362363
}
363364
})

‎coderd/templates_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -536,7 +536,7 @@ func TestTemplateMetrics(t *testing.T) {
536536
})
537537
template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID)
538538
require.Equal(t, -1, template.ActiveUserCount)
539-
require.Empty(t, template.BuildTimeStats)
539+
require.Empty(t, template.BuildTimeStats[codersdk.WorkspaceTransitionStart])
540540

541541
coderdtest.AwaitTemplateVersionJob(t, client, version.ID)
542542
workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID)
@@ -607,7 +607,7 @@ func TestTemplateMetrics(t *testing.T) {
607607
require.Eventuallyf(t, func() bool {
608608
template, err = client.Template(ctx, template.ID)
609609
require.NoError(t, err)
610-
startMs := template.BuildTimeStats.StartMillis
610+
startMs := template.BuildTimeStats[codersdk.WorkspaceTransitionStart].P50
611611
return startMs != nil && *startMs > 1
612612
},
613613
testutil.WaitShort, testutil.IntervalFast,

‎codersdk/templates.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,12 @@ type Template struct {
3333
CreatedByName string `json:"created_by_name"`
3434
}
3535

36-
type TemplateBuildTimeStats struct {
37-
StartMillis *int64 `json:"start_ms"`
38-
StopMillis *int64 `json:"stop_ms"`
39-
DeleteMillis *int64 `json:"delete_ms"`
36+
type TransitionStats struct {
37+
P50 *int64
38+
P95 *int64
4039
}
4140

41+
type TemplateBuildTimeStats map[WorkspaceTransition]TransitionStats
4242
type UpdateActiveTemplateVersion struct {
4343
ID uuid.UUID `json:"id" validate:"required"`
4444
}

‎site/src/api/typesGenerated.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -651,11 +651,10 @@ export interface TemplateACL {
651651
}
652652

653653
// From codersdk/templates.go
654-
export interface TemplateBuildTimeStats {
655-
readonly start_ms?: number
656-
readonly stop_ms?: number
657-
readonly delete_ms?: number
658-
}
654+
export type TemplateBuildTimeStats = Record<
655+
WorkspaceTransition,
656+
TransitionStats
657+
>
659658

660659
// From codersdk/templates.go
661660
export interface TemplateDAUsResponse {
@@ -697,6 +696,12 @@ export interface TraceConfig {
697696
readonly capture_logs: DeploymentConfigField<boolean>
698697
}
699698

699+
// From codersdk/templates.go
700+
export interface TransitionStats {
701+
readonly P50?: number
702+
readonly P95?: number
703+
}
704+
700705
// From codersdk/templates.go
701706
export interface UpdateActiveTemplateVersion {
702707
readonly id: string

‎site/src/components/TemplateStats/TemplateStats.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ export const TemplateStats: FC<TemplateStatsProps> = ({
4242
/>
4343
<StatsItem
4444
label={Language.buildTimeLabel}
45-
value={formatTemplateBuildTime(template.build_time_stats.start_ms)}
45+
value={formatTemplateBuildTime(template.build_time_stats.start.P50)}
4646
/>
4747
<StatsItem
4848
label={Language.activeVersionLabel}

‎site/src/components/Workspace/Workspace.tsx

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import { WorkspaceStats } from "../WorkspaceStats/WorkspaceStats"
1919
import { AlertBanner } from "../AlertBanner/AlertBanner"
2020
import { useTranslation } from "react-i18next"
2121
import {
22-
EstimateTransitionTime,
22+
ActiveTransition,
2323
WorkspaceBuildProgress,
2424
} from "components/WorkspaceBuildProgress/WorkspaceBuildProgress"
2525
import { AgentRow } from "components/Resources/AgentRow"
@@ -115,13 +115,9 @@ export const Workspace: FC<React.PropsWithChildren<WorkspaceProps>> = ({
115115
/>
116116
)
117117

118-
let buildTimeEstimate: number | undefined = undefined
119-
let isTransitioning: boolean | undefined = undefined
118+
let transitionStats: TypesGen.TransitionStats | undefined = undefined
120119
if (template !== undefined) {
121-
;[buildTimeEstimate, isTransitioning] = EstimateTransitionTime(
122-
template,
123-
workspace,
124-
)
120+
transitionStats = ActiveTransition(template, workspace)
125121
}
126122

127123
return (
@@ -195,10 +191,10 @@ export const Workspace: FC<React.PropsWithChildren<WorkspaceProps>> = ({
195191
handleUpdate={handleUpdate}
196192
/>
197193

198-
{isTransitioning !== undefined && isTransitioning && (
194+
{transitionStats !== undefined && (
199195
<WorkspaceBuildProgress
200196
workspace={workspace}
201-
buildEstimate={buildTimeEstimate}
197+
transitionStats={transitionStats}
202198
/>
203199
)}
204200

‎site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.stories.tsx

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@ const Template: Story<WorkspaceBuildProgressProps> = (args) => (
2121

2222
export const Starting = Template.bind({})
2323
Starting.args = {
24-
buildEstimate: 10000,
24+
transitionStats: {
25+
P50: 10000,
26+
P95: 10010,
27+
},
2528
workspace: {
2629
...MockStartingWorkspace,
2730
latest_build: {
@@ -39,11 +42,17 @@ Starting.args = {
3942
export const StartingUnknown = Template.bind({})
4043
StartingUnknown.args = {
4144
...Starting.args,
42-
buildEstimate: undefined,
45+
transitionStats: undefined,
4346
}
4447

4548
export const StartingPassedEstimate = Template.bind({})
4649
StartingPassedEstimate.args = {
4750
...Starting.args,
48-
buildEstimate: 1000,
51+
transitionStats: { P50: 1000, P95: 1000 },
52+
}
53+
54+
export const StartingHighVariaton = Template.bind({})
55+
StartingHighVariaton.args = {
56+
...Starting.args,
57+
transitionStats: { P50: 10000, P95: 20000 },
4958
}

0 commit comments

Comments
 (0)