Repository: cockroachdb/pebble
Pull request #3808 (2024-07-30 15:43:32)

manifest: fix incorrect Annotator pointer aggregation
cockroachdb#3760 contained a bug which causes Annotator
values to be incorrectly aggregated when pointer values
should be overwritten. This is because the `vtyped`
variable was not being modified due to being on the
stack.

This change fixes this and adds a unit test for
`PickFileAggregator` to catch this issue in the future.

cockroachdb#3759 should already not be affected by
this due to the different way it handles aggregation.

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/3808)
<!-- Reviewable:end -->


Patch:
diff --git a/internal/compact/run.go b/internal/compact/run.go
index f1db931c..02a638da 100644
--- a/internal/compact/run.go
+++ b/internal/compact/run.go
@@ -185,7 +185,7 @@ func (r *Runner) writeKeysToTable(tw *sstable.RawWriter) (splitKey []byte, _ err
 		case base.InternalKeyKindRangeDelete:
 			// The previous span (if any) must end at or before this key, since the
 			// spans we receive are non-overlapping.
-			if err := tw.EncodeSpan(&r.lastRangeDelSpan); r.err != nil {
+			if err := tw.EncodeSpan(r.lastRangeDelSpan); r.err != nil {
 				return nil, err
 			}
 			r.lastRangeDelSpan.CopyFrom(r.iter.Span())
@@ -194,7 +194,7 @@ func (r *Runner) writeKeysToTable(tw *sstable.RawWriter) (splitKey []byte, _ err
 		case base.InternalKeyKindRangeKeySet, base.InternalKeyKindRangeKeyUnset, base.InternalKeyKindRangeKeyDelete:
 			// The previous span (if any) must end at or before this key, since the
 			// spans we receive are non-overlapping.
-			if err := tw.EncodeSpan(&r.lastRangeKeySpan); err != nil {
+			if err := tw.EncodeSpan(r.lastRangeKeySpan); err != nil {
 				return nil, err
 			}
 			r.lastRangeKeySpan.CopyFrom(r.iter.Span())
diff --git a/internal/compact/spans.go b/internal/compact/spans.go
index 37cfe486..8ca8c13a 100644
--- a/internal/compact/spans.go
+++ b/internal/compact/spans.go
@@ -189,7 +189,7 @@ func SplitAndEncodeSpan(
 	}
 
 	if upToKey == nil || cmp(span.End, upToKey) <= 0 {
-		if err := tw.EncodeSpan(span); err != nil {
+		if err := tw.EncodeSpan(*span); err != nil {
 			return err
 		}
 		span.Reset()
@@ -202,12 +202,11 @@ func SplitAndEncodeSpan(
 	}
 
 	// Split the span at upToKey and encode the first part.
-	splitSpan := keyspan.Span{
+	if err := tw.EncodeSpan(keyspan.Span{
 		Start: span.Start,
 		End:   upToKey,
 		Keys:  span.Keys,
-	}
-	if err := tw.EncodeSpan(&splitSpan); err != nil {
+	}); err != nil {
 		return err
 	}
 	span.Start = append(span.Start[:0], upToKey...)
diff --git a/internal/manifest/annotator.go b/internal/manifest/annotator.go
index cfbe2f4f..edab24db 100644
--- a/internal/manifest/annotator.go
+++ b/internal/manifest/annotator.go
@@ -84,33 +84,36 @@ func (a *Annotator[T]) findAnnotation(n *node) *annotation {
 // nodeAnnotation computes this annotator's annotation of this node across all
 // files in the node's subtree. The second return value indicates whether the
 // annotation is stable and thus cacheable.
-func (a *Annotator[T]) nodeAnnotation(n *node) (v *T, cacheOK bool) {
+func (a *Annotator[T]) nodeAnnotation(n *node) (_ *T, cacheOK bool) {
 	annot := a.findAnnotation(n)
-	vtyped := annot.v.(*T)
+	t := annot.v.(*T)
 	// If the annotation is already marked as valid, we can return it without
 	// recomputing anything.
 	if annot.valid {
-		return vtyped, true
+		return t, true
 	}
 
-	annot.v = a.Aggregator.Zero(vtyped)
-	annot.valid = true
+	t = a.Aggregator.Zero(t)
+	valid := true
 
 	for i := int16(0); i <= n.count; i++ {
 		if !n.leaf {
 			v, ok := a.nodeAnnotation(n.children[i])
-			annot.v = a.Aggregator.Merge(v, vtyped)
-			annot.valid = annot.valid && ok
+			t = a.Aggregator.Merge(v, t)
+			valid = valid && ok
 		}
 
 		if i < n.count {
-			v, ok := a.Aggregator.Accumulate(n.items[i], vtyped)
-			annot.v = v
-			annot.valid = annot.valid && ok
+			var ok bool
+			t, ok = a.Aggregator.Accumulate(n.items[i], t)
+			valid = valid && ok
 		}
 	}
 
-	return annot.v.(*T), annot.valid
+	annot.v = t
+	annot.valid = valid
+
+	return t, annot.valid
 }
 
 // InvalidateAnnotation removes any existing cached annotations from this
diff --git a/internal/rangedel/rangedel.go b/internal/rangedel/rangedel.go
index f5ea96e7..7184d95b 100644
--- a/internal/rangedel/rangedel.go
+++ b/internal/rangedel/rangedel.go
@@ -14,7 +14,7 @@ import (
 // closure with the encoded internal keys that represent the Span's state. The
 // keys and values passed to emit are only valid until the closure returns.  If
 // emit returns an error, Encode stops and returns the error.
-func Encode(s *keyspan.Span, emit func(k base.InternalKey, v []byte) error) error {
+func Encode(s keyspan.Span, emit func(k base.InternalKey, v []byte) error) error {
 	for _, k := range s.Keys {
 		if k.Kind() != base.InternalKeyKindRangeDelete {
 			return base.CorruptionErrorf("pebble: rangedel.Encode cannot encode %s key", k.Kind())
diff --git a/internal/rangekey/rangekey.go b/internal/rangekey/rangekey.go
index 2f834c57..59d555d0 100644
--- a/internal/rangekey/rangekey.go
+++ b/internal/rangekey/rangekey.go
@@ -61,7 +61,7 @@ import (
 // closure with the encoded internal keys that represent the Span's state. The
 // keys and values passed to emit are only valid until the closure returns.
 // If emit returns an error, Encode stops and returns the error.
-func Encode(s *keyspan.Span, emit func(k base.InternalKey, v []byte) error) error {
+func Encode(s keyspan.Span, emit func(k base.InternalKey, v []byte) error) error {
 	enc := Encoder{Emit: emit}
 	return enc.Encode(s)
 }
@@ -82,7 +82,7 @@ type Encoder struct {
 //
 // The encoded key-value pair passed to Emit is only valid until the closure
 // completes.
-func (e *Encoder) Encode(s *keyspan.Span) error {
+func (e *Encoder) Encode(s keyspan.Span) error {
 	if s.Empty() {
 		return nil
 	}
@@ -127,7 +127,7 @@ func (e *Encoder) Encode(s *keyspan.Span) error {
 
 // flush constructs internal keys for accumulated key state, and emits the
 // internal keys.
-func (e *Encoder) flush(s *keyspan.Span, seqNum base.SeqNum, del bool) error {
+func (e *Encoder) flush(s keyspan.Span, seqNum base.SeqNum, del bool) error {
 	if len(e.sets) > 0 {
 		ik := base.MakeInternalKey(s.Start, seqNum, base.InternalKeyKindRangeKeySet)
 		l := EncodedSetValueLen(s.End, e.sets)
diff --git a/metamorphic/build.go b/metamorphic/build.go
index 5555e436..d627dbd4 100644
--- a/metamorphic/build.go
+++ b/metamorphic/build.go
@@ -142,7 +142,7 @@ func writeSSTForIngestion(
 				collapsed.Keys[i].Trailer = base.MakeTrailer(0, collapsed.Keys[i].Kind())
 			}
 			keyspan.SortKeysByTrailer(&collapsed.Keys)
-			if err := w.Raw().EncodeSpan(&collapsed); err != nil {
+			if err := w.Raw().EncodeSpan(collapsed); err != nil {
 				return nil, err
 			}
 		}
diff --git a/metamorphic/ops.go b/metamorphic/ops.go
index 6b771097..f1f2cf26 100644
--- a/metamorphic/ops.go
+++ b/metamorphic/ops.go
@@ -1934,12 +1934,11 @@ func (r *replicateOp) runSharedReplicate(
 			return w.DeleteRange(start, end)
 		},
 		func(start, end []byte, keys []keyspan.Key) error {
-			s := keyspan.Span{
+			return w.Raw().EncodeSpan(keyspan.Span{
 				Start: start,
 				End:   end,
 				Keys:  keys,
-			}
-			return w.Raw().EncodeSpan(&s)
+			})
 		},
 		func(sst *pebble.SharedSSTMeta) error {
 			sharedSSTs = append(sharedSSTs, *sst)
@@ -1998,12 +1997,11 @@ func (r *replicateOp) runExternalReplicate(
 			return w.DeleteRange(start, end)
 		},
 		func(start, end []byte, keys []keyspan.Key) error {
-			s := keyspan.Span{
+			return w.Raw().EncodeSpan(keyspan.Span{
 				Start: start,
 				End:   end,
 				Keys:  keys,
-			}
-			return w.Raw().EncodeSpan(&s)
+			})
 		},
 		nil,
 		func(sst *pebble.ExternalFile) error {
@@ -2107,7 +2105,7 @@ func (r *replicateOp) run(t *Test, h historyRecorder) {
 			rangeKeys := iter.RangeKeys()
 			rkStart, rkEnd := iter.RangeBounds()
 
-			span := &keyspan.Span{Start: rkStart, End: rkEnd, Keys: make([]keyspan.Key, len(rangeKeys))}
+			span := keyspan.Span{Start: rkStart, End: rkEnd, Keys: make([]keyspan.Key, len(rangeKeys))}
 			for i := range rangeKeys {
 				span.Keys[i] = keyspan.Key{
 					Trailer: base.MakeTrailer(0, base.InternalKeyKindRangeKeySet),
diff --git a/replay/replay.go b/replay/replay.go
index 6a1aef32..c6e47285 100644
--- a/replay/replay.go
+++ b/replay/replay.go
@@ -1026,7 +1026,7 @@ func loadFlushedSSTableKeys(
 				defer iter.Close()
 				s, err := iter.First()
 				for ; s != nil; s, err = iter.Next() {
-					if err := rangedel.Encode(s, func(k base.InternalKey, v []byte) error {
+					if err := rangedel.Encode(*s, func(k base.InternalKey, v []byte) error {
 						var key flushedKey
 						key.Trailer = k.Trailer
 						bufs.alloc, key.UserKey = bufs.alloc.Copy(k.UserKey)
@@ -1049,7 +1049,7 @@ func loadFlushedSSTableKeys(
 				defer iter.Close()
 				s, err := iter.First()
 				for ; s != nil; s, err = iter.Next() {
-					if err := rangekey.Encode(s, func(k base.InternalKey, v []byte) error {
+					if err := rangekey.Encode(*s, func(k base.InternalKey, v []byte) error {
 						var key flushedKey
 						key.Trailer = k.Trailer
 						bufs.alloc, key.UserKey = bufs.alloc.Copy(k.UserKey)
diff --git a/sstable/raw_writer.go b/sstable/raw_writer.go
index 75bc4804..ddb38ed5 100644
--- a/sstable/raw_writer.go
+++ b/sstable/raw_writer.go
@@ -1037,7 +1037,7 @@ func (w *RawWriter) encodeFragmentedRangeKeySpan(span keyspan.Span) {
 	w.rangeKeySpan = span
 	w.rangeKeySpan.Keys = w.rangeKeysBySuffix.Keys
 	if w.err == nil {
-		w.err = w.EncodeSpan(&w.rangeKeySpan)
+		w.err = w.EncodeSpan(w.rangeKeySpan)
 	}
 }
 
@@ -1660,7 +1660,7 @@ func (w *RawWriter) UnsafeLastPointUserKey() []byte {
 //
 // This is a low-level API that bypasses the fragmenter. The spans passed to
 // this function must be fragmented and ordered.
-func (w *RawWriter) EncodeSpan(span *keyspan.Span) error {
+func (w *RawWriter) EncodeSpan(span keyspan.Span) error {
 	if span.Empty() {
 		return nil
 	}
@@ -1668,7 +1668,7 @@ func (w *RawWriter) EncodeSpan(span *keyspan.Span) error {
 		return rangedel.Encode(span, w.Add)
 	}
 	for i := range w.blockPropCollectors {
-		if err := w.blockPropCollectors[i].AddRangeKeys(*span); err != nil {
+		if err := w.blockPropCollectors[i].AddRangeKeys(span); err != nil {
 			return err
 		}
 	}
@@ -1892,19 +1892,9 @@ func (w *RawWriter) Metadata() (*WriterMetadata, error) {
 	return &w.meta, nil
 }
 
-// WriterOption provide an interface to do work on Writer while it is being
-// opened.
-type WriterOption interface {
-	// writerApply is called on the writer during opening in order to set
-	// internal parameters.
-	writerApply(*RawWriter)
-}
-
 // NewRawWriter returns a new table writer for the file. Closing the writer will
 // close the file.
-func NewRawWriter(
-	writable objstorage.Writable, o WriterOptions, extraOpts ...WriterOption,
-) *RawWriter {
+func NewRawWriter(writable objstorage.Writable, o WriterOptions) *RawWriter {
 	o = o.ensureDefaults()
 	w := &RawWriter{
 		layout: makeLayoutWriter(writable, o),
@@ -1963,16 +1953,6 @@ func NewRawWriter(
 		return w
 	}
 
-	// Note that WriterOptions are applied in two places; the ones with a
-	// preApply() method are applied here. The rest are applied down below after
-	// default properties are set.
-	type preApply interface{ preApply() }
-	for _, opt := range extraOpts {
-		if _, ok := opt.(preApply); ok {
-			opt.writerApply(w)
-		}
-	}
-
 	if o.FilterPolicy != nil {
 		switch o.FilterType {
 		case TableFilter:
@@ -2017,14 +1997,6 @@ func NewRawWriter(
 		w.props.PropertyCollectorNames = buf.String()
 	}
 
-	// Apply the remaining WriterOptions that do not have a preApply() method.
-	for _, opt := range extraOpts {
-		if _, ok := opt.(preApply); ok {
-			continue
-		}
-		opt.writerApply(w)
-	}
-
 	// Initialize the range key fragmenter and encoder.
 	w.rangeKeyEncoder.Emit = w.addRangeKey
 	return w
diff --git a/sstable/suffix_rewriter.go b/sstable/suffix_rewriter.go
index 53efd4c8..99269106 100644
--- a/sstable/suffix_rewriter.go
+++ b/sstable/suffix_rewriter.go
@@ -415,7 +415,7 @@ func rewriteRangeKeyBlockToWriter(r *Reader, w *RawWriter, from, to []byte) erro
 			s.Keys[i].Suffix = to
 		}
 
-		if err := w.EncodeSpan(s); err != nil {
+		if err := w.EncodeSpan(*s); err != nil {
 			return err
 		}
 	}
diff --git a/sstable/writer.go b/sstable/writer.go
index 549daaa3..d354b6e2 100644
--- a/sstable/writer.go
+++ b/sstable/writer.go
@@ -18,8 +18,9 @@ type Writer struct {
 	// be added to the Writer, a keyspan.Fragmenter is used to retain the keys
 	// and values, emitting fragmented, coalesced spans as appropriate. Range
 	// keys must be added in order of their start user-key.
-	fragmenter keyspan.Fragmenter
-	rkBuf      []byte
+	fragmenter  keyspan.Fragmenter
+	rkBuf       []byte
+	keyspanKeys []keyspan.Key
 }
 
 // NewWriter returns a new table writer intended for building external sstables
@@ -27,9 +28,9 @@ type Writer struct {
 // writer will close the file.
 //
 // Internal clients should generally prefer NewRawWriter.
-func NewWriter(writable objstorage.Writable, o WriterOptions, extraOpts ...WriterOption) *Writer {
+func NewWriter(writable objstorage.Writable, o WriterOptions) *Writer {
 	o = o.ensureDefaults()
-	rw := NewRawWriter(writable, o, extraOpts...)
+	rw := NewRawWriter(writable, o)
 	return &Writer{
 		rw: rw,
 		fragmenter: keyspan.Fragmenter{
@@ -83,12 +84,21 @@ func (w *Writer) Delete(key []byte) error {
 // 0. Intended for use to externally construct an sstable before ingestion into
 // a DB.
 //
+// Calls to DeleteRange must be made using already-fragmented (non-overlapping)
+// spans and in sorted order.
+//
 // TODO(peter): untested
 func (w *Writer) DeleteRange(start, end []byte) error {
 	if w.rw.err != nil {
 		return w.rw.err
 	}
-	return w.rw.addTombstone(base.MakeInternalKey(start, 0, InternalKeyKindRangeDelete), end)
+	return w.rw.EncodeSpan(keyspan.Span{
+		Start: start,
+		End:   end,
+		Keys: append(w.keyspanKeys[:0], keyspan.Key{
+			Trailer: base.MakeTrailer(0, base.InternalKeyKindRangeDelete),
+		}),
+	})
 }
 
 // Merge adds an action to the DB that merges the value at key with the new
diff --git a/tool/find.go b/tool/find.go
index 874babcf..7b6879eb 100644
--- a/tool/find.go
+++ b/tool/find.go
@@ -539,7 +539,7 @@ func (f *findT) searchTables(stdout io.Writer, searchKey []byte, refs []findRef)
 				} else {
 					// Use rangedel.Encode to add a reference for each key
 					// within the span.
-					err := rangedel.Encode(rangeDel, func(k base.InternalKey, v []byte) error {
+					err := rangedel.Encode(*rangeDel, func(k base.InternalKey, v []byte) error {
 						refs = append(refs, findRef{
 							key:      k.Clone(),
 							value:    slices.Clone(v),
diff --git a/tool/sstable.go b/tool/sstable.go
index fcfd001b..f8bed160 100644
--- a/tool/sstable.go
+++ b/tool/sstable.go
@@ -472,7 +472,7 @@ func (s *sstableT) runScan(cmd *cobra.Command, args []string) {
 						bytes.HasPrefix(rangeDel.Start, s.filter)) &&
 						r.Compare(s.filter, rangeDel.End) < 0) {
 					fmt.Fprint(stdout, prefix)
-					if err := rangedel.Encode(rangeDel, func(k base.InternalKey, v []byte) error {
+					if err := rangedel.Encode(*rangeDel, func(k base.InternalKey, v []byte) error {
 						formatKeyValue(stdout, s.fmtKey, s.fmtValue, &k, v)
 						return nil
 					}); err != nil {


Test patch:
diff --git a/data_test.go b/data_test.go
index ebcbe154..d6048bbf 100644
--- a/data_test.go
+++ b/data_test.go
@@ -597,7 +597,7 @@ func runBuildRemoteCmd(td *datadriven.TestData, d *DB, storage remote.Storage) e
 	if rdi := b.newRangeDelIter(nil, math.MaxUint64); rdi != nil {
 		s, err := rdi.First()
 		for ; s != nil && err == nil; s, err = rdi.Next() {
-			err = rangedel.Encode(s, func(k base.InternalKey, v []byte) error {
+			err = rangedel.Encode(*s, func(k base.InternalKey, v []byte) error {
 				k.SetSeqNum(0)
 				return w.Raw().Add(k, v)
 			})
@@ -691,7 +691,7 @@ func runBuildCmd(td *datadriven.TestData, d *DB, fs vfs.FS) error {
 	if rdi := b.newRangeDelIter(nil, math.MaxUint64); rdi != nil {
 		s, err := rdi.First()
 		for ; s != nil && err == nil; s, err = rdi.Next() {
-			err = rangedel.Encode(s, func(k base.InternalKey, v []byte) error {
+			err = rangedel.Encode(*s, func(k base.InternalKey, v []byte) error {
 				k.SetSeqNum(0)
 				return w.Raw().Add(k, v)
 			})
@@ -1086,7 +1086,7 @@ func runDBDefineCmdReuseFS(td *datadriven.TestData, opts *Options) (*DB, error)
 			}
 			if data[:i] == "rangekey" {
 				span := keyspan.ParseSpan(data[i:])
-				err := rangekey.Encode(&span, func(k base.InternalKey, v []byte) error {
+				err := rangekey.Encode(span, func(k base.InternalKey, v []byte) error {
 					return mem.set(k, v)
 				})
 				if err != nil {
diff --git a/ingest_test.go b/ingest_test.go
index b8216b9d..d544f71b 100644
--- a/ingest_test.go
+++ b/ingest_test.go
@@ -103,8 +103,7 @@ func TestIngestLoad(t *testing.T) {
 			for _, data := range strings.Split(td.Input, "\n") {
 				if strings.HasPrefix(data, "rangekey: ") {
 					data = strings.TrimPrefix(data, "rangekey: ")
-					s := keyspan.ParseSpan(data)
-					err := w.EncodeSpan(&s)
+					err := w.EncodeSpan(keyspan.ParseSpan(data))
 					if err != nil {
 						return err.Error()
 					}
@@ -1104,7 +1103,7 @@ func testIngestSharedImpl(
 					return nil
 				},
 				func(start, end []byte, seqNum base.SeqNum) error {
-					require.NoError(t, w.EncodeSpan(&keyspan.Span{
+					require.NoError(t, w.EncodeSpan(keyspan.Span{
 						Start: start,
 						End:   end,
 						Keys:  []keyspan.Key{{Trailer: base.MakeTrailer(0, base.InternalKeyKindRangeDelete)}},
@@ -1112,13 +1111,12 @@ func testIngestSharedImpl(
 					return nil
 				},
 				func(start, end []byte, keys []keyspan.Key) error {
-					s := keyspan.Span{
+					require.NoError(t, w.EncodeSpan(keyspan.Span{
 						Start:     start,
 						End:       end,
 						Keys:      keys,
 						KeysOrder: 0,
-					}
-					require.NoError(t, w.EncodeSpan(&s))
+					}))
 					return nil
 				},
 				func(sst *SharedSSTMeta) error {
@@ -1604,7 +1602,7 @@ func TestConcurrentExcise(t *testing.T) {
 					return nil
 				},
 				func(start, end []byte, seqNum base.SeqNum) error {
-					require.NoError(t, w.EncodeSpan(&keyspan.Span{
+					require.NoError(t, w.EncodeSpan(keyspan.Span{
 						Start: start,
 						End:   end,
 						Keys:  []keyspan.Key{{Trailer: base.MakeTrailer(0, base.InternalKeyKindRangeDelete)}},
@@ -1612,13 +1610,12 @@ func TestConcurrentExcise(t *testing.T) {
 					return nil
 				},
 				func(start, end []byte, keys []keyspan.Key) error {
-					s := keyspan.Span{
+					require.NoError(t, w.EncodeSpan(keyspan.Span{
 						Start:     start,
 						End:       end,
 						Keys:      keys,
 						KeysOrder: 0,
-					}
-					require.NoError(t, w.EncodeSpan(&s))
+					}))
 					return nil
 				},
 				func(sst *SharedSSTMeta) error {
@@ -2041,7 +2038,7 @@ func TestIngestExternal(t *testing.T) {
 					return nil
 				},
 				func(start, end []byte, seqNum base.SeqNum) error {
-					require.NoError(t, w.EncodeSpan(&keyspan.Span{
+					require.NoError(t, w.EncodeSpan(keyspan.Span{
 						Start: start,
 						End:   end,
 						Keys:  []keyspan.Key{{Trailer: base.MakeTrailer(0, base.InternalKeyKindRangeDelete)}},
@@ -2049,13 +2046,12 @@ func TestIngestExternal(t *testing.T) {
 					return nil
 				},
 				func(start, end []byte, keys []keyspan.Key) error {
-					s := keyspan.Span{
+					require.NoError(t, w.EncodeSpan(keyspan.Span{
 						Start:     start,
 						End:       end,
 						Keys:      keys,
 						KeysOrder: 0,
-					}
-					require.NoError(t, w.EncodeSpan(&s))
+					}))
 					return nil
 				},
 				nil,
@@ -3236,8 +3232,7 @@ func TestIngest_UpdateSequenceNumber(t *testing.T) {
 		for _, data := range strings.Split(input, "\n") {
 			if strings.HasPrefix(data, "rangekey: ") {
 				data = strings.TrimPrefix(data, "rangekey: ")
-				s := keyspan.ParseSpan(data)
-				err := w.EncodeSpan(&s)
+				err := w.EncodeSpan(keyspan.ParseSpan(data))
 				if err != nil {
 					return nil, err
 				}
diff --git a/internal/manifest/annotator_test.go b/internal/manifest/annotator_test.go
index 1765188b..07badf6a 100644
--- a/internal/manifest/annotator_test.go
+++ b/internal/manifest/annotator_test.go
@@ -56,3 +56,26 @@ func BenchmarkNumFilesAnnotator(b *testing.B) {
 		require.EqualValues(b, uint64(i), numFiles)
 	}
 }
+
+func TestPickFileAggregator(t *testing.T) {
+	const count = 1000
+	a := Annotator[FileMetadata]{
+		Aggregator: PickFileAggregator{
+			Filter: func(f *FileMetadata) (eligible bool, cacheOK bool) {
+				return true, true
+			},
+			Compare: func(f1 *FileMetadata, f2 *FileMetadata) bool {
+				return base.DefaultComparer.Compare(f1.Smallest.UserKey, f2.Smallest.UserKey) < 0
+			},
+		},
+	}
+
+	lm, files := makeTestLevelMetadata(1)
+
+	for i := 1; i <= count; i++ {
+		lm.tree.Insert(newItem(key(i)))
+		pickedFile := a.LevelAnnotation(lm)
+		// The picked file should always be the one with the smallest key.
+		require.Same(t, files[0], pickedFile)
+	}
+}
diff --git a/level_checker_test.go b/level_checker_test.go
index 8a8218fd..00a80c25 100644
--- a/level_checker_test.go
+++ b/level_checker_test.go
@@ -214,7 +214,7 @@ func TestCheckLevelsCornerCases(t *testing.T) {
 				}
 				frag.Finish()
 				for _, v := range tombstones {
-					if err := rangedel.Encode(&v, w.Add); err != nil {
+					if err := rangedel.Encode(v, w.Add); err != nil {
 						return err.Error()
 					}
 				}
diff --git a/level_iter_test.go b/level_iter_test.go
index 9efed178..9b105e1e 100644
--- a/level_iter_test.go
+++ b/level_iter_test.go
@@ -246,7 +246,7 @@ func (lt *levelIterTest) runBuild(d *datadriven.TestData) string {
 			if err != nil {
 				return err.Error()
 			}
-			if err := w.EncodeSpan(&span); err != nil {
+			if err := w.EncodeSpan(span); err != nil {
 				return err.Error()
 			}
 		default:
@@ -257,7 +257,7 @@ func (lt *levelIterTest) runBuild(d *datadriven.TestData) string {
 	}
 	f.Finish()
 	for _, v := range tombstones {
-		if err := rangedel.Encode(&v, w.Add); err != nil {
+		if err := rangedel.Encode(v, w.Add); err != nil {
 			return err.Error()
 		}
 	}
diff --git a/merging_iter_test.go b/merging_iter_test.go
index 4bc721c9..7a0b912f 100644
--- a/merging_iter_test.go
+++ b/merging_iter_test.go
@@ -247,7 +247,7 @@ func TestMergingIterCornerCases(t *testing.T) {
 				}
 				frag.Finish()
 				for _, v := range tombstones {
-					if err := rangedel.Encode(&v, w.Add); err != nil {
+					if err := rangedel.Encode(v, w.Add); err != nil {
 						return err.Error()
 					}
 				}
diff --git a/scan_internal_test.go b/scan_internal_test.go
index 8b355250..0158c0a7 100644
--- a/scan_internal_test.go
+++ b/scan_internal_test.go
@@ -426,8 +426,11 @@ func TestScanInternal(t *testing.T) {
 							keys[i].Trailer = base.MakeTrailer(0, keys[i].Kind())
 						}
 						keyspan.SortKeysByTrailer(&keys)
-						newSpan := &keyspan.Span{Start: span.Start, End: span.End, Keys: keys}
-						require.NoError(t, w.Raw().EncodeSpan(newSpan))
+						require.NoError(t, w.Raw().EncodeSpan(keyspan.Span{
+							Start: span.Start,
+							End:   span.End,
+							Keys:  keys,
+						}))
 					}
 					require.NoError(t, err)
 				}
diff --git a/sstable/block_property_test.go b/sstable/block_property_test.go
index e2c4b646..0c297c46 100644
--- a/sstable/block_property_test.go
+++ b/sstable/block_property_test.go
@@ -1352,7 +1352,7 @@ func (p *keyCountCollector) AddPointKey(k InternalKey, _ []byte) error {
 }
 
 func (p *keyCountCollector) AddRangeKeys(span Span) error {
-	return rangekey.Encode(&span, func(k base.InternalKey, v []byte) error {
+	return rangekey.Encode(span, func(k base.InternalKey, v []byte) error {
 		p.table++
 		return nil
 	})
diff --git a/sstable/rowblk/rowblk_fragment_iter_test.go b/sstable/rowblk/rowblk_fragment_iter_test.go
index 8e484053..5145d80f 100644
--- a/sstable/rowblk/rowblk_fragment_iter_test.go
+++ b/sstable/rowblk/rowblk_fragment_iter_test.go
@@ -58,9 +58,9 @@ func TestBlockFragmentIterator(t *testing.T) {
 			}
 			for _, s := range spans {
 				if s.Keys[0].Kind() == base.InternalKeyKindRangeDelete {
-					rangedel.Encode(&s, emitFn)
+					rangedel.Encode(s, emitFn)
 				} else {
-					rangekey.Encode(&s, emitFn)
+					rangekey.Encode(s, emitFn)
 				}
 			}
 			blockData := w.Finish()
diff --git a/table_stats_test.go b/table_stats_test.go
index 16b9e06a..f2cb0d2c 100644
--- a/table_stats_test.go
+++ b/table_stats_test.go
@@ -209,7 +209,7 @@ func TestTableRangeDeletionIter(t *testing.T) {
 			for _, line := range strings.Split(td.Input, "\n") {
 				s := keyspan.ParseSpan(line)
 				// Range dels can be written sequentially. Range keys must be collected.
-				rKeySpan := &keyspan.Span{Start: s.Start, End: s.End}
+				rKeySpan := keyspan.Span{Start: s.Start, End: s.End}
 				for _, k := range s.Keys {
 					if rangekey.IsRangeKey(k.Kind()) {
 						rKeySpan.Keys = append(rKeySpan.Keys, k)