Merge #156016 #156303 #156655

156016: sql/*: load external statement hints from the hints cache r=DrewKimball a=michae2

**sql/\*: load external statement hints from the hints cache**

When making a `sql.Statement`, call `MaybeGetStatementHints` to load any
applicable external statement hints from the hint cache. We don't yet do
anything with the hints once they're loaded, but we do save them in prepared
statements.

A few notable code paths are not yet loading hints from the hints cache:
- SQL statements within UDFs and SPs
- Views

Informs: #153633

Release note: None

---

**sql, sqltelemetry: add telemetry counter for external statement hints**

Informs: #153633

Release note: None

---

**sql/hints, builtins: add two more statement_hints_cache builtins**

Add new builtins `crdb_internal.clear_statement_hints_cache` and
`crdb_internal.await_statement_hints_cache` which will be useful for
testing.

The first builtin clears the hintCache, similar to
`clear_query_plan_cache` and `clear_table_stats_cache`. (I debated
whether this builtin should also set hintedHashes to nil and restart the
changefeed, but that seems too different from
`clear_table_stats_cache`.)

The second builtin blocks until the hints cache rangefeed watcher has
caught up with the present, which usually takes a couple of
seconds. This is handy when we want to test a statement using the hint
immediately after adding the hint to system.statement_hints.

Informs: #153633

Release note: None

---

**logictest: add tests for loading external statement hints**

Test that we're loading external statement hints correctly.

Informs: #153633

Release note: None

156303: kvserver: use Pebble snapshot for catchup scans r=stevendanna a=sumeerbhola

The snapshot is used to create an iterator, which is recreated based on the storage.snapshot.recreate_iter_duration cluster setting, which defaults to 20s.

This is mostly plumbing changes, except for catchup_scan.go.

Fixes #133851

Epic: none

Release note (ops change): The cluster setting
storage.snapshot.recreate_iter_duration (default 20s) controls how frequently a long-lived engine iterator, backed by an engine snapshot, will be closed and recreated. Currently, it is only used for iterators used in rangefeed catchup scans.

156655: sql: update cluster version checks in ElementCreationMetadata r=shghasemi a=shghasemi

This change adds `In_26_1OrLater` to the ElementCreationMetadata.
It will be used in future commits to enable/disable new schema changer
elements. It also removes `in_23_1OrLater` as it's no longer in use.

Part of: #142918

Release note: None

Co-authored-by: Michael Erickson <[email protected]>
Co-authored-by: sumeerbhola <[email protected]>
Co-authored-by: Shadi Ghasemitaheri <[email protected]>

Author: 4 people

pkg/backup/backupsink/file_sst_sink.go

@@ -361,7 +361,7 @@ func (s *FileSSTSink) copyPointKeys(ctx context.Context, dataSST []byte) (roachp
 
 	empty := true
 	for iter.SeekGE(storage.MVCCKey{Key: keys.MinKey}); ; iter.Next() {
-		if err := s.pacer.Pace(ctx); err != nil {
+		if _, err := s.pacer.Pace(ctx); err != nil {
 			return nil, err
 		}
 		if valid, err := iter.Valid(); !valid || err != nil {

pkg/backup/compaction_processor.go

@@ -422,7 +422,7 @@ func compactSpanEntry(
 	scratch = append(scratch, prefix...)
 	iter := sstIter.iter
 	for iter.SeekGE(trimmedStart); ; iter.NextKey() {
-		if err := pacer.Pace(ctx); err != nil {
+		if _, err := pacer.Pace(ctx); err != nil {
 			return err
 		}
 		var key storage.MVCCKey

pkg/ccl/changefeedccl/batching_sink.go

@@ -476,7 +476,7 @@ func (s *batchingSink) runBatchingWorker(ctx context.Context) {
 			// TODO(yevgeniy): rework this function: this function should simply
 			// return an error, and not rely on "handleError".
 			// It's hard to reason about this functions correctness otherwise.
-			_ = s.pacer.Pace(ctx)
+			_, _ = s.pacer.Pace(ctx)
 
 			switch r := req.(type) {
 			case *rowEvent:

pkg/ccl/changefeedccl/event_processing.go

@@ -343,7 +343,7 @@ func (c *kvEventToRowConsumer) ConsumeEvent(ctx context.Context, ev kvevent.Even
 	// Request CPU time to use for event consumption, block if this time is
 	// unavailable. If there is unused CPU time left from the last call to
 	// Pace, then use that time instead of blocking.
-	if err := c.pacer.Pace(ctx); err != nil {
+	if _, err := c.pacer.Pace(ctx); err != nil {
 		return err
 	}
 

pkg/ccl/schemachangerccl/testdata/decomp/multiregion

@@ -230,8 +230,8 @@ ElementState:
     columnId: 1
     computeExpr: null
     elementCreationMetadata:
-      in231OrLater: true
       in243OrLater: true
+      in261OrLater: true
     familyId: 0
     isVirtual: false
     tableId: 110
@@ -259,8 +259,8 @@ ElementState:
     columnId: 2
     computeExpr: null
     elementCreationMetadata:
-      in231OrLater: true
       in243OrLater: true
+      in261OrLater: true
     familyId: 0
     isVirtual: false
     tableId: 110
@@ -288,8 +288,8 @@ ElementState:
     columnId: 4.294967292e+09
     computeExpr: null
     elementCreationMetadata:
-      in231OrLater: true
       in243OrLater: true
+      in261OrLater: true
     familyId: 0
     isVirtual: false
     tableId: 110
@@ -317,8 +317,8 @@ ElementState:
     columnId: 4.294967293e+09
     computeExpr: null
     elementCreationMetadata:
-      in231OrLater: true
       in243OrLater: true
+      in261OrLater: true
     familyId: 0
     isVirtual: false
     tableId: 110
@@ -346,8 +346,8 @@ ElementState:
     columnId: 4.294967294e+09
     computeExpr: null
     elementCreationMetadata:
-      in231OrLater: true
       in243OrLater: true
+      in261OrLater: true
     familyId: 0
     isVirtual: false
     tableId: 110
@@ -375,8 +375,8 @@ ElementState:
     columnId: 4.294967295e+09
     computeExpr: null
     elementCreationMetadata:
-      in231OrLater: true
       in243OrLater: true
+      in261OrLater: true
     familyId: 0
     isVirtual: false
     tableId: 110
@@ -624,8 +624,8 @@ ElementState:
     columnId: 1
     computeExpr: null
     elementCreationMetadata:
-      in231OrLater: true
       in243OrLater: true
+      in261OrLater: true
     familyId: 0
     isVirtual: false
     tableId: 109
@@ -653,8 +653,8 @@ ElementState:
     columnId: 2
     computeExpr: null
     elementCreationMetadata:
-      in231OrLater: true
       in243OrLater: true
+      in261OrLater: true
     familyId: 0
     isVirtual: false
     tableId: 109
@@ -682,8 +682,8 @@ ElementState:
     columnId: 4.294967292e+09
     computeExpr: null
     elementCreationMetadata:
-      in231OrLater: true
       in243OrLater: true
+      in261OrLater: true
     familyId: 0
     isVirtual: false
     tableId: 109
@@ -711,8 +711,8 @@ ElementState:
     columnId: 4.294967293e+09
     computeExpr: null
     elementCreationMetadata:
-      in231OrLater: true
       in243OrLater: true
+      in261OrLater: true
     familyId: 0
     isVirtual: false
     tableId: 109
@@ -740,8 +740,8 @@ ElementState:
     columnId: 4.294967294e+09
     computeExpr: null
     elementCreationMetadata:
-      in231OrLater: true
       in243OrLater: true
+      in261OrLater: true
     familyId: 0
     isVirtual: false
     tableId: 109
@@ -769,8 +769,8 @@ ElementState:
     columnId: 4.294967295e+09
     computeExpr: null
     elementCreationMetadata:
-      in231OrLater: true
       in243OrLater: true
+      in261OrLater: true
     familyId: 0
     isVirtual: false
     tableId: 109
@@ -1074,8 +1074,8 @@ ElementState:
     columnId: 3
     computeExpr: null
     elementCreationMetadata:
-      in231OrLater: true
       in243OrLater: true
+      in261OrLater: true
     familyId: 0
     isVirtual: false
     tableId: 108
@@ -1104,8 +1104,8 @@ ElementState:
     columnId: 1
     computeExpr: null
     elementCreationMetadata:
-      in231OrLater: true
       in243OrLater: true
+      in261OrLater: true
     familyId: 0
     isVirtual: false
     tableId: 108
@@ -1133,8 +1133,8 @@ ElementState:
     columnId: 2
     computeExpr: null
     elementCreationMetadata:
-      in231OrLater: true
       in243OrLater: true
+      in261OrLater: true
     familyId: 0
     isVirtual: false
     tableId: 108
@@ -1162,8 +1162,8 @@ ElementState:
     columnId: 4.294967292e+09
     computeExpr: null
     elementCreationMetadata:
-      in231OrLater: true
       in243OrLater: true
+      in261OrLater: true
     familyId: 0
     isVirtual: false
     tableId: 108
@@ -1191,8 +1191,8 @@ ElementState:
     columnId: 4.294967293e+09
     computeExpr: null
     elementCreationMetadata:
-      in231OrLater: true
       in243OrLater: true
+      in261OrLater: true
     familyId: 0
     isVirtual: false
     tableId: 108
@@ -1220,8 +1220,8 @@ ElementState:
     columnId: 4.294967294e+09
     computeExpr: null
     elementCreationMetadata:
-      in231OrLater: true
       in243OrLater: true
+      in261OrLater: true
     familyId: 0
     isVirtual: false
     tableId: 108
@@ -1249,8 +1249,8 @@ ElementState:
     columnId: 4.294967295e+09
     computeExpr: null
     elementCreationMetadata:
-      in231OrLater: true
       in243OrLater: true
+      in261OrLater: true
     familyId: 0
     isVirtual: false
     tableId: 108

pkg/ccl/schemachangerccl/testdata/decomp/partitioning

@@ -161,8 +161,8 @@ ElementState:
     columnId: 1
     computeExpr: null
     elementCreationMetadata:
-      in231OrLater: true
       in243OrLater: true
+      in261OrLater: true
     familyId: 0
     isVirtual: false
     tableId: 104
@@ -190,8 +190,8 @@ ElementState:
     columnId: 2
     computeExpr: null
     elementCreationMetadata:
-      in231OrLater: true
       in243OrLater: true
+      in261OrLater: true
     familyId: 0
     isVirtual: false
     tableId: 104
@@ -219,8 +219,8 @@ ElementState:
     columnId: 3
     computeExpr: null
     elementCreationMetadata:
-      in231OrLater: true
       in243OrLater: true
+      in261OrLater: true
     familyId: 0
     isVirtual: false
     tableId: 104
@@ -248,8 +248,8 @@ ElementState:
     columnId: 4.294967292e+09
     computeExpr: null
     elementCreationMetadata:
-      in231OrLater: true
       in243OrLater: true
+      in261OrLater: true
     familyId: 0
     isVirtual: false
     tableId: 104
@@ -277,8 +277,8 @@ ElementState:
     columnId: 4.294967293e+09
     computeExpr: null
     elementCreationMetadata:
-      in231OrLater: true
       in243OrLater: true
+      in261OrLater: true
     familyId: 0
     isVirtual: false
     tableId: 104
@@ -306,8 +306,8 @@ ElementState:
     columnId: 4.294967294e+09
     computeExpr: null
     elementCreationMetadata:
-      in231OrLater: true
       in243OrLater: true
+      in261OrLater: true
     familyId: 0
     isVirtual: false
     tableId: 104
@@ -335,8 +335,8 @@ ElementState:
     columnId: 4.294967295e+09
     computeExpr: null
     elementCreationMetadata:
-      in231OrLater: true
       in243OrLater: true
+      in261OrLater: true
     familyId: 0
     isVirtual: false
     tableId: 104
@@ -666,8 +666,8 @@ ElementState:
     columnId: 1
     computeExpr: null
     elementCreationMetadata:
-      in231OrLater: true
       in243OrLater: true
+      in261OrLater: true
     familyId: 0
     isVirtual: false
     tableId: 105
@@ -695,8 +695,8 @@ ElementState:
     columnId: 2
     computeExpr: null
     elementCreationMetadata:
-      in231OrLater: true
       in243OrLater: true
+      in261OrLater: true
     familyId: 0
     isVirtual: false
     tableId: 105
@@ -724,8 +724,8 @@ ElementState:
     columnId: 4.294967292e+09
     computeExpr: null
     elementCreationMetadata:
-      in231OrLater: true
       in243OrLater: true
+      in261OrLater: true
     familyId: 0
     isVirtual: false
     tableId: 105
@@ -753,8 +753,8 @@ ElementState:
     columnId: 4.294967293e+09
     computeExpr: null
     elementCreationMetadata:
-      in231OrLater: true
       in243OrLater: true
+      in261OrLater: true
     familyId: 0
     isVirtual: false
     tableId: 105
@@ -782,8 +782,8 @@ ElementState:
     columnId: 4.294967294e+09
     computeExpr: null
     elementCreationMetadata:
-      in231OrLater: true
       in243OrLater: true
+      in261OrLater: true
     familyId: 0
     isVirtual: false
     tableId: 105
@@ -811,8 +811,8 @@ ElementState:
     columnId: 4.294967295e+09
     computeExpr: null
     elementCreationMetadata:
-      in231OrLater: true
       in243OrLater: true
+      in261OrLater: true
     familyId: 0
     isVirtual: false
     tableId: 105

pkg/kv/bulk/sst_batcher.go

@@ -444,7 +444,7 @@ func (b *SSTBatcher) AddMVCCKeyLDR(ctx context.Context, key storage.MVCCKey, val
 // Keys must be added in order.
 func (b *SSTBatcher) AddMVCCKey(ctx context.Context, key storage.MVCCKey, value []byte) error {
 	// Pace based on admission control before adding the key.
-	if err := b.pacer.Pace(ctx); err != nil {
+	if _, err := b.pacer.Pace(ctx); err != nil {
 		return err
 	}
 

pkg/kv/kvclient/rangefeed/rangefeed_external_test.go

@@ -841,6 +841,11 @@ func TestWithOnDeleteRange(t *testing.T) {
 		// should be visible, because catchup scans do emit tombstones. The range
 		// tombstone should be ordered after the initial point, but before the foo
 		// catchup point, and the previous values should respect the range tombstones.
+		//
+		// NB: When RaceEnabled=true, the range key will be emitted multiple
+		// times, since CatchUpSnapshot.CatchUpScan recreates the iterator at
+		// every step. These duplications are harmless and are de-duped by
+		// testEvents when printing.
 		require.NoError(t, db.Put(ctx, makeKey(srv.Codec(), "covered"), "covered"))
 		require.NoError(t, db.Put(ctx, makeKey(srv.Codec(), "foo"), "covered"))
 		require.NoError(t, db.DelRangeUsingTombstone(ctx, makeKey(srv.Codec(), "a"), makeKey(srv.Codec(), "z")))

pkg/kv/kvserver/rangefeed/BUILD.bazel

@@ -40,6 +40,7 @@ go_library(
         "//pkg/storage",
         "//pkg/storage/enginepb",
         "//pkg/storage/fs",
+        "//pkg/util",
         "//pkg/util/admission",
         "//pkg/util/bufalloc",
         "//pkg/util/buildutil",