sql: deflake TestDropDatabaseDeleteData

This commit deflakes the test by blocking the full reconciliation from
starting before we drop the database. This is used to avoid a race
condition where the full reconciliation starts after we drop the
database, it will ignore the dropped database. Then, there is a race
between the SQLWatcher and the GC job where the SQLWatcher might write a
zone config of table1 before table2, while the GC queue might not know
that this range needs to be split because it has different span config.

Release note: None

Fixes: #138185

Author: iskettaneh

pkg/spanconfig/spanconfigreconciler/reconciler.go

@@ -217,6 +217,10 @@ type fullReconciler struct {
 func (f *fullReconciler) reconcile(
 	ctx context.Context,
 ) (storeWithLatestSpanConfigs *spanconfigstore.Store, _ hlc.Timestamp, _ error) {
+	if f.knobs != nil && f.knobs.OnFullReconcilerStart != nil {
+		f.knobs.OnFullReconcilerStart()
+	}
+
 	storeWithExistingSpanConfigs, err := f.fetchExistingSpanConfigs(ctx)
 	if err != nil {
 		return nil, hlc.Timestamp{}, err

pkg/spanconfig/testing_knobs.go

@@ -127,6 +127,9 @@ type TestingKnobs struct {
 	// fallback config that will be applied to the span.
 	OverrideFallbackConf func(roachpb.SpanConfig) roachpb.SpanConfig
 
+	// OnFullReconcilerStart is invoked when full reconciliation starts.
+	OnFullReconcilerStart func()
+
 	// OnWatchForZoneConfigUpdatesEstablished is invoked when the RangeFeed over
 	// system.zones starts.
 	OnWatchForZoneConfigUpdatesEstablished func()

pkg/sql/drop_test.go

@@ -267,17 +267,26 @@ func TestDropDatabaseDeleteData(t *testing.T) {
 	// Speed up mvcc queue scan.
 	params.ScanMaxIdleTime = time.Millisecond
 
+	// fullReconcilierStarted is used to block the full reconciliation from
+	// starting before we drop the database. This is used to avoid a race
+	// condition where the full reconciliation starts after we drop the database,
+	// it will ignore the dropped database. Then, there is a race between the
+	// SQLWatcher and the GC job where the SQLWatcher might write a zone config
+	// of table1 before table2, while the GC queue might not know that this range
+	// needs to be split because it has different span config.
+	//
 	// zoneCfgRangeFeedStarted is used to signal that the zone config range feed
-	// started. This is used to avoid a race where we update `system.zones` before
-	// the full reconciliation of zone configs has started. The full
-	// reconciliation ignores dropped databases, and if we write to
-	// `system.zones` before that, our write will not be reconciled. By delaying
-	// the test until the zone config range feed starts, we ensure that the full
-	// reconciliation has finished, and the range feed is ready to stream our
-	// changes.
-	var zoneCfgRangeFeedStarted sync.WaitGroup
+	// started and that the full reconciliation has finished. This is used to
+	// avoid a race where we update `system.zones` before the full reconciliation
+	// of zone configs has finished. If we write to `system.zones` before that,
+	// our write will not be reconciled.
+	var fullReconcilierStarted, zoneCfgRangeFeedStarted sync.WaitGroup
+	fullReconcilierStarted.Add(1)
 	zoneCfgRangeFeedStarted.Add(1)
 	params.Knobs.SpanConfig = &spanconfig.TestingKnobs{
+		OnFullReconcilerStart: func() {
+			fullReconcilierStarted.Wait()
+		},
 		OnWatchForZoneConfigUpdatesEstablished: func() {
 			zoneCfgRangeFeedStarted.Done()
 		},
@@ -306,9 +315,6 @@ func TestDropDatabaseDeleteData(t *testing.T) {
 	_, err = systemDB.Exec(`SET CLUSTER SETTING spanconfig.tenant_coalesce_adjacent.enabled = 'false';`)
 	require.NoError(t, err)
 
-	// Wait for the zone config range feed to start.
-	zoneCfgRangeFeedStarted.Wait()
-
 	// Disable strict GC TTL enforcement because we're going to shove a zero-value
 	// TTL into the system with AddImmediateGCZoneConfig.
 	defer sqltestutils.DisableGCTTLStrictEnforcement(t, systemDB)()
@@ -347,6 +353,12 @@ INSERT INTO t.kv2 VALUES ('c', 'd'), ('a', 'b'), ('e', 'a');
 		t.Fatal(err)
 	}
 
+	// Unblock the full reconciliation.
+	fullReconcilierStarted.Done()
+
+	// Wait for the zone config range feed to start.
+	zoneCfgRangeFeedStarted.Wait()
+
 	if _, err := sqlDB.Exec(`DROP DATABASE t CASCADE`); err != nil {
 		t.Fatal(err)
 	}