Repository: cockroachdb/pebble
Pull request #3671 (2024-06-19 01:20:49)

keyspan: remove error from FragmentIterator.Close
#### sstable: remove unused closeHook in fragmentBlockIter


#### keyspan: remove error from FragmentIterator.Close

None of the actual `FragmentIterator` implementations generate an
error during Close. This change removes the error return, simplifying
relevant code.

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

RaduBerinde: TFTR!

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/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/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