Skip to content

Commit 4fab14b

Browse files
cstyancstyan
authored
fix: limit the scope of the template average build time query to the last 100 (#19648)
This PR should resolve coder/internal#719 by limiting the `workspace_builds` rows selected by the query to the most recent 100 builds of a template, as opposed to all builds in the last 30d. For our own internal templates with the most builds (1700-2000 in a 30d period) this should cut the query execution time by about 80%. Unless we have some restriction on keeping the 30d period, contract related or otherwise, this seems like a safe change to make. In addition to the execution speed improvements it also means the memory for the query is bounded as well. If we want to keep a 30d time period for the avg build time value I think it's worth exploring a purpose built solution such as histogram structures where the build times could be bucketized by template ID as they're observed. --------- Signed-off-by: Callum Styan <callumstyan@gmail.com>
1 parent 6574299 commit 4fab14b

File tree

8 files changed

+15
-24
lines changed

8 files changed

+15
-24
lines changed

‎coderd/database/dbauthz/dbauthz.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2792,7 +2792,7 @@ func (q *querier) GetTemplateAppInsightsByTemplate(ctx context.Context, arg data
27922792
}
27932793

27942794
// Only used by metrics cache.
2795-
func (q *querier) GetTemplateAverageBuildTime(ctx context.Context, arg database.GetTemplateAverageBuildTimeParams) (database.GetTemplateAverageBuildTimeRow, error) {
2795+
func (q *querier) GetTemplateAverageBuildTime(ctx context.Context, arg uuid.NullUUID) (database.GetTemplateAverageBuildTimeRow, error) {
27962796
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil {
27972797
return database.GetTemplateAverageBuildTimeRow{}, err
27982798
}

‎coderd/database/dbauthz/dbauthz_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3160,7 +3160,7 @@ func (s *MethodTestSuite) TestSystemFunctions() {
31603160
check.Args(arg).Asserts(rbac.ResourceSystem, policy.ActionRead).Returns(p)
31613161
}))
31623162
s.Run("GetTemplateAverageBuildTime", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) {
3163-
arg := database.GetTemplateAverageBuildTimeParams{}
3163+
arg := uuid.NullUUID{}
31643164
dbm.EXPECT().GetTemplateAverageBuildTime(gomock.Any(), arg).Return(database.GetTemplateAverageBuildTimeRow{}, nil).AnyTimes()
31653165
check.Args(arg).Asserts(rbac.ResourceSystem, policy.ActionRead)
31663166
}))

‎coderd/database/dbmetrics/querymetrics.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/dbmock/dbmock.go

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/querier.go

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries.sql.go

Lines changed: 3 additions & 8 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎coderd/database/queries/templates.sql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -203,11 +203,11 @@ JOIN provisioner_jobs pj ON
203203
WHERE
204204
template_versions.template_id = @template_id AND
205205
(pj.completed_at IS NOT NULL) AND (pj.started_at IS NOT NULL) AND
206-
(pj.started_at > @start_time) AND
207206
(pj.canceled_at IS NULL) AND
208207
((pj.error IS NULL) OR (pj.error = ''))
209208
ORDER BY
210209
workspace_builds.created_at DESC
210+
LIMIT 100
211211
)
212212
SELECT
213213
-- Postgres offers no clear way to DRY this short of a function or other

‎coderd/metricscache/metricscache.go

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -101,16 +101,12 @@ func (c *Cache) refreshTemplateBuildTimes(ctx context.Context) error {
101101
for _, template := range templates {
102102
ids = append(ids, template.ID)
103103

104-
templateAvgBuildTime, err := c.database.GetTemplateAverageBuildTime(ctx, database.GetTemplateAverageBuildTimeParams{
105-
TemplateID: uuid.NullUUID{
104+
templateAvgBuildTime, err := c.database.GetTemplateAverageBuildTime(ctx,
105+
uuid.NullUUID{
106106
UUID: template.ID,
107107
Valid: true,
108108
},
109-
StartTime: sql.NullTime{
110-
Time: dbtime.Time(c.clock.Now().AddDate(0, 0, -30)),
111-
Valid: true,
112-
},
113-
})
109+
)
114110
if err != nil {
115111
return err
116112
}

0 commit comments

Comments
 (0)