mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix: limit the scope of the template average build time query to the last 100 (#19648)
This PR should resolve https://github.com/coder/internal/issues/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>
This commit is contained in:
@@ -2792,7 +2792,7 @@ func (q *querier) GetTemplateAppInsightsByTemplate(ctx context.Context, arg data
|
||||
}
|
||||
|
||||
// Only used by metrics cache.
|
||||
func (q *querier) GetTemplateAverageBuildTime(ctx context.Context, arg database.GetTemplateAverageBuildTimeParams) (database.GetTemplateAverageBuildTimeRow, error) {
|
||||
func (q *querier) GetTemplateAverageBuildTime(ctx context.Context, arg uuid.NullUUID) (database.GetTemplateAverageBuildTimeRow, error) {
|
||||
if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceSystem); err != nil {
|
||||
return database.GetTemplateAverageBuildTimeRow{}, err
|
||||
}
|
||||
|
||||
@@ -3160,7 +3160,7 @@ func (s *MethodTestSuite) TestSystemFunctions() {
|
||||
check.Args(arg).Asserts(rbac.ResourceSystem, policy.ActionRead).Returns(p)
|
||||
}))
|
||||
s.Run("GetTemplateAverageBuildTime", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) {
|
||||
arg := database.GetTemplateAverageBuildTimeParams{}
|
||||
arg := uuid.NullUUID{}
|
||||
dbm.EXPECT().GetTemplateAverageBuildTime(gomock.Any(), arg).Return(database.GetTemplateAverageBuildTimeRow{}, nil).AnyTimes()
|
||||
check.Args(arg).Asserts(rbac.ResourceSystem, policy.ActionRead)
|
||||
}))
|
||||
|
||||
@@ -1447,7 +1447,7 @@ func (m queryMetricsStore) GetTemplateAppInsightsByTemplate(ctx context.Context,
|
||||
return r0, r1
|
||||
}
|
||||
|
||||
func (m queryMetricsStore) GetTemplateAverageBuildTime(ctx context.Context, arg database.GetTemplateAverageBuildTimeParams) (database.GetTemplateAverageBuildTimeRow, error) {
|
||||
func (m queryMetricsStore) GetTemplateAverageBuildTime(ctx context.Context, arg uuid.NullUUID) (database.GetTemplateAverageBuildTimeRow, error) {
|
||||
start := time.Now()
|
||||
buildTime, err := m.s.GetTemplateAverageBuildTime(ctx, arg)
|
||||
m.queryLatencies.WithLabelValues("GetTemplateAverageBuildTime").Observe(time.Since(start).Seconds())
|
||||
|
||||
@@ -3047,18 +3047,18 @@ func (mr *MockStoreMockRecorder) GetTemplateAppInsightsByTemplate(ctx, arg any)
|
||||
}
|
||||
|
||||
// GetTemplateAverageBuildTime mocks base method.
|
||||
func (m *MockStore) GetTemplateAverageBuildTime(ctx context.Context, arg database.GetTemplateAverageBuildTimeParams) (database.GetTemplateAverageBuildTimeRow, error) {
|
||||
func (m *MockStore) GetTemplateAverageBuildTime(ctx context.Context, templateID uuid.NullUUID) (database.GetTemplateAverageBuildTimeRow, error) {
|
||||
m.ctrl.T.Helper()
|
||||
ret := m.ctrl.Call(m, "GetTemplateAverageBuildTime", ctx, arg)
|
||||
ret := m.ctrl.Call(m, "GetTemplateAverageBuildTime", ctx, templateID)
|
||||
ret0, _ := ret[0].(database.GetTemplateAverageBuildTimeRow)
|
||||
ret1, _ := ret[1].(error)
|
||||
return ret0, ret1
|
||||
}
|
||||
|
||||
// GetTemplateAverageBuildTime indicates an expected call of GetTemplateAverageBuildTime.
|
||||
func (mr *MockStoreMockRecorder) GetTemplateAverageBuildTime(ctx, arg any) *gomock.Call {
|
||||
func (mr *MockStoreMockRecorder) GetTemplateAverageBuildTime(ctx, templateID any) *gomock.Call {
|
||||
mr.mock.ctrl.T.Helper()
|
||||
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetTemplateAverageBuildTime", reflect.TypeOf((*MockStore)(nil).GetTemplateAverageBuildTime), ctx, arg)
|
||||
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetTemplateAverageBuildTime", reflect.TypeOf((*MockStore)(nil).GetTemplateAverageBuildTime), ctx, templateID)
|
||||
}
|
||||
|
||||
// GetTemplateByID mocks base method.
|
||||
|
||||
@@ -325,7 +325,7 @@ type sqlcQuerier interface {
|
||||
// GetTemplateAppInsightsByTemplate is used for Prometheus metrics. Keep
|
||||
// in sync with GetTemplateAppInsights and UpsertTemplateUsageStats.
|
||||
GetTemplateAppInsightsByTemplate(ctx context.Context, arg GetTemplateAppInsightsByTemplateParams) ([]GetTemplateAppInsightsByTemplateRow, error)
|
||||
GetTemplateAverageBuildTime(ctx context.Context, arg GetTemplateAverageBuildTimeParams) (GetTemplateAverageBuildTimeRow, error)
|
||||
GetTemplateAverageBuildTime(ctx context.Context, templateID uuid.NullUUID) (GetTemplateAverageBuildTimeRow, error)
|
||||
GetTemplateByID(ctx context.Context, id uuid.UUID) (Template, error)
|
||||
GetTemplateByOrganizationAndName(ctx context.Context, arg GetTemplateByOrganizationAndNameParams) (Template, error)
|
||||
GetTemplateDAUs(ctx context.Context, arg GetTemplateDAUsParams) ([]GetTemplateDAUsRow, error)
|
||||
|
||||
@@ -11903,11 +11903,11 @@ JOIN provisioner_jobs pj ON
|
||||
WHERE
|
||||
template_versions.template_id = $1 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
|
||||
LIMIT 100
|
||||
)
|
||||
SELECT
|
||||
-- Postgres offers no clear way to DRY this short of a function or other
|
||||
@@ -11921,11 +11921,6 @@ SELECT
|
||||
FROM build_times
|
||||
`
|
||||
|
||||
type GetTemplateAverageBuildTimeParams struct {
|
||||
TemplateID uuid.NullUUID `db:"template_id" json:"template_id"`
|
||||
StartTime sql.NullTime `db:"start_time" json:"start_time"`
|
||||
}
|
||||
|
||||
type GetTemplateAverageBuildTimeRow struct {
|
||||
Start50 float64 `db:"start_50" json:"start_50"`
|
||||
Stop50 float64 `db:"stop_50" json:"stop_50"`
|
||||
@@ -11935,8 +11930,8 @@ type GetTemplateAverageBuildTimeRow struct {
|
||||
Delete95 float64 `db:"delete_95" json:"delete_95"`
|
||||
}
|
||||
|
||||
func (q *sqlQuerier) GetTemplateAverageBuildTime(ctx context.Context, arg GetTemplateAverageBuildTimeParams) (GetTemplateAverageBuildTimeRow, error) {
|
||||
row := q.db.QueryRowContext(ctx, getTemplateAverageBuildTime, arg.TemplateID, arg.StartTime)
|
||||
func (q *sqlQuerier) GetTemplateAverageBuildTime(ctx context.Context, templateID uuid.NullUUID) (GetTemplateAverageBuildTimeRow, error) {
|
||||
row := q.db.QueryRowContext(ctx, getTemplateAverageBuildTime, templateID)
|
||||
var i GetTemplateAverageBuildTimeRow
|
||||
err := row.Scan(
|
||||
&i.Start50,
|
||||
|
||||
@@ -203,11 +203,11 @@ JOIN provisioner_jobs pj ON
|
||||
WHERE
|
||||
template_versions.template_id = @template_id 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
|
||||
LIMIT 100
|
||||
)
|
||||
SELECT
|
||||
-- Postgres offers no clear way to DRY this short of a function or other
|
||||
|
||||
@@ -101,16 +101,12 @@ func (c *Cache) refreshTemplateBuildTimes(ctx context.Context) error {
|
||||
for _, template := range templates {
|
||||
ids = append(ids, template.ID)
|
||||
|
||||
templateAvgBuildTime, err := c.database.GetTemplateAverageBuildTime(ctx, database.GetTemplateAverageBuildTimeParams{
|
||||
TemplateID: uuid.NullUUID{
|
||||
templateAvgBuildTime, err := c.database.GetTemplateAverageBuildTime(ctx,
|
||||
uuid.NullUUID{
|
||||
UUID: template.ID,
|
||||
Valid: true,
|
||||
},
|
||||
StartTime: sql.NullTime{
|
||||
Time: dbtime.Time(c.clock.Now().AddDate(0, 0, -30)),
|
||||
Valid: true,
|
||||
},
|
||||
})
|
||||
)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user