From 0ba618dc2bf136f63000e656c93c20248d99f1d6 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Sun, 16 Oct 2022 02:31:29 +0000 Subject: [PATCH 01/11] Support `delete` & `stop` in build progress bar --- coderd/database/databasefake/databasefake.go | 41 ++++++++++++++------ coderd/database/querier.go | 2 +- coderd/database/queries.sql.go | 25 ++++++++---- coderd/database/queries/templates.sql | 11 ++++-- coderd/metricscache/metricscache.go | 29 ++++++++------ coderd/templates.go | 2 +- coderd/templates_test.go | 4 +- codersdk/templates.go | 23 ++++++----- 8 files changed, 91 insertions(+), 46 deletions(-) diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index 54314d221da1b..fe655dc9ca65e 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -235,15 +235,17 @@ func (q *fakeQuerier) GetTemplateDAUs(_ context.Context, templateID uuid.UUID) ( return rs, nil } -func (q *fakeQuerier) GetTemplateAverageBuildTime(ctx context.Context, arg database.GetTemplateAverageBuildTimeParams) (float64, error) { - var times []float64 +func (q *fakeQuerier) GetTemplateAverageBuildTime(ctx context.Context, arg database.GetTemplateAverageBuildTimeParams) (database.GetTemplateAverageBuildTimeRow, error) { + var emptyRow database.GetTemplateAverageBuildTimeRow + var ( + startTimes []float64 + stopTimes []float64 + deleteTimes []float64 + ) for _, wb := range q.workspaceBuilds { - if wb.Transition != database.WorkspaceTransitionStart { - continue - } version, err := q.GetTemplateVersionByID(ctx, wb.TemplateVersionID) if err != nil { - return -1, err + return emptyRow, err } if version.TemplateID != arg.TemplateID { continue @@ -251,17 +253,32 @@ func (q *fakeQuerier) GetTemplateAverageBuildTime(ctx context.Context, arg datab job, err := q.GetProvisionerJobByID(ctx, wb.JobID) if err != nil { - return -1, err + return emptyRow, err } if job.CompletedAt.Valid { - times = append(times, job.CompletedAt.Time.Sub(job.StartedAt.Time).Seconds()) + took := job.CompletedAt.Time.Sub(job.StartedAt.Time).Seconds() + if wb.Transition == database.WorkspaceTransitionStart { + startTimes = append(startTimes, took) + } else if wb.Transition == database.WorkspaceTransitionStop { + stopTimes = append(stopTimes, took) + } else if wb.Transition == database.WorkspaceTransitionDelete { + deleteTimes = append(deleteTimes, took) + } } } - sort.Float64s(times) - if len(times) == 0 { - return -1, nil + + tryMedian := func(fs []float64) float64 { + if len(fs) == 0 { + return -1 + } + sort.Float64s(fs) + return fs[len(startTimes)/2] } - return times[len(times)/2], nil + var row database.GetTemplateAverageBuildTimeRow + row.DeleteMedian = tryMedian(deleteTimes) + row.StopMedian = tryMedian(stopTimes) + row.StartMedian = tryMedian(startTimes) + return row, nil } func (q *fakeQuerier) ParameterValue(_ context.Context, id uuid.UUID) (database.ParameterValue, error) { diff --git a/coderd/database/querier.go b/coderd/database/querier.go index d3680675d51d5..ad26413873e04 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -67,7 +67,7 @@ type sqlcQuerier interface { GetProvisionerJobsByIDs(ctx context.Context, ids []uuid.UUID) ([]ProvisionerJob, error) GetProvisionerJobsCreatedAfter(ctx context.Context, createdAt time.Time) ([]ProvisionerJob, error) GetProvisionerLogsByIDBetween(ctx context.Context, arg GetProvisionerLogsByIDBetweenParams) ([]ProvisionerJobLog, error) - GetTemplateAverageBuildTime(ctx context.Context, arg GetTemplateAverageBuildTimeParams) (float64, error) + GetTemplateAverageBuildTime(ctx context.Context, arg GetTemplateAverageBuildTimeParams) (GetTemplateAverageBuildTimeRow, error) GetTemplateByID(ctx context.Context, id uuid.UUID) (Template, error) GetTemplateByOrganizationAndName(ctx context.Context, arg GetTemplateByOrganizationAndNameParams) (Template, error) GetTemplateDAUs(ctx context.Context, templateID uuid.UUID) ([]GetTemplateDAUsRow, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index e34c0fa841d30..41eb029b59a83 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -2600,7 +2600,8 @@ func (q *sqlQuerier) InsertDeploymentID(ctx context.Context, value string) error const getTemplateAverageBuildTime = `-- name: GetTemplateAverageBuildTime :one WITH build_times AS ( SELECT - EXTRACT(EPOCH FROM (pj.completed_at - pj.started_at))::FLOAT AS exec_time_sec + EXTRACT(EPOCH FROM (pj.completed_at - pj.started_at))::FLOAT AS exec_time_sec, + workspace_builds.transition FROM workspace_builds JOIN template_versions ON @@ -2609,7 +2610,6 @@ JOIN provisioner_jobs pj ON workspace_builds.job_id = pj.id WHERE template_versions.template_id = $1 AND - (workspace_builds.transition = 'start') AND (pj.completed_at IS NOT NULL) AND (pj.started_at IS NOT NULL) AND (pj.started_at > $2) AND (pj.canceled_at IS NULL) AND @@ -2617,7 +2617,12 @@ WHERE ORDER BY workspace_builds.created_at DESC ) -SELECT coalesce((PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec)), -1)::FLOAT +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 FROM build_times ` @@ -2626,11 +2631,17 @@ type GetTemplateAverageBuildTimeParams struct { StartTime sql.NullTime `db:"start_time" json:"start_time"` } -func (q *sqlQuerier) GetTemplateAverageBuildTime(ctx context.Context, arg GetTemplateAverageBuildTimeParams) (float64, error) { +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"` +} + +func (q *sqlQuerier) GetTemplateAverageBuildTime(ctx context.Context, arg GetTemplateAverageBuildTimeParams) (GetTemplateAverageBuildTimeRow, error) { row := q.db.QueryRowContext(ctx, getTemplateAverageBuildTime, arg.TemplateID, arg.StartTime) - var column_1 float64 - err := row.Scan(&column_1) - return column_1, err + var i GetTemplateAverageBuildTimeRow + err := row.Scan(&i.StartMedian, &i.StopMedian, &i.DeleteMedian) + return i, err } const getTemplateByID = `-- name: GetTemplateByID :one diff --git a/coderd/database/queries/templates.sql b/coderd/database/queries/templates.sql index 2bb6f933eeae5..06570fffb0998 100644 --- a/coderd/database/queries/templates.sql +++ b/coderd/database/queries/templates.sql @@ -109,7 +109,8 @@ RETURNING -- name: GetTemplateAverageBuildTime :one WITH build_times AS ( SELECT - EXTRACT(EPOCH FROM (pj.completed_at - pj.started_at))::FLOAT AS exec_time_sec + EXTRACT(EPOCH FROM (pj.completed_at - pj.started_at))::FLOAT AS exec_time_sec, + workspace_builds.transition FROM workspace_builds JOIN template_versions ON @@ -118,7 +119,6 @@ JOIN provisioner_jobs pj ON workspace_builds.job_id = pj.id WHERE template_versions.template_id = @template_id AND - (workspace_builds.transition = 'start') AND (pj.completed_at IS NOT NULL) AND (pj.started_at IS NOT NULL) AND (pj.started_at > @start_time) AND (pj.canceled_at IS NULL) AND @@ -126,6 +126,11 @@ WHERE ORDER BY workspace_builds.created_at DESC ) -SELECT coalesce((PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec)), -1)::FLOAT +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 FROM build_times ; diff --git a/coderd/metricscache/metricscache.go b/coderd/metricscache/metricscache.go index 1feb36ba2345b..419cd0f9624f1 100644 --- a/coderd/metricscache/metricscache.go +++ b/coderd/metricscache/metricscache.go @@ -29,7 +29,7 @@ type Cache struct { templateDAUResponses atomic.Pointer[map[uuid.UUID]codersdk.TemplateDAUsResponse] templateUniqueUsers atomic.Pointer[map[uuid.UUID]int] - templateAverageBuildTime atomic.Pointer[map[uuid.UUID]time.Duration] + templateAverageBuildTime atomic.Pointer[map[uuid.UUID]database.GetTemplateAverageBuildTimeRow] done chan struct{} cancel func() @@ -130,9 +130,9 @@ func (c *Cache) refresh(ctx context.Context) error { } var ( - templateDAUs = make(map[uuid.UUID]codersdk.TemplateDAUsResponse, len(templates)) - templateUniqueUsers = make(map[uuid.UUID]int) - templateAverageBuildTimeSec = make(map[uuid.UUID]time.Duration) + templateDAUs = make(map[uuid.UUID]codersdk.TemplateDAUsResponse, len(templates)) + templateUniqueUsers = make(map[uuid.UUID]int) + templateAverageBuildTimes = make(map[uuid.UUID]database.GetTemplateAverageBuildTimeRow) ) for _, template := range templates { rows, err := c.database.GetTemplateDAUs(ctx, template.ID) @@ -141,6 +141,7 @@ func (c *Cache) refresh(ctx context.Context) error { } templateDAUs[template.ID] = convertDAUResponse(rows) templateUniqueUsers[template.ID] = countUniqueUsers(rows) + templateAvgBuildTime, err := c.database.GetTemplateAverageBuildTime(ctx, database.GetTemplateAverageBuildTimeParams{ TemplateID: uuid.NullUUID{ UUID: template.ID, @@ -151,14 +152,15 @@ func (c *Cache) refresh(ctx context.Context) error { Valid: true, }, }) + if err != nil { return err } - templateAverageBuildTimeSec[template.ID] = time.Duration(float64(time.Second) * templateAvgBuildTime) + templateAverageBuildTimes[template.ID] = templateAvgBuildTime } c.templateDAUResponses.Store(&templateDAUs) c.templateUniqueUsers.Store(&templateUniqueUsers) - c.templateAverageBuildTime.Store(&templateAverageBuildTimeSec) + c.templateAverageBuildTime.Store(&templateAverageBuildTimes) return nil } @@ -239,17 +241,22 @@ func (c *Cache) TemplateUniqueUsers(id uuid.UUID) (int, bool) { return resp, true } -func (c *Cache) TemplateAverageBuildTime(id uuid.UUID) (time.Duration, bool) { +func (c *Cache) TemplateAverageBuildTime(id uuid.UUID) (*codersdk.TemplateBuildTimeStats, bool) { m := c.templateAverageBuildTime.Load() if m == nil { // Data loading. - return -1, false + return nil, false } resp, ok := (*m)[id] - if !ok || resp <= 0 { + if !ok || resp.StartMedian <= 0 || resp.StopMedian <= 0 { // No data or not enough builds. - return -1, false + return nil, false } - return resp, true + + return &codersdk.TemplateBuildTimeStats{ + StartMillis: int64(resp.StartMedian * 1000), + StopMillis: int64(resp.StopMedian * 1000), + DeleteMillis: int64(resp.DeleteMedian * 1000), + }, true } diff --git a/coderd/templates.go b/coderd/templates.go index 79fdc2091ffaf..3bb43c6d358a6 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -792,7 +792,7 @@ func (api *API) convertTemplate( ActiveVersionID: template.ActiveVersionID, WorkspaceOwnerCount: workspaceOwnerCount, ActiveUserCount: activeCount, - AverageBuildTimeMillis: averageBuildTimeMillis, + BuildTimeStats: averageBuildTimeMillis, Description: template.Description, Icon: template.Icon, MaxTTLMillis: time.Duration(template.MaxTtl).Milliseconds(), diff --git a/coderd/templates_test.go b/coderd/templates_test.go index b384108387f07..22bda5a51d3ad 100644 --- a/coderd/templates_test.go +++ b/coderd/templates_test.go @@ -594,7 +594,7 @@ func TestTemplateMetrics(t *testing.T) { }) template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) require.Equal(t, -1, template.ActiveUserCount) - require.EqualValues(t, -1, template.AverageBuildTimeMillis) + require.EqualValues(t, -1, template.BuildTimeStats) coderdtest.AwaitTemplateVersionJob(t, client, version.ID) workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) @@ -661,7 +661,7 @@ func TestTemplateMetrics(t *testing.T) { template, err = client.Template(ctx, template.ID) require.NoError(t, err) require.Equal(t, 1, template.ActiveUserCount) - require.Greater(t, template.AverageBuildTimeMillis, int64(1)) + require.Greater(t, template.BuildTimeStats, int64(1)) workspaces, err = client.Workspaces(ctx, codersdk.WorkspaceFilter{}) require.NoError(t, err) diff --git a/codersdk/templates.go b/codersdk/templates.go index 887c648b42954..0781a80a49c08 100644 --- a/codersdk/templates.go +++ b/codersdk/templates.go @@ -23,15 +23,20 @@ type Template struct { ActiveVersionID uuid.UUID `json:"active_version_id"` WorkspaceOwnerCount uint32 `json:"workspace_owner_count"` // ActiveUserCount is set to -1 when loading. - ActiveUserCount int `json:"active_user_count"` - // AverageBuildTimeMillis is set to -1 when there aren't enough recent builds. - AverageBuildTimeMillis int64 `json:"average_build_time_ms"` - Description string `json:"description"` - Icon string `json:"icon"` - MaxTTLMillis int64 `json:"max_ttl_ms"` - MinAutostartIntervalMillis int64 `json:"min_autostart_interval_ms"` - CreatedByID uuid.UUID `json:"created_by_id"` - CreatedByName string `json:"created_by_name"` + ActiveUserCount int `json:"active_user_count"` + BuildTimeStats TemplateBuildTimeStats `json:"build_time_stats"` + Description string `json:"description"` + Icon string `json:"icon"` + MaxTTLMillis int64 `json:"max_ttl_ms"` + MinAutostartIntervalMillis int64 `json:"min_autostart_interval_ms"` + CreatedByID uuid.UUID `json:"created_by_id"` + 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 UpdateActiveTemplateVersion struct { From 40769bdd0b7c8e9d673407e0e3e946c00a60a11e Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Sun, 16 Oct 2022 03:09:06 +0000 Subject: [PATCH 02/11] Use null types instead of -1 for simplicity --- coderd/database/databasefake/databasefake.go | 2 +- coderd/metricscache/metricscache.go | 28 +++++--- coderd/metricscache/metricscache_test.go | 69 ++++++++++++-------- coderd/templates.go | 10 +-- codersdk/templates.go | 6 +- site/src/api/typesGenerated.ts | 9 ++- 6 files changed, 75 insertions(+), 49 deletions(-) diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index fe655dc9ca65e..63239bdf4dfd3 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -272,7 +272,7 @@ func (q *fakeQuerier) GetTemplateAverageBuildTime(ctx context.Context, arg datab return -1 } sort.Float64s(fs) - return fs[len(startTimes)/2] + return fs[len(fs)/2] } var row database.GetTemplateAverageBuildTimeRow row.DeleteMedian = tryMedian(deleteTimes) diff --git a/coderd/metricscache/metricscache.go b/coderd/metricscache/metricscache.go index 419cd0f9624f1..a035acc2f211f 100644 --- a/coderd/metricscache/metricscache.go +++ b/coderd/metricscache/metricscache.go @@ -241,22 +241,32 @@ func (c *Cache) TemplateUniqueUsers(id uuid.UUID) (int, bool) { return resp, true } -func (c *Cache) TemplateAverageBuildTime(id uuid.UUID) (*codersdk.TemplateBuildTimeStats, bool) { +func (c *Cache) TemplateBuildTimeStats(id uuid.UUID) codersdk.TemplateBuildTimeStats { + var unknown codersdk.TemplateBuildTimeStats + m := c.templateAverageBuildTime.Load() if m == nil { // Data loading. - return nil, false + return unknown } resp, ok := (*m)[id] - if !ok || resp.StartMedian <= 0 || resp.StopMedian <= 0 { + if !ok { // No data or not enough builds. - return nil, false + return unknown } - return &codersdk.TemplateBuildTimeStats{ - StartMillis: int64(resp.StartMedian * 1000), - StopMillis: int64(resp.StopMedian * 1000), - DeleteMillis: int64(resp.DeleteMedian * 1000), - }, true + convertMedian := func(m float64) *int64 { + if m < 1 { + return nil + } + i := int64(m * 1000) + return &i + } + + return codersdk.TemplateBuildTimeStats{ + StartMillis: convertMedian(resp.StartMedian), + StopMillis: convertMedian(resp.StopMedian), + DeleteMillis: convertMedian(resp.DeleteMedian), + } } diff --git a/coderd/metricscache/metricscache_test.go b/coderd/metricscache/metricscache_test.go index 3ea2748331373..98e43c067f83b 100644 --- a/coderd/metricscache/metricscache_test.go +++ b/coderd/metricscache/metricscache_test.go @@ -7,6 +7,7 @@ import ( "time" "github.com/google/uuid" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "cdr.dev/slog/sloggers/slogtest" @@ -214,27 +215,30 @@ func TestCache_BuildTime(t *testing.T) { } type args struct { - rows []jobParams + rows []jobParams + transition database.WorkspaceTransition } type want struct { - buildTime time.Duration + buildTimeMs int64 + loads bool } tests := []struct { name string args args want want }{ - {"empty", args{}, want{-1}}, - {"one", args{ + {"empty", args{}, want{-1, false}}, + {"one/start", args{ rows: []jobParams{ { startedAt: clockTime(someDay, 10, 1, 0), completedAt: clockTime(someDay, 10, 1, 10), }, }, - }, want{time.Second * 10}, + transition: database.WorkspaceTransitionStart, + }, want{10 * 1000, true}, }, - {"two", args{ + {"two/stop", args{ rows: []jobParams{ { startedAt: clockTime(someDay, 10, 1, 0), @@ -245,9 +249,10 @@ func TestCache_BuildTime(t *testing.T) { completedAt: clockTime(someDay, 10, 1, 50), }, }, - }, want{time.Second * 50}, + transition: database.WorkspaceTransitionStop, + }, want{50 * 1000, true}, }, - {"three", args{ + {"three/delete", args{ rows: []jobParams{ { startedAt: clockTime(someDay, 10, 1, 0), @@ -261,7 +266,8 @@ func TestCache_BuildTime(t *testing.T) { completedAt: clockTime(someDay, 10, 1, 20), }, }, - }, want{time.Second * 20}, + transition: database.WorkspaceTransitionDelete, + }, want{20 * 1000, true}, }, } @@ -289,9 +295,8 @@ func TestCache_BuildTime(t *testing.T) { }) require.NoError(t, err) - gotBuildTime, ok := cache.TemplateAverageBuildTime(template.ID) - require.False(t, ok, "template shouldn't have loaded yet") - require.EqualValues(t, -1, gotBuildTime) + gotStats := cache.TemplateBuildTimeStats(template.ID) + require.Empty(t, gotStats, "should not have loaded yet") for _, row := range tt.args.rows { _, err := db.InsertProvisionerJob(ctx, database.InsertProvisionerJobParams{ @@ -311,7 +316,7 @@ func TestCache_BuildTime(t *testing.T) { _, err = db.InsertWorkspaceBuild(ctx, database.InsertWorkspaceBuildParams{ TemplateVersionID: templateVersion.ID, JobID: job.ID, - Transition: database.WorkspaceTransitionStart, + Transition: tt.args.transition, }) require.NoError(t, err) @@ -322,28 +327,38 @@ func TestCache_BuildTime(t *testing.T) { require.NoError(t, err) } - if tt.want.buildTime > 0 { + if tt.want.loads { require.Eventuallyf(t, func() bool { - _, ok := cache.TemplateAverageBuildTime(template.ID) - return ok + stats := cache.TemplateBuildTimeStats(template.ID) + return assert.NotEmpty(t, stats) }, testutil.WaitShort, testutil.IntervalMedium, - "TemplateDAUs never populated", + "BuildTime never populated", ) - gotBuildTime, ok = cache.TemplateAverageBuildTime(template.ID) - require.True(t, ok) - require.Equal(t, tt.want.buildTime, gotBuildTime) + 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) + } } else { require.Never(t, func() bool { - _, ok := cache.TemplateAverageBuildTime(template.ID) - return ok + stats := cache.TemplateBuildTimeStats(template.ID) + return !assert.Empty(t, stats) }, testutil.WaitShort/2, testutil.IntervalMedium, - "TemplateDAUs never populated", + "BuildTimeStats populated", ) - - gotBuildTime, ok = cache.TemplateAverageBuildTime(template.ID) - require.False(t, ok) - require.Less(t, gotBuildTime, time.Duration(0)) } }) } diff --git a/coderd/templates.go b/coderd/templates.go index 3bb43c6d358a6..7b08303dea096 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -774,13 +774,7 @@ func (api *API) convertTemplate( ) codersdk.Template { activeCount, _ := api.metricsCache.TemplateUniqueUsers(template.ID) - var averageBuildTimeMillis int64 - averageBuildTime, ok := api.metricsCache.TemplateAverageBuildTime(template.ID) - if !ok { - averageBuildTimeMillis = -1 - } else { - averageBuildTimeMillis = int64(averageBuildTime / time.Millisecond) - } + buildTimeStats := api.metricsCache.TemplateBuildTimeStats(template.ID) return codersdk.Template{ ID: template.ID, @@ -792,7 +786,7 @@ func (api *API) convertTemplate( ActiveVersionID: template.ActiveVersionID, WorkspaceOwnerCount: workspaceOwnerCount, ActiveUserCount: activeCount, - BuildTimeStats: averageBuildTimeMillis, + BuildTimeStats: buildTimeStats, Description: template.Description, Icon: template.Icon, MaxTTLMillis: time.Duration(template.MaxTtl).Milliseconds(), diff --git a/codersdk/templates.go b/codersdk/templates.go index 0781a80a49c08..22e707050aed3 100644 --- a/codersdk/templates.go +++ b/codersdk/templates.go @@ -34,9 +34,9 @@ type Template struct { } type TemplateBuildTimeStats struct { - StartMillis int64 `json:"start_ms"` - StopMillis int64 `json:"stop_ms"` - DeleteMillis int64 `json:"delete_ms"` + StartMillis *int64 `json:"start_ms"` + StopMillis *int64 `json:"stop_ms"` + DeleteMillis *int64 `json:"delete_ms"` } type UpdateActiveTemplateVersion struct { diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 519b828d30980..5347613e77e86 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -586,7 +586,7 @@ export interface Template { readonly active_version_id: string readonly workspace_owner_count: number readonly active_user_count: number - readonly average_build_time_ms: number + readonly build_time_stats: TemplateBuildTimeStats readonly description: string readonly icon: string readonly max_ttl_ms: number @@ -601,6 +601,13 @@ export interface TemplateACL { readonly group: TemplateGroup[] } +// From codersdk/templates.go +export interface TemplateBuildTimeStats { + readonly start_ms?: number + readonly stop_ms?: number + readonly delete_ms?: number +} + // From codersdk/templates.go export interface TemplateDAUsResponse { readonly entries: DAUEntry[] From 61941ed0315620c1d6738f48b0f19487834c2255 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Sun, 16 Oct 2022 03:17:28 +0000 Subject: [PATCH 03/11] Fix pgcrypto bug in migration 59 --- coderd/database/dump.sql | 4 ++++ coderd/database/migrations/000059_file_id.up.sql | 11 +++++++---- coderd/templates_test.go | 2 +- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 9e2c68dbf6ef3..de2d352a6a073 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -1,5 +1,9 @@ -- Code generated by 'make coderd/database/generate'. DO NOT EDIT. +CREATE EXTENSION IF NOT EXISTS pgcrypto WITH SCHEMA public; + +COMMENT ON EXTENSION pgcrypto IS 'cryptographic functions'; + CREATE TYPE api_key_scope AS ENUM ( 'all', 'application_connect' diff --git a/coderd/database/migrations/000059_file_id.up.sql b/coderd/database/migrations/000059_file_id.up.sql index 7e6e919fa9bb3..98090aff817f8 100644 --- a/coderd/database/migrations/000059_file_id.up.sql +++ b/coderd/database/migrations/000059_file_id.up.sql @@ -4,8 +4,8 @@ -- template to be able to push and read files used for template -- versions they create. -- Prior to this collisions on file.hash were not an issue --- since users who could push files could also read all files. --- +-- since users who could push files could also read all files. +-- -- This migration also adds a 'files.id' column as the primary -- key. As a side effect the provisioner_jobs must now reference -- the files.id column since the 'hash' column is now ambiguous. @@ -14,10 +14,13 @@ BEGIN; -- Drop the primary key on hash. ALTER TABLE files DROP CONSTRAINT files_pkey; +-- This extension is required by gen_random_uuid +CREATE EXTENSION pgcrypto; + -- Add an 'id' column and designate it the primary key. -ALTER TABLE files ADD COLUMN +ALTER TABLE files ADD COLUMN id uuid NOT NULL PRIMARY KEY DEFAULT gen_random_uuid (); - + -- Update the constraint to include the user who created it. ALTER TABLE files ADD UNIQUE(hash, created_by); diff --git a/coderd/templates_test.go b/coderd/templates_test.go index 22bda5a51d3ad..2d188a4fcf914 100644 --- a/coderd/templates_test.go +++ b/coderd/templates_test.go @@ -661,7 +661,7 @@ func TestTemplateMetrics(t *testing.T) { template, err = client.Template(ctx, template.ID) require.NoError(t, err) require.Equal(t, 1, template.ActiveUserCount) - require.Greater(t, template.BuildTimeStats, int64(1)) + require.Greater(t, template.BuildTimeStats.StartMillis, int64(1)) workspaces, err = client.Workspaces(ctx, codersdk.WorkspaceFilter{}) require.NoError(t, err) From 8d92c22f24484c4df9603a83406c3b54998b77d7 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Sun, 16 Oct 2022 21:04:43 +0000 Subject: [PATCH 04/11] fixup! Fix pgcrypto bug in migration 59 --- coderd/database/migrations/000059_file_id.up.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/coderd/database/migrations/000059_file_id.up.sql b/coderd/database/migrations/000059_file_id.up.sql index 98090aff817f8..640876e7b7d4c 100644 --- a/coderd/database/migrations/000059_file_id.up.sql +++ b/coderd/database/migrations/000059_file_id.up.sql @@ -15,7 +15,7 @@ BEGIN; ALTER TABLE files DROP CONSTRAINT files_pkey; -- This extension is required by gen_random_uuid -CREATE EXTENSION pgcrypto; +CREATE EXTENSION IF NOT EXISTS pgcrypto; -- Add an 'id' column and designate it the primary key. ALTER TABLE files ADD COLUMN From 0263b0956657987600c18486f70f00f8922f6c34 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Sun, 16 Oct 2022 21:05:41 +0000 Subject: [PATCH 05/11] Fix test bugs --- coderd/templates_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/coderd/templates_test.go b/coderd/templates_test.go index 2d188a4fcf914..089a19ed1e956 100644 --- a/coderd/templates_test.go +++ b/coderd/templates_test.go @@ -594,7 +594,7 @@ func TestTemplateMetrics(t *testing.T) { }) template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) require.Equal(t, -1, template.ActiveUserCount) - require.EqualValues(t, -1, template.BuildTimeStats) + require.Empty(t, template.BuildTimeStats) coderdtest.AwaitTemplateVersionJob(t, client, version.ID) workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) @@ -661,7 +661,7 @@ func TestTemplateMetrics(t *testing.T) { template, err = client.Template(ctx, template.ID) require.NoError(t, err) require.Equal(t, 1, template.ActiveUserCount) - require.Greater(t, template.BuildTimeStats.StartMillis, int64(1)) + require.Greater(t, *template.BuildTimeStats.StartMillis, int64(1)) workspaces, err = client.Workspaces(ctx, codersdk.WorkspaceFilter{}) require.NoError(t, err) From 30fee3f12e2b6d8d66d94e3250cb3d9603d0b21b Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Sun, 16 Oct 2022 21:52:28 +0000 Subject: [PATCH 06/11] Update frontend --- coderd/metricscache/metricscache.go | 2 +- coderd/templates_test.go | 1 + .../TemplateStats/TemplateStats.tsx | 2 +- site/src/components/Workspace/Workspace.tsx | 4 +- .../WorkspaceBuildProgress.tsx | 49 +++++++++++++------ .../pages/TemplatesPage/TemplatesPageView.tsx | 2 +- site/src/util/templates.ts | 4 +- 7 files changed, 42 insertions(+), 22 deletions(-) diff --git a/coderd/metricscache/metricscache.go b/coderd/metricscache/metricscache.go index a035acc2f211f..0adf5095961e5 100644 --- a/coderd/metricscache/metricscache.go +++ b/coderd/metricscache/metricscache.go @@ -257,7 +257,7 @@ func (c *Cache) TemplateBuildTimeStats(id uuid.UUID) codersdk.TemplateBuildTimeS } convertMedian := func(m float64) *int64 { - if m < 1 { + if m <= 0 { return nil } i := int64(m * 1000) diff --git a/coderd/templates_test.go b/coderd/templates_test.go index 089a19ed1e956..637ced633cdc1 100644 --- a/coderd/templates_test.go +++ b/coderd/templates_test.go @@ -661,6 +661,7 @@ func TestTemplateMetrics(t *testing.T) { template, err = client.Template(ctx, template.ID) require.NoError(t, err) require.Equal(t, 1, template.ActiveUserCount) + require.NotNil(t, template.BuildTimeStats.StartMillis, template.BuildTimeStats) require.Greater(t, *template.BuildTimeStats.StartMillis, int64(1)) workspaces, err = client.Workspaces(ctx, codersdk.WorkspaceFilter{}) diff --git a/site/src/components/TemplateStats/TemplateStats.tsx b/site/src/components/TemplateStats/TemplateStats.tsx index ba548937e5f88..3bc943479593c 100644 --- a/site/src/components/TemplateStats/TemplateStats.tsx +++ b/site/src/components/TemplateStats/TemplateStats.tsx @@ -46,7 +46,7 @@ export const TemplateStats: FC = ({ {Language.buildTimeLabel} - {formatTemplateBuildTime(template.average_build_time_ms)}{" "} + {formatTemplateBuildTime(template.build_time_stats.start_ms)}{" "}
diff --git a/site/src/components/Workspace/Workspace.tsx b/site/src/components/Workspace/Workspace.tsx index 6388b0ce12d23..5dc20b1948d8c 100644 --- a/site/src/components/Workspace/Workspace.tsx +++ b/site/src/components/Workspace/Workspace.tsx @@ -186,9 +186,7 @@ export const Workspace: FC> = ({ - {workspace.latest_build.status === "starting" && ( - - )} + {typeof resources !== "undefined" && resources.length > 0 && ( } + let buildEstimate: number | undefined = 0 + switch (workspace.latest_build.status) { + case "starting": + buildEstimate = template.build_time_stats.start_ms + break + case "stopping": + buildEstimate = template.build_time_stats.stop_ms + break + case "deleting": + buildEstimate = template.build_time_stats.delete_ms + break + default: + // Not in a transition state + return <> + } + const job = workspace.latest_build.job - const status = job.status + if (buildEstimate === undefined) { + return ( +
+ +
+
{`Build ${job.status}`}
+
Unknown ETA
+
+
+ ) + } return (
-
{`Build ${status}`}
+
{`Build ${job.status}`}
- {status === "running" && - estimateFinish( - dayjs(job.started_at), - template.average_build_time_ms, - )[1]} + {job.status === "running" && + estimateFinish(dayjs(job.started_at), buildEstimate)[1]}
diff --git a/site/src/pages/TemplatesPage/TemplatesPageView.tsx b/site/src/pages/TemplatesPage/TemplatesPageView.tsx index b1904c58fe7f8..cbfe68d55039c 100644 --- a/site/src/pages/TemplatesPage/TemplatesPageView.tsx +++ b/site/src/pages/TemplatesPage/TemplatesPageView.tsx @@ -237,7 +237,7 @@ export const TemplatesPageView: FC< style={{ color: theme.palette.text.secondary }} > {formatTemplateBuildTime( - template.average_build_time_ms, + template.build_time_stats.start_ms, )} diff --git a/site/src/util/templates.ts b/site/src/util/templates.ts index ebc3d8ae4a365..3f9aae69e5818 100644 --- a/site/src/util/templates.ts +++ b/site/src/util/templates.ts @@ -13,8 +13,8 @@ export const formatTemplateActiveDevelopers = (num?: number): string => { return num.toString() } -export const formatTemplateBuildTime = (buildTimeMs: number): string => { - return buildTimeMs < 0 +export const formatTemplateBuildTime = (buildTimeMs?: number): string => { + return buildTimeMs === undefined ? "Unknown" : `${Math.round(dayjs.duration(buildTimeMs, "milliseconds").asSeconds())}s` } From 4957b137f4c42e1aed8308d1b6312d4eb1e8a2c3 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Sun, 16 Oct 2022 22:13:35 +0000 Subject: [PATCH 07/11] Add stories --- .../WorkspaceBuildProgress.stories.tsx | 66 +++++++++++++++++++ .../WorkspaceBuildProgress.tsx | 14 ++-- site/src/testHelpers/entities.ts | 6 +- 3 files changed, 78 insertions(+), 8 deletions(-) create mode 100644 site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.stories.tsx diff --git a/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.stories.tsx b/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.stories.tsx new file mode 100644 index 0000000000000..b357c62e48012 --- /dev/null +++ b/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.stories.tsx @@ -0,0 +1,66 @@ +import { ComponentMeta, Story } from "@storybook/react" +import dayjs from "dayjs" +import { + MockProvisionerJob, + MockStartingWorkspace, + MockTemplate, + MockWorkspace, + MockWorkspaceBuild, +} from "../../testHelpers/renderHelpers" +import { + WorkspaceBuildProgress, + WorkspaceBuildProgressProps, +} from "./WorkspaceBuildProgress" + +export default { + title: "components/WorkspaceBuildProgress", + component: WorkspaceBuildProgress, +} as ComponentMeta + +const Template: Story = (args) => ( + +) + +export const Starting = Template.bind({}) +Starting.args = { + template: { + ...MockTemplate, + build_time_stats: { + start_ms: 10000, + }, + }, + workspace: { + ...MockStartingWorkspace, + latest_build: { + ...MockWorkspaceBuild, + status: "starting", + job: { + ...MockProvisionerJob, + started_at: dayjs().add(-5, "second").format(), + status: "running", + }, + }, + }, +} + +export const StartingUnknown = Template.bind({}) +StartingUnknown.args = { + ...Starting.args, + template: { + ...MockTemplate, + build_time_stats: { + start_ms: undefined, + }, + }, +} + +export const StartingPassedEstimate = Template.bind({}) +StartingPassedEstimate.args = { + ...Starting.args, + template: { + ...MockTemplate, + build_time_stats: { + start_ms: 1000, + }, + }, +} diff --git a/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.tsx b/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.tsx index 546e6fd58953e..113a727d9a10f 100644 --- a/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.tsx +++ b/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.tsx @@ -6,7 +6,6 @@ import { FC } from "react" import { MONOSPACE_FONT_FAMILY } from "theme/constants" import duration from "dayjs/plugin/duration" -import { Running } from "components/WorkspaceStatusBadge/WorkspaceStatusBadge.stories" dayjs.extend(duration) @@ -14,10 +13,6 @@ const estimateFinish = ( startedAt: Dayjs, templateAverage: number, ): [number, string] => { - // Buffer the template average to prevent the progress bar from waiting at end. - // Over-promise, under-deliver. - templateAverage *= 1.2 - const realPercentage = dayjs().diff(startedAt) / templateAverage // Showing a full bar is frustrating. @@ -33,10 +28,15 @@ const estimateFinish = ( ] } -export const WorkspaceBuildProgress: FC<{ +export interface WorkspaceBuildProgressProps { workspace: Workspace template?: Template -}> = ({ workspace, template }) => { +} + +export const WorkspaceBuildProgress: FC = ({ + workspace, + template, +}) => { const styles = useStyles() // Template stats not loaded or non-existent diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 3362e968cf607..59abb4a913222 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -185,7 +185,11 @@ export const MockTemplate: TypesGen.Template = { active_version_id: MockTemplateVersion.id, workspace_owner_count: 2, active_user_count: 1, - average_build_time_ms: 123, + build_time_stats: { + start_ms: 1000, + stop_ms: 2000, + delete_ms: 3000, + }, description: "This is a test description.", max_ttl_ms: 24 * 60 * 60 * 1000, min_autostart_interval_ms: 60 * 60 * 1000, From de58d5339859ea76e3ca543d084cd37bbc3ab3b1 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Sun, 16 Oct 2022 22:14:20 +0000 Subject: [PATCH 08/11] fmt --- .../WorkspaceBuildProgress/WorkspaceBuildProgress.stories.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.stories.tsx b/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.stories.tsx index b357c62e48012..7311da5fd301d 100644 --- a/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.stories.tsx +++ b/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.stories.tsx @@ -4,7 +4,6 @@ import { MockProvisionerJob, MockStartingWorkspace, MockTemplate, - MockWorkspace, MockWorkspaceBuild, } from "../../testHelpers/renderHelpers" import { From 6b21f00f7f930e5cb127e17bfdaa9b9d96568df9 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Sun, 16 Oct 2022 22:55:42 +0000 Subject: [PATCH 09/11] Remove visual stutter --- site/src/components/Workspace/Workspace.tsx | 21 +++++- .../WorkspaceBuildProgress.stories.tsx | 22 +------ .../WorkspaceBuildProgress.tsx | 66 +++++++++++-------- 3 files changed, 61 insertions(+), 48 deletions(-) diff --git a/site/src/components/Workspace/Workspace.tsx b/site/src/components/Workspace/Workspace.tsx index 5dc20b1948d8c..f2c22d3864c13 100644 --- a/site/src/components/Workspace/Workspace.tsx +++ b/site/src/components/Workspace/Workspace.tsx @@ -20,7 +20,10 @@ import { WorkspaceSection } from "../WorkspaceSection/WorkspaceSection" import { WorkspaceStats } from "../WorkspaceStats/WorkspaceStats" import { AlertBanner } from "../AlertBanner/AlertBanner" import { useTranslation } from "react-i18next" -import { WorkspaceBuildProgress } from "components/WorkspaceBuildProgress/WorkspaceBuildProgress" +import { + EstimateTransitionTime, + WorkspaceBuildProgress, +} from "components/WorkspaceBuildProgress/WorkspaceBuildProgress" export enum WorkspaceErrors { GET_RESOURCES_ERROR = "getResourcesError", @@ -115,6 +118,15 @@ export const Workspace: FC> = ({ /> ) + let buildTimeEstimate: number | undefined = undefined + let isTransitioning: boolean | undefined = undefined + if (template !== undefined) { + ;[buildTimeEstimate, isTransitioning] = EstimateTransitionTime( + template, + workspace, + ) + } + return ( > = ({ - + {isTransitioning !== undefined && isTransitioning && ( + + )} {typeof resources !== "undefined" && resources.length > 0 && ( = (args) => ( export const Starting = Template.bind({}) Starting.args = { - template: { - ...MockTemplate, - build_time_stats: { - start_ms: 10000, - }, - }, + buildEstimate: 10000, workspace: { ...MockStartingWorkspace, latest_build: { @@ -45,21 +39,11 @@ Starting.args = { export const StartingUnknown = Template.bind({}) StartingUnknown.args = { ...Starting.args, - template: { - ...MockTemplate, - build_time_stats: { - start_ms: undefined, - }, - }, + buildEstimate: undefined, } export const StartingPassedEstimate = Template.bind({}) StartingPassedEstimate.args = { ...Starting.args, - template: { - ...MockTemplate, - build_time_stats: { - start_ms: 1000, - }, - }, + buildEstimate: 1000, } diff --git a/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.tsx b/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.tsx index 113a727d9a10f..4a2374f3c33e0 100644 --- a/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.tsx +++ b/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.tsx @@ -2,17 +2,21 @@ import LinearProgress from "@material-ui/core/LinearProgress" import makeStyles from "@material-ui/core/styles/makeStyles" import { Template, Workspace } from "api/typesGenerated" import dayjs, { Dayjs } from "dayjs" -import { FC } from "react" +import { FC, useEffect, useState } from "react" import { MONOSPACE_FONT_FAMILY } from "theme/constants" import duration from "dayjs/plugin/duration" +import { Transition } from "xstate" dayjs.extend(duration) const estimateFinish = ( startedAt: Dayjs, - templateAverage: number, + templateAverage?: number, ): [number, string] => { + if (templateAverage === undefined) { + return [0, "Unknown"] + } const realPercentage = dayjs().diff(startedAt) / templateAverage // Showing a full bar is frustrating. @@ -30,37 +34,49 @@ const estimateFinish = ( export interface WorkspaceBuildProgressProps { workspace: Workspace - template?: Template + buildEstimate?: number } -export const WorkspaceBuildProgress: FC = ({ - workspace, - template, -}) => { - const styles = useStyles() - - // Template stats not loaded or non-existent - if (!template) { - return <> - } - - let buildEstimate: number | undefined = 0 +// 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": - buildEstimate = template.build_time_stats.start_ms - break + return [template.build_time_stats.start_ms, true] case "stopping": - buildEstimate = template.build_time_stats.stop_ms - break + return [template.build_time_stats.stop_ms, true] case "deleting": - buildEstimate = template.build_time_stats.delete_ms - break + return [template.build_time_stats.delete_ms, true] default: // Not in a transition state - return <> + return [undefined, false] } +} + +export const WorkspaceBuildProgress: FC = ({ + workspace, + buildEstimate, +}) => { + const styles = useStyles() const job = workspace.latest_build.job + + const [progressValue, setProgressValue] = useState(0) + // 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 = () => { + setProgressValue( + estimateFinish(dayjs(job.started_at), buildEstimate)[0] * 100, + ) + } + setTimeout(updateProgress, 100) + }, [progressValue, job.started_at, buildEstimate]) + if (buildEstimate === undefined) { return (
@@ -76,11 +92,7 @@ export const WorkspaceBuildProgress: FC = ({ return (
From 75b32c043c124fe10426e69c4ac59f8b1fb78040 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Sun, 16 Oct 2022 23:00:15 +0000 Subject: [PATCH 10/11] Comment --- .../WorkspaceBuildProgress.tsx | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.tsx b/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.tsx index 4a2374f3c33e0..bb70e3c7196dc 100644 --- a/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.tsx +++ b/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.tsx @@ -20,8 +20,9 @@ const estimateFinish = ( const realPercentage = dayjs().diff(startedAt) / templateAverage // Showing a full bar is frustrating. - if (realPercentage > 0.95) { - return [0.95, "Any moment now..."] + const maxPercentage = 0.99 + if (realPercentage > maxPercentage) { + return [maxPercentage, "Any moment now..."] } return [ @@ -61,22 +62,27 @@ export const WorkspaceBuildProgress: FC = ({ buildEstimate, }) => { const styles = useStyles() - const job = workspace.latest_build.job - const [progressValue, setProgressValue] = useState(0) + // 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") { + setProgressValue(0) + return + } setProgressValue( estimateFinish(dayjs(job.started_at), buildEstimate)[0] * 100, ) } setTimeout(updateProgress, 100) - }, [progressValue, job.started_at, buildEstimate]) + }, [progressValue, job, buildEstimate]) + // buildEstimate may be undefined if the template is new or coderd hasn't + // finished initial metrics collection. if (buildEstimate === undefined) { return (
From c4181bc23cf48997a00799c7596cc4391cb6c242 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Sun, 16 Oct 2022 23:17:42 +0000 Subject: [PATCH 11/11] Fix linting issue --- .../components/WorkspaceBuildProgress/WorkspaceBuildProgress.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.tsx b/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.tsx index bb70e3c7196dc..f9685b8951da4 100644 --- a/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.tsx +++ b/site/src/components/WorkspaceBuildProgress/WorkspaceBuildProgress.tsx @@ -6,7 +6,6 @@ import { FC, useEffect, useState } from "react" import { MONOSPACE_FONT_FAMILY } from "theme/constants" import duration from "dayjs/plugin/duration" -import { Transition } from "xstate" dayjs.extend(duration)