Repository: cockroachdb/pebble
Pull request #4295 (2025-01-27 20:54:07)

sstable: always use target buffer for CompressAndChecksum
Currently `CompressAndChecksum` can alias the original data buffer if
we do not compress the data (either because compression is disabled,
or the data was not compressible enough). In most cases, we write out
the resulting data which can mangle the buffer. This leads most
callers to check if the buffer is not compressed and make a copy.

This change moves the copy into `CompressAndChecksum`; we always use
the dst buffer, even if we don't compress. This simplifies the callers
and makes things less fragile.

cockroach-teamcity: 

<!-- Reviewable:start -->
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/cockroachdb/pebble/4295)
<!-- Reviewable:end -->


Patch:
diff --git a/cockroachkvs/cockroachkvs.go b/cockroachkvs/cockroachkvs.go
index 3bdae183..1ac57fd5 100644
--- a/cockroachkvs/cockroachkvs.go
+++ b/cockroachkvs/cockroachkvs.go
@@ -70,6 +70,9 @@ var Comparer = base.Comparer{
 	FormatKey: FormatKey,
 
 	Separator: func(dst, a, b []byte) []byte {
+		if len(a) == 0 || len(b) == 0 {
+			panic(errors.AssertionFailedf("cannot separate empty keys"))
+		}
 		aKey, ok := getKeyPartFromEngineKey(a)
 		if !ok {
 			return append(dst, a...)
@@ -94,6 +97,13 @@ var Comparer = base.Comparer{
 	},
 
 	Successor: func(dst, a []byte) []byte {
+		// If a is empty, the key consisting of just a single 0x00 byte is the
+		// smallest non-empty key ≥ a. We don't return an empty key because
+		// empty keys are problematic when combined with a synthetic prefix
+		// (they form a key that has no trailing sentinel byte).
+		if len(a) == 0 {
+			return append(dst, 0x00)
+		}
 		aKey, ok := getKeyPartFromEngineKey(a)
 		if !ok {
 			return append(dst, a...)
@@ -122,6 +132,8 @@ var Comparer = base.Comparer{
 		return append(append(dst, a...), 0)
 	},
 
+	ValidateKey: validateEngineKey,
+
 	Name: "cockroach_comparator",
 }
 
@@ -266,7 +278,7 @@ func Split(key []byte) int {
 		return 0
 	}
 	if invariants.Enabled {
-		checkEngineKey(key)
+		validateEngineKey.MustValidate(key)
 	}
 	// Last byte is the version length + 1 when there is a version, else it is
 	// 0.
@@ -281,8 +293,8 @@ func Compare(a, b []byte) int {
 		return cmp.Compare(len(a), len(b))
 	}
 	if invariants.Enabled {
-		checkEngineKey(a)
-		checkEngineKey(b)
+		validateEngineKey.MustValidate(a)
+		validateEngineKey.MustValidate(b)
 	}
 	aSuffixLen := int(a[len(a)-1])
 	aSuffixStart := len(a) - aSuffixLen
@@ -343,8 +355,8 @@ func Equal(a, b []byte) bool {
 		return len(a) == len(b)
 	}
 	if invariants.Enabled {
-		checkEngineKey(a)
-		checkEngineKey(b)
+		validateEngineKey.MustValidate(a)
+		validateEngineKey.MustValidate(b)
 	}
 	aSuffixLen := int(a[len(a)-1])
 	aSuffixStart := len(a) - aSuffixLen
@@ -743,34 +755,60 @@ func (ks *cockroachKeySeeker) IsLowerBound(k []byte, syntheticSuffix []byte) boo
 	if len(syntheticSuffix) > 0 {
 		return ComparePointSuffixes(syntheticSuffix, k[len(roachKey)+1:]) >= 0
 	}
+	firstRowWall := ks.mvccWallTimes.At(0)
+	firstLogical := ks.mvccLogical.At(0)
+	// If the first row's WallTime and LogicalTime are both zero, the first row
+	// is either (a) unversioned (and sorts before all other suffixes) or (b) is
+	// a non-MVCC key with an untyped version.
+	if firstRowWall == 0 && firstLogical == 0 {
+		return ComparePointSuffixes(ks.untypedVersions.At(0), version) >= 0
+	}
+
 	var wallTime uint64
 	var logicalTime uint32
 	switch len(version) {
 	case engineKeyNoVersion:
+		// The zero-length version is the smallest possible suffix.
+		return true
 	case engineKeyVersionWallTimeLen:
 		wallTime = binary.BigEndian.Uint64(version[:8])
 	case engineKeyVersionWallAndLogicalTimeLen, engineKeyVersionWallLogicalAndSyntheticTimeLen:
 		wallTime = binary.BigEndian.Uint64(version[:8])
 		logicalTime = binary.BigEndian.Uint32(version[8:12])
 	default:
-		// The provided key `k` is not a MVCC key. Assert that the first key in
-		// the block is also not an MVCC key. If it were, that would mean there
-		// exists both a MVCC key and a non-MVCC key with the same prefix.
+		// The provided key `k` is not a MVCC key.
 		//
-		// TODO(jackson): Double check that we'll never produce index separators
-		// that are invalid version lengths.
-		if invariants.Enabled && ks.mvccWallTimes.At(0) != 0 {
-			panic("comparing timestamp with untyped suffix")
+		// The first row IS an MVCC key. We don't expect this to happen in
+		// practice, so this path is not performance sensitive. We reconsistute
+		// the first row's MVCC suffix and then compare it to the provided key's
+		// non-MVCC suffix.
+
+		//gcassert:noescape
+		var buf [13]byte
+		//gcassert:inline
+		binary.BigEndian.PutUint64(buf[:], firstRowWall)
+		var firstRowSuffix []byte
+		if firstLogical == 0 {
+			buf[8] = 9
+			firstRowSuffix = buf[:9]
+		} else {
+			//gcassert:inline
+			binary.BigEndian.PutUint32(buf[8:], uint32(firstLogical))
+			buf[12] = 13
+			firstRowSuffix = buf[:13]
 		}
-		return ComparePointSuffixes(ks.untypedVersions.At(0), version) >= 0
+		return ComparePointSuffixes(firstRowSuffix, version) >= 0
 	}
 
 	// NB: The sign comparison is inverted because suffixes are sorted such that
 	// the largest timestamps are "smaller" in the lexicographical ordering.
-	if v := cmp.Compare(ks.mvccWallTimes.At(0), wallTime); v != 0 {
+	if v := cmp.Compare(firstRowWall, wallTime); v != 0 {
+		// The empty-suffixed zero-timestamped key sorts first. If the first row
+		// has zero wall and logical times, it sorts before k and k is not a
+		// lower bound.
 		return v < 0
 	}
-	return cmp.Compare(uint32(ks.mvccLogical.At(0)), logicalTime) <= 0
+	return cmp.Compare(uint32(firstLogical), logicalTime) <= 0
 }
 
 // SeekGE is part of the KeySeeker interface.
@@ -983,16 +1021,20 @@ func (ks *cockroachKeySeeker) MaterializeUserKeyWithSyntheticSuffix(
 //go:linkname memmove runtime.memmove
 func memmove(to, from unsafe.Pointer, n uintptr)
 
-func checkEngineKey(k []byte) {
+var validateEngineKey base.ValidateKey = func(k []byte) error {
 	if len(k) == 0 {
-		panic(errors.AssertionFailedf("empty key"))
+		// It would be nice to assert that the key is not empty, but
+		// unfortunately separator keys within index blocks may be empty in the
+		// case that a sstable (and its single data block) has no keys.
+		return nil
 	}
 	if int(k[len(k)-1]) >= len(k) {
-		panic(errors.AssertionFailedf("malformed key terminator byte: %x", k))
+		return errors.AssertionFailedf("malformed key terminator byte: %x", k)
 	}
 	if k[len(k)-1] == 1 {
-		panic(errors.AssertionFailedf("invalid key terminator byte 1"))
+		return errors.AssertionFailedf("invalid key terminator byte 1 in key: %x", k)
 	}
+	return nil
 }
 
 // FormatKey returns a formatter for the user key.
@@ -1003,6 +1045,10 @@ func FormatKey(key []byte) fmt.Formatter {
 type formatKey []byte
 
 func (fk formatKey) Format(f fmt.State, c rune) {
+	if len(fk) == 0 {
+		fmt.Fprint(f, "<empty>")
+		return
+	}
 	i := Split(fk)
 	fmt.Fprintf(f, "%s", []byte(fk)[:i-1])
 	if i == len(fk) {
@@ -1051,7 +1097,15 @@ func (fk formatKeySuffix) Format(f fmt.State, c rune) {
 //     c. <WALLTIME-SECONDS>,<LOGICAL>
 //     d. <WALLTIME-SECONDS>.<WALLTIME-NANOS>,<LOGICAL>
 //  2. A lock table key in the format: <STRENGTH-BYTE>,<TXNUUID>
+//  3. A raw hex-encoded string prefixed with "hex:".
 func ParseFormattedKey(formattedKey string) []byte {
+	if strings.HasPrefix(formattedKey, "hex:") {
+		decoded, err := hex.DecodeString(formattedKey[4:])
+		if err != nil {
+			panic(errors.Newf("invalid hex-encoded key: %w", err))
+		}
+		return decoded
+	}
 	i := strings.IndexByte(formattedKey, '@')
 	if i == -1 {
 		// Suffix-less.
diff --git a/compaction.go b/compaction.go
index 780283f2..b39ab15d 100644
--- a/compaction.go
+++ b/compaction.go
@@ -1500,7 +1500,7 @@ func (d *DB) flush1() (bytesFlushed uint64, err error) {
 		Err:        err,
 	}
 	if err == nil {
-		validateVersionEdit(ve, d.opts.Experimental.KeyValidationFunc, d.opts.Comparer.FormatKey, d.opts.Logger)
+		validateVersionEdit(ve, d.opts.Comparer.ValidateKey, d.opts.Comparer.FormatKey, d.opts.Logger)
 		for i := range ve.NewFiles {
 			e := &ve.NewFiles[i]
 			info.Output = append(info.Output, e.Meta.TableInfo())
@@ -2356,7 +2356,7 @@ func (d *DB) compact1(c *compaction, errChannel chan error) (err error) {
 
 	info.Duration = d.timeNow().Sub(startTime)
 	if err == nil {
-		validateVersionEdit(ve, d.opts.Experimental.KeyValidationFunc, d.opts.Comparer.FormatKey, d.opts.Logger)
+		validateVersionEdit(ve, d.opts.Comparer.ValidateKey, d.opts.Comparer.FormatKey, d.opts.Logger)
 		err = func() error {
 			var err error
 			d.mu.versions.logLock()
@@ -3241,10 +3241,10 @@ func (d *DB) newCompactionOutput(
 // validateVersionEdit validates that start and end keys across new and deleted
 // files in a versionEdit pass the given validation function.
 func validateVersionEdit(
-	ve *versionEdit, validateFn func([]byte) error, format base.FormatKey, logger Logger,
+	ve *versionEdit, vk base.ValidateKey, format base.FormatKey, logger Logger,
 ) {
 	validateKey := func(f *manifest.FileMetadata, key []byte) {
-		if err := validateFn(key); err != nil {
+		if err := vk.Validate(key); err != nil {
 			logger.Fatalf("pebble: version edit validation failed (key=%s file=%s): %v", format(key), f, err)
 		}
 	}
diff --git a/ingest.go b/ingest.go
index 08491994..c409c289 100644
--- a/ingest.go
+++ b/ingest.go
@@ -98,6 +98,10 @@ func ingestValidateKey(opts *Options, key *InternalKey) error {
 		return base.CorruptionErrorf("pebble: external sstable has non-zero seqnum: %s",
 			key.Pretty(opts.Comparer.FormatKey))
 	}
+	if err := opts.Comparer.ValidateKey.Validate(key.UserKey); err != nil {
+		return base.CorruptionErrorf("pebble: external sstable has corrupted key: %s, %w",
+			key.Pretty(opts.Comparer.FormatKey), err)
+	}
 	return nil
 }
 
diff --git a/internal/base/comparer.go b/internal/base/comparer.go
index 8e277e60..92713bad 100644
--- a/internal/base/comparer.go
+++ b/internal/base/comparer.go
@@ -12,6 +12,7 @@ import (
 	"strconv"
 	"unicode/utf8"
 
+	"github.com/cockroachdb/crlib/crbytes"
 	"github.com/cockroachdb/errors"
 )
 
@@ -124,11 +125,18 @@ type FormatValue func(key, value []byte) fmt.Formatter
 //
 // For example, if a and b are the []byte equivalents of the strings "black" and
 // "blue", then the function may append "blb" to dst.
+//
+// Callers must guarantee that len(a) > 0 and len(b) > 0.
 type Separator func(dst, a, b []byte) []byte
 
 // Successor appends to dst a shortened key k given a key a such that
 // Compare(a, k) <= 0. A simple implementation may return a unchanged.
 // The appended key k must be valid to pass to Compare.
+//
+// The parameter a may be an empty slice even if an empty slice was never
+// committed to Pebble and is not a valid key representation. If a is the empty
+// slice, Successor must return a valid key but otherwise has no other
+// constraints.
 type Successor func(dst, a []byte) []byte
 
 // ImmediateSuccessor is invoked with a prefix key ([Split(a) == len(a)]) and
@@ -229,6 +237,10 @@ type Comparer struct {
 	// FormatValue is optional.
 	FormatValue FormatValue
 
+	// ValidateKey is an optional function that determines whether a key is
+	// valid according to this Comparer's key encoding.
+	ValidateKey ValidateKey
+
 	// Name is the name of the comparer.
 	//
 	// The on-disk format stores the comparer name, and opening a database with a
@@ -308,14 +320,15 @@ var DefaultComparer = &Comparer{
 	FormatKey: DefaultFormatter,
 
 	Separator: func(dst, a, b []byte) []byte {
-		i, n := SharedPrefixLen(a, b), len(dst)
+		if len(a) == 0 || len(b) == 0 {
+			panic(errors.AssertionFailedf("empty keys"))
+		}
+
+		i := crbytes.CommonPrefix(a, b)
+		n := len(dst)
 		dst = append(dst, a...)
 
-		min := len(a)
-		if min > len(b) {
-			min = len(b)
-		}
-		if i >= min {
+		if i == len(a) || i == len(b) {
 			// Do not shorten if one string is a prefix of the other.
 			return dst
 		}
@@ -362,25 +375,6 @@ var DefaultComparer = &Comparer{
 	Name: "leveldb.BytewiseComparator",
 }
 
-// SharedPrefixLen returns the largest i such that a[:i] equals b[:i].
-// This function can be useful in implementing the Comparer interface.
-func SharedPrefixLen(a, b []byte) int {
-	i, n := 0, len(a)
-	if n > len(b) {
-		n = len(b)
-	}
-	asUint64 := func(c []byte, i int) uint64 {
-		return binary.LittleEndian.Uint64(c[i:])
-	}
-	for i < n-7 && asUint64(a, i) == asUint64(b, i) {
-		i += 8
-	}
-	for i < n && a[i] == b[i] {
-		i++
-	}
-	return i
-}
-
 // MinUserKey returns the smaller of two user keys. If one of the keys is nil,
 // the other one is returned.
 func MinUserKey(cmp Compare, a, b []byte) []byte {
@@ -439,13 +433,54 @@ func MakeAssertComparer(c Comparer) Comparer {
 		ComparePointSuffixes: c.ComparePointSuffixes,
 		CompareRangeSuffixes: c.CompareRangeSuffixes,
 		AbbreviatedKey:       c.AbbreviatedKey,
-		Separator:            c.Separator,
-		Successor:            c.Successor,
-		ImmediateSuccessor:   c.ImmediateSuccessor,
-		FormatKey:            c.FormatKey,
-		Split:                c.Split,
-		FormatValue:          c.FormatValue,
-		Name:                 c.Name,
+		Separator: func(dst, a, b []byte) []byte {
+			if len(a) == 0 || len(b) == 0 {
+				panic(errors.AssertionFailedf("empty keys"))
+			}
+			ret := c.Separator(dst, a, b)
+			// The Separator func must return a valid key.
+			c.ValidateKey.MustValidate(ret)
+			return ret
+		},
+		Successor: func(dst, a []byte) []byte {
+			ret := c.Successor(dst, a)
+			// The Successor func must return a valid key.
+			c.ValidateKey.MustValidate(ret)
+			return ret
+		},
+		ImmediateSuccessor: func(dst, a []byte) []byte {
+			ret := c.ImmediateSuccessor(dst, a)
+			// The ImmediateSuccessor func must return a valid key.
+			c.ValidateKey.MustValidate(ret)
+			return ret
+		},
+		FormatKey:   c.FormatKey,
+		Split:       c.Split,
+		FormatValue: c.FormatValue,
+		ValidateKey: c.ValidateKey,
+		Name:        c.Name,
+	}
+}
+
+// ValidateKey is a func that determines whether a key is valid according to a
+// particular key encoding. Returns nil if the provided key is a valid, full
+// user key. Implementations must be careful to not mutate the provided key.
+type ValidateKey func([]byte) error
+
+// Validate validates the provided user key. If the func is nil, Validate
+// returns nil.
+func (v ValidateKey) Validate(key []byte) error {
+	if v == nil {
+		return nil
+	}
+	return v(key)
+}
+
+// MustValidate validates the provided user key, panicking if the key is
+// invalid.
+func (v ValidateKey) MustValidate(key []byte) {
+	if err := v.Validate(key); err != nil {
+		panic(err)
 	}
 }
 
@@ -489,6 +524,13 @@ func CheckComparer(c *Comparer, prefixes [][]byte, suffixes [][]byte) error {
 		for i := 1; i < len(suffixes); i++ {
 			a := slices.Concat(p, suffixes[i-1])
 			b := slices.Concat(p, suffixes[i])
+			if err := c.ValidateKey.Validate(a); err != nil {
+				return err
+			}
+			if err := c.ValidateKey.Validate(b); err != nil {
+				return err
+			}
+
 			// Make sure the Compare function agrees with ComparePointSuffixes.
 			if cmp := c.Compare(a, b); cmp > 0 {
 				return errors.Errorf("Compare(%s, %s)=%d, expected <= 0", c.FormatKey(a), c.FormatKey(b), cmp)
@@ -500,9 +542,15 @@ func CheckComparer(c *Comparer, prefixes [][]byte, suffixes [][]byte) error {
 	for _, ap := range prefixes {
 		for _, as := range suffixes {
 			a := slices.Concat(ap, as)
+			if err := c.ValidateKey.Validate(a); err != nil {
+				return err
+			}
 			for _, bp := range prefixes {
 				for _, bs := range suffixes {
 					b := slices.Concat(bp, bs)
+					if err := c.ValidateKey.Validate(b); err != nil {
+						return err
+					}
 					result := c.Compare(a, b)
 					if (result == 0) != c.Equal(a, b) {
 						return errors.Errorf("Equal(%s, %s) doesn't agree with Compare", c.FormatKey(a), c.FormatKey(b))
diff --git a/internal/base/internal.go b/internal/base/internal.go
index 0c123eb4..bd6381bf 100644
--- a/internal/base/internal.go
+++ b/internal/base/internal.go
@@ -12,6 +12,8 @@ import (
 	"strings"
 	"sync/atomic"
 
+	"github.com/cockroachdb/errors"
+	"github.com/cockroachdb/pebble/internal/invariants"
 	"github.com/cockroachdb/redact"
 )
 
@@ -398,6 +400,9 @@ func (k InternalKey) EncodeTrailer() [8]byte {
 func (k InternalKey) Separator(
 	cmp Compare, sep Separator, buf []byte, other InternalKey,
 ) InternalKey {
+	if invariants.Enabled && (len(k.UserKey) == 0 || len(other.UserKey) == 0) {
+		panic(errors.AssertionFailedf("empty keys passed to Separator: %s, %s", k, other))
+	}
 	buf = sep(buf, k.UserKey, other.UserKey)
 	if len(buf) <= len(k.UserKey) && cmp(k.UserKey, buf) < 0 {
 		// The separator user key is physically shorter than k.UserKey (if it is
@@ -416,7 +421,7 @@ func (k InternalKey) Separator(
 // InternalKey.UserKey, though it is valid to pass a nil.
 func (k InternalKey) Successor(cmp Compare, succ Successor, buf []byte) InternalKey {
 	buf = succ(buf, k.UserKey)
-	if len(buf) <= len(k.UserKey) && cmp(k.UserKey, buf) < 0 {
+	if (len(k.UserKey) == 0 || len(buf) <= len(k.UserKey)) && cmp(k.UserKey, buf) < 0 {
 		// The successor user key is physically shorter that k.UserKey (if it is
 		// longer, we'll continue to use "k"), but logically after. Tack on the max
 		// sequence number to the shortened user key. Note that we could tack on
diff --git a/internal/cache/clockpro_normal.go b/internal/cache/clockpro_normal.go
index ae49938e..c8b250c6 100644
--- a/internal/cache/clockpro_normal.go
+++ b/internal/cache/clockpro_normal.go
@@ -3,7 +3,6 @@
 // the LICENSE file.
 
 //go:build !tracing
-// +build !tracing
 
 package cache
 
diff --git a/internal/cache/clockpro_tracing.go b/internal/cache/clockpro_tracing.go
index d14c1cdc..ccc592b3 100644
--- a/internal/cache/clockpro_tracing.go
+++ b/internal/cache/clockpro_tracing.go
@@ -3,7 +3,6 @@
 // the LICENSE file.
 
 //go:build tracing
-// +build tracing
 
 package cache
 
diff --git a/internal/cache/refcnt_normal.go b/internal/cache/refcnt_normal.go
index 116c2047..a1592672 100644
--- a/internal/cache/refcnt_normal.go
+++ b/internal/cache/refcnt_normal.go
@@ -3,7 +3,6 @@
 // the LICENSE file.
 
 //go:build !tracing
-// +build !tracing
 
 package cache
 
diff --git a/internal/cache/refcnt_tracing.go b/internal/cache/refcnt_tracing.go
index 1d5e6c02..151c68d0 100644
--- a/internal/cache/refcnt_tracing.go
+++ b/internal/cache/refcnt_tracing.go
@@ -3,7 +3,6 @@
 // the LICENSE file.
 
 //go:build tracing
-// +build tracing
 
 package cache
 
diff --git a/internal/devtools/tools.go b/internal/devtools/tools.go
index 132cc746..516cd9d2 100644
--- a/internal/devtools/tools.go
+++ b/internal/devtools/tools.go
@@ -1,5 +1,4 @@
 //go:build tools
-// +build tools
 
 package tools
 
diff --git a/internal/manual/manual_nocgo.go b/internal/manual/manual_nocgo.go
index 5a732b3c..5c38f674 100644
--- a/internal/manual/manual_nocgo.go
+++ b/internal/manual/manual_nocgo.go
@@ -3,7 +3,6 @@
 // the LICENSE file.
 
 //go:build !cgo
-// +build !cgo
 
 package manual
 
diff --git a/internal/rawalloc/rawalloc_32bit.go b/internal/rawalloc/rawalloc_32bit.go
index 3112cc9e..1046d397 100644
--- a/internal/rawalloc/rawalloc_32bit.go
+++ b/internal/rawalloc/rawalloc_32bit.go
@@ -13,7 +13,6 @@
 // permissions and limitations under the License.
 
 //go:build 386 || amd64p32 || arm || armbe || ppc || sparc
-// +build 386 amd64p32 arm armbe ppc sparc
 
 package rawalloc
 
diff --git a/internal/rawalloc/rawalloc_64bit.go b/internal/rawalloc/rawalloc_64bit.go
index 6660462e..f07420dc 100644
--- a/internal/rawalloc/rawalloc_64bit.go
+++ b/internal/rawalloc/rawalloc_64bit.go
@@ -13,7 +13,6 @@
 // permissions and limitations under the License.
 
 //go:build amd64 || arm64 || arm64be || ppc64 || ppc64le || mips64 || mips64le || s390x || sparc64 || riscv64 || loong64
-// +build amd64 arm64 arm64be ppc64 ppc64le mips64 mips64le s390x sparc64 riscv64 loong64
 
 package rawalloc
 
diff --git a/internal/rawalloc/rawalloc_gccgo.go b/internal/rawalloc/rawalloc_gccgo.go
index f2db79c7..716fe6c8 100644
--- a/internal/rawalloc/rawalloc_gccgo.go
+++ b/internal/rawalloc/rawalloc_gccgo.go
@@ -13,7 +13,6 @@
 // permissions and limitations under the License.
 
 //go:build gccgo
-// +build gccgo
 
 package rawalloc
 
diff --git a/internal/rawalloc/rawalloc_go1.9.go b/internal/rawalloc/rawalloc_go1.9.go
index 65da4361..055f6714 100644
--- a/internal/rawalloc/rawalloc_go1.9.go
+++ b/internal/rawalloc/rawalloc_go1.9.go
@@ -13,7 +13,6 @@
 // permissions and limitations under the License.
 
 //go:build gc && go1.9
-// +build gc,go1.9
 
 package rawalloc
 
diff --git a/internal/rawalloc/rawalloc_mipsall.go b/internal/rawalloc/rawalloc_mipsall.go
index 55b45eb1..badb0367 100644
--- a/internal/rawalloc/rawalloc_mipsall.go
+++ b/internal/rawalloc/rawalloc_mipsall.go
@@ -13,7 +13,6 @@
 // permissions and limitations under the License.
 
 //go:build mips || mipsle || mips64p32 || mips64p32le
-// +build mips mipsle mips64p32 mips64p32le
 
 package rawalloc
 
diff --git a/metamorphic/build.go b/metamorphic/build.go
index 4b5af83a..8c8bee8c 100644
--- a/metamorphic/build.go
+++ b/metamorphic/build.go
@@ -97,6 +97,7 @@ func writeSSTForIngestion(
 		if err != nil {
 			return nil, err
 		}
+		t.opts.Comparer.ValidateKey.MustValidate(k.K.UserKey)
 		if err := w.Raw().AddWithForceObsolete(k.K, valBytes, false); err != nil {
 			return nil, err
 		}
@@ -111,7 +112,11 @@ func writeSSTForIngestion(
 			if syntheticSuffix.IsSet() {
 				panic("synthetic suffix with RangeDel")
 			}
-			if err := w.DeleteRange(outputKey(span.Start, nil), outputKey(span.End, nil)); err != nil {
+			start := outputKey(span.Start, nil)
+			end := outputKey(span.End, nil)
+			t.opts.Comparer.ValidateKey.MustValidate(start)
+			t.opts.Comparer.ValidateKey.MustValidate(end)
+			if err := w.DeleteRange(start, end); err != nil {
 				return nil, err
 			}
 		}
@@ -139,6 +144,8 @@ func writeSSTForIngestion(
 				End:   outputKey(span.End, nil),
 				Keys:  make([]keyspan.Key, 0, len(span.Keys)),
 			}
+			t.opts.Comparer.ValidateKey.MustValidate(collapsed.Start)
+			t.opts.Comparer.ValidateKey.MustValidate(collapsed.End)
 			keys := span.Keys
 			if syntheticSuffix.IsSet() {
 				keys = slices.Clone(span.Keys)
diff --git a/metamorphic/key_manager.go b/metamorphic/key_manager.go
index a3622be5..5c1cbd06 100644
--- a/metamorphic/key_manager.go
+++ b/metamorphic/key_manager.go
@@ -389,6 +389,7 @@ func newKeyManager(numInstances int, kf KeyFormat) *keyManager {
 // addNewKey adds the given key to the key manager for global key tracking.
 // Returns false iff this is not a new key.
 func (k *keyManager) addNewKey(key []byte) bool {
+	k.kf.Comparer.ValidateKey.MustValidate(key)
 	if k.globalKeysMap[string(key)] {
 		return false
 	}
@@ -409,6 +410,7 @@ func (k *keyManager) addNewKey(key []byte) bool {
 // getOrInit returns the keyMeta for the (objID, key) pair, if it exists, else
 // allocates, initializes and returns a new value.
 func (k *keyManager) getOrInit(id objID, key []byte) *keyMeta {
+	k.kf.Comparer.ValidateKey.MustValidate(key)
 	objKeys := k.objKeyMeta(id)
 	m, ok := objKeys.keys[string(key)]
 	if ok {
diff --git a/metamorphic/ops.go b/metamorphic/ops.go
index 0a4599ba..5b59e186 100644
--- a/metamorphic/ops.go
+++ b/metamorphic/ops.go
@@ -811,6 +811,7 @@ func (o *ingestOp) collapseBatch(
 	if pointIter != nil {
 		var lastUserKey []byte
 		for kv := pointIter.First(); kv != nil; kv = pointIter.Next() {
+			t.opts.Comparer.ValidateKey.MustValidate(kv.K.UserKey)
 			// Ignore duplicate keys.
 			//
 			// Note: this is necessary due to MERGE keys, otherwise it would be
@@ -882,6 +883,7 @@ func (o *ingestOp) collapseBatch(
 				continue
 			}
 			ik := base.MakeInternalKey(key, 0, kind)
+			t.opts.Comparer.ValidateKey.MustValidate(ik.UserKey)
 			if err := collapsed.AddInternalKey(&ik, value, nil); err != nil {
 				return nil, err
 			}
@@ -943,6 +945,8 @@ func (o *ingestAndExciseOp) run(t *Test, h historyRecorder) {
 		return
 	}
 
+	t.opts.Comparer.ValidateKey.MustValidate(o.exciseStart)
+	t.opts.Comparer.ValidateKey.MustValidate(o.exciseEnd)
 	if t.testOpts.useExcise {
 		err = firstError(err, t.withRetries(func() error {
 			_, err := db.IngestAndExcise(context.Background(), []string{path}, nil /* shared */, nil /* external */, pebble.KeyRange{
@@ -1043,6 +1047,9 @@ func (o *ingestExternalFilesOp) run(t *Test, h historyRecorder) {
 	} else {
 		external := make([]pebble.ExternalFile, len(o.objs))
 		for i, obj := range o.objs {
+			t.opts.Comparer.ValidateKey.MustValidate(obj.bounds.Start)
+			t.opts.Comparer.ValidateKey.MustValidate(obj.bounds.End)
+
 			meta := t.getExternalObj(obj.externalObjID)
 			external[i] = pebble.ExternalFile{
 				Locator:           "external",
@@ -1449,9 +1456,9 @@ type iterSeekGEOp struct {
 	derivedReaderID objID
 }
 
-func iteratorPos(i *retryableIter) string {
+func iteratorPos(kf KeyFormat, i *retryableIter) string {
 	var buf bytes.Buffer
-	fmt.Fprintf(&buf, "%q", i.Key())
+	fmt.Fprintf(&buf, "%q", kf.FormatKey(i.Key()))
 	hasPoint, hasRange := i.HasPointAndRange()
 	if hasPoint {
 		fmt.Fprintf(&buf, ",%q", i.Value())
@@ -1460,12 +1467,12 @@ func iteratorPos(i *retryableIter) string {
 	}
 	if hasRange {
 		start, end := i.RangeBounds()
-		fmt.Fprintf(&buf, ",[%q,%q)=>{", start, end)
+		fmt.Fprintf(&buf, ",[%q,%q)=>{", kf.FormatKey(start), kf.FormatKey(end))
 		for i, rk := range i.RangeKeys() {
 			if i > 0 {
 				fmt.Fprint(&buf, ",")
 			}
-			fmt.Fprintf(&buf, "%q=%q", rk.Suffix, rk.Value)
+			fmt.Fprintf(&buf, "%q=%q", kf.FormatKeySuffix(rk.Suffix), rk.Value)
 		}
 		fmt.Fprint(&buf, "}")
 	} else {
@@ -1506,7 +1513,7 @@ func (o *iterSeekGEOp) run(t *Test, h historyRecorder) {
 	}
 	if valid {
 		h.Recordf("%s // [%s,%s] %v", o.formattedString(t.testOpts.KeyFormat),
-			validStr, iteratorPos(i), i.Error())
+			validStr, iteratorPos(t.testOpts.KeyFormat, i), i.Error())
 	} else {
 		h.Recordf("%s // [%s] %v",
 			o.formattedString(t.testOpts.KeyFormat), validStr, i.Error())
@@ -1551,7 +1558,7 @@ func (o *iterSeekPrefixGEOp) run(t *Test, h historyRecorder) {
 	valid := i.SeekPrefixGE(o.key)
 	if valid {
 		h.Recordf("%s // [%t,%s] %v", o.formattedString(t.testOpts.KeyFormat),
-			valid, iteratorPos(i), i.Error())
+			valid, iteratorPos(t.testOpts.KeyFormat, i), i.Error())
 	} else {
 		h.Recordf("%s // [%t] %v", o.formattedString(t.testOpts.KeyFormat), valid, i.Error())
 	}
@@ -1592,7 +1599,7 @@ func (o *iterSeekLTOp) run(t *Test, h historyRecorder) {
 	}
 	if valid {
 		h.Recordf("%s // [%s,%s] %v", o.formattedString(t.testOpts.KeyFormat),
-			validStr, iteratorPos(i), i.Error())
+			validStr, iteratorPos(t.testOpts.KeyFormat, i), i.Error())
 	} else {
 		h.Recordf("%s // [%s] %v", o.formattedString(t.testOpts.KeyFormat),
 			validStr, i.Error())
@@ -1627,7 +1634,7 @@ func (o *iterFirstOp) run(t *Test, h historyRecorder) {
 	valid := i.First()
 	if valid {
 		h.Recordf("%s // [%t,%s] %v", o.formattedString(t.testOpts.KeyFormat),
-			valid, iteratorPos(i), i.Error())
+			valid, iteratorPos(t.testOpts.KeyFormat, i), i.Error())
 	} else {
 		h.Recordf("%s // [%t] %v", o.formattedString(t.testOpts.KeyFormat),
 			valid, i.Error())
@@ -1653,7 +1660,7 @@ func (o *iterLastOp) run(t *Test, h historyRecorder) {
 	valid := i.Last()
 	if valid {
 		h.Recordf("%s // [%t,%s] %v", o.formattedString(t.testOpts.KeyFormat),
-			valid, iteratorPos(i), i.Error())
+			valid, iteratorPos(t.testOpts.KeyFormat, i), i.Error())
 	} else {
 		h.Recordf("%s // [%t] %v", o.formattedString(t.testOpts.KeyFormat),
 			valid, i.Error())
@@ -1687,7 +1694,7 @@ func (o *iterNextOp) run(t *Test, h historyRecorder) {
 	}
 	if valid {
 		h.Recordf("%s // [%s,%s] %v", o.formattedString(t.testOpts.KeyFormat),
-			validStr, iteratorPos(i), i.Error())
+			validStr, iteratorPos(t.testOpts.KeyFormat, i), i.Error())
 	} else {
 		h.Recordf("%s // [%s] %v", o.formattedString(t.testOpts.KeyFormat),
 			validStr, i.Error())
@@ -1716,7 +1723,7 @@ func (o *iterNextPrefixOp) run(t *Test, h historyRecorder) {
 	validStr := validBoolToStr(valid)
 	if valid {
 		h.Recordf("%s // [%s,%s] %v", o.formattedString(t.testOpts.KeyFormat),
-			validStr, iteratorPos(i), i.Error())
+			validStr, iteratorPos(t.testOpts.KeyFormat, i), i.Error())
 	} else {
 		h.Recordf("%s // [%s] %v", o.formattedString(t.testOpts.KeyFormat),
 			validStr, i.Error())
@@ -1787,7 +1794,7 @@ func (o *iterPrevOp) run(t *Test, h historyRecorder) {
 	}
 	if valid {
 		h.Recordf("%s // [%s,%s] %v", o.formattedString(t.testOpts.KeyFormat),
-			validStr, iteratorPos(i), i.Error())
+			validStr, iteratorPos(t.testOpts.KeyFormat, i), i.Error())
 	} else {
 		h.Recordf("%s // [%s] %v", o.formattedString(t.testOpts.KeyFormat),
 			validStr, i.Error())
@@ -2059,12 +2066,17 @@ func (r *replicateOp) runExternalReplicate(
 			if err != nil {
 				panic(err)
 			}
+			t.opts.Comparer.ValidateKey.MustValidate(key.UserKey)
 			return w.Raw().AddWithForceObsolete(base.MakeInternalKey(key.UserKey, 0, key.Kind()), val, false)
 		},
 		func(start, end []byte, seqNum base.SeqNum) error {
+			t.opts.Comparer.ValidateKey.MustValidate(start)
+			t.opts.Comparer.ValidateKey.MustValidate(end)
 			return w.DeleteRange(start, end)
 		},
 		func(start, end []byte, keys []keyspan.Key) error {
+			t.opts.Comparer.ValidateKey.MustValidate(start)
+			t.opts.Comparer.ValidateKey.MustValidate(end)
 			return w.Raw().EncodeSpan(keyspan.Span{
 				Start: start,
 				End:   end,
@@ -2107,6 +2119,9 @@ func (r *replicateOp) runExternalReplicate(
 		return
 	}
 
+	t.opts.Comparer.ValidateKey.MustValidate(r.start)
+	t.opts.Comparer.ValidateKey.MustValidate(r.end)
+
 	_, err = dest.IngestAndExcise(context.Background(), []string{sstPath}, nil, externalSSTs /* external */, pebble.KeyRange{Start: r.start, End: r.end})
 	h.Recordf("%s // %v", r.formattedString(t.testOpts.KeyFormat), err)
 }
@@ -2139,6 +2154,8 @@ func (r *replicateOp) run(t *Test, h historyRecorder) {
 		return
 	}
 
+	t.opts.Comparer.ValidateKey.MustValidate(r.start)
+	t.opts.Comparer.ValidateKey.MustValidate(r.end)
 	// First, do a RangeKeyDelete and DeleteRange on the whole span.
 	if err := dest.RangeKeyDelete(r.start, r.end, t.writeOpts); err != nil {
 		h.Recordf("%s // %v", r.formattedString(t.testOpts.KeyFormat), err)
@@ -2173,6 +2190,8 @@ func (r *replicateOp) run(t *Test, h historyRecorder) {
 			rangeKeys := iter.RangeKeys()
 			rkStart, rkEnd := iter.RangeBounds()
 
+			t.opts.Comparer.ValidateKey.MustValidate(rkStart)
+			t.opts.Comparer.ValidateKey.MustValidate(rkEnd)
 			span := keyspan.Span{Start: rkStart, End: rkEnd, Keys: make([]keyspan.Key, len(rangeKeys))}
 			for i := range rangeKeys {
 				span.Keys[i] = keyspan.Key{
diff --git a/objstorage/objstorageprovider/objiotracing/obj_io_tracing_off.go b/objstorage/objstorageprovider/objiotracing/obj_io_tracing_off.go
index a4923ab6..3857b418 100644
--- a/objstorage/objstorageprovider/objiotracing/obj_io_tracing_off.go
+++ b/objstorage/objstorageprovider/objiotracing/obj_io_tracing_off.go
@@ -3,7 +3,6 @@
 // the LICENSE file.
 
 //go:build !pebble_obj_io_tracing
-// +build !pebble_obj_io_tracing
 
 package objiotracing
 
diff --git a/objstorage/objstorageprovider/objiotracing/obj_io_tracing_on.go b/objstorage/objstorageprovider/objiotracing/obj_io_tracing_on.go
index d3f8b1bb..be4c8e74 100644
--- a/objstorage/objstorageprovider/objiotracing/obj_io_tracing_on.go
+++ b/objstorage/objstorageprovider/objiotracing/obj_io_tracing_on.go
@@ -3,7 +3,6 @@
 // the LICENSE file.
 
 //go:build pebble_obj_io_tracing
-// +build pebble_obj_io_tracing
 
 package objiotracing
 
diff --git a/options.go b/options.go
index c991648f..f5c8eb54 100644
--- a/options.go
+++ b/options.go
@@ -654,19 +654,6 @@ type Options struct {
 		// limited by runtime.GOMAXPROCS.
 		FileCacheShards int
 
-		// KeyValidationFunc is a function to validate a user key in an SSTable.
-		//
-		// Currently, this function is used to validate the smallest and largest
-		// keys in an SSTable undergoing compaction. In this case, returning an
-		// error from the validation function will result in a panic at runtime,
-		// given that there is rarely any way of recovering from malformed keys
-		// present in compacted files. By default, validation is not performed.
-		//
-		// Additional use-cases may be added in the future.
-		//
-		// NOTE: callers should take care to not mutate the key being validated.
-		KeyValidationFunc func(userKey []byte) error
-
 		// ValidateOnIngest schedules validation of sstables after they have
 		// been ingested.
 		//
@@ -1165,9 +1152,6 @@ func (o *Options) EnsureDefaults() *Options {
 	if o.Experimental.CompactionLimiter == nil {
 		o.Experimental.CompactionLimiter = &base.DefaultCompactionLimiter{}
 	}
-	if o.Experimental.KeyValidationFunc == nil {
-		o.Experimental.KeyValidationFunc = func([]byte) error { return nil }
-	}
 	if o.KeySchema == "" && len(o.KeySchemas) == 0 {
 		ks := colblk.DefaultKeySchema(o.Comparer, 16 /* bundleSize */)
 		o.KeySchema = ks.Name
diff --git a/sstable/block/compression.go b/sstable/block/compression.go
index 15b079b4..4ab6a643 100644
--- a/sstable/block/compression.go
+++ b/sstable/block/compression.go
@@ -212,18 +212,6 @@ func (b PhysicalBlock) Clone() PhysicalBlock {
 	return PhysicalBlock{data: data, trailer: b.trailer}
 }
 
-// CloneUsingBuf makes a copy of the block data, using the given slice if it has
-// enough capacity.
-func (b PhysicalBlock) CloneUsingBuf(buf []byte) (_ PhysicalBlock, newBuf []byte) {
-	newBuf = append(buf[:0], b.data...)
-	return PhysicalBlock{data: newBuf, trailer: b.trailer}, newBuf
-}
-
-// IsCompressed returns true if the block is compressed.
-func (b *PhysicalBlock) IsCompressed() bool {
-	return CompressionIndicator(b.trailer[0]) != NoCompressionIndicator
-}
-
 // WriteTo writes the block (including its trailer) to the provided Writable. If
 // err == nil, n is the number of bytes successfully written to the Writable.
 //
@@ -247,35 +235,37 @@ func (b *PhysicalBlock) WriteTo(w objstorage.Writable) (n int, err error) {
 }
 
 // CompressAndChecksum compresses and checksums the provided block, returning
-// the compressed block and its trailer. The dst argument is used for the
-// compressed payload if it's sufficiently large. If it's not, a new buffer is
-// allocated and *dst is updated to point to it.
+// the compressed block and its trailer. The result is appended to the dst
+// argument.
 //
 // If the compressed block is not sufficiently smaller than the original block,
-// the compressed payload is discarded and the original, uncompressed block is
-// used to avoid unnecessary decompression overhead at read time.
+// the compressed payload is discarded and the original, uncompressed block data
+// is used to avoid unnecessary decompression overhead at read time.
 func CompressAndChecksum(
-	dst *[]byte, block []byte, compression Compression, checksummer *Checksummer,
+	dst *[]byte, blockData []byte, compression Compression, checksummer *Checksummer,
 ) PhysicalBlock {
+	buf := (*dst)[:0]
 	// Compress the buffer, discarding the result if the improvement isn't at
 	// least 12.5%.
 	algo := NoCompressionIndicator
 	if compression != NoCompression {
-		var compressed []byte
-		algo, compressed = compress(compression, block, *dst)
-		if algo != NoCompressionIndicator && cap(compressed) > cap(*dst) {
-			*dst = compressed[:cap(compressed)]
-		}
-		if len(compressed) < len(block)-len(block)/8 {
-			block = compressed
-		} else {
+		algo, buf = compress(compression, blockData, buf)
+		if len(buf) >= len(blockData)-len(blockData)/8 {
 			algo = NoCompressionIndicator
 		}
 	}
+	if algo == NoCompressionIndicator {
+		// We don't want to use the given blockData buffer directly: typically the
+		// result will be written to disk and that can mangle the buffer, leading to
+		// fragile code.
+		buf = append(buf[:0], blockData...)
+	}
+
+	*dst = buf
 
 	// Calculate the checksum.
-	pb := PhysicalBlock{data: block}
-	checksum := checksummer.Checksum(block, byte(algo))
+	pb := PhysicalBlock{data: buf}
+	checksum := checksummer.Checksum(buf, byte(algo))
 	pb.trailer = MakeTrailer(byte(algo), checksum)
 	return pb
 }
@@ -375,19 +365,8 @@ func (b *Buffer) CompressAndChecksum() (PhysicalBlock, *BufHandle) {
 	// Grab a buffer to use as the destination for compression.
 	compressedBuf := compressedBuffers.Get()
 	pb := CompressAndChecksum(&compressedBuf.b, b.h.b, b.compression, &b.checksummer)
-	if pb.IsCompressed() {
-		// Compression was fruitful, and pb's data points into compressedBuf. We
-		// can reuse b.Buffer because we've copied the compressed data.
-		b.h.b = b.h.b[:0]
-		return pb, compressedBuf
-	}
-	// Compression was not fruitful, and pb's data points into b.h. The
-	// compressedBuf we retrieved from the pool isn't needed, but our b.h is.
-	// Use the compressedBuf as the new b.h.
-	pbHandle := b.h
-	b.h = compressedBuf
 	b.h.b = b.h.b[:0]
-	return pb, pbHandle
+	return pb, compressedBuf
 }
 
 // SetCompression changes the compression algorithm used by CompressAndChecksum.
diff --git a/sstable/block/compression_cgo.go b/sstable/block/compression_cgo.go
index 76f6b66b..4ef0bda7 100644
--- a/sstable/block/compression_cgo.go
+++ b/sstable/block/compression_cgo.go
@@ -3,7 +3,6 @@
 // the LICENSE file.
 
 //go:build cgo
-// +build cgo
 
 package block
 
diff --git a/sstable/block/compression_nocgo.go b/sstable/block/compression_nocgo.go
index 0c787a05..4d34aa46 100644
--- a/sstable/block/compression_nocgo.go
+++ b/sstable/block/compression_nocgo.go
@@ -3,7 +3,6 @@
 // the LICENSE file.
 
 //go:build !cgo
-// +build !cgo
 
 package block
 
diff --git a/sstable/colblk_writer.go b/sstable/colblk_writer.go
index 35fa1b9a..9a63bc85 100644
--- a/sstable/colblk_writer.go
+++ b/sstable/colblk_writer.go
@@ -667,14 +667,6 @@ func (w *RawColumnWriter) enqueueDataBlock(
 		w.opts.Compression,
 		&cb.blockBuf.checksummer,
 	)
-	if !cb.physical.IsCompressed() {
-		// If the block isn't compressed, cb.physical's underlying data points
-		// directly into a buffer owned by w.dataBlock. Clone it before passing
-		// it to the write queue to be asynchronously written to disk.
-		// TODO(jackson): Should we try to avoid this clone by tracking the
-		// lifetime of the DataBlockWriters?
-		cb.physical, cb.blockBuf.dataBuf = cb.physical.CloneUsingBuf(cb.blockBuf.dataBuf)
-	}
 	return w.enqueuePhysicalBlock(cb, separator)
 }
 
@@ -1164,14 +1156,6 @@ func (w *RawColumnWriter) addDataBlock(b, sep []byte, bhp block.HandleWithProper
 		w.opts.Compression,
 		&cb.blockBuf.checksummer,
 	)
-	if !cb.physical.IsCompressed() {
-		// If the block isn't compressed, cb.physical's underlying data points
-		// directly into a buffer owned by w.dataBlock. Clone it before passing
-		// it to the write queue to be asynchronously written to disk.
-		// TODO(jackson): Should we try to avoid this clone by tracking the
-		// lifetime of the DataBlockWriters?
-		cb.physical, cb.blockBuf.dataBuf = cb.physical.CloneUsingBuf(cb.blockBuf.dataBuf)
-	}
 	if err := w.enqueuePhysicalBlock(cb, sep); err != nil {
 		return err
 	}
diff --git a/sstable/layout.go b/sstable/layout.go
index 6c8c0003..a8571d9e 100644
--- a/sstable/layout.go
+++ b/sstable/layout.go
@@ -18,7 +18,6 @@ import (
 	"github.com/cockroachdb/pebble/internal/base"
 	"github.com/cockroachdb/pebble/internal/binfmt"
 	"github.com/cockroachdb/pebble/internal/bytealloc"
-	"github.com/cockroachdb/pebble/internal/invariants"
 	"github.com/cockroachdb/pebble/internal/sstableinternal"
 	"github.com/cockroachdb/pebble/internal/treeprinter"
 	"github.com/cockroachdb/pebble/objstorage"
@@ -675,8 +674,7 @@ func (w *layoutWriter) Abort() {
 }
 
 // WriteDataBlock constructs a trailer for the provided data block and writes
-// the block and trailer to the writer. It returns the block's handle. It can
-// mangle b.
+// the block and trailer to the writer. It returns the block's handle.
 func (w *layoutWriter) WriteDataBlock(b []byte, buf *blockBuf) (block.Handle, error) {
 	return w.writeBlock(b, w.compression, buf)
 }
@@ -692,8 +690,6 @@ func (w *layoutWriter) WritePrecompressedDataBlock(blk block.PhysicalBlock) (blo
 // second-level) and writes the block and trailer to the writer. It remembers
 // the last-written index block's handle and adds it to the file's meta index
 // when the writer is finished.
-//
-// WriteIndexBlock can mangle b.
 func (w *layoutWriter) WriteIndexBlock(b []byte) (block.Handle, error) {
 	h, err := w.writeBlock(b, w.compression, &w.buf)
 	if err == nil {
@@ -716,8 +712,6 @@ func (w *layoutWriter) WriteFilterBlock(f filterWriter) (bh block.Handle, err er
 // WritePropertiesBlock constructs a trailer for the provided properties block
 // and writes the block and trailer to the writer. It automatically adds the
 // properties block to the file's meta index when the writer is finished.
-//
-// WritePropertiesBlock can mangle b.
 func (w *layoutWriter) WritePropertiesBlock(b []byte) (block.Handle, error) {
 	return w.writeNamedBlock(b, metaPropertiesName)
 }
@@ -725,8 +719,6 @@ func (w *layoutWriter) WritePropertiesBlock(b []byte) (block.Handle, error) {
 // WriteRangeKeyBlock constructs a trailer for the provided range key block and
 // writes the block and trailer to the writer. It automatically adds the range
 // key block to the file's meta index when the writer is finished.
-//
-// WriteRangeKeyBlock can mangle the block data.
 func (w *layoutWriter) WriteRangeKeyBlock(b []byte) (block.Handle, error) {
 	return w.writeNamedBlock(b, metaRangeKeyName)
 }
@@ -735,13 +727,10 @@ func (w *layoutWriter) WriteRangeKeyBlock(b []byte) (block.Handle, error) {
 // block and writes the block and trailer to the writer. It automatically adds
 // the range deletion block to the file's meta index when the writer is
 // finished.
-//
-// WriteRangeDeletionBlock can mangle the block data.
 func (w *layoutWriter) WriteRangeDeletionBlock(b []byte) (block.Handle, error) {
 	return w.writeNamedBlock(b, metaRangeDelV2Name)
 }
 
-// writeNamedBlock can mangle the block data.
 func (w *layoutWriter) writeNamedBlock(b []byte, name string) (bh block.Handle, err error) {
 	bh, err = w.writeBlock(b, block.NoCompression, &w.buf)
 	if err == nil {
@@ -770,20 +759,12 @@ func (w *layoutWriter) WriteValueIndexBlock(
 	return h, nil
 }
 
-// writeBlock checksums, compresses, and writes out a block. It can mangle b.
+// writeBlock checksums, compresses, and writes out a block.
 func (w *layoutWriter) writeBlock(
 	b []byte, compression block.Compression, buf *blockBuf,
 ) (block.Handle, error) {
 	pb := block.CompressAndChecksum(&buf.dataBuf, b, compression, &buf.checksummer)
 	h, err := w.writePrecompressedBlock(pb)
-	// This method is allowed to mangle b, but that only happens when the block
-	// data is not compressible. Mangle it anyway in invariant builds to catch
-	// callers that don't handle this.
-	if invariants.Enabled && invariants.Sometimes(1) {
-		for i := range b {
-			b[i] = 0xFF
-		}
-	}
 	return h, err
 }
 
diff --git a/sstable/rowblk/rowblk_rewrite.go b/sstable/rowblk/rowblk_rewrite.go
index d1667ee1..48b64cbc 100644
--- a/sstable/rowblk/rowblk_rewrite.go
+++ b/sstable/rowblk/rowblk_rewrite.go
@@ -10,6 +10,7 @@ import (
 	"github.com/cockroachdb/errors"
 	"github.com/cockroachdb/pebble/internal/base"
 	"github.com/cockroachdb/pebble/internal/bytealloc"
+	"github.com/cockroachdb/pebble/internal/invariants"
 	"github.com/cockroachdb/pebble/sstable/block"
 )
 
@@ -78,6 +79,9 @@ func (r *Rewriter) RewriteSuffixes(
 		// in the block, which includes the 1-byte prefix. This is fine since bw
 		// also does not know about the prefix and will preserve it in bw.add.
 		v := kv.InPlaceValue()
+		if invariants.Enabled && invariants.Sometimes(10) {
+			r.comparer.ValidateKey.MustValidate(r.scratchKey.UserKey)
+		}
 		r.writer.Add(r.scratchKey, v)
 		if start.UserKey == nil {
 			// Copy the first key.
diff --git a/sstable/rowblk_writer.go b/sstable/rowblk_writer.go
index 4ce0a9ef..97d237dd 100644
--- a/sstable/rowblk_writer.go
+++ b/sstable/rowblk_writer.go
@@ -51,6 +51,7 @@ type RawRowWriter struct {
 	compression          block.Compression
 	separator            Separator
 	successor            Successor
+	validateKey          base.ValidateKey
 	tableFormat          TableFormat
 	isStrictObsolete     bool
 	writingToLowestLevel bool
@@ -1186,6 +1187,12 @@ func (w *RawRowWriter) indexEntrySep(
 	} else {
 		sep = prevKey.Separator(w.compare, w.separator, dataBlockBuf.sepScratch[:0], key)
 	}
+	if invariants.Enabled && invariants.Sometimes(25) {
+		if w.compare(prevKey.UserKey, sep.UserKey) > 0 {
+			panic(errors.AssertionFailedf("prevKey.UserKey > sep.UserKey: %s > %s",
+				prevKey.UserKey, sep.UserKey))
+		}
+	}
 	return sep
 }
 
@@ -1214,6 +1221,11 @@ func (w *RawRowWriter) addIndexEntry(
 		// In particular, it must have a non-zero length.
 		return nil
 	}
+	if invariants.Enabled {
+		if err := w.validateKey.Validate(sep.UserKey); err != nil {
+			return err
+		}
+	}
 
 	encoded := bhp.EncodeVarints(tmp)
 	if flushIndexBuf != nil {
@@ -1532,6 +1544,8 @@ func (w *RawRowWriter) Close() (err error) {
 			return err
 		}
 		prevKey := w.dataBlockBuf.dataBlock.CurKey()
+		// NB: prevKey.UserKey may be nil if there are no keys and we're forcing
+		// the creation of an empty data block.
 		if err := w.addIndexEntrySync(prevKey, InternalKey{}, bhp, w.dataBlockBuf.tmp[:]); err != nil {
 			return err
 		}
@@ -1713,6 +1727,7 @@ func newRowWriter(writable objstorage.Writable, o WriterOptions) *RawRowWriter {
 		compression:                o.Compression,
 		separator:                  o.Comparer.Separator,
 		successor:                  o.Comparer.Successor,
+		validateKey:                o.Comparer.ValidateKey,
 		tableFormat:                o.TableFormat,
 		isStrictObsolete:           o.IsStrictObsolete,
 		writingToLowestLevel:       o.WritingToLowestLevel,
@@ -1953,13 +1968,9 @@ func (w *RawRowWriter) addDataBlock(b, sep []byte, bhp block.HandleWithPropertie
 		w.layout.compression,
 		&blockBuf.checksummer,
 	)
-	if !pb.IsCompressed() {
-		// If the block isn't compressed, pb's underlying data points
-		// directly b. Clone it before writing it, as writing can mangle the buffer.
-		pb, blockBuf.dataBuf = pb.CloneUsingBuf(blockBuf.dataBuf)
-	}
 
 	// layout.WriteDataBlock keeps layout.offset up-to-date for us.
+	// Note that this can mangle the pb data.
 	bh, err := w.layout.writePrecompressedBlock(pb)
 	if err != nil {
 		return err
diff --git a/sstable/suffix_rewriter.go b/sstable/suffix_rewriter.go
index 8387f189..67b0f34f 100644
--- a/sstable/suffix_rewriter.go
+++ b/sstable/suffix_rewriter.go
@@ -178,6 +178,12 @@ func rewriteDataBlocksInParallel(
 					if err != nil {
 						return err
 					}
+					if err := r.Comparer.ValidateKey.Validate(output[i].start.UserKey); err != nil {
+						return err
+					}
+					if err := r.Comparer.ValidateKey.Validate(output[i].end.UserKey); err != nil {
+						return err
+					}
 					compressedBuf = compressedBuf[:cap(compressedBuf)]
 					finished := block.CompressAndChecksum(&compressedBuf, outputBlock, opts.Compression, &checksummer)
 					output[i].physical = finished.CloneWithByteAlloc(&blockAlloc)
@@ -219,6 +225,12 @@ func rewriteRangeKeyBlockToWriter(r *Reader, w RawWriter, from, to []byte) error
 		if !s.Valid() {
 			break
 		}
+		if err := r.Comparer.ValidateKey.Validate(s.Start); err != nil {
+			return err
+		}
+		if err := r.Comparer.ValidateKey.Validate(s.End); err != nil {
+			return err
+		}
 		for i := range s.Keys {
 			if s.Keys[i].Kind() != base.InternalKeyKindRangeKeySet {
 				return errBadKind
@@ -328,6 +340,9 @@ func RewriteKeySuffixesViaWriter(
 		if err != nil {
 			return nil, err
 		}
+		if invariants.Enabled && invariants.Sometimes(10) {
+			r.Comparer.ValidateKey.MustValidate(scratch.UserKey)
+		}
 		if err := w.AddWithForceObsolete(scratch, val, false); err != nil {
 			return nil, err
 		}
diff --git a/tool/make_incorrect_manifests.go b/tool/make_incorrect_manifests.go
index 90aa87d0..f0ae2030 100644
--- a/tool/make_incorrect_manifests.go
+++ b/tool/make_incorrect_manifests.go
@@ -3,7 +3,6 @@
 // the LICENSE file.
 
 //go:build make_incorrect_manifests
-// +build make_incorrect_manifests
 
 // Run using: go run -tags make_incorrect_manifests ./tool/make_incorrect_manifests.go
 package main
diff --git a/vfs/default_linux.go b/vfs/default_linux.go
index b505ce0c..2b4894e9 100644
--- a/vfs/default_linux.go
+++ b/vfs/default_linux.go
@@ -3,7 +3,6 @@
 // the LICENSE file.
 
 //go:build linux
-// +build linux
 
 package vfs
 
diff --git a/vfs/default_unix.go b/vfs/default_unix.go
index 34a4a3e7..593d9d06 100644
--- a/vfs/default_unix.go
+++ b/vfs/default_unix.go
@@ -3,7 +3,6 @@
 // the LICENSE file.
 
 //go:build darwin || dragonfly || freebsd || netbsd || openbsd || solaris
-// +build darwin dragonfly freebsd netbsd openbsd solaris
 
 package vfs
 
diff --git a/vfs/default_windows.go b/vfs/default_windows.go
index 05777f20..9882df14 100644
--- a/vfs/default_windows.go
+++ b/vfs/default_windows.go
@@ -3,7 +3,6 @@
 // the LICENSE file.
 
 //go:build windows
-// +build windows
 
 package vfs
 
diff --git a/vfs/disk_usage_linux.go b/vfs/disk_usage_linux.go
index 4f0bcd9d..c967a66f 100644
--- a/vfs/disk_usage_linux.go
+++ b/vfs/disk_usage_linux.go
@@ -3,7 +3,6 @@
 // the LICENSE file.
 
 //go:build linux
-// +build linux
 
 package vfs
 
diff --git a/vfs/disk_usage_netbsd.go b/vfs/disk_usage_netbsd.go
index 8b7e9dd9..88398781 100644
--- a/vfs/disk_usage_netbsd.go
+++ b/vfs/disk_usage_netbsd.go
@@ -3,7 +3,6 @@
 // the LICENSE file.
 
 //go:build netbsd
-// +build netbsd
 
 package vfs
 
diff --git a/vfs/disk_usage_openbsd.go b/vfs/disk_usage_openbsd.go
index 19e9c5cf..536ad1ef 100644
--- a/vfs/disk_usage_openbsd.go
+++ b/vfs/disk_usage_openbsd.go
@@ -3,7 +3,6 @@
 // the LICENSE file.
 
 //go:build openbsd
-// +build openbsd
 
 package vfs
 
diff --git a/vfs/disk_usage_unix.go b/vfs/disk_usage_unix.go
index 95d9a7c3..a025f746 100644
--- a/vfs/disk_usage_unix.go
+++ b/vfs/disk_usage_unix.go
@@ -3,7 +3,6 @@
 // the LICENSE file.
 
 //go:build darwin || dragonfly || freebsd
-// +build darwin dragonfly freebsd
 
 package vfs
 
diff --git a/vfs/disk_usage_windows.go b/vfs/disk_usage_windows.go
index a380bcbf..600877f2 100644
--- a/vfs/disk_usage_windows.go
+++ b/vfs/disk_usage_windows.go
@@ -3,7 +3,6 @@
 // the LICENSE file.
 
 //go:build windows
-// +build windows
 
 package vfs
 
diff --git a/vfs/errors_unix.go b/vfs/errors_unix.go
index 2d05e14b..eea5f80f 100644
--- a/vfs/errors_unix.go
+++ b/vfs/errors_unix.go
@@ -3,7 +3,6 @@
 // the LICENSE file.
 
 //go:build darwin || dragonfly || freebsd || linux || openbsd || netbsd
-// +build darwin dragonfly freebsd linux openbsd netbsd
 
 package vfs
 
diff --git a/vfs/errors_windows.go b/vfs/errors_windows.go
index 65920fc7..4ba99824 100644
--- a/vfs/errors_windows.go
+++ b/vfs/errors_windows.go
@@ -3,7 +3,6 @@
 // the LICENSE file.
 
 //go:build windows
-// +build windows
 
 package vfs
 
diff --git a/vfs/fadvise_generic.go b/vfs/fadvise_generic.go
index d6b9b41a..b92cb5f4 100644
--- a/vfs/fadvise_generic.go
+++ b/vfs/fadvise_generic.go
@@ -3,7 +3,6 @@
 // the LICENSE file.
 
 //go:build !linux
-// +build !linux
 
 package vfs
 
diff --git a/vfs/fadvise_linux.go b/vfs/fadvise_linux.go
index 6bb4db13..5aa75a22 100644
--- a/vfs/fadvise_linux.go
+++ b/vfs/fadvise_linux.go
@@ -3,7 +3,6 @@
 // the LICENSE file.
 
 //go:build linux
-// +build linux
 
 package vfs
 
diff --git a/vfs/file_lock_generic.go b/vfs/file_lock_generic.go
index 6177da69..1b936536 100644
--- a/vfs/file_lock_generic.go
+++ b/vfs/file_lock_generic.go
@@ -3,7 +3,6 @@
 // the LICENSE file.
 
 //go:build !darwin && !dragonfly && !freebsd && !linux && !netbsd && !openbsd && !solaris && !windows
-// +build !darwin,!dragonfly,!freebsd,!linux,!netbsd,!openbsd,!solaris,!windows
 
 package vfs
 
diff --git a/vfs/file_lock_unix.go b/vfs/file_lock_unix.go
index 4d05e8ce..4f93e3cc 100644
--- a/vfs/file_lock_unix.go
+++ b/vfs/file_lock_unix.go
@@ -3,7 +3,6 @@
 // the LICENSE file.
 
 //go:build darwin || dragonfly || freebsd || linux || netbsd || openbsd || solaris
-// +build darwin dragonfly freebsd linux netbsd openbsd solaris
 
 package vfs
 
diff --git a/vfs/file_lock_windows.go b/vfs/file_lock_windows.go
index 31fd17a8..d0237081 100644
--- a/vfs/file_lock_windows.go
+++ b/vfs/file_lock_windows.go
@@ -3,7 +3,6 @@
 // the LICENSE file.
 
 //go:build windows
-// +build windows
 
 package vfs
 


Test patch:
diff --git a/cockroachkvs/cockroachkvs_test.go b/cockroachkvs/cockroachkvs_test.go
index 7227a8ea..1c4cb860 100644
--- a/cockroachkvs/cockroachkvs_test.go
+++ b/cockroachkvs/cockroachkvs_test.go
@@ -79,6 +79,46 @@ func TestComparer(t *testing.T) {
 	}
 }
 
+func TestComparerFuncs(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+
+	var buf bytes.Buffer
+	datadriven.RunTest(t, "testdata/comparer_funcs",
+		func(t *testing.T, td *datadriven.TestData) string {
+			buf.Reset()
+			switch td.Cmd {
+			case "separator":
+				var keys [][]byte
+				var dst []byte
+				for _, line := range crstrings.Lines(td.Input) {
+					keys = keys[:0]
+					for _, formattedKey := range strings.Fields(line) {
+						k := ParseFormattedKey(formattedKey)
+						keys = append(keys, k)
+					}
+					if len(keys) != 2 {
+						t.Fatalf("expected two keys, but found %d", len(keys))
+					}
+					dst = Comparer.Separator(dst[:0], keys[0], keys[1])
+					fmt.Fprintf(&buf, "Separator(%q [%x], %q [%x]) = %q [%x]\n",
+						FormatKey(keys[0]), keys[0], FormatKey(keys[1]), keys[1], FormatKey(dst), dst)
+				}
+				return buf.String()
+			case "successor":
+				var dst []byte
+				for _, line := range crstrings.Lines(td.Input) {
+					k := ParseFormattedKey(line)
+					dst = Comparer.Successor(dst[:0], k)
+					fmt.Fprintf(&buf, "Successor(%q [%x]) = %q [%x]\n",
+						FormatKey(k), k, FormatKey(dst), dst)
+				}
+				return buf.String()
+			default:
+				panic(fmt.Sprintf("unknown command: %s", td.Cmd))
+			}
+		})
+}
+
 func TestKeySchema_KeyWriter(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 
@@ -633,7 +673,7 @@ func parseUserKey(userKeyStr string) []byte {
 		}
 	}
 	k := append(append([]byte(roachKey), 0), parseVersion(versionStr)...)
-	checkEngineKey(k)
+	validateEngineKey.MustValidate(k)
 	return k
 }
 
@@ -657,7 +697,7 @@ func parseVersion(versionStr string) []byte {
 		if len(ret) != 14 && len(ret) != 10 {
 			panic(fmt.Sprintf("expected 10 or 14-length ret got %d", len(ret)))
 		}
-		checkEngineKey(ret)
+		validateEngineKey.MustValidate(ret)
 		// TODO(jackson): Refactor to allow us to generate a suffix without a
 		// sentinel byte rather than stripping it like this.
 		return ret[1:]
diff --git a/cockroachkvs/key_schema_test.go b/cockroachkvs/key_schema_test.go
index c3309bce..dcaae8f3 100644
--- a/cockroachkvs/key_schema_test.go
+++ b/cockroachkvs/key_schema_test.go
@@ -161,7 +161,7 @@ func TestKeySchema_RandomKeys(t *testing.T) {
 		if n := len(kv.K.UserKey); n > len(keys[k]) {
 			t.Fatalf("key %q is longer than original key %q", kv.K.UserKey, keys[k])
 		}
-		checkEngineKey(kv.K.UserKey)
+		validateEngineKey.MustValidate(kv.K.UserKey)
 
 		// We write keys[k] as the value too, so check that it's verbatim equal.
 		value, callerOwned, err := kv.V.Value(valBuf)
diff --git a/cockroachkvs/testdata/comparer_funcs b/cockroachkvs/testdata/comparer_funcs
new file mode 100644
index 00000000..f181a323
--- /dev/null
+++ b/cockroachkvs/testdata/comparer_funcs
@@ -0,0 +1,36 @@
+successor
+foo
+bar
+foo@1000000,0
+foo@1000000.000051212,0
+foo@1000000,8
+foo@1000000.000051212,7
+foo@01,cf379e85-6371-42e6-acc8-ed5259fa177a
+----
+Successor(foo [666f6f00]) = g [6700]
+Successor(bar [62617200]) = c [6300]
+Successor(foo@1000000,0 [666f6f0000038d7ea4c6800009]) = g [6700]
+Successor(foo@1000000.000051212,0 [666f6f0000038d7ea4c7480c09]) = g [6700]
+Successor(foo@1000000,8 [666f6f0000038d7ea4c68000000000080d]) = g [6700]
+Successor(foo@1000000.000051212,7 [666f6f0000038d7ea4c7480c000000070d]) = g [6700]
+Successor(foo@01,cf379e85-6371-42e6-acc8-ed5259fa177a [666f6f0001cf379e85637142e6acc8ed5259fa177a12]) = g [6700]
+
+# The successor for an empty string should be a key consisting only of the 0x00
+# byte.
+successor
+hex:
+----
+Successor(<empty> []) =  [00]
+
+separator
+bar foo
+foo@1000000,0 foo@1000000,0
+foo@1000000,1 foo@1000000,0
+foo@01,a65a118c-5dcb-452b-bd82-7dd17e121138 foo@01,cf379e85-6371-42e6-acc8-ed5259fa177a
+fax@01,a65a118c-5dcb-452b-bd82-7dd17e121138 foo@01,cf379e85-6371-42e6-acc8-ed5259fa177a
+----
+Separator(bar [62617200], foo [666f6f00]) = c [6300]
+Separator(foo@1000000,0 [666f6f0000038d7ea4c6800009], foo@1000000,0 [666f6f0000038d7ea4c6800009]) = foo@1000000,0 [666f6f0000038d7ea4c6800009]
+Separator(foo@1000000,1 [666f6f0000038d7ea4c68000000000010d], foo@1000000,0 [666f6f0000038d7ea4c6800009]) = foo@1000000,1 [666f6f0000038d7ea4c68000000000010d]
+Separator(foo@01,a65a118c-5dcb-452b-bd82-7dd17e121138 [666f6f0001a65a118c5dcb452bbd827dd17e12113812], foo@01,cf379e85-6371-42e6-acc8-ed5259fa177a [666f6f0001cf379e85637142e6acc8ed5259fa177a12]) = foo@01,a65a118c-5dcb-452b-bd82-7dd17e121138 [666f6f0001a65a118c5dcb452bbd827dd17e12113812]
+Separator(fax@01,a65a118c-5dcb-452b-bd82-7dd17e121138 [6661780001a65a118c5dcb452bbd827dd17e12113812], foo@01,cf379e85-6371-42e6-acc8-ed5259fa177a [666f6f0001cf379e85637142e6acc8ed5259fa177a12]) = fb [666200]
diff --git a/cockroachkvs/testdata/key_schema_key_seeker b/cockroachkvs/testdata/key_schema_key_seeker
index 9509b416..5eff3c64 100644
--- a/cockroachkvs/testdata/key_schema_key_seeker
+++ b/cockroachkvs/testdata/key_schema_key_seeker
@@ -169,3 +169,75 @@ MaterializeUserKey(-1, 0) = hex:6d6f6f0000000000b2d05e0109
 MaterializeUserKey(0, 1) = hex:6d6f6f0000000000b2d05e00000000020d
 MaterializeUserKey(1, 2) = hex:6d6f6f0000000000b2d05e00000000010d
 MaterializeUserKey(2, 3) = hex:6d6f6f0000000000b2d05e0009
+
+define-block
+moo
+moo @ 3000000002,1
+moo @ 3000000001,1
+moo @ 0000000001,0
+----
+Parse(moo) = hex:6d6f6f00
+Parse(moo @ 3000000002,1) = hex:6d6f6f0000000000b2d05e02000000010d
+Parse(moo @ 3000000001,1) = hex:6d6f6f0000000000b2d05e01000000010d
+Parse(moo @ 0000000001,0) = hex:6d6f6f00000000000000000109
+
+# IsLowerBound should return false for all keys because the first key of the
+# block sorts before all the provided keys. Previously, a bug did not account
+# for the empty suffix sorting first.
+
+is-lower-bound
+moo @ 9000000000,2
+moo @ 8000000000,2
+moo @ 8000000000,1
+moo @ 8000000000,0
+moo @ 7000000000,9
+moo @ 3000000000,2
+moo @ 3000000000,1
+moo @ 3000000000,0
+moo @ 0000000000,1
+----
+IsLowerBound(moo @ 9000000000,2, "") = false
+IsLowerBound(moo @ 8000000000,2, "") = false
+IsLowerBound(moo @ 8000000000,1, "") = false
+IsLowerBound(moo @ 8000000000,0, "") = false
+IsLowerBound(moo @ 7000000000,9, "") = false
+IsLowerBound(moo @ 3000000000,2, "") = false
+IsLowerBound(moo @ 3000000000,1, "") = false
+IsLowerBound(moo @ 3000000000,0, "") = false
+IsLowerBound(moo @ 0000000000,1, "") = false
+
+define-block
+moo @ 0000000000,1
+moo @ 3000000002,1
+moo @ 3000000001,1
+moo @ 0000000001,0
+----
+Parse(moo @ 0000000000,1) = hex:6d6f6f000000000000000000000000010d
+Parse(moo @ 3000000002,1) = hex:6d6f6f0000000000b2d05e02000000010d
+Parse(moo @ 3000000001,1) = hex:6d6f6f0000000000b2d05e01000000010d
+Parse(moo @ 0000000001,0) = hex:6d6f6f00000000000000000109
+
+is-lower-bound
+moo
+moo @ 9000000000,2
+moo @ 8000000000,2
+moo @ 8000000000,1
+moo @ 8000000000,0
+moo @ 7000000000,9
+moo @ 3000000000,2
+moo @ 3000000000,1
+moo @ 3000000000,0
+moo @ 0000000000,1
+moo @ 02073a83c45688420eaf97824255790f1e12
+----
+IsLowerBound(moo, "") = true
+IsLowerBound(moo @ 9000000000,2, "") = true
+IsLowerBound(moo @ 8000000000,2, "") = true
+IsLowerBound(moo @ 8000000000,1, "") = true
+IsLowerBound(moo @ 8000000000,0, "") = true
+IsLowerBound(moo @ 7000000000,9, "") = true
+IsLowerBound(moo @ 3000000000,2, "") = true
+IsLowerBound(moo @ 3000000000,1, "") = true
+IsLowerBound(moo @ 3000000000,0, "") = true
+IsLowerBound(moo @ 0000000000,1, "") = true
+IsLowerBound(moo @ 02073a83c45688420eaf97824255790f1e12, "") = true
diff --git a/internal/arenaskl/race_test.go b/internal/arenaskl/race_test.go
index b9310c92..dfff4957 100644
--- a/internal/arenaskl/race_test.go
+++ b/internal/arenaskl/race_test.go
@@ -1,5 +1,4 @@
 //go:build race
-// +build race
 
 // Copyright 2020 The LevelDB-Go and Pebble Authors. All rights reserved. Use
 // of this source code is governed by a BSD-style license that can be found in
diff --git a/internal/base/comparer_test.go b/internal/base/comparer_test.go
index e16d85ea..31db932b 100644
--- a/internal/base/comparer_test.go
+++ b/internal/base/comparer_test.go
@@ -12,15 +12,11 @@ import (
 	"time"
 )
 
-func TestDefAppendSeparator(t *testing.T) {
+func TestDefaultComparer_Separator(t *testing.T) {
 	testCases := []struct {
 		a, b, want string
 	}{
-		// Examples from the doc comments.
 		{"black", "blue", "blb"},
-		{"green", "", "green"},
-		// Non-empty b values. The C++ Level-DB code calls these separators.
-		{"", "2", ""},
 		{"1", "2", "1"},
 		{"1", "29", "2"},
 		{"13", "19", "14"},
@@ -33,22 +29,37 @@ func TestDefAppendSeparator(t *testing.T) {
 		{"1\xff\xff", "19", "1\xff\xff"},
 		{"1\xff\xff", "2", "1\xff\xff"},
 		{"1\xff\xff", "9", "2"},
-		// Empty b values. The C++ Level-DB code calls these successors.
-		{"", "", ""},
-		{"1", "", "1"},
-		{"11", "", "11"},
-		{"11\xff", "", "11\xff"},
-		{"1\xff", "", "1\xff"},
-		{"1\xff\xff", "", "1\xff\xff"},
-		{"\xff", "", "\xff"},
-		{"\xff\xff", "", "\xff\xff"},
-		{"\xff\xff\xff", "", "\xff\xff\xff"},
 	}
 	for _, tc := range testCases {
 		t.Run("", func(t *testing.T) {
 			got := string(DefaultComparer.Separator(nil, []byte(tc.a), []byte(tc.b)))
 			if got != tc.want {
-				t.Errorf("a, b = %q, %q: got %q, want %q", tc.a, tc.b, got, tc.want)
+				t.Errorf("DefaultComparer.Separator(nil, %q, %q) = %q, want %q", tc.a, tc.b, got, tc.want)
+			}
+		})
+	}
+}
+
+func TestDefaultComparer_Successor(t *testing.T) {
+	testCases := []struct {
+		a, want string
+	}{
+		{"green", "h"},
+		{"", ""},
+		{"1", "2"},
+		{"11", "2"},
+		{"11\xff", "2"},
+		{"1\xff", "2"},
+		{"1\xff\xff", "2"},
+		{"\xff", "\xff"},
+		{"\xff\xff", "\xff\xff"},
+		{"\xff\xff\xff", "\xff\xff\xff"},
+	}
+	for _, tc := range testCases {
+		t.Run("", func(t *testing.T) {
+			got := string(DefaultComparer.Successor(nil, []byte(tc.a)))
+			if got != tc.want {
+				t.Errorf("DefaultComparer.Successor(nil, %q) = %q, want %q", tc.a, got, tc.want)
 			}
 		})
 	}
diff --git a/internal/testkeys/testkeys.go b/internal/testkeys/testkeys.go
index 35eb6144..4ddab98b 100644
--- a/internal/testkeys/testkeys.go
+++ b/internal/testkeys/testkeys.go
@@ -22,6 +22,7 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/cockroachdb/errors"
 	"github.com/cockroachdb/pebble/internal/base"
 )
 
@@ -106,7 +107,20 @@ var Comparer = &base.Comparer{
 		return append(append(dst, a...), 0x00)
 	},
 	Split: split,
-	Name:  "pebble.internal.testkeys",
+	ValidateKey: func(k []byte) error {
+		// Ensure that if the key has a suffix, it's a valid integer
+		// (potentially modulo a faux synthetic bit suffix).
+		k = bytes.TrimSuffix(k, ignoreTimestampSuffix)
+		i := split(k)
+		if i == len(k) {
+			return nil
+		}
+		if _, err := parseUintBytes(k[i+1:], 10, 64); err != nil {
+			return errors.Wrapf(err, "invalid key %q", k)
+		}
+		return nil
+	},
+	Name: "pebble.internal.testkeys",
 }
 
 // The comparator is similar to the one in Cockroach; when the prefixes are
diff --git a/tool/make_test_find_db.go b/tool/make_test_find_db.go
index 21f4f0b0..d8d8e2a2 100644
--- a/tool/make_test_find_db.go
+++ b/tool/make_test_find_db.go
@@ -3,7 +3,6 @@
 // the LICENSE file.
 
 //go:build make_test_find_db
-// +build make_test_find_db
 
 // Run using: go run -tags make_test_find_db ./tool/make_test_find_db.go
 package main
diff --git a/tool/make_test_remotecat.go b/tool/make_test_remotecat.go
index fb16accf..d35e5b28 100644
--- a/tool/make_test_remotecat.go
+++ b/tool/make_test_remotecat.go
@@ -3,7 +3,6 @@
 // the LICENSE file.
 
 //go:build make_test_remotecat
-// +build make_test_remotecat
 
 // Run using: go run -tags make_test_remotecat ./tool/make_test_remotecat.go
 package main
diff --git a/tool/make_test_sstables.go b/tool/make_test_sstables.go
index 9070452f..f155ca0e 100644
--- a/tool/make_test_sstables.go
+++ b/tool/make_test_sstables.go
@@ -3,7 +3,6 @@
 // the LICENSE file.
 
 //go:build make_test_sstables
-// +build make_test_sstables
 
 // Run using: go run -tags make_test_sstables ./tool/make_test_sstables.go
 package main
diff --git a/vfs/errors_unix_test.go b/vfs/errors_unix_test.go
index 7a1c1dc4..ea441bc3 100644
--- a/vfs/errors_unix_test.go
+++ b/vfs/errors_unix_test.go
@@ -3,7 +3,6 @@
 // the LICENSE file.
 
 //go:build darwin || dragonfly || freebsd || linux || openbsd || netbsd
-// +build darwin dragonfly freebsd linux openbsd netbsd
 
 package vfs
 
diff --git a/vfs/syncing_file_linux_test.go b/vfs/syncing_file_linux_test.go
index 83458a3b..1b69e0ef 100644
--- a/vfs/syncing_file_linux_test.go
+++ b/vfs/syncing_file_linux_test.go
@@ -3,7 +3,6 @@
 // the LICENSE file.
 
 //go:build linux && !arm
-// +build linux,!arm
 
 package vfs