Merge #38004

38004: storage/sql: change sticky bit to represent time of expiration r=jeffrey-xiao a=jeffrey-xiao

This PR does the following:

- Revert the cluster setting for manual split ttl. It was agreed that this is probably too coarse to be useful.
- Change sticky bit to represent time of expiration instead of time of split.
- The expiration time displayed in `crdb_internal.ranges` is `NULL` if the range is permanent (I.E. has sticky bit of `hlc.MaxTimestamp`), otherwise it is the expiration time. This means that for static splits and splits performed by the split queue, it is displayed as `1970-01-01 00:00:00+00:00` (`hlc.Timestamp{}`).
- Add `WITH EXPIRATION` option to `SPLIT AT` that accepts the same values as `AS OF SYSTEM TIME`.

Co-authored-by: Jeffrey Xiao <[email protected]>

docs/generated/settings/settings.html

@@ -43,7 +43,6 @@
 <tr><td><code>kv.raft_log.disable_synchronization_unsafe</code></td><td>boolean</td><td><code>false</code></td><td>set to true to disable synchronization on Raft log writes to persistent storage. Setting to true risks data loss or data corruption on server crashes. The setting is meant for internal testing only and SHOULD NOT be used in production.</td></tr>
 <tr><td><code>kv.range.backpressure_range_size_multiplier</code></td><td>float</td><td><code>2</code></td><td>multiple of range_max_bytes that a range is allowed to grow to without splitting before writes to that range are blocked, or 0 to disable</td></tr>
 <tr><td><code>kv.range_descriptor_cache.size</code></td><td>integer</td><td><code>1000000</code></td><td>maximum number of entries in the range descriptor and leaseholder caches</td></tr>
-<tr><td><code>kv.range_merge.manual_split.ttl</code></td><td>duration</td><td><code>0s</code></td><td>if nonzero, manual splits older than this duration will be considered for automatic range merging</td></tr>
 <tr><td><code>kv.range_merge.queue_enabled</code></td><td>boolean</td><td><code>true</code></td><td>whether the automatic merge queue is enabled</td></tr>
 <tr><td><code>kv.range_merge.queue_interval</code></td><td>duration</td><td><code>1s</code></td><td>how long the merge queue waits between processing replicas (WARNING: may compromise cluster stability or correctness; do not edit without supervision)</td></tr>
 <tr><td><code>kv.range_split.by_load_enabled</code></td><td>boolean</td><td><code>true</code></td><td>allow automatic splits of ranges based on where load is concentrated</td></tr>

docs/generated/sql/bnf/split_index_at.bnf

@@ -1,3 +1,5 @@
 alter_split_index_stmt ::=
 	'ALTER' 'INDEX' table_name '@' index_name 'SPLIT' 'AT' select_stmt
 	| 'ALTER' 'INDEX' standalone_index_name 'SPLIT' 'AT' select_stmt
+	| 'ALTER' 'INDEX' table_name '@' index_name 'SPLIT' 'AT' select_stmt 'WITH' 'EXPIRATION' a_expr
+	| 'ALTER' 'INDEX' standalone_index_name 'SPLIT' 'AT' select_stmt 'WITH' 'EXPIRATION' a_expr

docs/generated/sql/bnf/split_table_at.bnf

@@ -1,2 +1,3 @@
 alter_split_stmt ::=
 	'ALTER' 'TABLE' table_name 'SPLIT' 'AT' select_stmt
+	| 'ALTER' 'TABLE' table_name 'SPLIT' 'AT' select_stmt 'WITH' 'EXPIRATION' a_expr

docs/generated/sql/bnf/stmt_block.bnf

@@ -620,6 +620,7 @@ unreserved_keyword ::=
 	| 'EXPERIMENTAL_RANGES'
 	| 'EXPERIMENTAL_RELOCATE'
 	| 'EXPERIMENTAL_REPLICA'
+	| 'EXPIRATION'
 	| 'EXPLAIN'
 	| 'EXPORT'
 	| 'EXTENSION'
@@ -1228,6 +1229,7 @@ alter_onetable_stmt ::=
 
 alter_split_stmt ::=
 	'ALTER' 'TABLE' table_name 'SPLIT' 'AT' select_stmt
+	| 'ALTER' 'TABLE' table_name 'SPLIT' 'AT' select_stmt 'WITH' 'EXPIRATION' a_expr
 
 alter_unsplit_stmt ::=
 	'ALTER' 'TABLE' table_name 'UNSPLIT' 'AT' select_stmt
@@ -1250,6 +1252,7 @@ alter_oneindex_stmt ::=
 
 alter_split_index_stmt ::=
 	'ALTER' 'INDEX' table_index_name 'SPLIT' 'AT' select_stmt
+	| 'ALTER' 'INDEX' table_index_name 'SPLIT' 'AT' select_stmt 'WITH' 'EXPIRATION' a_expr
 
 alter_unsplit_index_stmt ::=
 	'ALTER' 'INDEX' table_index_name 'UNSPLIT' 'AT' select_stmt

pkg/ccl/backupccl/restore.go

@@ -793,7 +793,7 @@ func splitAndScatter(
 			// TODO(dan): Really, this should be splitting the Key of the first
 			// entry in the _next_ chunk.
 			log.VEventf(restoreCtx, 1, "presplitting chunk %d of %d", idx, len(importSpanChunks))
-			if err := db.AdminSplit(ctx, chunkKey, chunkKey, false /* manual */); err != nil {
+			if err := db.AdminSplit(ctx, chunkKey, chunkKey, hlc.Timestamp{} /* expirationTime */); err != nil {
 				return err
 			}
 
@@ -849,7 +849,7 @@ func splitAndScatter(
 					// TODO(dan): Really, this should be splitting the Key of
 					// the _next_ entry.
 					log.VEventf(restoreCtx, 1, "presplitting %d of %d", idx, len(importSpans))
-					if err := db.AdminSplit(ctx, newSpanKey, newSpanKey, false /* manual */); err != nil {
+					if err := db.AdminSplit(ctx, newSpanKey, newSpanKey, hlc.Timestamp{} /* expirationTime */); err != nil {
 						return err
 					}
 

pkg/ccl/importccl/bench_test.go

@@ -27,6 +27,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/testutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
 	"github.com/cockroachdb/cockroach/pkg/util/ctxgroup"
+	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/timeutil"
 	"github.com/cockroachdb/cockroach/pkg/workload"
 	"github.com/cockroachdb/cockroach/pkg/workload/tpcc"
@@ -125,7 +126,7 @@ func benchmarkAddSSTable(b *testing.B, dir string, tables []tableSSTable) {
 		}}
 		s, _, kvDB := serverutils.StartServer(b, args)
 		for _, t := range tables {
-			if err := kvDB.AdminSplit(ctx, t.span.Key, t.span.Key, false /* manual */); err != nil {
+			if err := kvDB.AdminSplit(ctx, t.span.Key, t.span.Key, hlc.Timestamp{} /* expirationTime */); err != nil {
 				b.Fatal(err)
 			}
 		}

pkg/ccl/importccl/read_import_proc.go

@@ -487,7 +487,7 @@ func (cp *readImportDataProcessor) doRun(ctx context.Context) error {
 	if cp.spec.IngestDirectly {
 		for _, tbl := range cp.spec.Tables {
 			for _, span := range tbl.AllIndexSpans() {
-				if err := cp.flowCtx.ClientDB.AdminSplit(ctx, span.Key, span.Key, false /* manual */); err != nil {
+				if err := cp.flowCtx.ClientDB.AdminSplit(ctx, span.Key, span.Key, hlc.Timestamp{} /* expirationTime */); err != nil {
 					return err
 				}
 

pkg/ccl/importccl/sst_writer_proc.go

@@ -29,6 +29,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/storage/diskmap"
 	"github.com/cockroachdb/cockroach/pkg/storage/engine"
 	"github.com/cockroachdb/cockroach/pkg/util/ctxgroup"
+	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/protoutil"
 	"github.com/cockroachdb/cockroach/pkg/util/tracing"
@@ -176,7 +177,7 @@ func (sp *sstWriter) Run(ctx context.Context) {
 						end = sst.span.EndKey
 					}
 
-					if err := sp.db.AdminSplit(ctx, end, end, false /* manual */); err != nil {
+					if err := sp.db.AdminSplit(ctx, end, end, hlc.Timestamp{} /* expirationTime */); err != nil {
 						return err
 					}
 

pkg/ccl/storageccl/import_test.go

@@ -318,10 +318,10 @@ func runTestImport(t *testing.T, init func(*cluster.Settings)) {
 				t.Fatalf("failed to rewrite key: %s", reqMidKey2)
 			}
 
-			if err := kvDB.AdminSplit(ctx, reqMidKey1, reqMidKey1, true /* manual */); err != nil {
+			if err := kvDB.AdminSplit(ctx, reqMidKey1, reqMidKey1, hlc.MaxTimestamp /* expirationTime */); err != nil {
 				t.Fatal(err)
 			}
-			if err := kvDB.AdminSplit(ctx, reqMidKey2, reqMidKey2, true /* manual */); err != nil {
+			if err := kvDB.AdminSplit(ctx, reqMidKey2, reqMidKey2, hlc.MaxTimestamp /* expirationTime */); err != nil {
 				t.Fatal(err)
 			}
 

pkg/internal/client/batch.go

@@ -16,6 +16,7 @@ import (
 	"context"
 
 	"github.com/cockroachdb/cockroach/pkg/roachpb"
+	"github.com/cockroachdb/cockroach/pkg/util/hlc"
 	"github.com/pkg/errors"
 )
 
@@ -586,7 +587,7 @@ func (b *Batch) adminMerge(key interface{}) {
 
 // adminSplit is only exported on DB. It is here for symmetry with the
 // other operations.
-func (b *Batch) adminSplit(spanKeyIn, splitKeyIn interface{}, manual bool) {
+func (b *Batch) adminSplit(spanKeyIn, splitKeyIn interface{}, expirationTime hlc.Timestamp) {
 	spanKey, err := marshalKey(spanKeyIn)
 	if err != nil {
 		b.initResult(0, 0, notRaw, err)
@@ -601,8 +602,8 @@ func (b *Batch) adminSplit(spanKeyIn, splitKeyIn interface{}, manual bool) {
 		RequestHeader: roachpb.RequestHeader{
 			Key: spanKey,
 		},
-		SplitKey: splitKey,
-		Manual:   manual,
+		SplitKey:       splitKey,
+		ExpirationTime: expirationTime,
 	}
 	b.appendReqs(req)
 	b.initResult(1, 0, notRaw, nil)

pkg/internal/client/db.go

@@ -475,14 +475,18 @@ func (db *DB) AdminMerge(ctx context.Context, key interface{}) error {
 // #16008 for details, and #16344 for the tracking issue to clean this mess up
 // properly.
 //
-// When manual is true, the sticky bit associated with the split range is set.
-// Any ranges with the sticky bit set will be skipped by the merge queue when
-// scanning for potential ranges to merge.
+// expirationTime is the timestamp when the split expires and is eligible for
+// automatic merging by the merge queue. To specify that a split should
+// immediately be eligible for automatic merging, set expirationTime to
+// hlc.Timestamp{} (I.E. the zero timestamp). To specify that a split should
+// never be eligible, set expirationTime to hlc.MaxTimestamp.
 //
 // The keys can be either byte slices or a strings.
-func (db *DB) AdminSplit(ctx context.Context, spanKey, splitKey interface{}, manual bool) error {
+func (db *DB) AdminSplit(
+	ctx context.Context, spanKey, splitKey interface{}, expirationTime hlc.Timestamp,
+) error {
 	b := &Batch{}
-	b.adminSplit(spanKey, splitKey, manual)
+	b.adminSplit(spanKey, splitKey, expirationTime)
 	return getOneErr(db.Run(ctx, b), b)
 }
 

pkg/kv/dist_sender_server_test.go

@@ -118,7 +118,7 @@ func TestRangeLookupWithOpenTransaction(t *testing.T) {
 func setupMultipleRanges(ctx context.Context, db *client.DB, splitAt ...string) error {
 	// Split the keyspace at the given keys.
 	for _, key := range splitAt {
-		if err := db.AdminSplit(ctx, key /* spanKey */, key /* splitKey */, true /* manual */); err != nil {
+		if err := db.AdminSplit(ctx, key /* spanKey */, key /* splitKey */, hlc.MaxTimestamp /* expirationTime */); err != nil {
 			return err
 		}
 	}
@@ -1046,7 +1046,7 @@ func TestParallelSender(t *testing.T) {
 	// Split into multiple ranges.
 	splitKeys := []string{"a", "b", "c", "d", "e", "f", "g", "h", "i", "j"}
 	for _, key := range splitKeys {
-		if err := db.AdminSplit(context.TODO(), key, key, true /* manual */); err != nil {
+		if err := db.AdminSplit(context.TODO(), key, key, hlc.MaxTimestamp /* expirationTime */); err != nil {
 			t.Fatal(err)
 		}
 	}
@@ -1091,7 +1091,7 @@ func initReverseScanTestEnv(s serverutils.TestServerInterface, t *testing.T) *cl
 	// ["", "b"),["b", "e") ,["e", "g") and ["g", "\xff\xff").
 	for _, key := range []string{"b", "e", "g"} {
 		// Split the keyspace at the given key.
-		if err := db.AdminSplit(context.TODO(), key, key, true /* manual */); err != nil {
+		if err := db.AdminSplit(context.TODO(), key, key, hlc.MaxTimestamp /* expirationTime */); err != nil {
 			t.Fatal(err)
 		}
 	}
@@ -1530,7 +1530,7 @@ func TestBatchPutWithConcurrentSplit(t *testing.T) {
 	// Split first using the default client and scan to make sure that
 	// the range descriptor cache reflects the split.
 	for _, key := range []string{"b", "f"} {
-		if err := db.AdminSplit(context.TODO(), key, key, true /* manual */); err != nil {
+		if err := db.AdminSplit(context.TODO(), key, key, hlc.MaxTimestamp /* expirationTime */); err != nil {
 			t.Fatal(err)
 		}
 	}
@@ -1555,7 +1555,8 @@ func TestBatchPutWithConcurrentSplit(t *testing.T) {
 			RequestHeader: roachpb.RequestHeader{
 				Key: roachpb.Key(key),
 			},
-			SplitKey: roachpb.Key(key),
+			SplitKey:       roachpb.Key(key),
+			ExpirationTime: hlc.MaxTimestamp,
 		}
 		if _, err := client.SendWrapped(context.Background(), ds, req); err != nil {
 			t.Fatal(err)
@@ -1584,7 +1585,7 @@ func TestReverseScanWithSplitAndMerge(t *testing.T) {
 
 	// Case 1: An encounter with a range split.
 	// Split the range ["b", "e") at "c".
-	if err := db.AdminSplit(context.TODO(), "c", "c", true /* manual */); err != nil {
+	if err := db.AdminSplit(context.TODO(), "c", "c", hlc.MaxTimestamp /* expirationTime */); err != nil {
 		t.Fatal(err)
 	}
 

pkg/kv/split_test.go

@@ -110,7 +110,7 @@ func TestRangeSplitMeta(t *testing.T) {
 	for _, splitRKey := range splitKeys {
 		splitKey := roachpb.Key(splitRKey)
 		log.Infof(ctx, "starting split at key %q...", splitKey)
-		if err := s.DB.AdminSplit(ctx, splitKey, splitKey, true /* manual */); err != nil {
+		if err := s.DB.AdminSplit(ctx, splitKey, splitKey, hlc.MaxTimestamp /* expirationTime */); err != nil {
 			t.Fatal(err)
 		}
 		log.Infof(ctx, "split at key %q complete", splitKey)
@@ -156,7 +156,7 @@ func TestRangeSplitsWithConcurrentTxns(t *testing.T) {
 			<-txnChannel
 		}
 		log.Infof(ctx, "starting split at key %q...", splitKey)
-		if pErr := s.DB.AdminSplit(context.TODO(), splitKey, splitKey, true /* manual */); pErr != nil {
+		if pErr := s.DB.AdminSplit(context.TODO(), splitKey, splitKey, hlc.MaxTimestamp /* expirationTime */); pErr != nil {
 			t.Error(pErr)
 		}
 		log.Infof(ctx, "split at key %q complete", splitKey)
@@ -249,19 +249,19 @@ func TestRangeSplitsWithSameKeyTwice(t *testing.T) {
 
 	splitKey := roachpb.Key("aa")
 	log.Infof(ctx, "starting split at key %q...", splitKey)
-	if err := s.DB.AdminSplit(ctx, splitKey, splitKey, true /* manual */); err != nil {
+	if err := s.DB.AdminSplit(ctx, splitKey, splitKey, hlc.MaxTimestamp /* expirationTime */); err != nil {
 		t.Fatal(err)
 	}
 	log.Infof(ctx, "split at key %q first time complete", splitKey)
-	if err := s.DB.AdminSplit(ctx, splitKey, splitKey, true /* manual */); err != nil {
+	if err := s.DB.AdminSplit(ctx, splitKey, splitKey, hlc.MaxTimestamp /* expirationTime */); err != nil {
 		t.Fatal(err)
 	}
 }
 
 // TestSplitStickyBit checks that the sticky bit is set when performing a manual
 // split. There are two cases to consider:
-// 1. Range is split so sticky bit is set on RHS.
-// 2. Range is already split and split key is the start key of a range, so set
+// 1. Range is split so sticky bit is updated on RHS.
+// 2. Range is already split and split key is the start key of a range, so update
 //    the sticky bit of that range, but no range is split.
 func TestRangeSplitsStickyBit(t *testing.T) {
 	defer leaktest.AfterTest(t)()
@@ -277,7 +277,7 @@ func TestRangeSplitsStickyBit(t *testing.T) {
 	descKey := keys.RangeDescriptorKey(splitKey)
 
 	// Splitting range.
-	if err := s.DB.AdminSplit(ctx, splitKey.AsRawKey(), splitKey.AsRawKey(), true /* manual */); err != nil {
+	if err := s.DB.AdminSplit(ctx, splitKey.AsRawKey(), splitKey.AsRawKey(), hlc.MaxTimestamp /* expirationTime */); err != nil {
 		t.Fatal(err)
 	}
 
@@ -287,7 +287,7 @@ func TestRangeSplitsStickyBit(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if desc.StickyBit == nil {
+	if (desc.StickyBit == hlc.Timestamp{}) {
 		t.Fatal("Sticky bit not set after splitting")
 	}
 
@@ -297,7 +297,7 @@ func TestRangeSplitsStickyBit(t *testing.T) {
 	}
 
 	// Splitting range.
-	if err := s.DB.AdminSplit(ctx, splitKey.AsRawKey(), splitKey.AsRawKey(), true /* manual */); err != nil {
+	if err := s.DB.AdminSplit(ctx, splitKey.AsRawKey(), splitKey.AsRawKey(), hlc.MaxTimestamp /* expirationTime */); err != nil {
 		t.Fatal(err)
 	}
 
@@ -306,7 +306,7 @@ func TestRangeSplitsStickyBit(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if desc.StickyBit == nil {
+	if (desc.StickyBit == hlc.Timestamp{}) {
 		t.Fatal("Sticky bit not set after splitting")
 	}
 }

pkg/kv/txn_coord_sender_test.go

@@ -317,7 +317,7 @@ func TestTxnCoordSenderHeartbeat(t *testing.T) {
 	keyA := roachpb.Key("a")
 	keyC := roachpb.Key("c")
 	splitKey := roachpb.Key("b")
-	if err := s.DB.AdminSplit(ctx, splitKey /* spanKey */, splitKey /* splitKey */, true /* manual */); err != nil {
+	if err := s.DB.AdminSplit(ctx, splitKey /* spanKey */, splitKey /* splitKey */, hlc.MaxTimestamp /* expirationTimestamp */); err != nil {
 		t.Fatal(err)
 	}