Repository: cockroachdb/pebble
Pull request #3668 (2024-06-18 02:13:55)

provider: consult readahead config when initializing handle
We currently retrieve the readahead config when we create a `Readable`
and then we use that config for all read handles. This makes the
settings not responsive, as any tables already in the table cache will
have the `Readable` created.

We change `ReadaheadConfig` to an atomic that is loaded every time we
initialize a read handle.

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


Patch:
diff --git a/compaction.go b/compaction.go
index 94ec4871..536fb0f5 100644
--- a/compaction.go
+++ b/compaction.go
@@ -8,7 +8,6 @@ import (
 	"bytes"
 	"context"
 	"fmt"
-	"io"
 	"math"
 	"runtime/pprof"
 	"slices"
@@ -77,9 +76,7 @@ type noCloseIter struct {
 	keyspan.FragmentIterator
 }
 
-func (i noCloseIter) Close() error {
-	return nil
-}
+func (i *noCloseIter) Close() {}
 
 type compactionLevel struct {
 	level int
@@ -228,10 +225,10 @@ type compaction struct {
 	smallest InternalKey
 	largest  InternalKey
 
-	// A list of objects to close when the compaction finishes. Used by input
-	// iteration to keep rangeDelIters open for the lifetime of the compaction,
-	// and only close them when the compaction finishes.
-	closers []io.Closer
+	// A list of fragment iterators to close when the compaction finishes. Used by
+	// input iteration to keep rangeDelIters open for the lifetime of the
+	// compaction, and only close them when the compaction finishes.
+	closers []*noCloseIter
 
 	// grandparents are the tables in level+2 that overlap with the files being
 	// compacted. Used to determine output table boundaries. Do not assume that the actual files
@@ -788,8 +785,7 @@ func (c *compaction) newInputIters(
 			// mergingIter.
 			iter := level.files.Iter()
 			for f := iter.First(); f != nil; f = iter.Next() {
-				rangeDelIter, closer, err := c.newRangeDelIter(
-					newIters, iter.Take(), iterOpts, l)
+				rangeDelIter, err := c.newRangeDelIter(newIters, iter.Take(), iterOpts, l)
 				if err != nil {
 					// The error will already be annotated with the BackingFileNum, so
 					// we annotate it with the FileNum.
@@ -799,7 +795,7 @@ func (c *compaction) newInputIters(
 					continue
 				}
 				rangeDelIters = append(rangeDelIters, rangeDelIter)
-				c.closers = append(c.closers, closer)
+				c.closers = append(c.closers, rangeDelIter)
 			}
 
 			// Check if this level has any range keys.
@@ -813,25 +809,25 @@ func (c *compaction) newInputIters(
 			if hasRangeKeys {
 				li := &keyspanimpl.LevelIter{}
 				newRangeKeyIterWrapper := func(file *manifest.FileMetadata, iterOptions keyspan.SpanIterOptions) (keyspan.FragmentIterator, error) {
-					iter, err := newRangeKeyIter(file, iterOptions)
+					rangeKeyIter, err := newRangeKeyIter(file, iterOptions)
 					if err != nil {
 						return nil, err
-					} else if iter == nil {
+					} else if rangeKeyIter == nil {
 						return emptyKeyspanIter, nil
 					}
 					// Ensure that the range key iter is not closed until the compaction is
 					// finished. This is necessary because range key processing
 					// requires the range keys to be held in memory for up to the
 					// lifetime of the compaction.
-					c.closers = append(c.closers, iter)
-					iter = noCloseIter{iter}
+					noCloseIter := &noCloseIter{rangeKeyIter}
+					c.closers = append(c.closers, noCloseIter)
 
 					// We do not need to truncate range keys to sstable boundaries, or
 					// only read within the file's atomic compaction units, unlike with
 					// range tombstones. This is because range keys were added after we
 					// stopped splitting user keys across sstables, so all the range keys
 					// in this sstable must wholly lie within the file's bounds.
-					return iter, err
+					return noCloseIter, err
 				}
 				li.Init(keyspan.SpanIterOptions{}, c.cmp, newRangeKeyIterWrapper, level.files.Iter(), l, manifest.KeyTypeRange)
 				rangeKeyIters = append(rangeKeyIters, li)
@@ -902,7 +898,7 @@ func (c *compaction) newInputIters(
 
 func (c *compaction) newRangeDelIter(
 	newIters tableNewIters, f manifest.LevelFile, opts IterOptions, l manifest.Level,
-) (keyspan.FragmentIterator, io.Closer, error) {
+) (*noCloseIter, error) {
 	opts.level = l
 	iterSet, err := newIters(context.Background(), f.FileMetadata, &opts,
 		internalIterOpts{
@@ -910,16 +906,16 @@ func (c *compaction) newRangeDelIter(
 			bufferPool: &c.bufferPool,
 		}, iterRangeDeletions)
 	if err != nil {
-		return nil, nil, err
+		return nil, err
 	} else if iterSet.rangeDeletion == nil {
 		// The file doesn't contain any range deletions.
-		return nil, nil, nil
+		return nil, nil
 	}
 	// Ensure that rangeDelIter is not closed until the compaction is
 	// finished. This is necessary because range tombstone processing
 	// requires the range tombstones to be held in memory for up to the
 	// lifetime of the compaction.
-	return noCloseIter{iterSet.rangeDeletion}, iterSet.rangeDeletion, nil
+	return &noCloseIter{iterSet.rangeDeletion}, nil
 }
 
 func (c *compaction) String() string {
@@ -2497,7 +2493,7 @@ func (d *DB) compactAndWrite(
 	pointIter, rangeDelIter, rangeKeyIter, err := c.newInputIters(d.newIters, d.tableNewRangeKeyIter)
 	defer func() {
 		for _, closer := range c.closers {
-			result.Err = firstError(result.Err, closer.Close())
+			closer.FragmentIterator.Close()
 		}
 	}()
 	if err != nil {
diff --git a/error_iter.go b/error_iter.go
index 4ae02811..b9f4c5ba 100644
--- a/error_iter.go
+++ b/error_iter.go
@@ -85,6 +85,6 @@ func (i *errorKeyspanIter) First() (*keyspan.Span, error)            { return ni
 func (i *errorKeyspanIter) Last() (*keyspan.Span, error)             { return nil, i.err }
 func (i *errorKeyspanIter) Next() (*keyspan.Span, error)             { return nil, i.err }
 func (i *errorKeyspanIter) Prev() (*keyspan.Span, error)             { return nil, i.err }
-func (i *errorKeyspanIter) Close() error                             { return i.err }
+func (i *errorKeyspanIter) Close()                                   {}
 func (*errorKeyspanIter) String() string                             { return "error" }
 func (*errorKeyspanIter) WrapChildren(wrap keyspan.WrapFn)           {}
diff --git a/flushable.go b/flushable.go
index 85aef6b4..241ddcfc 100644
--- a/flushable.go
+++ b/flushable.go
@@ -7,7 +7,6 @@ package pebble
 import (
 	"context"
 	"fmt"
-	"io"
 	"sync/atomic"
 	"time"
 
@@ -347,9 +346,9 @@ func computePossibleOverlapsGenericImpl[F flushable](
 ) {
 	iter := f.newIter(nil)
 	rangeDelIter := f.newRangeDelIter(nil)
-	rkeyIter := f.newRangeKeyIter(nil)
+	rangeKeyIter := f.newRangeKeyIter(nil)
 	for _, b := range bounded {
-		overlap, err := determineOverlapAllIters(cmp, b.UserKeyBounds(), iter, rangeDelIter, rkeyIter)
+		overlap, err := determineOverlapAllIters(cmp, b.UserKeyBounds(), iter, rangeDelIter, rangeKeyIter)
 		if invariants.Enabled && err != nil {
 			panic(errors.AssertionFailedf("expected iterator to be infallible: %v", err))
 		}
@@ -360,15 +359,19 @@ func computePossibleOverlapsGenericImpl[F flushable](
 		}
 	}
 
-	for _, c := range [3]io.Closer{iter, rangeDelIter, rkeyIter} {
-		if c != nil {
-			if err := c.Close(); err != nil {
-				// This implementation must be used in circumstances where
-				// reading through the iterator is infallible.
-				panic(err)
-			}
+	if iter != nil {
+		if err := iter.Close(); err != nil {
+			// This implementation must be used in circumstances where
+			// reading through the iterator is infallible.
+			panic(err)
 		}
 	}
+	if rangeDelIter != nil {
+		rangeDelIter.Close()
+	}
+	if rangeKeyIter != nil {
+		rangeKeyIter.Close()
+	}
 }
 
 // determineOverlapAllIters checks for overlap in a point iterator, range
diff --git a/get_iter.go b/get_iter.go
index ee2b4044..44a4c890 100644
--- a/get_iter.go
+++ b/get_iter.go
@@ -288,8 +288,6 @@ func (g *getIter) maybeSetTombstone(rangeDelIter keyspan.FragmentIterator) (ok b
 	// care about the most recent range deletion that's visible because it's the
 	// "most powerful."
 	g.tombstonedSeqNum, g.tombstoned = t.LargestVisibleSeqNum(g.snapshot)
-	if g.err = firstError(g.err, rangeDelIter.Close()); g.err != nil {
-		return false
-	}
+	rangeDelIter.Close()
 	return true
 }
diff --git a/internal/keyspan/assert_iter.go b/internal/keyspan/assert_iter.go
index 3a56b680..6fb6b59b 100644
--- a/internal/keyspan/assert_iter.go
+++ b/internal/keyspan/assert_iter.go
@@ -163,8 +163,8 @@ func (i *assertIter) Prev() (*Span, error) {
 }
 
 // Close implements FragmentIterator.
-func (i *assertIter) Close() error {
-	return i.iter.Close()
+func (i *assertIter) Close() {
+	i.iter.Close()
 }
 
 // WrapChildren implements FragmentIterator.
diff --git a/internal/keyspan/bounded.go b/internal/keyspan/bounded.go
index 3ca452ea..6b0e88a2 100644
--- a/internal/keyspan/bounded.go
+++ b/internal/keyspan/bounded.go
@@ -202,8 +202,8 @@ func (i *BoundedIter) Prev() (*Span, error) {
 }
 
 // Close implements FragmentIterator.
-func (i *BoundedIter) Close() error {
-	return i.iter.Close()
+func (i *BoundedIter) Close() {
+	i.iter.Close()
 }
 
 // SetBounds modifies the FragmentIterator's bounds.
diff --git a/internal/keyspan/defragment.go b/internal/keyspan/defragment.go
index 4e05e204..5f6402ce 100644
--- a/internal/keyspan/defragment.go
+++ b/internal/keyspan/defragment.go
@@ -196,8 +196,8 @@ func (i *DefragmentingIter) Init(
 }
 
 // Close closes the underlying iterators.
-func (i *DefragmentingIter) Close() error {
-	return i.iter.Close()
+func (i *DefragmentingIter) Close() {
+	i.iter.Close()
 }
 
 // SeekGE moves the iterator to the first span covering a key greater than or
diff --git a/internal/keyspan/filter.go b/internal/keyspan/filter.go
index 436750a1..55a8d6f0 100644
--- a/internal/keyspan/filter.go
+++ b/internal/keyspan/filter.go
@@ -91,8 +91,8 @@ func (i *filteringIter) Prev() (*Span, error) {
 }
 
 // Close implements FragmentIterator.
-func (i *filteringIter) Close() error {
-	return i.iter.Close()
+func (i *filteringIter) Close() {
+	i.iter.Close()
 }
 
 // filter uses the filterFn (if configured) to filter and possibly mutate the
diff --git a/internal/keyspan/interleaving_iter.go b/internal/keyspan/interleaving_iter.go
index a6eee31e..dda0da6d 100644
--- a/internal/keyspan/interleaving_iter.go
+++ b/internal/keyspan/interleaving_iter.go
@@ -1152,9 +1152,9 @@ func (i *InterleavingIter) Error() error {
 
 // Close implements (base.InternalIterator).Close.
 func (i *InterleavingIter) Close() error {
-	perr := i.pointIter.Close()
-	rerr := i.keyspanIter.Close()
-	return firstError(perr, rerr)
+	err := i.pointIter.Close()
+	i.keyspanIter.Close()
+	return err
 }
 
 // String implements (base.InternalIterator).String.
diff --git a/internal/keyspan/iter.go b/internal/keyspan/iter.go
index 325fb377..3ff971d6 100644
--- a/internal/keyspan/iter.go
+++ b/internal/keyspan/iter.go
@@ -55,11 +55,9 @@ type FragmentIterator interface {
 	// previous call to SeekLT or Prev returned an invalid span.
 	Prev() (*Span, error)
 
-	// Close closes the iterator and returns any accumulated error. Exhausting
-	// the iterator is not considered to be an error. It is valid to call Close
-	// multiple times. Other methods should not be called after the iterator has
-	// been closed.
-	Close() error
+	// Close closes the iterator. It is valid to call Close multiple times. Other
+	// methods should not be called after the iterator has been closed.
+	Close()
 
 	// WrapChildren wraps any child iterators using the given function. The
 	// function can call WrapChildren to recursively wrap an entire iterator
@@ -207,9 +205,7 @@ func (i *Iter) Prev() (*Span, error) {
 }
 
 // Close implements FragmentIterator.Close.
-func (i *Iter) Close() error {
-	return nil
-}
+func (i *Iter) Close() {}
 
 func (i *Iter) String() string {
 	return "fragmented-spans"
diff --git a/internal/keyspan/keyspanimpl/level_iter.go b/internal/keyspan/keyspanimpl/level_iter.go
index b45c4340..5be37b9b 100644
--- a/internal/keyspan/keyspanimpl/level_iter.go
+++ b/internal/keyspan/keyspanimpl/level_iter.go
@@ -146,9 +146,7 @@ func (l *LevelIter) loadFile(file *manifest.FileMetadata, dir int) loadFileRetur
 	}
 
 	// Note that LevelIter.Close() can be called multiple times.
-	if err := l.Close(); err != nil {
-		return noFileLoaded
-	}
+	l.Close()
 
 	l.iterFile = file
 	l.iter = nil
@@ -197,9 +195,7 @@ func (l *LevelIter) SeekGE(key []byte) (*keyspan.Span, error) {
 			// cases similar to the above, while still retaining correctness.
 			// Return a straddling key instead of loading the file.
 			l.iterFile = f
-			if l.err = l.Close(); l.err != nil {
-				return l.verify(nil, l.err)
-			}
+			l.Close()
 			l.straddleDir = +1
 			l.straddle = keyspan.Span{
 				Start: prevFile.LargestRangeKey.UserKey,
@@ -250,9 +246,7 @@ func (l *LevelIter) SeekLT(key []byte) (*keyspan.Span, error) {
 			// cases similar to the above, while still retaining correctness.
 			// Return a straddling key instead of loading the file.
 			l.iterFile = f
-			if l.err = l.Close(); l.err != nil {
-				return l.verify(nil, l.err)
-			}
+			l.Close()
 			l.straddleDir = -1
 			l.straddle = keyspan.Span{
 				Start: f.LargestRangeKey.UserKey,
@@ -360,9 +354,7 @@ func (l *LevelIter) skipEmptyFileForward() (*keyspan.Span, error) {
 		// a "straddle span" in l.straddle and return that.
 		//
 		// Straddle spans are not created in rangedel mode.
-		if l.err = l.Close(); l.err != nil {
-			return l.verify(nil, l.err)
-		}
+		l.Close()
 		startKey := l.iterFile.LargestRangeKey.UserKey
 		// Resetting l.iterFile without loading the file into l.iter is okay and
 		// does not change the logic in loadFile() as long as l.iter is also nil;
@@ -423,9 +415,7 @@ func (l *LevelIter) skipEmptyFileBackward() (*keyspan.Span, error) {
 	// Straddle spans are not created in rangedel mode.
 	if l.straddleDir == 0 && l.keyType == manifest.KeyTypeRange &&
 		l.iterFile != nil && l.iter != nil {
-		if l.err = l.Close(); l.err != nil {
-			return l.verify(nil, l.err)
-		}
+		l.Close()
 		endKey := l.iterFile.SmallestRangeKey.UserKey
 		// Resetting l.iterFile without loading the file into l.iter is okay and
 		// does not change the logic in loadFile() as long as l.iter is also nil;
@@ -503,12 +493,11 @@ func (l *LevelIter) Error() error {
 }
 
 // Close implements keyspan.FragmentIterator.
-func (l *LevelIter) Close() error {
+func (l *LevelIter) Close() {
 	if l.iter != nil {
-		l.err = l.iter.Close()
+		l.iter.Close()
 		l.iter = nil
 	}
-	return l.err
 }
 
 // String implements keyspan.FragmentIterator.
diff --git a/internal/keyspan/keyspanimpl/merging_iter.go b/internal/keyspan/keyspanimpl/merging_iter.go
index 8840754e..ad54e725 100644
--- a/internal/keyspan/keyspanimpl/merging_iter.go
+++ b/internal/keyspan/keyspanimpl/merging_iter.go
@@ -676,14 +676,12 @@ func (m *MergingIter) Prev() (*keyspan.Span, error) {
 }
 
 // Close closes the iterator, releasing all acquired resources.
-func (m *MergingIter) Close() error {
-	var err error
+func (m *MergingIter) Close() {
 	for i := range m.levels {
-		err = firstError(err, m.levels[i].iter.Close())
+		m.levels[i].iter.Close()
 	}
 	m.levels = nil
 	m.heap.items = m.heap.items[:0]
-	return err
 }
 
 // String implements fmt.Stringer.
@@ -1240,10 +1238,3 @@ func (k boundKey) String() string {
 	fmt.Fprint(&buf, "]")
 	return buf.String()
 }
-
-func firstError(e1, e2 error) error {
-	if e1 == nil {
-		return e2
-	}
-	return e1
-}
diff --git a/internal/keyspan/logging_iter.go b/internal/keyspan/logging_iter.go
index 0536f578..219fd825 100644
--- a/internal/keyspan/logging_iter.go
+++ b/internal/keyspan/logging_iter.go
@@ -133,15 +133,10 @@ func (i *loggingIter) Prev() (*Span, error) {
 }
 
 // Close implements FragmentIterator.
-func (i *loggingIter) Close() error {
+func (i *loggingIter) Close() {
 	opEnd := i.opStartf("Close()")
-	err := i.iter.Close()
-	if err != nil {
-		opEnd(err)
-	} else {
-		opEnd()
-	}
-	return err
+	i.iter.Close()
+	opEnd()
 }
 
 // WrapChildren implements FragmentIterator.
diff --git a/internal/keyspan/transformer.go b/internal/keyspan/transformer.go
index b85d29fe..fe1f1fd0 100644
--- a/internal/keyspan/transformer.go
+++ b/internal/keyspan/transformer.go
@@ -134,6 +134,6 @@ func (t *TransformerIter) Prev() (*Span, error) {
 }
 
 // Close implements the FragmentIterator interface.
-func (t *TransformerIter) Close() error {
-	return t.FragmentIterator.Close()
+func (t *TransformerIter) Close() {
+	t.FragmentIterator.Close()
 }
diff --git a/internal/keyspan/truncate.go b/internal/keyspan/truncate.go
index 63aa0ea4..898c51a8 100644
--- a/internal/keyspan/truncate.go
+++ b/internal/keyspan/truncate.go
@@ -112,8 +112,8 @@ func (i *truncatingIter) Prev() (*Span, error) {
 }
 
 // Close implements FragmentIterator.
-func (i *truncatingIter) Close() error {
-	return i.iter.Close()
+func (i *truncatingIter) Close() {
+	i.iter.Close()
 }
 
 // nextSpanWithinBounds returns the first span (starting with the given span and
diff --git a/iterator.go b/iterator.go
index 2957ecf1..76e45b06 100644
--- a/iterator.go
+++ b/iterator.go
@@ -2347,7 +2347,7 @@ func (i *Iterator) Close() error {
 			i.err = firstError(i.err, i.pointIter.Close())
 		}
 		if i.rangeKey != nil && i.rangeKey.rangeKeyIter != nil {
-			i.err = firstError(i.err, i.rangeKey.rangeKeyIter.Close())
+			i.rangeKey.rangeKeyIter.Close()
 		}
 	}
 	err := i.err
@@ -2591,7 +2591,7 @@ func (i *Iterator) SetOptions(o *IterOptions) {
 	}
 	if i.rangeKey != nil {
 		if closeBoth || len(o.RangeKeyFilters) > 0 || len(i.opts.RangeKeyFilters) > 0 {
-			i.err = firstError(i.err, i.rangeKey.rangeKeyIter.Close())
+			i.rangeKey.rangeKeyIter.Close()
 			i.rangeKey = nil
 		} else {
 			// If there's still a range key iterator stack, invalidate the
@@ -2649,7 +2649,7 @@ func (i *Iterator) SetOptions(o *IterOptions) {
 					// iterator stack. We need to reconstruct the range key
 					// iterator to add i.batchRangeKeyIter into the iterator
 					// stack.
-					i.err = firstError(i.err, i.rangeKey.rangeKeyIter.Close())
+					i.rangeKey.rangeKeyIter.Close()
 					i.rangeKey = nil
 				} else {
 					// There are range keys in the batch and we already
diff --git a/level_checker.go b/level_checker.go
index b2f0936b..28692e8f 100644
--- a/level_checker.go
+++ b/level_checker.go
@@ -449,9 +449,9 @@ func addTombstonesFromIter(
 	seqNum uint64,
 	cmp Compare,
 	formatKey base.FormatKey,
-) (_ []tombstoneWithLevel, err error) {
+) ([]tombstoneWithLevel, error) {
 	defer func() {
-		err = firstError(err, iter.Close())
+		iter.Close()
 	}()
 
 	var prevTombstone keyspan.Span
@@ -594,7 +594,7 @@ func checkLevelsInternal(c *checkConfig) (err error) {
 				l.iter = nil
 			}
 			if l.rangeDelIter != nil {
-				err = firstError(err, l.rangeDelIter.Close())
+				l.rangeDelIter.Close()
 				l.rangeDelIter = nil
 			}
 		}
diff --git a/level_iter.go b/level_iter.go
index 568e38e5..f461800a 100644
--- a/level_iter.go
+++ b/level_iter.go
@@ -921,7 +921,7 @@ func (l *levelIter) Close() error {
 	}
 	if l.rangeDelIterPtr != nil {
 		if t := l.rangeDelIterCopy; t != nil {
-			l.err = firstError(l.err, t.Close())
+			t.Close()
 		}
 		*l.rangeDelIterPtr = nil
 		l.rangeDelIterCopy = nil
diff --git a/merging_iter.go b/merging_iter.go
index 2a1781bf..aef58565 100644
--- a/merging_iter.go
+++ b/merging_iter.go
@@ -1291,9 +1291,7 @@ func (m *mergingIter) Close() error {
 			m.err = err
 		}
 		if rangeDelIter := m.levels[i].rangeDelIter; rangeDelIter != nil {
-			if err := rangeDelIter.Close(); err != nil && m.err == nil {
-				m.err = err
-			}
+			rangeDelIter.Close()
 		}
 	}
 	m.levels = nil
diff --git a/metamorphic/build.go b/metamorphic/build.go
index 623c6267..24a87e0d 100644
--- a/metamorphic/build.go
+++ b/metamorphic/build.go
@@ -41,11 +41,15 @@ func writeSSTForIngestion(
 	}
 	w := sstable.NewWriter(writable, writerOpts)
 	pointIterCloser := base.CloseHelper(pointIter)
-	defer pointIterCloser.Close()
-	rangeDelIterCloser := base.CloseHelper(rangeDelIter)
-	defer rangeDelIterCloser.Close()
-	rangeKeyIterCloser := base.CloseHelper(rangeKeyIter)
-	defer rangeKeyIterCloser.Close()
+	defer func() {
+		_ = pointIterCloser.Close()
+		if rangeDelIter != nil {
+			rangeDelIter.Close()
+		}
+		if rangeKeyIter != nil {
+			rangeKeyIter.Close()
+		}
+	}()
 
 	outputKey := func(key []byte) []byte {
 		if !syntheticPrefix.IsSet() && !syntheticSuffix.IsSet() {
@@ -110,9 +114,8 @@ func writeSSTForIngestion(
 		if err != nil {
 			return nil, err
 		}
-		if err := rangeDelIterCloser.Close(); err != nil {
-			return nil, err
-		}
+		rangeDelIter.Close()
+		rangeDelIter = nil
 	}
 
 	if rangeKeyIter != nil {
@@ -146,9 +149,8 @@ func writeSSTForIngestion(
 		if err != nil {
 			return nil, err
 		}
-		if err := rangeKeyIterCloser.Close(); err != nil {
-			return nil, err
-		}
+		rangeKeyIter.Close()
+		rangeKeyIter = nil
 	}
 
 	if err := w.Close(); err != nil {
diff --git a/metamorphic/ops.go b/metamorphic/ops.go
index 0db675fc..daf132f2 100644
--- a/metamorphic/ops.go
+++ b/metamorphic/ops.go
@@ -772,9 +772,7 @@ func (o *ingestOp) collapseBatch(
 		if err != nil {
 			return nil, err
 		}
-		if err := rangeDelIter.Close(); err != nil {
-			return nil, err
-		}
+		rangeDelIter.Close()
 		rangeDelIter = nil
 	}
 
@@ -856,9 +854,7 @@ func (o *ingestOp) collapseBatch(
 				return nil, err
 			}
 		}
-		if err := rangeKeyIter.Close(); err != nil {
-			return nil, err
-		}
+		rangeKeyIter.Close()
 		rangeKeyIter = nil
 	}
 
diff --git a/objstorage/objstorageprovider/provider.go b/objstorage/objstorageprovider/provider.go
index 729f61a5..e50f2c82 100644
--- a/objstorage/objstorageprovider/provider.go
+++ b/objstorage/objstorageprovider/provider.go
@@ -11,6 +11,7 @@ import (
 	"os"
 	"slices"
 	"sync"
+	"sync/atomic"
 
 	"github.com/cockroachdb/errors"
 	"github.com/cockroachdb/errors/oserror"
@@ -92,10 +93,9 @@ type Settings struct {
 	Local struct {
 		// TODO(radu): move FSCleaner, NoSyncOnClose, BytesPerSync here.
 
-		// ReadaheadConfigFn is a function used to retrieve the current readahead
-		// mode. This function is run whenever a local object is open for reading.
-		// If it is nil, DefaultReadaheadConfig is used.
-		ReadaheadConfigFn func() ReadaheadConfig
+		// ReadaheadConfig is used to retrieve the current readahead mode; it is
+		// consulted whenever a read handle is initialized.
+		ReadaheadConfig *ReadaheadConfig
 	}
 
 	// Fields here are set only if the provider is to support remote objects
@@ -135,22 +135,49 @@ type Settings struct {
 	}
 }
 
-// ReadaheadConfig controls the use of read-ahead.
+// ReadaheadConfig is a container for the settings that control the use of
+// read-ahead.
+//
+// It stores two ReadaheadModes:
+//   - Informed is the type of read-ahead for operations that are known to read a
+//     large consecutive chunk of a file.
+//   - Speculative is the type of read-ahead used automatically, when consecutive
+//     reads are detected.
+//
+// The settings can be changed and read atomically.
 type ReadaheadConfig struct {
-	// Informed is the type of read-ahead for operations that are known to read a
-	// large consecutive chunk of a file.
-	Informed ReadaheadMode
+	value atomic.Uint32
+}
+
+// These are the default readahead modes when a config is not specified.
+const (
+	defaultReadaheadInformed    = FadviseSequential
+	defaultReadaheadSpeculative = FadviseSequential
+)
 
-	// Speculative is the type of read-ahead used automatically, when consecutive
-	// reads are detected.
-	Speculative ReadaheadMode
+// NewReadaheadConfig returns a new readahead config container initialized with
+// default values.
+func NewReadaheadConfig() *ReadaheadConfig {
+	rc := &ReadaheadConfig{}
+	rc.Set(defaultReadaheadInformed, defaultReadaheadSpeculative)
+	return rc
 }
 
-// DefaultReadaheadConfig is the readahead config used when ReadaheadConfigFn is
-// not specified.
-var DefaultReadaheadConfig = ReadaheadConfig{
-	Informed:    FadviseSequential,
-	Speculative: FadviseSequential,
+// Set the informed and speculative readahead modes.
+func (rc *ReadaheadConfig) Set(informed, speculative ReadaheadMode) {
+	rc.value.Store(uint32(speculative)<<8 | uint32(informed))
+}
+
+// Informed returns the type of read-ahead for operations that are known to read
+// a large consecutive chunk of a file.
+func (rc *ReadaheadConfig) Informed() ReadaheadMode {
+	return ReadaheadMode(rc.value.Load() & 0xff)
+}
+
+// Speculative returns the type of read-ahead used automatically, when
+// consecutive reads are detected.
+func (rc *ReadaheadConfig) Speculative() ReadaheadMode {
+	return ReadaheadMode(rc.value.Load() >> 8)
 }
 
 // ReadaheadMode indicates the type of read-ahead to use, either for informed
@@ -208,6 +235,10 @@ func open(settings Settings) (p *provider, _ error) {
 		}
 	}()
 
+	if settings.Local.ReadaheadConfig == nil {
+		settings.Local.ReadaheadConfig = NewReadaheadConfig()
+	}
+
 	p = &provider{
 		st:    settings,
 		fsDir: fsDir,
diff --git a/objstorage/objstorageprovider/remote_readable.go b/objstorage/objstorageprovider/remote_readable.go
index 1780ba51..cfa79c4d 100644
--- a/objstorage/objstorageprovider/remote_readable.go
+++ b/objstorage/objstorageprovider/remote_readable.go
@@ -93,7 +93,7 @@ func (r *remoteReadable) NewReadHandle(
 	ctx context.Context, readBeforeSize objstorage.ReadBeforeSize,
 ) objstorage.ReadHandle {
 	rh := remoteReadHandlePool.Get().(*remoteReadHandle)
-	*rh = remoteReadHandle{readable: r, readBeforeSize: readBeforeSize}
+	*rh = remoteReadHandle{readable: r, readBeforeSize: readBeforeSize, buffered: rh.buffered}
 	rh.readAheadState = makeReadaheadState(remoteMaxReadaheadSize)
 	return rh
 }
@@ -250,7 +250,9 @@ func (r *remoteReadHandle) readToBuffer(ctx context.Context, offset int64, lengt
 
 // Close is part of the objstorage.ReadHandle interface.
 func (r *remoteReadHandle) Close() error {
+	buf := r.buffered.data[:0]
 	*r = remoteReadHandle{}
+	r.buffered.data = buf
 	remoteReadHandlePool.Put(r)
 	return nil
 }
diff --git a/objstorage/objstorageprovider/vfs.go b/objstorage/objstorageprovider/vfs.go
index 2ce4175c..b3d2326f 100644
--- a/objstorage/objstorageprovider/vfs.go
+++ b/objstorage/objstorageprovider/vfs.go
@@ -31,11 +31,7 @@ func (p *provider) vfsOpenForReading(
 		}
 		return nil, err
 	}
-	readaheadConfig := DefaultReadaheadConfig
-	if f := p.st.Local.ReadaheadConfigFn; f != nil {
-		readaheadConfig = f()
-	}
-	return newFileReadable(file, p.st.FS, readaheadConfig, filename)
+	return newFileReadable(file, p.st.FS, p.st.Local.ReadaheadConfig, filename)
 }
 
 func (p *provider) vfsCreate(
diff --git a/objstorage/objstorageprovider/vfs_readable.go b/objstorage/objstorageprovider/vfs_readable.go
index ddc0786e..f9c902b3 100644
--- a/objstorage/objstorageprovider/vfs_readable.go
+++ b/objstorage/objstorageprovider/vfs_readable.go
@@ -25,17 +25,18 @@ type fileReadable struct {
 	file vfs.File
 	size int64
 
+	readaheadConfig *ReadaheadConfig
+
 	// The following fields are used to possibly open the file again using the
 	// sequential reads option (see vfsReadHandle).
-	filename        string
-	fs              vfs.FS
-	readaheadConfig ReadaheadConfig
+	filename string
+	fs       vfs.FS
 }
 
 var _ objstorage.Readable = (*fileReadable)(nil)
 
 func newFileReadable(
-	file vfs.File, fs vfs.FS, readaheadConfig ReadaheadConfig, filename string,
+	file vfs.File, fs vfs.FS, readaheadConfig *ReadaheadConfig, filename string,
 ) (*fileReadable, error) {
 	info, err := file.Stat()
 	if err != nil {
@@ -118,7 +119,7 @@ func (rh *vfsReadHandle) init(r *fileReadable) {
 	*rh = vfsReadHandle{
 		r:             r,
 		rs:            makeReadaheadState(fileMaxReadaheadSize),
-		readaheadMode: r.readaheadConfig.Speculative,
+		readaheadMode: r.readaheadConfig.Speculative(),
 	}
 }
 
@@ -163,7 +164,7 @@ func (rh *vfsReadHandle) ReadAt(_ context.Context, p []byte, offset int64) error
 
 // SetupForCompaction is part of the objstorage.ReadHandle interface.
 func (rh *vfsReadHandle) SetupForCompaction() {
-	rh.readaheadMode = rh.r.readaheadConfig.Informed
+	rh.readaheadMode = rh.r.readaheadConfig.Informed()
 	if rh.readaheadMode == FadviseSequential {
 		rh.switchToOSReadahead()
 	}
diff --git a/open.go b/open.go
index cb986299..cebfb235 100644
--- a/open.go
+++ b/open.go
@@ -297,7 +297,7 @@ func Open(dirname string, opts *Options) (db *DB, err error) {
 		NoSyncOnClose:       opts.NoSyncOnClose,
 		BytesPerSync:        opts.BytesPerSync,
 	}
-	providerSettings.Local.ReadaheadConfigFn = opts.Local.ReadaheadConfigFn
+	providerSettings.Local.ReadaheadConfig = opts.Local.ReadaheadConfig
 	providerSettings.Remote.StorageFactory = opts.Experimental.RemoteStorage
 	providerSettings.Remote.CreateOnShared = opts.Experimental.CreateOnShared
 	providerSettings.Remote.CreateOnSharedLocator = opts.Experimental.CreateOnSharedLocator
diff --git a/options.go b/options.go
index c2e1140d..39643cca 100644
--- a/options.go
+++ b/options.go
@@ -497,9 +497,9 @@ type Options struct {
 
 	// Local contains option that pertain to files stored on the local filesystem.
 	Local struct {
-		// ReadaheadConfigFn is a function used to retrieve the current readahead
-		// mode. This function is consulted when a table enters the table cache.
-		ReadaheadConfigFn func() ReadaheadConfig
+		// ReadaheadConfig is used to retrieve the current readahead mode; it is
+		// consulted whenever a read handle is initialized.
+		ReadaheadConfig *ReadaheadConfig
 
 		// TODO(radu): move BytesPerSync, LoadBlockSema, Cleaner here.
 	}
diff --git a/sstable/block_fragment_iter.go b/sstable/block_fragment_iter.go
index 42106367..91948581 100644
--- a/sstable/block_fragment_iter.go
+++ b/sstable/block_fragment_iter.go
@@ -36,13 +36,14 @@ type fragmentBlockIter struct {
 	keyBuf    [2]keyspan.Key
 	span      keyspan.Span
 	dir       int8
-	closeHook func(i keyspan.FragmentIterator) error
 
 	// elideSameSeqnum, if true, returns only the first-occurring (in forward
 	// order) Key for each sequence number.
 	elideSameSeqnum bool
 }
 
+var _ keyspan.FragmentIterator = (*fragmentBlockIter)(nil)
+
 func (i *fragmentBlockIter) Init(elideSameSeqnum bool) {
 	// Use the i.keyBuf array to back the Keys slice to prevent an allocation
 	// when the spans contain few keys.
@@ -184,13 +185,8 @@ func (i *fragmentBlockIter) gatherBackward(kv *base.InternalKV) (*keyspan.Span,
 }
 
 // Close implements (keyspan.FragmentIterator).Close.
-func (i *fragmentBlockIter) Close() error {
-	var err error
-	if i.closeHook != nil {
-		err = i.closeHook(i)
-	}
-	err = firstError(err, i.blockIter.Close())
-	return err
+func (i *fragmentBlockIter) Close() {
+	i.blockIter.Close()
 }
 
 // First implements (keyspan.FragmentIterator).First
@@ -301,10 +297,5 @@ func (i *fragmentBlockIter) String() string {
 	return "fragment-block-iter"
 }
 
-// SetCloseHook implements sstable.FragmentIterator.
-func (i *fragmentBlockIter) SetCloseHook(fn func(i keyspan.FragmentIterator) error) {
-	i.closeHook = fn
-}
-
 // WrapChildren implements FragmentIterator.
 func (i *fragmentBlockIter) WrapChildren(wrap keyspan.WrapFn) {}
diff --git a/sstable/reader.go b/sstable/reader.go
index bbb6792c..fb711ce8 100644
--- a/sstable/reader.go
+++ b/sstable/reader.go
@@ -496,11 +496,10 @@ type rangeKeyFragmentBlockIter struct {
 	fragmentBlockIter
 }
 
-func (i *rangeKeyFragmentBlockIter) Close() error {
-	err := i.fragmentBlockIter.Close()
+func (i *rangeKeyFragmentBlockIter) Close() {
+	i.fragmentBlockIter.Close()
 	i.fragmentBlockIter.ResetForReuse()
 	rangeKeyFragmentBlockIterPool.Put(i)
-	return err
 }
 
 func (r *Reader) readIndex(
diff --git a/table_cache.go b/table_cache.go
index a8f9154b..60b64bd4 100644
--- a/table_cache.go
+++ b/table_cache.go
@@ -1272,10 +1272,10 @@ func (s *iterSet) CloseAll() error {
 		err = s.point.Close()
 	}
 	if s.rangeDeletion != nil {
-		err = firstError(err, s.rangeDeletion.Close())
+		s.rangeDeletion.Close()
 	}
 	if s.rangeKey != nil {
-		err = firstError(err, s.rangeKey.Close())
+		s.rangeKey.Close()
 	}
 	return err
 }


Test patch:
diff --git a/flushable_test.go b/flushable_test.go
index 50b7420d..82b4420d 100644
--- a/flushable_test.go
+++ b/flushable_test.go
@@ -138,7 +138,7 @@ func TestIngestedSSTFlushableAPI(t *testing.T) {
 					buf.WriteString(span.String())
 					buf.WriteString("\n")
 				}
-				err = firstError(err, iter.Close())
+				iter.Close()
 				if err != nil {
 					fmt.Fprintf(&buf, "err=%q", err.Error())
 				}
@@ -153,7 +153,7 @@ func TestIngestedSSTFlushableAPI(t *testing.T) {
 					buf.WriteString(span.String())
 					buf.WriteString("\n")
 				}
-				err = firstError(err, iter.Close())
+				iter.Close()
 				if err != nil {
 					fmt.Fprintf(&buf, "err=%q", err.Error())
 				}
diff --git a/internal/keyspan/keyspanimpl/level_iter_test.go b/internal/keyspan/keyspanimpl/level_iter_test.go
index e852f188..60a4f0ce 100644
--- a/internal/keyspan/keyspanimpl/level_iter_test.go
+++ b/internal/keyspan/keyspanimpl/level_iter_test.go
@@ -428,7 +428,7 @@ func TestLevelIter(t *testing.T) {
 		case "num-files":
 			return fmt.Sprintf("%d", len(level))
 		case "close-iter":
-			_ = iter.Close()
+			iter.Close()
 			iter = nil
 			return "ok"
 		case "iter":
diff --git a/internal/keyspan/logging_iter_test.go b/internal/keyspan/logging_iter_test.go
index 630de9c3..162a68b4 100644
--- a/internal/keyspan/logging_iter_test.go
+++ b/internal/keyspan/logging_iter_test.go
@@ -12,7 +12,6 @@ import (
 
 	"github.com/cockroachdb/datadriven"
 	"github.com/cockroachdb/pebble/internal/base"
-	"github.com/stretchr/testify/require"
 )
 
 func TestLoggingIter(t *testing.T) {
@@ -34,7 +33,7 @@ func TestLoggingIter(t *testing.T) {
 			iter = Assert(iter, base.DefaultComparer.Compare)
 			iter = InjectLogging(iter, l)
 			RunFragmentIteratorCmd(iter, d.Input, nil)
-			require.NoError(t, iter.Close())
+			iter.Close()
 			out := l.String()
 			// Hide pointer values.
 			r := regexp.MustCompile(`\(0x[0-9a-f]+\)`)
diff --git a/internal/keyspan/test_utils.go b/internal/keyspan/test_utils.go
index 7a1a599c..e9c1ef93 100644
--- a/internal/keyspan/test_utils.go
+++ b/internal/keyspan/test_utils.go
@@ -364,13 +364,12 @@ func (p *probeIterator) Prev() (*Span, error) {
 	return p.handleOp(op)
 }
 
-func (p *probeIterator) Close() error {
+func (p *probeIterator) Close() {
 	op := op{Kind: OpClose}
 	if p.iter != nil {
-		op.Err = p.iter.Close()
+		p.iter.Close()
 	}
-	_, err := p.handleOp(op)
-	return err
+	_, _ = p.handleOp(op)
 }
 
 func (p *probeIterator) WrapChildren(wrap WrapFn) {
@@ -551,7 +550,7 @@ func (i *invalidatingIter) First() (*Span, error)            { return i.invalida
 func (i *invalidatingIter) Last() (*Span, error)             { return i.invalidate(i.iter.Last()) }
 func (i *invalidatingIter) Next() (*Span, error)             { return i.invalidate(i.iter.Next()) }
 func (i *invalidatingIter) Prev() (*Span, error)             { return i.invalidate(i.iter.Prev()) }
-func (i *invalidatingIter) Close() error                     { return i.iter.Close() }
+func (i *invalidatingIter) Close()                           { i.iter.Close() }
 func (i *invalidatingIter) WrapChildren(wrap WrapFn) {
 	i.iter = wrap(i.iter)
 }
diff --git a/keyspan_probe_test.go b/keyspan_probe_test.go
index 7ce39009..dbe9fb5b 100644
--- a/keyspan_probe_test.go
+++ b/keyspan_probe_test.go
@@ -368,11 +368,10 @@ func (p *probeKeyspanIterator) WrapChildren(wrap keyspan.WrapFn) {
 	p.iter = wrap(p.iter)
 }
 
-func (p *probeKeyspanIterator) Close() error {
+func (p *probeKeyspanIterator) Close() {
 	op := keyspanOp{Kind: opSpanClose}
 	if p.iter != nil {
-		op.Err = p.iter.Close()
+		p.iter.Close()
 	}
-	_, err := p.handleOp(op)
-	return err
+	_, _ = p.handleOp(op)
 }
diff --git a/objstorage/objstorageprovider/provider_test.go b/objstorage/objstorageprovider/provider_test.go
index 1e795791..a8440e5f 100644
--- a/objstorage/objstorageprovider/provider_test.go
+++ b/objstorage/objstorageprovider/provider_test.go
@@ -40,9 +40,9 @@ func TestProvider(t *testing.T) {
 		backings := make(map[string]objstorage.RemoteObjectBacking)
 		backingHandles := make(map[string]objstorage.RemoteObjectBackingHandle)
 		var curProvider objstorage.Provider
-		readaheadConfig := DefaultReadaheadConfig
+		readaheadConfig := NewReadaheadConfig()
 		datadriven.RunTest(t, path, func(t *testing.T, d *datadriven.TestData) string {
-			readaheadConfig = DefaultReadaheadConfig
+			readaheadConfig.Set(defaultReadaheadInformed, defaultReadaheadSpeculative)
 			scanArgs := func(desc string, args ...interface{}) {
 				t.Helper()
 				if len(d.CmdArgs) != len(args) {
@@ -70,9 +70,7 @@ func TestProvider(t *testing.T) {
 					st.Remote.CreateOnShared = remote.CreateOnSharedAll
 					st.Remote.CreateOnSharedLocator = ""
 				}
-				st.Local.ReadaheadConfigFn = func() ReadaheadConfig {
-					return readaheadConfig
-				}
+				st.Local.ReadaheadConfig = readaheadConfig
 				require.NoError(t, fs.MkdirAll(fsDir, 0755))
 				p, err := Open(st)
 				require.NoError(t, err)
@@ -189,9 +187,9 @@ func TestProvider(t *testing.T) {
 						d.Fatalf(t, "unknown readahead mode %s", arg.SingleVal(t))
 					}
 					if forCompaction {
-						readaheadConfig.Informed = mode
+						readaheadConfig.Set(mode, defaultReadaheadSpeculative)
 					} else {
-						readaheadConfig.Speculative = mode
+						readaheadConfig.Set(defaultReadaheadInformed, mode)
 					}
 				}
 
diff --git a/scan_internal_test.go b/scan_internal_test.go
index 949cf769..305bb7a2 100644
--- a/scan_internal_test.go
+++ b/scan_internal_test.go
@@ -416,7 +416,7 @@ func TestScanInternal(t *testing.T) {
 						require.NoError(t, w.DeleteRange(span.Start, span.End))
 					}
 					require.NoError(t, err)
-					require.NoError(t, rangeDels.Close())
+					rangeDels.Close()
 				}
 				{
 					span, err := rangeKeys.First()
@@ -432,7 +432,7 @@ func TestScanInternal(t *testing.T) {
 					}
 					require.NoError(t, err)
 				}
-				require.NoError(t, rangeKeys.Close())
+				rangeKeys.Close()
 				for kv := points.First(); kv != nil; kv = points.Next() {
 					t.Logf("writing %s", kv.K)
 					var value []byte