From 01c9e754f0a63e65c4769afe5b526fe290decfba Mon Sep 17 00:00:00 2001
From: Jackson Owens <jackson@cockroachlabs.com>
Date: Mon, 13 May 2024 17:38:42 -0400
Subject: [PATCH] db: add initMergingIterLevel to flushable interface

Add a new initMergingIterLevel method to the flushable interface for setting up
a flushable's level within the mergingIter. This is used to update ingested
flushables to use a levelIter to surface range deletions, like other levels of
the LSM do. This is more efficient, avoiding unnecessarily loading range
deletions in other files, and consistent.
---
 batch.go     |  8 ++++++++
 db.go        |  6 ++----
 flushable.go | 15 +++++++++++++--
 mem_table.go | 10 ++++++++++
 4 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/batch.go b/batch.go
index c77896d25a..2347d658fb 100644
--- a/batch.go
+++ b/batch.go
@@ -2043,6 +2043,14 @@ func (b *flushableBatch) Swap(i, j int) {
 	b.offsets[i], b.offsets[j] = b.offsets[j], b.offsets[i]
 }
 
+// initMergingIterLevel is part of the flushable interface.
+func (b *flushableBatch) initMergingIterLevel(
+	ctx context.Context, o *IterOptions, mil *mergingIterLevel,
+) {
+	mil.iter = b.newIter(o)
+	mil.rangeDelIter = b.newRangeDelIter(o)
+}
+
 // newIter is part of the flushable interface.
 func (b *flushableBatch) newIter(o *IterOptions) internalIterator {
 	return &flushableBatchIter{
diff --git a/db.go b/db.go
index b4b9acd0a4..9befc0e379 100644
--- a/db.go
+++ b/db.go
@@ -1451,10 +1451,8 @@ func (i *Iterator) constructPointIter(
 		// Next are the memtables.
 		for j := len(memtables) - 1; j >= 0; j-- {
 			mem := memtables[j]
-			mlevels = append(mlevels, mergingIterLevel{
-				iter:         mem.newIter(&i.opts),
-				rangeDelIter: mem.newRangeDelIter(&i.opts),
-			})
+			mlevels = append(mlevels, mergingIterLevel{})
+			mem.initMergingIterLevel(ctx, &i.opts, &mlevels[len(mlevels)-1])
 		}
 
 		// Next are the file levels: L0 sub-levels followed by lower levels.
diff --git a/flushable.go b/flushable.go
index a2ed68c4fb..8d8122e0fc 100644
--- a/flushable.go
+++ b/flushable.go
@@ -13,6 +13,7 @@ import (
 
 	"github.com/cockroachdb/errors"
 	"github.com/cockroachdb/pebble/internal/base"
+	"github.com/cockroachdb/pebble/internal/invalidating"
 	"github.com/cockroachdb/pebble/internal/invariants"
 	"github.com/cockroachdb/pebble/internal/keyspan"
 	"github.com/cockroachdb/pebble/internal/keyspan/keyspanimpl"
@@ -21,6 +22,9 @@ import (
 
 // flushable defines the interface for immutable memtables.
 type flushable interface {
+	// initMergingIterLevel initializes a mergingIterLevel for iteration over
+	// the flushable's point keys and range deletions.
+	initMergingIterLevel(ctx context.Context, o *IterOptions, mil *mergingIterLevel)
 	newIter(o *IterOptions) internalIterator
 	newFlushIter(o *IterOptions) internalIterator
 	newRangeDelIter(o *IterOptions) keyspan.FragmentIterator
@@ -209,6 +213,15 @@ func newIngestedFlushable(
 // TODO(sumeer): ingestedFlushable iters also need to plumb context for
 // tracing.
 
+func (s *ingestedFlushable) initMergingIterLevel(
+	ctx context.Context, o *IterOptions, mil *mergingIterLevel,
+) {
+	li := newLevelIter(ctx, *o, s.comparer, s.newIters, s.slice.Iter(), manifest.Level(0), internalIterOpts{})
+	li.initRangeDel(&mil.rangeDelIter)
+	mil.levelIter = li
+	mil.iter = invalidating.MaybeWrapIfInvariants(li)
+}
+
 // newIter is part of the flushable interface.
 func (s *ingestedFlushable) newIter(o *IterOptions) internalIterator {
 	var opts IterOptions
@@ -244,8 +257,6 @@ func (s *ingestedFlushable) constructRangeDelIter(
 }
 
 // newRangeDelIter is part of the flushable interface.
-// TODO(bananabrick): Using a level iter instead of a keyspan level iter to
-// surface range deletes is more efficient.
 //
 // TODO(sumeer): *IterOptions are being ignored, so the index block load for
 // the point iterator in constructRangeDeIter is not tracked.
diff --git a/mem_table.go b/mem_table.go
index d1ed16e289..db01bdc7e8 100644
--- a/mem_table.go
+++ b/mem_table.go
@@ -6,6 +6,7 @@ package pebble
 
 import (
 	"bytes"
+	"context"
 	"fmt"
 	"os"
 	"sync"
@@ -252,6 +253,15 @@ func (m *memTable) apply(batch *Batch, seqNum uint64) error {
 	return nil
 }
 
+func (m *memTable) initMergingIterLevel(
+	ctx context.Context, o *IterOptions, mil *mergingIterLevel,
+) {
+	*mil = mergingIterLevel{
+		iter:         m.newIter(o),
+		rangeDelIter: m.newRangeDelIter(o),
+	}
+}
+
 // newIter is part of the flushable interface. It returns an iterator that is
 // unpositioned (Iterator.Valid() will return false). The iterator can be
 // positioned via a call to SeekGE, SeekLT, First or Last.