site: support high build time variation in progress bar (#4941)

This commit is contained in:
Ammar Bandukwala
2022-11-17 16:56:56 +00:00
committed by GitHub
parent 60cec022eb
commit acf34d4295
15 changed files with 187 additions and 120 deletions
+1
View File
@@ -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) {
+6 -5
View File
@@ -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
}
+20 -7
View File
@@ -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
}
+6 -3
View File
@@ -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
;
+18 -5
View File
@@ -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),
},
}
}
+22 -21
View File
@@ -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,
)
}
})
+2 -2
View File
@@ -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,
+4 -4
View File
@@ -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"`
}
+10 -5
View File
@@ -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<boolean>
}
// From codersdk/templates.go
export interface TransitionStats {
readonly P50?: number
readonly P95?: number
}
// From codersdk/templates.go
export interface UpdateActiveTemplateVersion {
readonly id: string
@@ -42,7 +42,7 @@ export const TemplateStats: FC<TemplateStatsProps> = ({
/>
<StatsItem
label={Language.buildTimeLabel}
value={formatTemplateBuildTime(template.build_time_stats.start_ms)}
value={formatTemplateBuildTime(template.build_time_stats.start.P50)}
/>
<StatsItem
label={Language.activeVersionLabel}
+5 -9
View File
@@ -19,7 +19,7 @@ import { WorkspaceStats } from "../WorkspaceStats/WorkspaceStats"
import { AlertBanner } from "../AlertBanner/AlertBanner"
import { useTranslation } from "react-i18next"
import {
EstimateTransitionTime,
ActiveTransition,
WorkspaceBuildProgress,
} from "components/WorkspaceBuildProgress/WorkspaceBuildProgress"
import { AgentRow } from "components/Resources/AgentRow"
@@ -115,13 +115,9 @@ export const Workspace: FC<React.PropsWithChildren<WorkspaceProps>> = ({
/>
)
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<React.PropsWithChildren<WorkspaceProps>> = ({
handleUpdate={handleUpdate}
/>
{isTransitioning !== undefined && isTransitioning && (
{transitionStats !== undefined && (
<WorkspaceBuildProgress
workspace={workspace}
buildEstimate={buildTimeEstimate}
transitionStats={transitionStats}
/>
)}
@@ -21,7 +21,10 @@ const Template: Story<WorkspaceBuildProgressProps> = (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 },
}
@@ -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<WorkspaceBuildProgressProps> = ({
workspace,
buildEstimate,
transitionStats: transitionStats,
}) => {
const styles = useStyles()
const job = workspace.latest_build.job
const [progressValue, setProgressValue] = useState<number | undefined>(0)
const [progressText, setProgressText] = useState<string | undefined>(
"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 (
<div className={styles.stack}>
@@ -84,9 +112,7 @@ export const WorkspaceBuildProgress: FC<WorkspaceBuildProgressProps> = ({
// (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<WorkspaceBuildProgressProps> = ({
/>
<div className={styles.barHelpers}>
<div className={styles.label}>{`Build ${job.status}`}</div>
<div className={styles.label}>
{(() => {
if (job.status !== "running") {
return ""
} else if (buildEstimate !== undefined) {
return estimateFinish(dayjs(job.started_at), buildEstimate)[1]
} else {
return "Unknown ETA"
}
})()}
</div>
<div className={styles.label}>{progressText}</div>
</div>
</div>
)
@@ -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,
)}
</span>
</TableCellLink>
+12 -3
View File
@@ -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,