From a96ce2cb7eb53e96e0912dd0767e95eb81fe7539 Mon Sep 17 00:00:00 2001
From: Arul Ajmani <arulajmani@gmail.com>
Date: Tue, 28 Mar 2023 00:07:46 +0000
Subject: [PATCH] spanconfigsqlwatcher: deflake TestSQLWatcherOnEventError

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
---
 .../spanconfigccl/spanconfigsqlwatcherccl/sqlwatcher_test.go  | 4 ++--
 pkg/spanconfig/spanconfigsqlwatcher/sqlwatcher.go             | 3 ++-
 pkg/spanconfig/testing_knobs.go                               | 4 ++++
 3 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/pkg/ccl/spanconfigccl/spanconfigsqlwatcherccl/sqlwatcher_test.go b/pkg/ccl/spanconfigccl/spanconfigsqlwatcherccl/sqlwatcher_test.go
index 800eb6858e43..a83d85d0b826 100644
--- a/pkg/ccl/spanconfigccl/spanconfigsqlwatcherccl/sqlwatcher_test.go
+++ b/pkg/ccl/spanconfigccl/spanconfigsqlwatcherccl/sqlwatcher_test.go
@@ -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,
 		},
 	)
 
diff --git a/pkg/spanconfig/spanconfigsqlwatcher/sqlwatcher.go b/pkg/spanconfig/spanconfigsqlwatcher/sqlwatcher.go
index 745117208136..b01ba29fefb2 100644
--- a/pkg/spanconfig/spanconfigsqlwatcher/sqlwatcher.go
+++ b/pkg/spanconfig/spanconfigsqlwatcher/sqlwatcher.go
@@ -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 {
diff --git a/pkg/spanconfig/testing_knobs.go b/pkg/spanconfig/testing_knobs.go
index 67f3c0f6843f..e6c19893a80e 100644
--- a/pkg/spanconfig/testing_knobs.go
+++ b/pkg/spanconfig/testing_knobs.go
@@ -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)