Skip to content

Commit

Permalink
spanconfigsqlwatcher: deflake TestSQLWatcherOnEventError
Browse files Browse the repository at this point in the history
Previously, this test was setting the no-op checkpoint duration to be
 every hour to effectively disable checkpoints. Doing so is integral to
what the test is testing. However, this was a lie, given how
`util.Every` works -- A call to `ShouldProcess` returns true the very
first time.

This patch achieves the original goal by introducing a new testing knob.
Previously, the test would fail in < 40 runs locally.  Have this running
strong for ~1000 runs.

Fixes #76765

Release note: None
  • Loading branch information
arulajmani committed Mar 28, 2023
1 parent 84fdc8d commit a96ce2c
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -437,18 +437,18 @@ func TestSQLWatcherOnEventError(t *testing.T) {
tdb.Exec(t, `SET CLUSTER SETTING kv.rangefeed.enabled = true`)
tdb.Exec(t, `SET CLUSTER SETTING kv.closed_timestamp.target_duration = '100ms'`)

noopCheckpointDuration := time.Hour // effectively disable no-op checkpoints
sqlWatcher := spanconfigsqlwatcher.New(
keys.SystemSQLCodec,
ts.ClusterSettings(),
ts.RangeFeedFactory().(*rangefeed.Factory),
1<<20, /* 1 MB, bufferMemLimit */
ts.Stopper(),
noopCheckpointDuration,
time.Second, // doesn't matter
&spanconfig.TestingKnobs{
SQLWatcherOnEventInterceptor: func() error {
return errors.New("boom")
},
SQLWatcherSkipNoopCheckpoints: true,
},
)

Expand Down
3 changes: 2 additions & 1 deletion pkg/spanconfig/spanconfigsqlwatcher/sqlwatcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,8 @@ func (s *SQLWatcher) watch(
if err != nil {
return err
}
if len(sqlUpdates) == 0 && !checkpointNoops.ShouldProcess(timeutil.Now()) {
if len(sqlUpdates) == 0 &&
(!checkpointNoops.ShouldProcess(timeutil.Now()) || s.knobs.SQLWatcherSkipNoopCheckpoints) {
continue
}
if err := handler(ctx, sqlUpdates, combinedFrontierTS); err != nil {
Expand Down
4 changes: 4 additions & 0 deletions pkg/spanconfig/testing_knobs.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ type TestingKnobs struct {
// often the SQLWatcher checkpoints noops.
SQLWatcherCheckpointNoopsEveryDurationOverride time.Duration

// SQLWatcherSkipNoopCheckpoints allows tests to skip no-op checkpoints
// entirely.
SQLWatcherSkipNoopCheckpoints bool

// SplitterStepLogger is used to capture internal steps the splitter is
// making, for debugging and test-readability purposes.
SplitterStepLogger func(string)
Expand Down

0 comments on commit a96ce2c

Please sign in to comment.