keys: assert that schema is immutable

This commit adds a test which ensures that storage schema in doc.go does
not change. If it does, the test fails and forces the author to think
harder about the implications of the change.

Context / example of a concern: in replica destruction code
(kvstorage.DestroyReplica and variants), we should destroy, among other
things, the “unreplicated” keys. With separated engines this is tricky
because the raft and state machine keys are inconveniently interleaved.
Instead of using various ranged deletions that cover the entire
keyspace, we can manually clear each key (which we all know). But if we
do so, there is a risk someone adds a new key to the unreplicated
section and forgets to update the destruction code. This at best could
lead to a small disk space leak, and at worst a more serious bug in
replica lifecycle.

Another example: the unreplicated space does not have ranged Pebble
keys. Some code relies on this, e.g. applySnapshot does not clear ranged
keys when rewriting the replica state (as an optimization, because there
are none). It will have to start doing so if ranged keys are ever added
to the unreplicated span.

Epic: none
Release note: none

Author: pav-kv

pkg/keys/BUILD.bazel

@@ -29,6 +29,7 @@ go_test(
     name = "keys_test",
     size = "small",
     srcs = [
+        "doc_test.go",
         "keys_test.go",
         "printer_test.go",
         "sql_test.go",

pkg/keys/doc.go

@@ -146,7 +146,9 @@ package keys
 
 // NB: The sorting order of the symbols below map to the physical layout.
 // Preserve group-wise ordering when adding new constants.
-var _ = [...]interface{}{
+//
+// See TestSchemaIsStable for additional considerations when changing schema.
+var schema = [...]interface{}{
 	MinKey,
 
 	// There are five types of local key data enumerated below: replicated
@@ -176,19 +178,21 @@ var _ = [...]interface{}{
 	//   range as a whole. Though they are replicated, they are unaddressable.
 	//   Typical examples are MVCC stats and the abort span. They all share
 	//   `LocalRangeIDPrefix` and `LocalRangeIDReplicatedInfix`.
-	AbortSpanKey,             // "abc-"
-	RangeGCThresholdKey,      // "lgc-"
-	RangeAppliedStateKey,     // "rask"
-	RangeForceFlushKey,       // "rffk"
-	RangeLeaseKey,            // "rll-"
-	RangePriorReadSummaryKey, // "rprs"
+	LocalRangeIDReplicatedInfix,                 // "r"
+	AbortSpanKey,                                // "abc-"
+	RangeGCThresholdKey,                         // "lgc-"
+	RangeAppliedStateKey,                        // "rask"
+	RangeForceFlushKey,                          // "rffk"
+	RangeLeaseKey,                               // "rll-"
+	RangePriorReadSummaryKey,                    // "rprs"
 	ReplicatedSharedLocksTransactionLatchingKey, // "rsl-"
-	RangeVersionKey, // "rver"
+	RangeVersionKey,                             // "rver"
 
 	//   2. Unreplicated range-ID local keys: These contain metadata that
 	//   pertain to just one replica of a range. They are unreplicated and
 	//   unaddressable. The typical example is the Raft log. They all share
 	//   `LocalRangeIDPrefix` and `localRangeIDUnreplicatedInfix`.
+	localRangeIDUnreplicatedInfix,  // "u"
 	RangeTombstoneKey,              // "rftb"
 	RaftHardStateKey,               // "rfth"
 	RaftLogKey,                     // "rftl"
@@ -200,6 +204,7 @@ var _ = [...]interface{}{
 	//   as a whole. They are replicated and addressable. Typical examples are
 	//   the range descriptor and transaction records. They all share
 	//   `LocalRangePrefix`.
+	LocalRangePrefix,      // "k"
 	RangeProbeKey,         // "prbe"
 	QueueLastProcessedKey, // "qlpt"
 	RangeDescriptorKey,    // "rdsc"
@@ -208,6 +213,7 @@ var _ = [...]interface{}{
 	//   4. Store local keys: These contain metadata about an individual store.
 	//   They are unreplicated and unaddressable. The typical example is the
 	//   store 'ident' record. They all share `localStorePrefix`.
+	LocalStorePrefix,                 // "s"
 	DeprecatedStoreClusterVersionKey, // "cver"
 	StoreGossipKey,                   // "goss"
 	StoreHLCUpperBoundKey,            // "hlcu"
@@ -229,6 +235,7 @@ var _ = [...]interface{}{
 	//   Single key locks use a byte, LockTableSingleKeyInfix, that follows
 	//   the LocalRangeLockTablePrefix. This is to keep the single-key locks
 	//   separate from (future) range locks.
+	LocalRangeLockTablePrefix, // "z"
 	LockTableSingleKey,
 
 	// The global keyspace includes the meta{1,2}, system, system tenant SQL

pkg/keys/doc_test.go

@@ -0,0 +1,92 @@
+// Copyright 2025 The Cockroach Authors.
+//
+// Use of this software is governed by the CockroachDB Software License
+// included in the /LICENSE file.
+
+package keys
+
+import (
+	"testing"
+
+	"github.com/cockroachdb/cockroach/pkg/roachpb"
+	"github.com/stretchr/testify/require"
+)
+
+// TestSchemaIsStable asserts that the storage schema in doc.go does not change.
+// If it does (e.g. there is a new key), this test fails and nudges the author
+// to think through the implications.
+//
+// When changing the schema, it is a good idea to read the comments in doc.go
+// around the modified sections, and understand the lifecycle of the relevant
+// keys. For example, look at the code where the related keys are read, written,
+// modified, and removed.
+//
+// Note that it can be tricky: e.g. some code might be clearing a whole span of
+// keys (like unreplicated RangeID-local space), without explicitly calling out
+// individual keys, so the author should be aware of this to understand where
+// the lifetime of their new key ends.
+//
+// Conversely, some key spans can be cleared key-by-key, for efficiency or
+// simplicity reasons. New keys might need to be explicitly cleared in those
+// places, so that there is no storage space leakage. The author should not
+// assume that keys are cleared auto-magically in the right places. There are
+// "gaps" in the keyspace that don't have proper management because the code
+// assumes they are always empty.
+//
+// Another important aspect to consider is version compatibility, i.e. how
+// adjacent CRDB versions handle the added or removed keys. It might be
+// necessary to complete a migration to make a transition safe.
+//
+// Once this due diligence is done, it is ok to trivially fix this test.
+//
+// TODO(pav-kv): make a proper "structured" schema instead of a flat list of
+// things, and make the assertions stronger (currently they only count "lines"
+// in the schema).
+func TestSchemaIsStable(t *testing.T) {
+	pos := make(map[string]struct {
+		first int
+		last  int
+	})
+	add := func(k string, v int) {
+		p, found := pos[k]
+		if found {
+			p.last++
+			require.Equal(t, p.last, v)
+		} else {
+			p.first = v
+			p.last = v
+		}
+		pos[k] = p
+	}
+	for i, ref := range schema {
+		switch ref := ref.(type) {
+		case []byte:
+			add(string(ref), i)
+		case roachpb.Key:
+			add(string(ref), i)
+		}
+	}
+
+	section := func(from, to string, items int) {
+		t.Helper()
+		fromPos, ok := pos[from]
+		require.True(t, ok)
+		toPos, ok := pos[to]
+		require.True(t, ok)
+		require.Greater(t, toPos.first, fromPos.last)
+		require.Equal(t, items, toPos.first-fromPos.last-1)
+	}
+
+	// The replicated RangeID-local keys section.
+	// WARNING: if this line fails, don't just fix it, read the test comment.
+	section(string(LocalRangeIDReplicatedInfix), string(localRangeIDUnreplicatedInfix), 8)
+	// The unreplicated RangeID-local keys section.
+	// WARNING: if this line fails, don't just fix it, read the test comment.
+	section(string(localRangeIDUnreplicatedInfix), string(LocalRangePrefix), 6)
+	// The replicated range-local keys section.
+	// WARNING: if this line fails, don't just fix it, read the test comment.
+	section(string(LocalRangePrefix), string(LocalStorePrefix), 4)
+	// The store-local keys section.
+	// WARNING: if this line fails, don't just fix it, read the test comment.
+	section(string(LocalStorePrefix), string(LocalRangeLockTablePrefix), 12)
+}