spanconfig/{watcher,translator}: add testing knobs

We introduce two testing knobs that we'll use in future commits:

- SQLWatcherCheckpointNoopsEveryDurationOverride, if set, will override
  how often the SQLWatcher checkpoints no-ops (follow up to cockroachdb#73171)

- ExcludeDroppedDescriptorsFromLookup will control whether the
  SQLTranslator ignores dropped descriptors. We'll also use this knob
  for the Reconciler (cockroachdb#71994), for the same purpose. Ignoring dropped
  descriptors in tests is a convenient (and faster!) alternative to
  waiting for the descriptor to actually get GC-ed by the GC job. It
  also makes it easier to order events transactionally -- it's
  sufficient for a checkpoint to cross the timestamp when the table is
  dropped, instead of waiting for the GC job to pick it up post TTL.
  This latter form is used in our current sqltranslator tests where we
  sleep arbitrarily:

    # Sleep for 5 seconds, which is more than the TTL on db.t1, so that
    # the gc job can delete the descriptor.
    sleep duration=5

  This is unfortunate and makes for slower tests than necessary. We can
  get rid of it in a subsequent commit.

Release note: None

pkg/ccl/spanconfigccl/BUILD.bazel

@@ -18,6 +18,7 @@ go_test(
         "//pkg/security/securitytest",
         "//pkg/server",
         "//pkg/spanconfig",
+        "//pkg/spanconfig/spanconfigsqltranslator",
         "//pkg/sql",
         "//pkg/sql/catalog/catalogkv",
         "//pkg/sql/catalog/descpb",

pkg/ccl/spanconfigccl/datadriven_test.go

@@ -22,6 +22,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/kv"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/spanconfig"
+	"github.com/cockroachdb/cockroach/pkg/spanconfig/spanconfigsqltranslator"
 	"github.com/cockroachdb/cockroach/pkg/sql"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/catalogkv"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
@@ -142,7 +143,7 @@ func TestSQLTranslatorDataDriven(t *testing.T) {
 				require.NoError(t, err)
 				return datadrivenTranslationResult(entries)
 			case "full-translate":
-				entries, _, err := spanconfig.FullTranslate(ctx, sqlTranslator)
+				entries, _, err := spanconfigsqltranslator.FullTranslate(ctx, sqlTranslator)
 				require.NoError(t, err)
 				return datadrivenTranslationResult(entries)
 			case "sleep":

pkg/server/server_sql.go

@@ -856,7 +856,7 @@ func newSQLServer(ctx context.Context, cfg sqlServerArgs) (*SQLServer, error) {
 		// Instantiate a span config manager. If we're the host tenant we'll
 		// only do it if COCKROACH_EXPERIMENTAL_SPAN_CONFIGS is set.
 		spanConfigKnobs, _ := cfg.TestingKnobs.SpanConfig.(*spanconfig.TestingKnobs)
-		sqlTranslator := spanconfigsqltranslator.New(execCfg, codec)
+		sqlTranslator := spanconfigsqltranslator.New(execCfg, codec, spanConfigKnobs)
 		sqlWatcher := spanconfigsqlwatcher.New(
 			codec,
 			cfg.Settings,

pkg/spanconfig/BUILD.bazel

@@ -10,7 +10,6 @@ go_library(
     visibility = ["//visibility:public"],
     deps = [
         "//pkg/base",
-        "//pkg/keys",
         "//pkg/roachpb:with-mocks",
         "//pkg/sql/catalog",
         "//pkg/sql/catalog/descpb",

pkg/spanconfig/spanconfig.go

@@ -13,7 +13,6 @@ package spanconfig
 import (
 	"context"
 
-	"github.com/cockroachdb/cockroach/pkg/keys"
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog"
 	"github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb"
@@ -106,18 +105,6 @@ type SQLTranslator interface {
 	Translate(ctx context.Context, ids descpb.IDs) ([]roachpb.SpanConfigEntry, hlc.Timestamp, error)
 }
 
-// FullTranslate translates the entire SQL zone configuration state to the
-// span configuration state. The timestamp at which such a translation is valid
-// is also returned.
-func FullTranslate(
-	ctx context.Context, s SQLTranslator,
-) ([]roachpb.SpanConfigEntry, hlc.Timestamp, error) {
-	// As RANGE DEFAULT is the root of all zone configurations (including
-	// other named zones for the system tenant), we can construct the entire
-	// span configuration state by starting from RANGE DEFAULT.
-	return s.Translate(ctx, descpb.IDs{keys.RootNamespaceID})
-}
-
 // SQLWatcher watches for events on system.zones and system.descriptors.
 type SQLWatcher interface {
 	// WatchForSQLUpdates watches for updates to zones and descriptors starting
@@ -143,17 +130,6 @@ type SQLWatcher interface {
 	) error
 }
 
-// DescriptorUpdate captures the ID and type of a descriptor or zone that the
-// SQLWatcher has observed updated.
-type DescriptorUpdate struct {
-	// ID of the descriptor/zone that has been updated.
-	ID descpb.ID
-
-	// DescriptorType of the descriptor/zone that has been updated. Could be either
-	// the specific type or catalog.Any if no information is available.
-	DescriptorType catalog.DescriptorType
-}
-
 // ReconciliationDependencies captures what's needed by the span config
 // reconciliation job to perform its task. The job is responsible for
 // reconciling a tenant's zone configurations with the clusters span
@@ -259,6 +235,17 @@ type StoreReader interface {
 	GetSpanConfigForKey(ctx context.Context, key roachpb.RKey) (roachpb.SpanConfig, error)
 }
 
+// DescriptorUpdate captures the ID and type of a descriptor or zone that the
+// SQLWatcher has observed updated.
+type DescriptorUpdate struct {
+	// ID of the descriptor/zone that has been updated.
+	ID descpb.ID
+
+	// DescriptorType of the descriptor/zone that has been updated. Could be either
+	// the specific type or catalog.Any if no information is available.
+	DescriptorType catalog.DescriptorType
+}
+
 // Update captures a span and the corresponding config change. It's the unit of
 // what can be applied to a StoreWriter.
 type Update struct {

pkg/spanconfig/spanconfigsqltranslator/sqltranslator.go

@@ -37,13 +37,20 @@ var _ spanconfig.SQLTranslator = &SQLTranslator{}
 type SQLTranslator struct {
 	execCfg *sql.ExecutorConfig
 	codec   keys.SQLCodec
+	knobs   *spanconfig.TestingKnobs
 }
 
 // New constructs and returns a SQLTranslator.
-func New(execCfg *sql.ExecutorConfig, codec keys.SQLCodec) *SQLTranslator {
+func New(
+	execCfg *sql.ExecutorConfig, codec keys.SQLCodec, knobs *spanconfig.TestingKnobs,
+) *SQLTranslator {
+	if knobs == nil {
+		knobs = &spanconfig.TestingKnobs{}
+	}
 	return &SQLTranslator{
 		execCfg: execCfg,
 		codec:   codec,
+		knobs:   knobs,
 	}
 }
 
@@ -64,7 +71,7 @@ func (s *SQLTranslator) Translate(
 
 		// For every ID we want to translate, first expand it to descendant leaf
 		// IDs that have span configurations associated for them. We also
-		// de-duplicate leaf IDs so as to not generate redundant entries.
+		// de-duplicate leaf IDs to not generate redundant entries.
 		seen := make(map[descpb.ID]struct{})
 		var leafIDs descpb.IDs
 		for _, id := range ids {
@@ -80,8 +87,7 @@ func (s *SQLTranslator) Translate(
 			}
 		}
 
-		// For every leaf ID, which has been de-duplicated, generate span
-		// configurations.
+		// For every unique leaf ID, generate span configurations.
 		for _, leafID := range leafIDs {
 			translatedEntries, err := s.generateSpanConfigurations(ctx, leafID, txn, descsCol)
 			if err != nil {
@@ -116,11 +122,13 @@ func (s *SQLTranslator) generateSpanConfigurations(
 	})
 	if err != nil {
 		if errors.Is(err, catalog.ErrDescriptorNotFound) {
-			// The descriptor has been deleted. Nothing to do here.
-			return nil, nil
+			return nil, nil // the descriptor has been deleted; nothing to do here
 		}
 		return nil, err
 	}
+	if s.knobs.ExcludeDroppedDescriptorsFromLookup && desc.Dropped() {
+		return nil, nil // we're excluding this descriptor; nothing to do here
+	}
 
 	if desc.DescriptorType() != catalog.Table {
 		return nil, errors.AssertionFailedf(
@@ -297,11 +305,13 @@ func (s *SQLTranslator) findDescendantLeafIDsForDescriptor(
 	})
 	if err != nil {
 		if errors.Is(err, catalog.ErrDescriptorNotFound) {
-			// The descriptor has been deleted. Nothing to do here.
-			return nil, nil
+			return nil, nil // the descriptor has been deleted; nothing to do here
 		}
 		return nil, err
 	}
+	if s.knobs.ExcludeDroppedDescriptorsFromLookup && desc.Dropped() {
+		return nil, nil // we're excluding this descriptor; nothing to do here
+	}
 
 	switch desc.DescriptorType() {
 	case catalog.Type, catalog.Schema:
@@ -386,3 +396,15 @@ func (s *SQLTranslator) findDescendantLeafIDsForNamedZone(
 	}
 	return descendantIDs, nil
 }
+
+// FullTranslate translates the entire SQL zone configuration state to the
+// span configuration state. The timestamp at which such a translation is valid
+// is also returned.
+func FullTranslate(
+	ctx context.Context, s spanconfig.SQLTranslator,
+) ([]roachpb.SpanConfigEntry, hlc.Timestamp, error) {
+	// As RANGE DEFAULT is the root of all zone configurations (including
+	// other named zones for the system tenant), we can construct the entire
+	// span configuration state by starting from RANGE DEFAULT.
+	return s.Translate(ctx, descpb.IDs{keys.RootNamespaceID})
+}

pkg/spanconfig/spanconfigsqlwatcher/sqlwatcher.go

@@ -90,7 +90,6 @@ func (s *SQLWatcher) watch(
 	startTS hlc.Timestamp,
 	handler func(context.Context, []spanconfig.DescriptorUpdate, hlc.Timestamp) error,
 ) error {
-
 	// The callbacks below are invoked by both the rangefeeds we establish, both
 	// of which run on separate goroutines. We serialize calls to the handler
 	// function by invoking in this single watch thread (instead of pushing it
@@ -148,7 +147,12 @@ func (s *SQLWatcher) watch(
 	}
 	defer zonesRF.Close()
 
-	checkpointNoops := util.Every(s.checkpointNoopsEvery)
+	noopCheckpointInterval := s.checkpointNoopsEvery
+	if override := s.knobs.SQLWatcherCheckpointNoopsEveryDurationOverride; override.Nanoseconds() != 0 {
+		noopCheckpointInterval = override
+	}
+	checkpointNoops := util.Every(noopCheckpointInterval)
+
 	for {
 		select {
 		case <-ctx.Done():

pkg/spanconfig/testing_knobs.go

@@ -11,6 +11,8 @@
 package spanconfig
 
 import (
+	"time"
+
 	"github.com/cockroachdb/cockroach/pkg/base"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 )
@@ -57,6 +59,16 @@ type TestingKnobs struct {
 	// SQLWatcherOnEventInterceptor, if set, is invoked when the SQLWatcher
 	// receives an event on one of its rangefeeds.
 	SQLWatcherOnEventInterceptor func() error
+
+	// SQLWatcherCheckpointNoopsEveryDurationOverride, if set, overrides how
+	// often the SQLWatcher checkpoints noops.
+	SQLWatcherCheckpointNoopsEveryDurationOverride time.Duration
+
+	// ExcludeDroppedDescriptorsFromLookup is used to control if the
+	// SQLTranslator ignores dropped descriptors. If enabled, dropped
+	// descriptors appear missing -- a convenient+faster alternative to waiting
+	// for the descriptor to actually get GC-ed in tests.
+	ExcludeDroppedDescriptorsFromLookup bool
 }
 
 // ModuleTestingKnobs is part of the base.ModuleTestingKnobs interface.