diff --git a/cli/loadtest_test.go b/cli/loadtest_test.go index 6eb9d58d73..63acd7c0b0 100644 --- a/cli/loadtest_test.go +++ b/cli/loadtest_test.go @@ -26,6 +26,7 @@ import ( ) func TestLoadTest(t *testing.T) { + t.Skipf("This test is flakey. See https://github.com/coder/coder/issues/4942") t.Parallel() t.Run("PlaceboFromStdin", func(t *testing.T) { diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index 51d4016c4f..953e8db9f8 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -300,17 +300,18 @@ func (q *fakeQuerier) GetTemplateAverageBuildTime(ctx context.Context, arg datab } } - tryMedian := func(fs []float64) float64 { + tryPercentile := func(fs []float64, p float64) float64 { if len(fs) == 0 { return -1 } sort.Float64s(fs) - return fs[len(fs)/2] + return fs[int(float64(len(fs))*p/100)] } + var row database.GetTemplateAverageBuildTimeRow - row.DeleteMedian = tryMedian(deleteTimes) - row.StopMedian = tryMedian(stopTimes) - row.StartMedian = tryMedian(startTimes) + row.Delete50, row.Delete95 = tryPercentile(deleteTimes, 50), tryPercentile(deleteTimes, 95) + row.Stop50, row.Stop95 = tryPercentile(stopTimes, 50), tryPercentile(stopTimes, 95) + row.Start50, row.Start95 = tryPercentile(startTimes, 50), tryPercentile(startTimes, 95) return row, nil } diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 207fe622bb..60932097bb 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -3090,9 +3090,12 @@ ORDER BY SELECT -- Postgres offers no clear way to DRY this short of a function or other -- complexities. - coalesce((PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'start')), -1)::FLOAT AS start_median, - coalesce((PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'stop')), -1)::FLOAT AS stop_median, - coalesce((PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'delete')), -1)::FLOAT AS delete_median + coalesce((PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'start')), -1)::FLOAT AS start_50, + coalesce((PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'stop')), -1)::FLOAT AS stop_50, + coalesce((PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'delete')), -1)::FLOAT AS delete_50, + coalesce((PERCENTILE_DISC(0.95) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'start')), -1)::FLOAT AS start_95, + coalesce((PERCENTILE_DISC(0.95) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'stop')), -1)::FLOAT AS stop_95, + coalesce((PERCENTILE_DISC(0.95) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'delete')), -1)::FLOAT AS delete_95 FROM build_times ` @@ -3102,15 +3105,25 @@ type GetTemplateAverageBuildTimeParams struct { } type GetTemplateAverageBuildTimeRow struct { - StartMedian float64 `db:"start_median" json:"start_median"` - StopMedian float64 `db:"stop_median" json:"stop_median"` - DeleteMedian float64 `db:"delete_median" json:"delete_median"` + Start50 float64 `db:"start_50" json:"start_50"` + Stop50 float64 `db:"stop_50" json:"stop_50"` + Delete50 float64 `db:"delete_50" json:"delete_50"` + Start95 float64 `db:"start_95" json:"start_95"` + Stop95 float64 `db:"stop_95" json:"stop_95"` + Delete95 float64 `db:"delete_95" json:"delete_95"` } func (q *sqlQuerier) GetTemplateAverageBuildTime(ctx context.Context, arg GetTemplateAverageBuildTimeParams) (GetTemplateAverageBuildTimeRow, error) { row := q.db.QueryRowContext(ctx, getTemplateAverageBuildTime, arg.TemplateID, arg.StartTime) var i GetTemplateAverageBuildTimeRow - err := row.Scan(&i.StartMedian, &i.StopMedian, &i.DeleteMedian) + err := row.Scan( + &i.Start50, + &i.Stop50, + &i.Delete50, + &i.Start95, + &i.Stop95, + &i.Delete95, + ) return i, err } diff --git a/coderd/database/queries/templates.sql b/coderd/database/queries/templates.sql index 0757f823d6..902f4ecf99 100644 --- a/coderd/database/queries/templates.sql +++ b/coderd/database/queries/templates.sql @@ -142,8 +142,11 @@ ORDER BY SELECT -- Postgres offers no clear way to DRY this short of a function or other -- complexities. - coalesce((PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'start')), -1)::FLOAT AS start_median, - coalesce((PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'stop')), -1)::FLOAT AS stop_median, - coalesce((PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'delete')), -1)::FLOAT AS delete_median + coalesce((PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'start')), -1)::FLOAT AS start_50, + coalesce((PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'stop')), -1)::FLOAT AS stop_50, + coalesce((PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'delete')), -1)::FLOAT AS delete_50, + coalesce((PERCENTILE_DISC(0.95) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'start')), -1)::FLOAT AS start_95, + coalesce((PERCENTILE_DISC(0.95) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'stop')), -1)::FLOAT AS stop_95, + coalesce((PERCENTILE_DISC(0.95) WITHIN GROUP(ORDER BY exec_time_sec) FILTER (WHERE transition = 'delete')), -1)::FLOAT AS delete_95 FROM build_times ; diff --git a/coderd/metricscache/metricscache.go b/coderd/metricscache/metricscache.go index 0adf509596..58536958e5 100644 --- a/coderd/metricscache/metricscache.go +++ b/coderd/metricscache/metricscache.go @@ -242,7 +242,11 @@ func (c *Cache) TemplateUniqueUsers(id uuid.UUID) (int, bool) { } func (c *Cache) TemplateBuildTimeStats(id uuid.UUID) codersdk.TemplateBuildTimeStats { - var unknown codersdk.TemplateBuildTimeStats + unknown := codersdk.TemplateBuildTimeStats{ + codersdk.WorkspaceTransitionStart: {}, + codersdk.WorkspaceTransitionStop: {}, + codersdk.WorkspaceTransitionDelete: {}, + } m := c.templateAverageBuildTime.Load() if m == nil { @@ -256,7 +260,7 @@ func (c *Cache) TemplateBuildTimeStats(id uuid.UUID) codersdk.TemplateBuildTimeS return unknown } - convertMedian := func(m float64) *int64 { + convertMillis := func(m float64) *int64 { if m <= 0 { return nil } @@ -265,8 +269,17 @@ func (c *Cache) TemplateBuildTimeStats(id uuid.UUID) codersdk.TemplateBuildTimeS } return codersdk.TemplateBuildTimeStats{ - StartMillis: convertMedian(resp.StartMedian), - StopMillis: convertMedian(resp.StopMedian), - DeleteMillis: convertMedian(resp.DeleteMedian), + codersdk.WorkspaceTransitionStart: { + P50: convertMillis(resp.Start50), + P95: convertMillis(resp.Start95), + }, + codersdk.WorkspaceTransitionStop: { + P50: convertMillis(resp.Stop50), + P95: convertMillis(resp.Stop95), + }, + codersdk.WorkspaceTransitionDelete: { + P50: convertMillis(resp.Delete50), + P95: convertMillis(resp.Delete95), + }, } } diff --git a/coderd/metricscache/metricscache_test.go b/coderd/metricscache/metricscache_test.go index a01d71b9ee..cdcd1c88b3 100644 --- a/coderd/metricscache/metricscache_test.go +++ b/coderd/metricscache/metricscache_test.go @@ -7,7 +7,6 @@ import ( "time" "github.com/google/uuid" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "cdr.dev/slog/sloggers/slogtest" @@ -204,6 +203,12 @@ func clockTime(t time.Time, hour, minute, sec int) time.Time { return time.Date(t.Year(), t.Month(), t.Day(), hour, minute, sec, t.Nanosecond(), t.Location()) } +func requireBuildTimeStatsEmpty(t *testing.T, stats codersdk.TemplateBuildTimeStats) { + require.Empty(t, stats[codersdk.WorkspaceTransitionStart]) + require.Empty(t, stats[codersdk.WorkspaceTransitionStop]) + require.Empty(t, stats[codersdk.WorkspaceTransitionDelete]) +} + func TestCache_BuildTime(t *testing.T) { t.Parallel() @@ -296,7 +301,7 @@ func TestCache_BuildTime(t *testing.T) { require.NoError(t, err) gotStats := cache.TemplateBuildTimeStats(template.ID) - require.Empty(t, gotStats, "should not have loaded yet") + requireBuildTimeStatsEmpty(t, gotStats) for _, row := range tt.args.rows { _, err := db.InsertProvisionerJob(ctx, database.InsertProvisionerJobParams{ @@ -328,36 +333,32 @@ func TestCache_BuildTime(t *testing.T) { } if tt.want.loads { + wantTransition := codersdk.WorkspaceTransition(tt.args.transition) require.Eventuallyf(t, func() bool { stats := cache.TemplateBuildTimeStats(template.ID) - return stats != codersdk.TemplateBuildTimeStats{} + return stats[wantTransition] != codersdk.TransitionStats{} }, testutil.WaitLong, testutil.IntervalMedium, "BuildTime never populated", ) gotStats = cache.TemplateBuildTimeStats(template.ID) - - if tt.args.transition == database.WorkspaceTransitionDelete { - require.Nil(t, gotStats.StopMillis) - require.Nil(t, gotStats.StartMillis) - require.Equal(t, tt.want.buildTimeMs, *gotStats.DeleteMillis) - } - if tt.args.transition == database.WorkspaceTransitionStart { - require.Nil(t, gotStats.StopMillis) - require.Nil(t, gotStats.DeleteMillis) - require.Equal(t, tt.want.buildTimeMs, *gotStats.StartMillis) - } - if tt.args.transition == database.WorkspaceTransitionStop { - require.Nil(t, gotStats.StartMillis) - require.Nil(t, gotStats.DeleteMillis) - require.Equal(t, tt.want.buildTimeMs, *gotStats.StopMillis) + for transition, stats := range gotStats { + if transition == wantTransition { + require.Equal(t, tt.want.buildTimeMs, *stats.P50) + } else { + require.Empty( + t, stats, "%v", transition, + ) + } } } else { + var stats codersdk.TemplateBuildTimeStats require.Never(t, func() bool { - stats := cache.TemplateBuildTimeStats(template.ID) - return !assert.Empty(t, stats) + stats = cache.TemplateBuildTimeStats(template.ID) + requireBuildTimeStatsEmpty(t, stats) + return t.Failed() }, testutil.WaitShort/2, testutil.IntervalMedium, - "BuildTimeStats populated", + "BuildTimeStats populated", stats, ) } }) diff --git a/coderd/templates_test.go b/coderd/templates_test.go index 94bc5383bf..5b38dc83f3 100644 --- a/coderd/templates_test.go +++ b/coderd/templates_test.go @@ -536,7 +536,7 @@ func TestTemplateMetrics(t *testing.T) { }) template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) require.Equal(t, -1, template.ActiveUserCount) - require.Empty(t, template.BuildTimeStats) + require.Empty(t, template.BuildTimeStats[codersdk.WorkspaceTransitionStart]) coderdtest.AwaitTemplateVersionJob(t, client, version.ID) workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) @@ -607,7 +607,7 @@ func TestTemplateMetrics(t *testing.T) { require.Eventuallyf(t, func() bool { template, err = client.Template(ctx, template.ID) require.NoError(t, err) - startMs := template.BuildTimeStats.StartMillis + startMs := template.BuildTimeStats[codersdk.WorkspaceTransitionStart].P50 return startMs != nil && *startMs > 1 }, testutil.WaitShort, testutil.IntervalFast, diff --git a/codersdk/templates.go b/codersdk/templates.go index 786984a1ce..1d5a642b6f 100644 --- a/codersdk/templates.go +++ b/codersdk/templates.go @@ -33,12 +33,12 @@ type Template struct { CreatedByName string `json:"created_by_name"` } -type TemplateBuildTimeStats struct { - StartMillis *int64 `json:"start_ms"` - StopMillis *int64 `json:"stop_ms"` - DeleteMillis *int64 `json:"delete_ms"` +type TransitionStats struct { + P50 *int64 + P95 *int64 } +type TemplateBuildTimeStats map[WorkspaceTransition]TransitionStats type UpdateActiveTemplateVersion struct { ID uuid.UUID `json:"id" validate:"required"` } diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 75c85198b0..e745225e1c 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -651,11 +651,10 @@ export interface TemplateACL { } // From codersdk/templates.go -export interface TemplateBuildTimeStats { - readonly start_ms?: number - readonly stop_ms?: number - readonly delete_ms?: number -} +export type TemplateBuildTimeStats = Record< + WorkspaceTransition, + TransitionStats +> // From codersdk/templates.go export interface TemplateDAUsResponse { @@ -697,6 +696,12 @@ export interface TraceConfig { readonly capture_logs: DeploymentConfigField } +// From codersdk/templates.go +export interface TransitionStats { + readonly P50?: number + readonly P95?: number +} + // From codersdk/templates.go export interface UpdateActiveTemplateVersion { readonly id: string diff --git a/site/src/components/TemplateStats/TemplateStats.tsx b/site/src/components/TemplateStats/TemplateStats.tsx index f342c03fa5..444a1bad4f 100644 --- a/site/src/components/TemplateStats/TemplateStats.tsx +++ b/site/src/components/TemplateStats/TemplateStats.tsx @@ -42,7 +42,7 @@ export const TemplateStats: FC = ({ /> > = ({ /> ) - let buildTimeEstimate: number | undefined = undefined - let isTransitioning: boolean | undefined = undefined + let transitionStats: TypesGen.TransitionStats | undefined = undefined if (template !== undefined) { - ;[buildTimeEstimate, isTransitioning] = EstimateTransitionTime( - template, - workspace, - ) + transitionStats = ActiveTransition(template, workspace) } return ( @@ -195,10 +191,10 @@ export const Workspace: FC> = ({ handleUpdate={handleUpdate} /> - {isTransitioning !== undefined && isTransitioning && ( + {transitionStats !== undefined && ( )} diff --git a/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.stories.tsx b/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.stories.tsx index 40f4a1d8ec..23c9ecf134 100644 --- a/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.stories.tsx +++ b/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.stories.tsx @@ -21,7 +21,10 @@ const Template: Story = (args) => ( export const Starting = Template.bind({}) Starting.args = { - buildEstimate: 10000, + transitionStats: { + P50: 10000, + P95: 10010, + }, workspace: { ...MockStartingWorkspace, latest_build: { @@ -39,11 +42,17 @@ Starting.args = { export const StartingUnknown = Template.bind({}) StartingUnknown.args = { ...Starting.args, - buildEstimate: undefined, + transitionStats: undefined, } export const StartingPassedEstimate = Template.bind({}) StartingPassedEstimate.args = { ...Starting.args, - buildEstimate: 1000, + transitionStats: { P50: 1000, P95: 1000 }, +} + +export const StartingHighVariaton = Template.bind({}) +StartingHighVariaton.args = { + ...Starting.args, + transitionStats: { P50: 10000, P95: 20000 }, } diff --git a/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.tsx b/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.tsx index f1d61ee26e..f661afb601 100644 --- a/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.tsx +++ b/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.tsx @@ -1,6 +1,6 @@ import LinearProgress from "@material-ui/core/LinearProgress" import makeStyles from "@material-ui/core/styles/makeStyles" -import { Template, Workspace } from "api/typesGenerated" +import { TransitionStats, Template, Workspace } from "api/typesGenerated" import dayjs, { Dayjs } from "dayjs" import { FC, useEffect, useState } from "react" import { MONOSPACE_FONT_FAMILY } from "theme/constants" @@ -9,71 +9,99 @@ import duration from "dayjs/plugin/duration" dayjs.extend(duration) +// ActiveTransition gets the build estimate for the workspace, +// if it is in a transition state. +export const ActiveTransition = ( + template: Template, + workspace: Workspace, +): TransitionStats | undefined => { + const status = workspace.latest_build.status + + switch (status) { + case "starting": + return template.build_time_stats.start + case "stopping": + return template.build_time_stats.stop + case "deleting": + return template.build_time_stats.delete + default: + return undefined + } +} + const estimateFinish = ( startedAt: Dayjs, - buildEstimate: number, -): [number, string] => { - const realPercentage = dayjs().diff(startedAt) / buildEstimate + p50: number, + p95: number, +): [number | undefined, string] => { + const sinceStart = dayjs().diff(startedAt) + const secondsLeft = (est: number) => + Math.max( + Math.ceil(dayjs.duration((1 - sinceStart / est) * est).asSeconds()), + 0, + ) - const maxPercentage = 1 - if (realPercentage > maxPercentage) { - return [maxPercentage * 100, "Any moment now..."] - } + const lowGuess = secondsLeft(p50) + const highGuess = secondsLeft(p95) - return [ - realPercentage * 100, - `~${Math.ceil( - dayjs.duration((1 - realPercentage) * buildEstimate).asSeconds(), - )} seconds remaining...`, + const anyMomentNow: [number | undefined, string] = [ + undefined, + "Any moment now...", ] + + const p50percent = (sinceStart * 100) / p50 + if (highGuess <= 0) { + return anyMomentNow + } + const diff = highGuess - lowGuess + if (diff < 3) { + // If there is sufficient consistency, keep display simple. + return [p50percent, `${highGuess} seconds remaining...`] + } + return [p50percent, `Up to ${highGuess} seconds remaining...`] } export interface WorkspaceBuildProgressProps { workspace: Workspace - buildEstimate?: number -} - -// EstimateTransitionTime gets the build estimate for the workspace, -// if it is in a transition state. -export const EstimateTransitionTime = ( - template: Template, - workspace: Workspace, -): [number | undefined, boolean] => { - switch (workspace.latest_build.status) { - case "starting": - return [template.build_time_stats.start_ms, true] - case "stopping": - return [template.build_time_stats.stop_ms, true] - case "deleting": - return [template.build_time_stats.delete_ms, true] - default: - // Not in a transition state - return [undefined, false] - } + transitionStats: TransitionStats } export const WorkspaceBuildProgress: FC = ({ workspace, - buildEstimate, + transitionStats: transitionStats, }) => { const styles = useStyles() const job = workspace.latest_build.job const [progressValue, setProgressValue] = useState(0) + const [progressText, setProgressText] = useState( + "Finding ETA...", + ) // By default workspace is updated every second, which can cause visual stutter // when the build estimate is a few seconds. The timer ensures no observable // stutter in all cases. useEffect(() => { const updateProgress = () => { - if (job.status !== "running" || buildEstimate === undefined) { + if ( + job.status !== "running" || + transitionStats.P50 === undefined || + transitionStats.P95 === undefined + ) { setProgressValue(undefined) + setProgressText(undefined) return } - const est = estimateFinish(dayjs(job.started_at), buildEstimate)[0] + + const [est, text] = estimateFinish( + dayjs(job.started_at), + transitionStats.P50, + transitionStats.P95, + ) setProgressValue(est) + setProgressText(text) } setTimeout(updateProgress, 5) - }, [progressValue, job, buildEstimate]) + }, [progressValue, job, transitionStats]) return (
@@ -84,9 +112,7 @@ export const WorkspaceBuildProgress: FC = ({ // (e.g. the build isn't yet running). If we flicker from the // indeterminate bar to the determinate bar, the vigilant user // perceives the bar jumping from 100% to 0%. - progressValue !== undefined && - progressValue < 100 && - buildEstimate !== undefined + progressValue !== undefined && progressValue < 100 ? "determinate" : "indeterminate" } @@ -97,17 +123,7 @@ export const WorkspaceBuildProgress: FC = ({ />
{`Build ${job.status}`}
-
- {(() => { - if (job.status !== "running") { - return "" - } else if (buildEstimate !== undefined) { - return estimateFinish(dayjs(job.started_at), buildEstimate)[1] - } else { - return "Unknown ETA" - } - })()} -
+
{progressText}
) diff --git a/site/src/pages/TemplatesPage/TemplatesPageView.tsx b/site/src/pages/TemplatesPage/TemplatesPageView.tsx index 0a5d103534..0ed9bea46a 100644 --- a/site/src/pages/TemplatesPage/TemplatesPageView.tsx +++ b/site/src/pages/TemplatesPage/TemplatesPageView.tsx @@ -241,7 +241,7 @@ export const TemplatesPageView: FC< style={{ color: theme.palette.text.secondary }} > {formatTemplateBuildTime( - template.build_time_stats.start_ms, + template.build_time_stats.start.P50, )} diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 7d1e521351..e56b5358a0 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -193,9 +193,18 @@ export const MockTemplate: TypesGen.Template = { workspace_owner_count: 2, active_user_count: 1, build_time_stats: { - start_ms: 1000, - stop_ms: 2000, - delete_ms: 3000, + start: { + P50: 1000, + P95: 1500, + }, + stop: { + P50: 1000, + P95: 1500, + }, + delete: { + P50: 1000, + P95: 1500, + }, }, description: "This is a test description.", default_ttl_ms: 24 * 60 * 60 * 1000,