Skip to content

Support high build time variation in progress bar #4941

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Nov 17, 2022
1 change: 1 addition & 0 deletions cli/loadtest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
)

func TestLoadTest(t *testing.T) {
t.Skipf("This test is flakey. See https://github.com/coder/coder/issues/4942")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I already disabled the flakey test in another PR.

t.Parallel()

t.Run("PlaceboFromStdin", func(t *testing.T) {
Expand Down
11 changes: 6 additions & 5 deletions coderd/database/databasefake/databasefake.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
27 changes: 20 additions & 7 deletions coderd/database/queries.sql.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 6 additions & 3 deletions coderd/database/queries/templates.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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
;
23 changes: 18 additions & 5 deletions coderd/metricscache/metricscache.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}
Expand All @@ -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),
},
}
}
43 changes: 22 additions & 21 deletions coderd/metricscache/metricscache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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,
)
}
})
Expand Down
4 changes: 2 additions & 2 deletions coderd/templates_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand Down
8 changes: 4 additions & 4 deletions codersdk/templates.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}
Expand Down
15 changes: 10 additions & 5 deletions site/src/api/typesGenerated.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion site/src/components/TemplateStats/TemplateStats.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Expand Down
14 changes: 5 additions & 9 deletions site/src/components/Workspace/Workspace.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 (
Expand Down Expand Up @@ -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}
/>
)}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand All @@ -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 },
}
Loading