From 4f33078ef0649773842098c684744e44bbc215ef Mon Sep 17 00:00:00 2001 From: Timur Iskhakov Date: Sun, 11 Sep 2022 21:27:33 +0100 Subject: [PATCH 1/7] Compute template average build time --- coderd/database/databasefake/databasefake.go | 8 +++ coderd/database/querier.go | 1 + coderd/database/queries.sql.go | 70 ++++++++++++++++++++ coderd/database/queries/templates.sql | 36 ++++++++++ coderd/metricscache/metricscache.go | 48 +++++++++++++- coderd/templates.go | 2 + codersdk/templates.go | 2 + 7 files changed, 164 insertions(+), 3 deletions(-) diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index b1ab38c600173..d43bdaaf96063 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -1102,6 +1102,14 @@ func (q *fakeQuerier) GetTemplateVersionByJobID(_ context.Context, jobID uuid.UU return database.TemplateVersion{}, sql.ErrNoRows } +func (q *fakeQuerier) GetTemplatesAverageBuildTime(_ context.Context, _ database.GetTemplatesAverageBuildTimeParams) ([]database.GetTemplatesAverageBuildTimeRow, error) { + q.mutex.RLock() + defer q.mutex.RUnlock() + + // TODO + return nil, nil +} + func (q *fakeQuerier) GetParameterSchemasByJobID(_ context.Context, jobID uuid.UUID) ([]database.ParameterSchema, error) { q.mutex.RLock() defer q.mutex.RUnlock() diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 8e270eb73dbe1..454ec001abaf3 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -66,6 +66,7 @@ type querier interface { GetTemplateVersionsByTemplateID(ctx context.Context, arg GetTemplateVersionsByTemplateIDParams) ([]TemplateVersion, error) GetTemplateVersionsCreatedAfter(ctx context.Context, createdAt time.Time) ([]TemplateVersion, error) GetTemplates(ctx context.Context) ([]Template, error) + GetTemplatesAverageBuildTime(ctx context.Context, arg GetTemplatesAverageBuildTimeParams) ([]GetTemplatesAverageBuildTimeRow, error) GetTemplatesWithFilter(ctx context.Context, arg GetTemplatesWithFilterParams) ([]Template, error) GetUnexpiredLicenses(ctx context.Context) ([]License, error) GetUserByEmailOrUsername(ctx context.Context, arg GetUserByEmailOrUsernameParams) (User, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 456d0ccf83305..0245175b58936 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -2172,6 +2172,76 @@ func (q *sqlQuerier) GetTemplates(ctx context.Context) ([]Template, error) { return items, nil } +const getTemplatesAverageBuildTime = `-- name: GetTemplatesAverageBuildTime :many +SELECT + t.id, + AVG(pj.exec_time_sec) AS avg_build_time_sec, + COUNT(pj.id) AS job_count +FROM + (SELECT + id + FROM + templates) AS t +LEFT JOIN + (SELECT + workspace_id, + template_version_id, + job_id + FROM + workspace_builds) AS wb +ON + t.id = wb.workspace_id AND t.active_version_id = wb.template_version_id +LEFT JOIN + (SELECT + id, + TIMESTAMPDIFF(second, started_at, completed_at) AS exec_time_sec + FROM + provisioner_jobs pj + WHERE + (pj.completed_at IS NOT NULL) AND + (pj.completed_at >= $1 AND pj.completed_at <= $2) AND + (pj.cancelled_at IS NULL) AND + ((pj.error IS NULL) OR (pj.error = ''))) AS pj +ON + wb.job_id = pj.id +GROUP BY + t.id +` + +type GetTemplatesAverageBuildTimeParams struct { + StartTs sql.NullTime `db:"start_ts" json:"start_ts"` + EndTs sql.NullTime `db:"end_ts" json:"end_ts"` +} + +type GetTemplatesAverageBuildTimeRow struct { + ID uuid.UUID `db:"id" json:"id"` + AvgBuildTimeSec string `db:"avg_build_time_sec" json:"avg_build_time_sec"` + JobCount int64 `db:"job_count" json:"job_count"` +} + +func (q *sqlQuerier) GetTemplatesAverageBuildTime(ctx context.Context, arg GetTemplatesAverageBuildTimeParams) ([]GetTemplatesAverageBuildTimeRow, error) { + rows, err := q.db.QueryContext(ctx, getTemplatesAverageBuildTime, arg.StartTs, arg.EndTs) + if err != nil { + return nil, err + } + defer rows.Close() + var items []GetTemplatesAverageBuildTimeRow + for rows.Next() { + var i GetTemplatesAverageBuildTimeRow + if err := rows.Scan(&i.ID, &i.AvgBuildTimeSec, &i.JobCount); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const getTemplatesWithFilter = `-- name: GetTemplatesWithFilter :many SELECT id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, max_ttl, min_autostart_interval, created_by, icon diff --git a/coderd/database/queries/templates.sql b/coderd/database/queries/templates.sql index 4d552443356fe..114d9e41a9790 100644 --- a/coderd/database/queries/templates.sql +++ b/coderd/database/queries/templates.sql @@ -105,3 +105,39 @@ WHERE id = $1 RETURNING *; + +-- name: GetTemplatesAverageBuildTime :many +SELECT + t.id, + AVG(pj.exec_time_sec) AS avg_build_time_sec, + COUNT(pj.id) AS job_count +FROM + (SELECT + id + FROM + templates) AS t +LEFT JOIN + (SELECT + workspace_id, + template_version_id, + job_id + FROM + workspace_builds) AS wb +ON + t.id = wb.workspace_id AND t.active_version_id = wb.template_version_id +LEFT JOIN + (SELECT + id, + TIMESTAMPDIFF(second, started_at, completed_at) AS exec_time_sec + FROM + provisioner_jobs pj + WHERE + (pj.completed_at IS NOT NULL) AND + (pj.completed_at >= @start_ts AND pj.completed_at <= @end_ts) AND + (pj.cancelled_at IS NULL) AND + ((pj.error IS NULL) OR (pj.error = ''))) AS pj +ON + wb.job_id = pj.id +GROUP BY + t.id +; diff --git a/coderd/metricscache/metricscache.go b/coderd/metricscache/metricscache.go index da38f54c840f0..b58f0c6ade571 100644 --- a/coderd/metricscache/metricscache.go +++ b/coderd/metricscache/metricscache.go @@ -2,6 +2,8 @@ package metricscache import ( "context" + "database/sql" + "strconv" "sync/atomic" "time" @@ -17,7 +19,7 @@ import ( "github.com/coder/retry" ) -// Cache holds the template DAU cache. +// Cache holds the template metrics. // The aggregation queries responsible for these values can take up to a minute // on large deployments. Even in small deployments, aggregation queries can // take a few hundred milliseconds, which would ruin page load times and @@ -26,8 +28,9 @@ type Cache struct { database database.Store log slog.Logger - templateDAUResponses atomic.Pointer[map[uuid.UUID]codersdk.TemplateDAUsResponse] - templateUniqueUsers atomic.Pointer[map[uuid.UUID]int] + templateDAUResponses atomic.Pointer[map[uuid.UUID]codersdk.TemplateDAUsResponse] + templateUniqueUsers atomic.Pointer[map[uuid.UUID]int] + templateAverageBuildTimeSec atomic.Pointer[map[uuid.UUID]float64] done chan struct{} cancel func() @@ -116,6 +119,24 @@ func countUniqueUsers(rows []database.GetTemplateDAUsRow) int { return len(seen) } +func (c *Cache) computeAverageBuildTime(ctx context.Context, now time.Time) (map[uuid.UUID]float64, error) { + records, err := c.database.GetTemplatesAverageBuildTime(ctx, database.GetTemplatesAverageBuildTimeParams{StartTs: sql.NullTime{Time: now.Add(-time.Hour * 24)}, EndTs: sql.NullTime{Time: now}}) + if err != nil { + return nil, err + } + + var ret = make(map[uuid.UUID]float64) + for _, record := range records { + if record.JobCount >= 10 { + val, err := strconv.ParseFloat(record.AvgBuildTimeSec, 64) + if err != nil { + ret[record.ID] = val + } + } + } + return ret, nil +} + func (c *Cache) refresh(ctx context.Context) error { err := c.database.DeleteOldAgentStats(ctx) if err != nil { @@ -142,6 +163,12 @@ func (c *Cache) refresh(ctx context.Context) error { c.templateDAUResponses.Store(&templateDAUs) c.templateUniqueUsers.Store(&templateUniqueUsers) + templateAverageBuildTime, err := c.computeAverageBuildTime(ctx, time.Now()) + if err != nil { + return err + } + c.templateAverageBuildTimeSec.Store(&templateAverageBuildTime) + return nil } @@ -220,3 +247,18 @@ func (c *Cache) TemplateUniqueUsers(id uuid.UUID) (int, bool) { } return resp, true } + +func (c *Cache) TemplateAverageBuildTimeSec(id uuid.UUID) (float64, bool) { + m := c.templateAverageBuildTimeSec.Load() + if m == nil { + // Data loading. + return -1, false + } + + resp, ok := (*m)[id] + if !ok { + // No data or not enough builds. + return -1, false + } + return resp, true +} diff --git a/coderd/templates.go b/coderd/templates.go index c48531a25c226..43bee7807fe63 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -759,6 +759,7 @@ func (api *API) convertTemplate( template database.Template, workspaceOwnerCount uint32, createdByName string, ) codersdk.Template { activeCount, _ := api.metricsCache.TemplateUniqueUsers(template.ID) + averageBuildTimeSec, _ := api.metricsCache.TemplateAverageBuildTimeSec(template.ID) return codersdk.Template{ ID: template.ID, CreatedAt: template.CreatedAt, @@ -769,6 +770,7 @@ func (api *API) convertTemplate( ActiveVersionID: template.ActiveVersionID, WorkspaceOwnerCount: workspaceOwnerCount, ActiveUserCount: activeCount, + AverageBuildTimeSec: averageBuildTimeSec, Description: template.Description, Icon: template.Icon, MaxTTLMillis: time.Duration(template.MaxTtl).Milliseconds(), diff --git a/codersdk/templates.go b/codersdk/templates.go index 3af058cc19719..cd515059f40e9 100644 --- a/codersdk/templates.go +++ b/codersdk/templates.go @@ -24,6 +24,8 @@ type Template struct { WorkspaceOwnerCount uint32 `json:"workspace_owner_count"` // ActiveUserCount is set to -1 when loading. ActiveUserCount int `json:"active_user_count"` + // AverageBuildTimeSec is set to -1 when there aren't enough recent builds. + AverageBuildTimeSec float64 `json:"average_build_time_sec"` Description string `json:"description"` Icon string `json:"icon"` MaxTTLMillis int64 `json:"max_ttl_ms"` From d0abdbe8ac1268e056fa3b31c55cf3ee94291109 Mon Sep 17 00:00:00 2001 From: Timur Iskhakov Date: Mon, 12 Sep 2022 00:14:40 +0100 Subject: [PATCH 2/7] Improve sql query & add front end --- coderd/database/queries.sql.go | 68 ++++++++++++------- coderd/database/queries/templates.sql | 52 ++++++++------ coderd/metricscache/metricscache.go | 18 +++-- site/src/api/typesGenerated.ts | 1 + .../pages/TemplatesPage/TemplatesPageView.tsx | 18 +++-- site/src/testHelpers/entities.ts | 1 + 6 files changed, 105 insertions(+), 53 deletions(-) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 0245175b58936..fcaae2910f920 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -2173,54 +2173,74 @@ func (q *sqlQuerier) GetTemplates(ctx context.Context) ([]Template, error) { } const getTemplatesAverageBuildTime = `-- name: GetTemplatesAverageBuildTime :many -SELECT - t.id, - AVG(pj.exec_time_sec) AS avg_build_time_sec, - COUNT(pj.id) AS job_count +WITH query_with_all_job_count AS (SELECT + DISTINCT t.id, + AVG(pj.exec_time_sec) + OVER(PARTITION BY t.id ORDER BY pj.completed_at ROWS BETWEEN $2::integer PRECEDING AND CURRENT ROW) + AS avg_build_time_sec, + COUNT(*) OVER(PARTITION BY t.id) as job_count FROM (SELECT - id + id, + active_version_id FROM templates) AS t LEFT JOIN (SELECT - workspace_id, - template_version_id, - job_id - FROM - workspace_builds) AS wb + workspace_id, + template_version_id, + job_id + FROM + workspace_builds) + AS + wb ON t.id = wb.workspace_id AND t.active_version_id = wb.template_version_id LEFT JOIN (SELECT id, - TIMESTAMPDIFF(second, started_at, completed_at) AS exec_time_sec + completed_at, + EXTRACT(EPOCH FROM (completed_at - started_at)) AS exec_time_sec FROM - provisioner_jobs pj + provisioner_jobs WHERE - (pj.completed_at IS NOT NULL) AND - (pj.completed_at >= $1 AND pj.completed_at <= $2) AND - (pj.cancelled_at IS NULL) AND - ((pj.error IS NULL) OR (pj.error = ''))) AS pj + (completed_at IS NOT NULL) AND (started_at IS NOT NULL) AND + (completed_at >= $3 AND completed_at <= $4) AND + (canceled_at IS NULL) AND + ((error IS NULL) OR (error = ''))) + AS + pj ON - wb.job_id = pj.id -GROUP BY - t.id + wb.job_id = pj.id) +SELECT + id, + avg_build_time_sec +FROM + query_with_all_job_count +WHERE + avg_build_time_sec IS NOT NULL AND + job_count >= GREATEST($1::integer, 1) ` type GetTemplatesAverageBuildTimeParams struct { - StartTs sql.NullTime `db:"start_ts" json:"start_ts"` - EndTs sql.NullTime `db:"end_ts" json:"end_ts"` + MinCompletedJobCount int32 `db:"min_completed_job_count" json:"min_completed_job_count"` + MovingAverageSize int32 `db:"moving_average_size" json:"moving_average_size"` + StartTs sql.NullTime `db:"start_ts" json:"start_ts"` + EndTs sql.NullTime `db:"end_ts" json:"end_ts"` } type GetTemplatesAverageBuildTimeRow struct { ID uuid.UUID `db:"id" json:"id"` AvgBuildTimeSec string `db:"avg_build_time_sec" json:"avg_build_time_sec"` - JobCount int64 `db:"job_count" json:"job_count"` } func (q *sqlQuerier) GetTemplatesAverageBuildTime(ctx context.Context, arg GetTemplatesAverageBuildTimeParams) ([]GetTemplatesAverageBuildTimeRow, error) { - rows, err := q.db.QueryContext(ctx, getTemplatesAverageBuildTime, arg.StartTs, arg.EndTs) + rows, err := q.db.QueryContext(ctx, getTemplatesAverageBuildTime, + arg.MinCompletedJobCount, + arg.MovingAverageSize, + arg.StartTs, + arg.EndTs, + ) if err != nil { return nil, err } @@ -2228,7 +2248,7 @@ func (q *sqlQuerier) GetTemplatesAverageBuildTime(ctx context.Context, arg GetTe var items []GetTemplatesAverageBuildTimeRow for rows.Next() { var i GetTemplatesAverageBuildTimeRow - if err := rows.Scan(&i.ID, &i.AvgBuildTimeSec, &i.JobCount); err != nil { + if err := rows.Scan(&i.ID, &i.AvgBuildTimeSec); err != nil { return nil, err } items = append(items, i) diff --git a/coderd/database/queries/templates.sql b/coderd/database/queries/templates.sql index 114d9e41a9790..cd819698eb5da 100644 --- a/coderd/database/queries/templates.sql +++ b/coderd/database/queries/templates.sql @@ -107,37 +107,51 @@ RETURNING *; -- name: GetTemplatesAverageBuildTime :many -SELECT - t.id, - AVG(pj.exec_time_sec) AS avg_build_time_sec, - COUNT(pj.id) AS job_count +WITH query_with_all_job_count AS (SELECT + DISTINCT t.id, + AVG(pj.exec_time_sec) + OVER(PARTITION BY t.id ORDER BY pj.completed_at ROWS BETWEEN @moving_average_size::integer PRECEDING AND CURRENT ROW) + AS avg_build_time_sec, + COUNT(*) OVER(PARTITION BY t.id) as job_count FROM (SELECT - id + id, + active_version_id FROM templates) AS t LEFT JOIN (SELECT - workspace_id, - template_version_id, - job_id - FROM - workspace_builds) AS wb + workspace_id, + template_version_id, + job_id + FROM + workspace_builds) + AS + wb ON t.id = wb.workspace_id AND t.active_version_id = wb.template_version_id LEFT JOIN (SELECT id, - TIMESTAMPDIFF(second, started_at, completed_at) AS exec_time_sec + completed_at, + EXTRACT(EPOCH FROM (completed_at - started_at)) AS exec_time_sec FROM - provisioner_jobs pj + provisioner_jobs WHERE - (pj.completed_at IS NOT NULL) AND - (pj.completed_at >= @start_ts AND pj.completed_at <= @end_ts) AND - (pj.cancelled_at IS NULL) AND - ((pj.error IS NULL) OR (pj.error = ''))) AS pj + (completed_at IS NOT NULL) AND (started_at IS NOT NULL) AND + (completed_at >= @start_ts AND completed_at <= @end_ts) AND + (canceled_at IS NULL) AND + ((error IS NULL) OR (error = ''))) + AS + pj ON - wb.job_id = pj.id -GROUP BY - t.id + wb.job_id = pj.id) +SELECT + id, + avg_build_time_sec +FROM + query_with_all_job_count +WHERE + avg_build_time_sec IS NOT NULL AND + job_count >= GREATEST(@min_completed_job_count::integer, 1) ; diff --git a/coderd/metricscache/metricscache.go b/coderd/metricscache/metricscache.go index b58f0c6ade571..486eb5d01a6cc 100644 --- a/coderd/metricscache/metricscache.go +++ b/coderd/metricscache/metricscache.go @@ -120,19 +120,25 @@ func countUniqueUsers(rows []database.GetTemplateDAUsRow) int { } func (c *Cache) computeAverageBuildTime(ctx context.Context, now time.Time) (map[uuid.UUID]float64, error) { - records, err := c.database.GetTemplatesAverageBuildTime(ctx, database.GetTemplatesAverageBuildTimeParams{StartTs: sql.NullTime{Time: now.Add(-time.Hour * 24)}, EndTs: sql.NullTime{Time: now}}) + records, err := c.database.GetTemplatesAverageBuildTime( + ctx, + database.GetTemplatesAverageBuildTimeParams{ + StartTs: sql.NullTime{Time: now.Add(-time.Hour * 24)}, + EndTs: sql.NullTime{Time: now}, + MinCompletedJobCount: 1, + MovingAverageSize: 10, + }) if err != nil { return nil, err } var ret = make(map[uuid.UUID]float64) for _, record := range records { - if record.JobCount >= 10 { - val, err := strconv.ParseFloat(record.AvgBuildTimeSec, 64) - if err != nil { - ret[record.ID] = val - } + val, err := strconv.ParseFloat(record.AvgBuildTimeSec, 64) + if err == nil { + return nil, err } + ret[record.ID] = val } return ret, nil } diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 9b4f4ad243e2d..3462a92964cea 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -379,6 +379,7 @@ export interface Template { readonly active_version_id: string readonly workspace_owner_count: number readonly active_user_count: number + readonly average_build_time_sec: number readonly description: string readonly icon: string readonly max_ttl_ms: number diff --git a/site/src/pages/TemplatesPage/TemplatesPageView.tsx b/site/src/pages/TemplatesPage/TemplatesPageView.tsx index b40dab7b21d2d..c933643fe1582 100644 --- a/site/src/pages/TemplatesPage/TemplatesPageView.tsx +++ b/site/src/pages/TemplatesPage/TemplatesPageView.tsx @@ -34,10 +34,13 @@ import { } from "../../components/Tooltips/HelpTooltip/HelpTooltip" export const Language = { + buildTime: (buildTime: number): string => + buildTime === -1 ? "Unknown" : `${buildTime / 60}:${buildTime % 60}`, developerCount: (activeCount: number): string => { return `${formatTemplateActiveDevelopers(activeCount)} developer${activeCount !== 1 ? "s" : ""}` }, nameLabel: "Name", + buildTimeLabel: "Build time", usedByLabel: "Used by", lastUpdatedLabel: "Last updated", emptyViewNoPerms: @@ -118,10 +121,11 @@ export const TemplatesPageView: FC - {Language.nameLabel} - {Language.usedByLabel} - {Language.lastUpdatedLabel} - {Language.createdByLabel} + {Language.nameLabel} + {Language.buildTimeLabel} + {Language.usedByLabel} + {Language.lastUpdatedLabel} + {Language.createdByLabel} @@ -175,6 +179,12 @@ export const TemplatesPageView: FC + + + {Language.buildTime(template.average_build_time_sec)} + + + {Language.developerCount(template.active_user_count)} diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 106036783571e..17ba74d5e05f0 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -177,6 +177,7 @@ export const MockTemplate: TypesGen.Template = { active_version_id: MockTemplateVersion.id, workspace_owner_count: 2, active_user_count: 1, + average_build_time_sec: 123, description: "This is a test description.", max_ttl_ms: 24 * 60 * 60 * 1000, min_autostart_interval_ms: 60 * 60 * 1000, From 5bac9fc8baff1197a1d2834ed49db0faab55db86 Mon Sep 17 00:00:00 2001 From: Timur Iskhakov Date: Mon, 12 Sep 2022 00:18:38 +0100 Subject: [PATCH 3/7] sql style --- coderd/database/queries/templates.sql | 33 +++++++++++++++------------ 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/coderd/database/queries/templates.sql b/coderd/database/queries/templates.sql index cd819698eb5da..667946854b711 100644 --- a/coderd/database/queries/templates.sql +++ b/coderd/database/queries/templates.sql @@ -108,9 +108,12 @@ RETURNING -- name: GetTemplatesAverageBuildTime :many WITH query_with_all_job_count AS (SELECT - DISTINCT t.id, + DISTINCT t.id, AVG(pj.exec_time_sec) - OVER(PARTITION BY t.id ORDER BY pj.completed_at ROWS BETWEEN @moving_average_size::integer PRECEDING AND CURRENT ROW) + OVER( + PARTITION BY t.id + ORDER BY pj.completed_at + ROWS BETWEEN @moving_average_size::integer PRECEDING AND CURRENT ROW) AS avg_build_time_sec, COUNT(*) OVER(PARTITION BY t.id) as job_count FROM @@ -121,33 +124,33 @@ FROM templates) AS t LEFT JOIN (SELECT - workspace_id, - template_version_id, - job_id + workspace_id, + template_version_id, + job_id FROM - workspace_builds) + workspace_builds) AS - wb + wb ON - t.id = wb.workspace_id AND t.active_version_id = wb.template_version_id + t.id = wb.workspace_id AND t.active_version_id = wb.template_version_id LEFT JOIN (SELECT - id, + id, completed_at, EXTRACT(EPOCH FROM (completed_at - started_at)) AS exec_time_sec FROM - provisioner_jobs + provisioner_jobs WHERE - (completed_at IS NOT NULL) AND (started_at IS NOT NULL) AND + (completed_at IS NOT NULL) AND (started_at IS NOT NULL) AND (completed_at >= @start_ts AND completed_at <= @end_ts) AND - (canceled_at IS NULL) AND - ((error IS NULL) OR (error = ''))) + (canceled_at IS NULL) AND + ((error IS NULL) OR (error = ''))) AS - pj + pj ON wb.job_id = pj.id) SELECT - id, + id, avg_build_time_sec FROM query_with_all_job_count From 151a76bdd738236ceb1dd8f94d0323b629130f83 Mon Sep 17 00:00:00 2001 From: Timur Iskhakov Date: Mon, 12 Sep 2022 13:22:07 +0100 Subject: [PATCH 4/7] Simplify sql query --- coderd/database/querier.go | 3 ++ coderd/database/queries.sql.go | 43 +++++++++++++++------------ coderd/database/queries/templates.sql | 10 ++++--- 3 files changed, 33 insertions(+), 23 deletions(-) diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 454ec001abaf3..798ae30d09be6 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -66,6 +66,9 @@ type querier interface { GetTemplateVersionsByTemplateID(ctx context.Context, arg GetTemplateVersionsByTemplateIDParams) ([]TemplateVersion, error) GetTemplateVersionsCreatedAfter(ctx context.Context, createdAt time.Time) ([]TemplateVersion, error) GetTemplates(ctx context.Context) ([]Template, error) + // Computes average build time for every template. + // Only considers last moving_average_size successful builds between start_ts and end_ts. + // If a template does not have at least min_completed_job_count such builds, it gets skipped. GetTemplatesAverageBuildTime(ctx context.Context, arg GetTemplatesAverageBuildTimeParams) ([]GetTemplatesAverageBuildTimeRow, error) GetTemplatesWithFilter(ctx context.Context, arg GetTemplatesWithFilterParams) ([]Template, error) GetUnexpiredLicenses(ctx context.Context) ([]License, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index fcaae2910f920..df872218cac58 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -2174,9 +2174,12 @@ func (q *sqlQuerier) GetTemplates(ctx context.Context) ([]Template, error) { const getTemplatesAverageBuildTime = `-- name: GetTemplatesAverageBuildTime :many WITH query_with_all_job_count AS (SELECT - DISTINCT t.id, + DISTINCT t.id, AVG(pj.exec_time_sec) - OVER(PARTITION BY t.id ORDER BY pj.completed_at ROWS BETWEEN $2::integer PRECEDING AND CURRENT ROW) + OVER( + PARTITION BY t.id + ORDER BY pj.completed_at + ROWS BETWEEN $2::integer PRECEDING AND CURRENT ROW) AS avg_build_time_sec, COUNT(*) OVER(PARTITION BY t.id) as job_count FROM @@ -2185,41 +2188,40 @@ FROM active_version_id FROM templates) AS t -LEFT JOIN +INNER JOIN (SELECT - workspace_id, - template_version_id, - job_id + workspace_id, + template_version_id, + job_id FROM - workspace_builds) + workspace_builds) AS - wb + wb ON - t.id = wb.workspace_id AND t.active_version_id = wb.template_version_id -LEFT JOIN + t.id = wb.workspace_id AND t.active_version_id = wb.template_version_id +INNER JOIN (SELECT - id, + id, completed_at, EXTRACT(EPOCH FROM (completed_at - started_at)) AS exec_time_sec FROM - provisioner_jobs + provisioner_jobs WHERE - (completed_at IS NOT NULL) AND (started_at IS NOT NULL) AND + (completed_at IS NOT NULL) AND (started_at IS NOT NULL) AND (completed_at >= $3 AND completed_at <= $4) AND - (canceled_at IS NULL) AND - ((error IS NULL) OR (error = ''))) + (canceled_at IS NULL) AND + ((error IS NULL) OR (error = ''))) AS - pj + pj ON wb.job_id = pj.id) SELECT - id, + id, avg_build_time_sec FROM query_with_all_job_count WHERE - avg_build_time_sec IS NOT NULL AND - job_count >= GREATEST($1::integer, 1) + job_count >= $1::integer ` type GetTemplatesAverageBuildTimeParams struct { @@ -2234,6 +2236,9 @@ type GetTemplatesAverageBuildTimeRow struct { AvgBuildTimeSec string `db:"avg_build_time_sec" json:"avg_build_time_sec"` } +// Computes average build time for every template. +// Only considers last moving_average_size successful builds between start_ts and end_ts. +// If a template does not have at least min_completed_job_count such builds, it gets skipped. func (q *sqlQuerier) GetTemplatesAverageBuildTime(ctx context.Context, arg GetTemplatesAverageBuildTimeParams) ([]GetTemplatesAverageBuildTimeRow, error) { rows, err := q.db.QueryContext(ctx, getTemplatesAverageBuildTime, arg.MinCompletedJobCount, diff --git a/coderd/database/queries/templates.sql b/coderd/database/queries/templates.sql index 667946854b711..88f45ea750de4 100644 --- a/coderd/database/queries/templates.sql +++ b/coderd/database/queries/templates.sql @@ -107,6 +107,9 @@ RETURNING *; -- name: GetTemplatesAverageBuildTime :many +-- Computes average build time for every template. +-- Only considers last moving_average_size successful builds between start_ts and end_ts. +-- If a template does not have at least min_completed_job_count such builds, it gets skipped. WITH query_with_all_job_count AS (SELECT DISTINCT t.id, AVG(pj.exec_time_sec) @@ -122,7 +125,7 @@ FROM active_version_id FROM templates) AS t -LEFT JOIN +INNER JOIN (SELECT workspace_id, template_version_id, @@ -133,7 +136,7 @@ LEFT JOIN wb ON t.id = wb.workspace_id AND t.active_version_id = wb.template_version_id -LEFT JOIN +INNER JOIN (SELECT id, completed_at, @@ -155,6 +158,5 @@ SELECT FROM query_with_all_job_count WHERE - avg_build_time_sec IS NOT NULL AND - job_count >= GREATEST(@min_completed_job_count::integer, 1) + job_count >= @min_completed_job_count::integer ; From 96b358f63cce6d8c7ec64729b69c8b8a8a757903 Mon Sep 17 00:00:00 2001 From: Timur Iskhakov Date: Mon, 12 Sep 2022 16:49:50 +0100 Subject: [PATCH 5/7] Use humanised duration --- site/src/pages/TemplatesPage/TemplatesPageView.tsx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/site/src/pages/TemplatesPage/TemplatesPageView.tsx b/site/src/pages/TemplatesPage/TemplatesPageView.tsx index c933643fe1582..ea66b92090008 100644 --- a/site/src/pages/TemplatesPage/TemplatesPageView.tsx +++ b/site/src/pages/TemplatesPage/TemplatesPageView.tsx @@ -10,6 +10,7 @@ import KeyboardArrowRight from "@material-ui/icons/KeyboardArrowRight" import useTheme from "@material-ui/styles/useTheme" import { FC } from "react" import { useNavigate } from "react-router-dom" +import dayjs from "dayjs" import { createDayString } from "util/createDayString" import { formatTemplateActiveDevelopers } from "util/templates" import * as TypesGen from "../../api/typesGenerated" @@ -34,8 +35,8 @@ import { } from "../../components/Tooltips/HelpTooltip/HelpTooltip" export const Language = { - buildTime: (buildTime: number): string => - buildTime === -1 ? "Unknown" : `${buildTime / 60}:${buildTime % 60}`, + buildTime: (buildTimeSec: number): string => + buildTimeSec === -1 ? "Unknown" : dayjs.duration(buildTimeSec, "seconds").humanize(), developerCount: (activeCount: number): string => { return `${formatTemplateActiveDevelopers(activeCount)} developer${activeCount !== 1 ? "s" : ""}` }, From 5c4227ad04f1905c304865b29b8a1c4f2679f4f3 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Fri, 14 Oct 2022 18:27:09 +0000 Subject: [PATCH 6/7] Write and test metricscache --- coderd/database/databasefake/databasefake.go | 37 ++++- coderd/database/querier.go | 5 +- coderd/database/queries.sql.go | 130 +++++----------- coderd/database/queries/templates.sql | 68 +++------ coderd/metricscache/metricscache.go | 63 +++----- coderd/metricscache/metricscache_test.go | 153 ++++++++++++++++++- coderd/templates.go | 4 +- codersdk/templates.go | 6 +- provisionerd/proto/provisionerd_drpc.pb.go | 2 +- provisionersdk/proto/provisioner_drpc.pb.go | 2 +- 10 files changed, 266 insertions(+), 204 deletions(-) diff --git a/coderd/database/databasefake/databasefake.go b/coderd/database/databasefake/databasefake.go index 351cc1b2e6ada..37b4a5a59318a 100644 --- a/coderd/database/databasefake/databasefake.go +++ b/coderd/database/databasefake/databasefake.go @@ -235,6 +235,35 @@ 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 + for _, wb := range q.workspaceBuilds { + if wb.Transition != database.WorkspaceTransitionStart { + continue + } + version, err := q.GetTemplateVersionByID(ctx, wb.TemplateVersionID) + if err != nil { + return -1, err + } + if version.TemplateID != arg.TemplateID { + continue + } + + job, err := q.GetProvisionerJobByID(ctx, wb.JobID) + if err != nil { + return -1, err + } + if job.CompletedAt.Valid { + times = append(times, job.CompletedAt.Time.Sub(job.StartedAt.Time).Seconds()) + } + } + sort.Float64s(times) + if len(times) == 0 { + return -1, nil + } + return times[len(times)/2], nil +} + func (q *fakeQuerier) ParameterValue(_ context.Context, id uuid.UUID) (database.ParameterValue, error) { q.mutex.Lock() defer q.mutex.Unlock() @@ -1326,14 +1355,6 @@ func (q *fakeQuerier) GetTemplateVersionByJobID(_ context.Context, jobID uuid.UU return database.TemplateVersion{}, sql.ErrNoRows } -func (q *fakeQuerier) GetTemplatesAverageBuildTime(_ context.Context, _ database.GetTemplatesAverageBuildTimeParams) ([]database.GetTemplatesAverageBuildTimeRow, error) { - q.mutex.RLock() - defer q.mutex.RUnlock() - - // TODO - return nil, nil -} - func (q *fakeQuerier) GetParameterSchemasByJobID(_ context.Context, jobID uuid.UUID) ([]database.ParameterSchema, error) { q.mutex.RLock() defer q.mutex.RUnlock() diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 099b355f82fe0..503bccfbdc855 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -66,6 +66,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) 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) @@ -75,10 +76,6 @@ type sqlcQuerier interface { GetTemplateVersionsByTemplateID(ctx context.Context, arg GetTemplateVersionsByTemplateIDParams) ([]TemplateVersion, error) GetTemplateVersionsCreatedAfter(ctx context.Context, createdAt time.Time) ([]TemplateVersion, error) GetTemplates(ctx context.Context) ([]Template, error) - // Computes average build time for every template. - // Only considers last moving_average_size successful builds between start_ts and end_ts. - // If a template does not have at least min_completed_job_count such builds, it gets skipped. - GetTemplatesAverageBuildTime(ctx context.Context, arg GetTemplatesAverageBuildTimeParams) ([]GetTemplatesAverageBuildTimeRow, error) GetTemplatesWithFilter(ctx context.Context, arg GetTemplatesWithFilterParams) ([]Template, error) GetUnexpiredLicenses(ctx context.Context) ([]License, error) GetUserByEmailOrUsername(ctx context.Context, arg GetUserByEmailOrUsernameParams) (User, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 8be8c349749b7..8f39449c5eedb 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -2551,6 +2551,41 @@ func (q *sqlQuerier) InsertDeploymentID(ctx context.Context, value string) error return err } +const getTemplateAverageBuildTime = `-- name: GetTemplateAverageBuildTime :one +WITH build_times AS ( +SELECT + EXTRACT(EPOCH FROM (pj.completed_at - pj.started_at)) AS exec_time_sec +FROM + workspace_builds +JOIN template_versions ON + workspace_builds.template_version_id = template_versions.id +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 + ((pj.error IS NULL) OR (pj.error = '')) +ORDER BY + workspace_builds.created_at DESC +) +SELECT CAST(PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec) AS FLOAT) FROM build_times +` + +type GetTemplateAverageBuildTimeParams struct { + TemplateID uuid.NullUUID `db:"template_id" json:"template_id"` + StartTime sql.NullTime `db:"start_time" json:"start_time"` +} + +func (q *sqlQuerier) GetTemplateAverageBuildTime(ctx context.Context, arg GetTemplateAverageBuildTimeParams) (float64, error) { + row := q.db.QueryRowContext(ctx, getTemplateAverageBuildTime, arg.TemplateID, arg.StartTime) + var column_1 float64 + err := row.Scan(&column_1) + return column_1, err +} + const getTemplateByID = `-- name: GetTemplateByID :one SELECT id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, max_ttl, min_autostart_interval, created_by, icon, user_acl, group_acl @@ -2671,101 +2706,6 @@ func (q *sqlQuerier) GetTemplates(ctx context.Context) ([]Template, error) { return items, nil } -const getTemplatesAverageBuildTime = `-- name: GetTemplatesAverageBuildTime :many -WITH query_with_all_job_count AS (SELECT - DISTINCT t.id, - AVG(pj.exec_time_sec) - OVER( - PARTITION BY t.id - ORDER BY pj.completed_at - ROWS BETWEEN $2::integer PRECEDING AND CURRENT ROW) - AS avg_build_time_sec, - COUNT(*) OVER(PARTITION BY t.id) as job_count -FROM - (SELECT - id, - active_version_id - FROM - templates) AS t -INNER JOIN - (SELECT - workspace_id, - template_version_id, - job_id - FROM - workspace_builds) - AS - wb -ON - t.id = wb.workspace_id AND t.active_version_id = wb.template_version_id -INNER JOIN - (SELECT - id, - completed_at, - EXTRACT(EPOCH FROM (completed_at - started_at)) AS exec_time_sec - FROM - provisioner_jobs - WHERE - (completed_at IS NOT NULL) AND (started_at IS NOT NULL) AND - (completed_at >= $3 AND completed_at <= $4) AND - (canceled_at IS NULL) AND - ((error IS NULL) OR (error = ''))) - AS - pj -ON - wb.job_id = pj.id) -SELECT - id, - avg_build_time_sec -FROM - query_with_all_job_count -WHERE - job_count >= $1::integer -` - -type GetTemplatesAverageBuildTimeParams struct { - MinCompletedJobCount int32 `db:"min_completed_job_count" json:"min_completed_job_count"` - MovingAverageSize int32 `db:"moving_average_size" json:"moving_average_size"` - StartTs sql.NullTime `db:"start_ts" json:"start_ts"` - EndTs sql.NullTime `db:"end_ts" json:"end_ts"` -} - -type GetTemplatesAverageBuildTimeRow struct { - ID uuid.UUID `db:"id" json:"id"` - AvgBuildTimeSec string `db:"avg_build_time_sec" json:"avg_build_time_sec"` -} - -// Computes average build time for every template. -// Only considers last moving_average_size successful builds between start_ts and end_ts. -// If a template does not have at least min_completed_job_count such builds, it gets skipped. -func (q *sqlQuerier) GetTemplatesAverageBuildTime(ctx context.Context, arg GetTemplatesAverageBuildTimeParams) ([]GetTemplatesAverageBuildTimeRow, error) { - rows, err := q.db.QueryContext(ctx, getTemplatesAverageBuildTime, - arg.MinCompletedJobCount, - arg.MovingAverageSize, - arg.StartTs, - arg.EndTs, - ) - if err != nil { - return nil, err - } - defer rows.Close() - var items []GetTemplatesAverageBuildTimeRow - for rows.Next() { - var i GetTemplatesAverageBuildTimeRow - if err := rows.Scan(&i.ID, &i.AvgBuildTimeSec); err != nil { - return nil, err - } - items = append(items, i) - } - if err := rows.Close(); err != nil { - return nil, err - } - if err := rows.Err(); err != nil { - return nil, err - } - return items, nil -} - const getTemplatesWithFilter = `-- name: GetTemplatesWithFilter :many SELECT id, created_at, updated_at, organization_id, deleted, name, provisioner, active_version_id, description, max_ttl, min_autostart_interval, created_by, icon, user_acl, group_acl diff --git a/coderd/database/queries/templates.sql b/coderd/database/queries/templates.sql index 88f45ea750de4..57092d54cd064 100644 --- a/coderd/database/queries/templates.sql +++ b/coderd/database/queries/templates.sql @@ -106,57 +106,25 @@ WHERE RETURNING *; --- name: GetTemplatesAverageBuildTime :many --- Computes average build time for every template. --- Only considers last moving_average_size successful builds between start_ts and end_ts. --- If a template does not have at least min_completed_job_count such builds, it gets skipped. -WITH query_with_all_job_count AS (SELECT - DISTINCT t.id, - AVG(pj.exec_time_sec) - OVER( - PARTITION BY t.id - ORDER BY pj.completed_at - ROWS BETWEEN @moving_average_size::integer PRECEDING AND CURRENT ROW) - AS avg_build_time_sec, - COUNT(*) OVER(PARTITION BY t.id) as job_count -FROM - (SELECT - id, - active_version_id - FROM - templates) AS t -INNER JOIN - (SELECT - workspace_id, - template_version_id, - job_id - FROM - workspace_builds) - AS - wb -ON - t.id = wb.workspace_id AND t.active_version_id = wb.template_version_id -INNER JOIN - (SELECT - id, - completed_at, - EXTRACT(EPOCH FROM (completed_at - started_at)) AS exec_time_sec - FROM - provisioner_jobs - WHERE - (completed_at IS NOT NULL) AND (started_at IS NOT NULL) AND - (completed_at >= @start_ts AND completed_at <= @end_ts) AND - (canceled_at IS NULL) AND - ((error IS NULL) OR (error = ''))) - AS - pj -ON - wb.job_id = pj.id) +-- name: GetTemplateAverageBuildTime :one +WITH build_times AS ( SELECT - id, - avg_build_time_sec + EXTRACT(EPOCH FROM (pj.completed_at - pj.started_at)) AS exec_time_sec FROM - query_with_all_job_count + workspace_builds +JOIN template_versions ON + workspace_builds.template_version_id = template_versions.id +JOIN provisioner_jobs pj ON + workspace_builds.job_id = pj.id WHERE - job_count >= @min_completed_job_count::integer + 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 + ((pj.error IS NULL) OR (pj.error = '')) +ORDER BY + workspace_builds.created_at DESC +) +SELECT CAST(PERCENTILE_DISC(0.5) WITHIN GROUP(ORDER BY exec_time_sec) AS FLOAT) FROM build_times ; diff --git a/coderd/metricscache/metricscache.go b/coderd/metricscache/metricscache.go index 486eb5d01a6cc..42b8abe9c9d1d 100644 --- a/coderd/metricscache/metricscache.go +++ b/coderd/metricscache/metricscache.go @@ -3,7 +3,6 @@ package metricscache import ( "context" "database/sql" - "strconv" "sync/atomic" "time" @@ -28,9 +27,9 @@ type Cache struct { database database.Store log slog.Logger - templateDAUResponses atomic.Pointer[map[uuid.UUID]codersdk.TemplateDAUsResponse] - templateUniqueUsers atomic.Pointer[map[uuid.UUID]int] - templateAverageBuildTimeSec atomic.Pointer[map[uuid.UUID]float64] + templateDAUResponses atomic.Pointer[map[uuid.UUID]codersdk.TemplateDAUsResponse] + templateUniqueUsers atomic.Pointer[map[uuid.UUID]int] + templateAverageBuildTime atomic.Pointer[map[uuid.UUID]time.Duration] done chan struct{} cancel func() @@ -119,30 +118,6 @@ func countUniqueUsers(rows []database.GetTemplateDAUsRow) int { return len(seen) } -func (c *Cache) computeAverageBuildTime(ctx context.Context, now time.Time) (map[uuid.UUID]float64, error) { - records, err := c.database.GetTemplatesAverageBuildTime( - ctx, - database.GetTemplatesAverageBuildTimeParams{ - StartTs: sql.NullTime{Time: now.Add(-time.Hour * 24)}, - EndTs: sql.NullTime{Time: now}, - MinCompletedJobCount: 1, - MovingAverageSize: 10, - }) - if err != nil { - return nil, err - } - - var ret = make(map[uuid.UUID]float64) - for _, record := range records { - val, err := strconv.ParseFloat(record.AvgBuildTimeSec, 64) - if err == nil { - return nil, err - } - ret[record.ID] = val - } - return ret, nil -} - func (c *Cache) refresh(ctx context.Context) error { err := c.database.DeleteOldAgentStats(ctx) if err != nil { @@ -155,8 +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) + templateDAUs = make(map[uuid.UUID]codersdk.TemplateDAUsResponse, len(templates)) + templateUniqueUsers = make(map[uuid.UUID]int) + templateAverageBuildTimeSec = make(map[uuid.UUID]time.Duration) ) for _, template := range templates { rows, err := c.database.GetTemplateDAUs(ctx, template.ID) @@ -165,15 +141,24 @@ func (c *Cache) refresh(ctx context.Context) error { } templateDAUs[template.ID] = convertDAUResponse(rows) templateUniqueUsers[template.ID] = countUniqueUsers(rows) + avgBuildTime, err := c.database.GetTemplateAverageBuildTime(ctx, database.GetTemplateAverageBuildTimeParams{ + TemplateID: uuid.NullUUID{ + UUID: template.ID, + Valid: true, + }, + StartTime: sql.NullTime{ + Time: database.Time(time.Now().AddDate(0, -30, 0)), + Valid: true, + }, + }) + if err != nil { + return err + } + templateAverageBuildTimeSec[template.ID] = time.Duration(float64(time.Second) * avgBuildTime) } c.templateDAUResponses.Store(&templateDAUs) c.templateUniqueUsers.Store(&templateUniqueUsers) - - templateAverageBuildTime, err := c.computeAverageBuildTime(ctx, time.Now()) - if err != nil { - return err - } - c.templateAverageBuildTimeSec.Store(&templateAverageBuildTime) + c.templateAverageBuildTime.Store(&templateAverageBuildTimeSec) return nil } @@ -254,15 +239,15 @@ func (c *Cache) TemplateUniqueUsers(id uuid.UUID) (int, bool) { return resp, true } -func (c *Cache) TemplateAverageBuildTimeSec(id uuid.UUID) (float64, bool) { - m := c.templateAverageBuildTimeSec.Load() +func (c *Cache) TemplateAverageBuildTime(id uuid.UUID) (time.Duration, bool) { + m := c.templateAverageBuildTime.Load() if m == nil { // Data loading. return -1, false } resp, ok := (*m)[id] - if !ok { + if !ok || resp <= 0 { // No data or not enough builds. return -1, false } diff --git a/coderd/metricscache/metricscache_test.go b/coderd/metricscache/metricscache_test.go index 70d926702e77f..3ea2748331373 100644 --- a/coderd/metricscache/metricscache_test.go +++ b/coderd/metricscache/metricscache_test.go @@ -2,6 +2,7 @@ package metricscache_test import ( "context" + "database/sql" "testing" "time" @@ -20,7 +21,7 @@ func date(year, month, day int) time.Time { return time.Date(year, time.Month(month), day, 0, 0, 0, 0, time.UTC) } -func TestCache(t *testing.T) { +func TestCache_TemplateUsers(t *testing.T) { t.Parallel() var ( @@ -197,3 +198,153 @@ func TestCache(t *testing.T) { }) } } + +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 TestCache_BuildTime(t *testing.T) { + t.Parallel() + + someDay := date(2022, 10, 1) + + type jobParams struct { + startedAt time.Time + completedAt time.Time + } + + type args struct { + rows []jobParams + } + type want struct { + buildTime time.Duration + } + tests := []struct { + name string + args args + want want + }{ + {"empty", args{}, want{-1}}, + {"one", args{ + rows: []jobParams{ + { + startedAt: clockTime(someDay, 10, 1, 0), + completedAt: clockTime(someDay, 10, 1, 10), + }, + }, + }, want{time.Second * 10}, + }, + {"two", args{ + rows: []jobParams{ + { + startedAt: clockTime(someDay, 10, 1, 0), + completedAt: clockTime(someDay, 10, 1, 10), + }, + { + startedAt: clockTime(someDay, 10, 1, 0), + completedAt: clockTime(someDay, 10, 1, 50), + }, + }, + }, want{time.Second * 50}, + }, + {"three", args{ + rows: []jobParams{ + { + startedAt: clockTime(someDay, 10, 1, 0), + completedAt: clockTime(someDay, 10, 1, 10), + }, + { + startedAt: clockTime(someDay, 10, 1, 0), + completedAt: clockTime(someDay, 10, 1, 50), + }, { + startedAt: clockTime(someDay, 10, 1, 0), + completedAt: clockTime(someDay, 10, 1, 20), + }, + }, + }, want{time.Second * 20}, + }, + } + + for _, tt := range tests { + tt := tt + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + ctx := context.Background() + + var ( + db = databasefake.New() + cache = metricscache.New(db, slogtest.Make(t, nil), testutil.IntervalFast) + ) + + defer cache.Close() + + template, err := db.InsertTemplate(ctx, database.InsertTemplateParams{ + ID: uuid.New(), + }) + require.NoError(t, err) + + templateVersion, err := db.InsertTemplateVersion(ctx, database.InsertTemplateVersionParams{ + ID: uuid.New(), + TemplateID: uuid.NullUUID{UUID: template.ID, Valid: true}, + }) + 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) + + for _, row := range tt.args.rows { + _, err := db.InsertProvisionerJob(ctx, database.InsertProvisionerJobParams{ + ID: uuid.New(), + Provisioner: database.ProvisionerTypeEcho, + }) + require.NoError(t, err) + + job, err := db.AcquireProvisionerJob(ctx, database.AcquireProvisionerJobParams{ + StartedAt: sql.NullTime{Time: row.startedAt, Valid: true}, + Types: []database.ProvisionerType{ + database.ProvisionerTypeEcho, + }, + }) + require.NoError(t, err) + + _, err = db.InsertWorkspaceBuild(ctx, database.InsertWorkspaceBuildParams{ + TemplateVersionID: templateVersion.ID, + JobID: job.ID, + Transition: database.WorkspaceTransitionStart, + }) + require.NoError(t, err) + + err = db.UpdateProvisionerJobWithCompleteByID(ctx, database.UpdateProvisionerJobWithCompleteByIDParams{ + ID: job.ID, + CompletedAt: sql.NullTime{Time: row.completedAt, Valid: true}, + }) + require.NoError(t, err) + } + + if tt.want.buildTime > 0 { + require.Eventuallyf(t, func() bool { + _, ok := cache.TemplateAverageBuildTime(template.ID) + return ok + }, testutil.WaitShort, testutil.IntervalMedium, + "TemplateDAUs never populated", + ) + + gotBuildTime, ok = cache.TemplateAverageBuildTime(template.ID) + require.True(t, ok) + require.Equal(t, tt.want.buildTime, gotBuildTime) + } else { + require.Never(t, func() bool { + _, ok := cache.TemplateAverageBuildTime(template.ID) + return ok + }, testutil.WaitShort/2, testutil.IntervalMedium, + "TemplateDAUs never 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 01c4b2b5372f0..bf2962c6acc8c 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -772,7 +772,7 @@ func (api *API) convertTemplate( template database.Template, workspaceOwnerCount uint32, createdByName string, ) codersdk.Template { activeCount, _ := api.metricsCache.TemplateUniqueUsers(template.ID) - averageBuildTimeSec, _ := api.metricsCache.TemplateAverageBuildTimeSec(template.ID) + averageBuildTime, _ := api.metricsCache.TemplateAverageBuildTimeSec(template.ID) return codersdk.Template{ ID: template.ID, CreatedAt: template.CreatedAt, @@ -783,7 +783,7 @@ func (api *API) convertTemplate( ActiveVersionID: template.ActiveVersionID, WorkspaceOwnerCount: workspaceOwnerCount, ActiveUserCount: activeCount, - AverageBuildTimeSec: averageBuildTimeSec, + AverageBuildTimeMillis: int64(averageBuildTime / time.Millisecond), Description: template.Description, Icon: template.Icon, MaxTTLMillis: time.Duration(template.MaxTtl).Milliseconds(), diff --git a/codersdk/templates.go b/codersdk/templates.go index 57a5d06641bf1..887c648b42954 100644 --- a/codersdk/templates.go +++ b/codersdk/templates.go @@ -23,9 +23,9 @@ 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"` - // AverageBuildTimeSec is set to -1 when there aren't enough recent builds. - AverageBuildTimeSec float64 `json:"average_build_time_sec"` + 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"` diff --git a/provisionerd/proto/provisionerd_drpc.pb.go b/provisionerd/proto/provisionerd_drpc.pb.go index 646f855eabc70..7ede95dcad75d 100644 --- a/provisionerd/proto/provisionerd_drpc.pb.go +++ b/provisionerd/proto/provisionerd_drpc.pb.go @@ -1,5 +1,5 @@ // Code generated by protoc-gen-go-drpc. DO NOT EDIT. -// protoc-gen-go-drpc version: v0.0.26 +// protoc-gen-go-drpc version: v0.0.33-0.20220923152156-858cfad9e41d // source: provisionerd/proto/provisionerd.proto package proto diff --git a/provisionersdk/proto/provisioner_drpc.pb.go b/provisionersdk/proto/provisioner_drpc.pb.go index c990f6f645b7f..9931b1d9a3d33 100644 --- a/provisionersdk/proto/provisioner_drpc.pb.go +++ b/provisionersdk/proto/provisioner_drpc.pb.go @@ -1,5 +1,5 @@ // Code generated by protoc-gen-go-drpc. DO NOT EDIT. -// protoc-gen-go-drpc version: v0.0.26 +// protoc-gen-go-drpc version: v0.0.33-0.20220923152156-858cfad9e41d // source: provisionersdk/proto/provisioner.proto package proto From 0f5f07bbc7c4c89a1a3d651985165547463b4ee7 Mon Sep 17 00:00:00 2001 From: Ammar Bandukwala Date: Fri, 14 Oct 2022 20:15:19 +0000 Subject: [PATCH 7/7] Complete tests --- coderd/database/queries.sql.go | 2 +- coderd/database/queries/templates.sql | 2 +- coderd/templates.go | 12 ++++++++++-- coderd/templates_test.go | 4 +++- 4 files changed, 15 insertions(+), 5 deletions(-) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 8f39449c5eedb..5897a7bec76a1 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -2563,7 +2563,7 @@ JOIN provisioner_jobs pj ON workspace_builds.job_id = pj.id WHERE template_versions.template_id = $1 AND - (workspace_builds.transition = "start") 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 diff --git a/coderd/database/queries/templates.sql b/coderd/database/queries/templates.sql index 57092d54cd064..cb4c00c23fc07 100644 --- a/coderd/database/queries/templates.sql +++ b/coderd/database/queries/templates.sql @@ -118,7 +118,7 @@ JOIN provisioner_jobs pj ON workspace_builds.job_id = pj.id WHERE template_versions.template_id = @template_id AND - (workspace_builds.transition = "start") 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 diff --git a/coderd/templates.go b/coderd/templates.go index bf2962c6acc8c..3d84044ffb44a 100644 --- a/coderd/templates.go +++ b/coderd/templates.go @@ -772,7 +772,15 @@ func (api *API) convertTemplate( template database.Template, workspaceOwnerCount uint32, createdByName string, ) codersdk.Template { activeCount, _ := api.metricsCache.TemplateUniqueUsers(template.ID) - averageBuildTime, _ := api.metricsCache.TemplateAverageBuildTimeSec(template.ID) + + var averageBuildTimeMillis int64 + averageBuildTime, ok := api.metricsCache.TemplateAverageBuildTime(template.ID) + if !ok { + averageBuildTimeMillis = -1 + } else { + averageBuildTimeMillis = int64(averageBuildTime / time.Millisecond) + } + return codersdk.Template{ ID: template.ID, CreatedAt: template.CreatedAt, @@ -783,7 +791,7 @@ func (api *API) convertTemplate( ActiveVersionID: template.ActiveVersionID, WorkspaceOwnerCount: workspaceOwnerCount, ActiveUserCount: activeCount, - AverageBuildTimeMillis: int64(averageBuildTime / time.Millisecond), + AverageBuildTimeMillis: 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 bf547c4d0eb9a..b384108387f07 100644 --- a/coderd/templates_test.go +++ b/coderd/templates_test.go @@ -561,7 +561,7 @@ func TestDeleteTemplate(t *testing.T) { }) } -func TestTemplateDAUs(t *testing.T) { +func TestTemplateMetrics(t *testing.T) { t.Parallel() client := coderdtest.New(t, &coderdtest.Options{ @@ -594,6 +594,7 @@ func TestTemplateDAUs(t *testing.T) { }) template := coderdtest.CreateTemplate(t, client, user.OrganizationID, version.ID) require.Equal(t, -1, template.ActiveUserCount) + require.EqualValues(t, -1, template.AverageBuildTimeMillis) coderdtest.AwaitTemplateVersionJob(t, client, version.ID) workspace := coderdtest.CreateWorkspace(t, client, user.OrganizationID, template.ID) @@ -660,6 +661,7 @@ func TestTemplateDAUs(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)) workspaces, err = client.Workspaces(ctx, codersdk.WorkspaceFilter{}) require.NoError(t, err)