Skip to content

Commit

Permalink
persistedsqlstats: speed up a test
Browse files Browse the repository at this point in the history
Previously, a single unit test could take on the order of 4 minutes (or
even exceed 5 minute timeout, rarely) because the job monitor checks
whether a cluster setting has been updated only every minute, and we
update the cluster setting twice in a unit test. This commit makes it so
that in a testing setup the check happens every second.

Release note: None
  • Loading branch information
yuzefovich committed Sep 22, 2022
1 parent 2b79929 commit ff682bb
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 5 deletions.
3 changes: 3 additions & 0 deletions pkg/sql/sqlstats/persistedsqlstats/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,9 @@ func New(cfg *Config, memSQLStats *sslocal.SQLStats) *PersistedSQLStats {
scanInterval: defaultScanInterval,
jitterFn: p.jitterInterval,
}
if cfg.Knobs != nil {
p.jobMonitor.testingKnobs.updateCheckInterval = cfg.Knobs.JobMonitorUpdateCheckInterval
}

return p
}
Expand Down
10 changes: 9 additions & 1 deletion pkg/sql/sqlstats/persistedsqlstats/scheduled_job_monitor.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ type jobMonitor struct {
db *kv.DB
scanInterval time.Duration
jitterFn func(time.Duration) time.Duration
testingKnobs struct {
updateCheckInterval time.Duration
}
}

func (j *jobMonitor) start(ctx context.Context, stopper *stop.Stopper) {
Expand All @@ -80,6 +83,11 @@ func (j *jobMonitor) start(ctx context.Context, stopper *stop.Stopper) {
timer.Reset(0)
defer timer.Stop()

updateCheckInterval := time.Minute
if j.testingKnobs.updateCheckInterval != 0 {
updateCheckInterval = j.testingKnobs.updateCheckInterval
}

// This loop runs every minute to check if we need to update the job schedule.
// We only hit the jobs table if the schedule needs to be updated due to a
// change in the recurrence cluster setting or as a scheduled check to
Expand All @@ -97,7 +105,7 @@ func (j *jobMonitor) start(ctx context.Context, stopper *stop.Stopper) {
currentRecurrence = SQLStatsCleanupRecurrence.Get(&j.st.SV)
}

timer.Reset(time.Minute)
timer.Reset(updateCheckInterval)
}
})
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -181,8 +181,7 @@ func TestSQLStatsScheduleOperations(t *testing.T) {
skip.UnderStressRace(t, "test is too slow to run under race")

ctx := context.Background()
helper, helperCleanup := newTestHelper(t, nil /* sqlStatsKnobs */)
helper.sqlDB.SucceedsSoonDuration = 2 * time.Minute
helper, helperCleanup := newTestHelper(t, &sqlstats.TestingKnobs{JobMonitorUpdateCheckInterval: time.Second})
defer helperCleanup()

schedID := getSQLStatsCompactionSchedule(t, helper).ScheduleID()
Expand Down Expand Up @@ -218,7 +217,7 @@ func TestSQLStatsScheduleOperations(t *testing.T) {
helper.sqlDB.Exec(t, "SET CLUSTER SETTING sql.stats.cleanup.recurrence = $1", expr)

var err error
testutils.SucceedsWithin(t, func() error {
testutils.SucceedsSoon(t, func() error {
// Reload schedule from DB.
sj := getSQLStatsCompactionSchedule(t, helper)
err = persistedsqlstats.CheckScheduleAnomaly(sj)
Expand All @@ -227,7 +226,7 @@ func TestSQLStatsScheduleOperations(t *testing.T) {
}
require.Equal(t, expr, sj.ScheduleExpr())
return nil
}, time.Minute*2)
})

require.True(t, errors.Is(
errors.Unwrap(err), persistedsqlstats.ErrScheduleIntervalTooLong),
Expand Down
5 changes: 5 additions & 0 deletions pkg/sql/sqlstats/test_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ type TestingKnobs struct {
// AOSTClause overrides the AS OF SYSTEM TIME clause in queries used in
// persistedsqlstats.
AOSTClause string

// JobMonitorUpdateCheckInterval if non-zero indicates the frequency at
// which the job monitor needs to check whether the schedule needs to be
// updated.
JobMonitorUpdateCheckInterval time.Duration
}

// ModuleTestingKnobs implements base.ModuleTestingKnobs interface.
Expand Down

0 comments on commit ff682bb

Please sign in to comment.