Repository: cockroachdb/pebble
Pull request #4269 (2025-01-17 18:55:47)

sstable: refactor ReaderOptions
Add block.ReaderOptions and refactor sstable.ReaderOptions to contain them, and the blob reader to also use block.Reader and block.ReaderOptions.

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


Patch:
diff --git a/file_cache.go b/file_cache.go
index 99b5b956..e09bda2c 100644
--- a/file_cache.go
+++ b/file_cache.go
@@ -1181,11 +1181,11 @@ func (v *fileCacheValue) load(
 	)
 	if err == nil {
 		o := dbOpts.readerOpts
-		o.SetInternalCacheOpts(sstableinternal.CacheOptions{
+		o.CacheOpts = sstableinternal.CacheOptions{
 			Cache:   dbOpts.cache,
 			CacheID: dbOpts.cacheID,
 			FileNum: backingFileNum,
-		})
+		}
 		r, err = sstable.NewReader(ctx, f, o)
 	}
 	if err == nil {
diff --git a/ingest.go b/ingest.go
index 3c450ac8..08491994 100644
--- a/ingest.go
+++ b/ingest.go
@@ -305,11 +305,11 @@ func ingestLoad1(
 	rangeKeyValidator rangeKeyIngestValidator,
 ) (meta *fileMetadata, lastRangeKey keyspan.Span, err error) {
 	o := opts.MakeReaderOptions()
-	o.SetInternalCacheOpts(sstableinternal.CacheOptions{
+	o.CacheOpts = sstableinternal.CacheOptions{
 		Cache:   opts.Cache,
 		CacheID: cacheID,
 		FileNum: base.PhysicalTableDiskFileNum(fileNum),
-	})
+	}
 	r, err := sstable.NewReader(ctx, readable, o)
 	if err != nil {
 		return nil, keyspan.Span{}, err
diff --git a/sstable/blob/blob.go b/sstable/blob/blob.go
index bcd1192b..fec88b2f 100644
--- a/sstable/blob/blob.go
+++ b/sstable/blob/blob.go
@@ -12,6 +12,7 @@ import (
 	"github.com/cockroachdb/errors"
 	"github.com/cockroachdb/pebble/internal/base"
 	"github.com/cockroachdb/pebble/objstorage"
+	"github.com/cockroachdb/pebble/objstorage/objstorageprovider"
 	"github.com/cockroachdb/pebble/sstable/block"
 	"github.com/cockroachdb/pebble/sstable/valblk"
 	"github.com/cockroachdb/redact"
@@ -323,23 +324,60 @@ func (f *fileFooter) encode(b []byte) {
 
 // FileReader reads a blob file.
 type FileReader struct {
-	r      objstorage.Readable
+	r      block.Reader
 	footer fileFooter
 }
 
+// FileReaderOptions configures a reader of a blob file.
+type FileReaderOptions struct {
+	block.ReaderOptions
+}
+
+func (o FileReaderOptions) ensureDefaults() FileReaderOptions {
+	if o.LoggerAndTracer == nil {
+		o.LoggerAndTracer = base.NoopLoggerAndTracer{}
+	}
+	return o
+}
+
 // NewFileReader opens a blob file for reading.
-func NewFileReader(ctx context.Context, r objstorage.Readable) (*FileReader, error) {
+func NewFileReader(
+	ctx context.Context, r objstorage.Readable, ro FileReaderOptions,
+) (*FileReader, error) {
+	ro = ro.ensureDefaults()
+
+	fileNum := ro.CacheOpts.FileNum
+
 	var footerBuf [fileFooterLength]byte
-	if err := r.ReadAt(ctx, footerBuf[:], r.Size()-fileFooterLength); err != nil {
+	size := r.Size()
+	off := size - fileFooterLength
+	if size < fileFooterLength {
+		return nil, base.CorruptionErrorf("pebble: invalid blob file %s (file size is too small)",
+			errors.Safe(fileNum))
+	}
+	var preallocRH objstorageprovider.PreallocatedReadHandle
+	rh := objstorageprovider.UsePreallocatedReadHandle(
+		r, objstorage.ReadBeforeForNewReader, &preallocRH)
+	defer rh.Close()
+
+	encodedFooter, err := block.ReadRaw(ctx, r, rh, ro.LoggerAndTracer, fileNum, footerBuf[:], off)
+	if err != nil {
 		return nil, err
 	}
-	fr := &FileReader{r: r}
-	if err := fr.footer.decode(footerBuf[:]); err != nil {
+
+	fr := &FileReader{}
+	if err := fr.footer.decode(encodedFooter); err != nil {
 		return nil, err
 	}
+	fr.r.Init(r, ro.ReaderOptions, fr.footer.checksum)
 	return fr, nil
 }
 
+// Close implements io.Closer, closing the underlying Readable.
+func (r *FileReader) Close() error {
+	return r.r.Close()
+}
+
 // lenLittleEndian returns the minimum number of bytes needed to encode v
 // using little endian encoding.
 func lenLittleEndian(v uint64) int {
diff --git a/sstable/block/block.go b/sstable/block/block.go
index 52df4268..06993e45 100644
--- a/sstable/block/block.go
+++ b/sstable/block/block.go
@@ -355,39 +355,42 @@ func (env *ReadEnv) BlockRead(blockLength uint64, readDuration time.Duration) {
 // A Reader reads blocks from a single file, handling caching, checksum
 // validation and decompression.
 type Reader struct {
-	readable      objstorage.Readable
-	cacheOpts     sstableinternal.CacheOptions
-	loadBlockSema *fifo.Semaphore
-	logger        base.LoggerAndTracer
-	checksumType  ChecksumType
+	readable     objstorage.Readable
+	opts         ReaderOptions
+	checksumType ChecksumType
+}
+
+// ReaderOptions configures a block reader.
+type ReaderOptions struct {
+	// CacheOpts contains the information needed to interact with the block
+	// cache.
+	CacheOpts sstableinternal.CacheOptions
+	// LoadBlockSema, if set, is used to limit the number of blocks that can be
+	// loaded (i.e. read from the filesystem) in parallel. Each load acquires
+	// one unit from the semaphore for the duration of the read.
+	LoadBlockSema *fifo.Semaphore
+	// LoggerAndTracer is an optional logger and tracer.
+	LoggerAndTracer base.LoggerAndTracer
 }
 
 // Init initializes the Reader to read blocks from the provided Readable.
-func (r *Reader) Init(
-	readable objstorage.Readable,
-	cacheOpts sstableinternal.CacheOptions,
-	sema *fifo.Semaphore,
-	logger base.LoggerAndTracer,
-	checksumType ChecksumType,
-) {
+func (r *Reader) Init(readable objstorage.Readable, ro ReaderOptions, checksumType ChecksumType) {
 	r.readable = readable
-	r.cacheOpts = cacheOpts
-	r.loadBlockSema = sema
-	r.logger = logger
+	r.opts = ro
 	r.checksumType = checksumType
-	if r.cacheOpts.Cache == nil {
-		r.cacheOpts.Cache = cache.New(0)
+	if r.opts.CacheOpts.Cache == nil {
+		r.opts.CacheOpts.Cache = cache.New(0)
 	} else {
-		r.cacheOpts.Cache.Ref()
+		r.opts.CacheOpts.Cache.Ref()
 	}
-	if r.cacheOpts.CacheID == 0 {
-		r.cacheOpts.CacheID = r.cacheOpts.Cache.NewID()
+	if r.opts.CacheOpts.CacheID == 0 {
+		r.opts.CacheOpts.CacheID = r.opts.CacheOpts.Cache.NewID()
 	}
 }
 
 // FileNum returns the file number of the file being read.
 func (r *Reader) FileNum() base.DiskFileNum {
-	return r.cacheOpts.FileNum
+	return r.opts.CacheOpts.FileNum
 }
 
 // ChecksumType returns the checksum type used by the reader.
@@ -410,10 +413,10 @@ func (r *Reader) Read(
 	if env.BufferPool == nil {
 		var errorDuration time.Duration
 		var err error
-		cv, crh, errorDuration, hit, err = r.cacheOpts.Cache.GetWithReadHandle(
-			ctx, r.cacheOpts.CacheID, r.cacheOpts.FileNum, bh.Offset)
-		if errorDuration > 5*time.Millisecond && r.logger.IsTracingEnabled(ctx) {
-			r.logger.Eventf(
+		cv, crh, errorDuration, hit, err = r.opts.CacheOpts.Cache.GetWithReadHandle(
+			ctx, r.opts.CacheOpts.CacheID, r.opts.CacheOpts.FileNum, bh.Offset)
+		if errorDuration > 5*time.Millisecond && r.opts.LoggerAndTracer.IsTracingEnabled(ctx) {
+			r.opts.LoggerAndTracer.Eventf(
 				ctx, "waited for turn when %s time wasted by failed reads", errorDuration.String())
 		}
 		// TODO(sumeer): consider tracing when waited longer than some duration
@@ -425,7 +428,7 @@ func (r *Reader) Read(
 		// The compaction path uses env.BufferPool, and does not coordinate read
 		// using a cache.ReadHandle. This is ok since only a single compaction is
 		// reading a block.
-		cv = r.cacheOpts.Cache.Get(r.cacheOpts.CacheID, r.cacheOpts.FileNum, bh.Offset)
+		cv = r.opts.CacheOpts.Cache.Get(r.opts.CacheOpts.CacheID, r.opts.CacheOpts.FileNum, bh.Offset)
 		if cv != nil {
 			hit = true
 		}
@@ -446,7 +449,7 @@ func (r *Reader) Read(
 	}
 
 	// Need to read. First acquire loadBlockSema, if needed.
-	if sema := r.loadBlockSema; sema != nil {
+	if sema := r.opts.LoadBlockSema; sema != nil {
 		if err := sema.Acquire(ctx, 1); err != nil {
 			// An error here can only come from the context.
 			return BufferHandle{}, err
@@ -460,7 +463,7 @@ func (r *Reader) Read(
 		}
 		return BufferHandle{}, err
 	}
-	h := value.MakeHandle(crh, r.cacheOpts.CacheID, r.cacheOpts.FileNum, bh.Offset)
+	h := value.MakeHandle(crh, r.opts.CacheOpts.CacheID, r.opts.CacheOpts.FileNum, bh.Offset)
 	return h, nil
 }
 
@@ -487,12 +490,12 @@ func (r *Reader) doRead(
 	readDuration := readStopwatch.stop()
 	// Call IsTracingEnabled to avoid the allocations of boxing integers into an
 	// interface{}, unless necessary.
-	if readDuration >= slowReadTracingThreshold && r.logger.IsTracingEnabled(ctx) {
+	if readDuration >= slowReadTracingThreshold && r.opts.LoggerAndTracer.IsTracingEnabled(ctx) {
 		_, file1, line1, _ := runtime.Caller(1)
 		_, file2, line2, _ := runtime.Caller(2)
-		r.logger.Eventf(ctx, "reading block of %d bytes took %s (fileNum=%s; %s/%s:%d -> %s/%s:%d)",
+		r.opts.LoggerAndTracer.Eventf(ctx, "reading block of %d bytes took %s (fileNum=%s; %s/%s:%d -> %s/%s:%d)",
 			int(bh.Length+TrailerLen), readDuration.String(),
-			r.cacheOpts.FileNum,
+			r.opts.CacheOpts.FileNum,
 			filepath.Base(filepath.Dir(file2)), filepath.Base(file2), line2,
 			filepath.Base(filepath.Dir(file1)), filepath.Base(file1), line1)
 	}
@@ -503,7 +506,7 @@ func (r *Reader) doRead(
 	env.BlockRead(bh.Length, readDuration)
 	if err = ValidateChecksum(r.checksumType, compressed.BlockData(), bh); err != nil {
 		compressed.Release()
-		err = errors.Wrapf(err, "pebble/table: table %s", r.cacheOpts.FileNum)
+		err = errors.Wrapf(err, "pebble/table: table %s", r.opts.CacheOpts.FileNum)
 		return Value{}, err
 	}
 	typ := CompressionIndicator(compressed.BlockData()[bh.Length])
@@ -545,7 +548,7 @@ func (r *Reader) Readable() objstorage.Readable {
 // Users should prefer using Read, which handles reading from object storage on
 // a cache miss.
 func (r *Reader) GetFromCache(bh Handle) *cache.Value {
-	return r.cacheOpts.Cache.Get(r.cacheOpts.CacheID, r.cacheOpts.FileNum, bh.Offset)
+	return r.opts.CacheOpts.Cache.Get(r.opts.CacheOpts.CacheID, r.opts.CacheOpts.FileNum, bh.Offset)
 }
 
 // UsePreallocatedReadHandle returns a ReadHandle that reads from the reader and
@@ -559,7 +562,7 @@ func (r *Reader) UsePreallocatedReadHandle(
 
 // Close releases resources associated with the Reader.
 func (r *Reader) Close() error {
-	r.cacheOpts.Cache.Unref()
+	r.opts.CacheOpts.Cache.Unref()
 	var err error
 	if r.readable != nil {
 		err = r.readable.Close()
diff --git a/sstable/options.go b/sstable/options.go
index 3d3ac9a8..b4f70bbf 100644
--- a/sstable/options.go
+++ b/sstable/options.go
@@ -7,7 +7,6 @@ package sstable
 import (
 	"fmt"
 
-	"github.com/cockroachdb/crlib/fifo"
 	"github.com/cockroachdb/pebble/internal/base"
 	"github.com/cockroachdb/pebble/internal/sstableinternal"
 	"github.com/cockroachdb/pebble/sstable/block"
@@ -81,10 +80,7 @@ func MakeKeySchemas(keySchemas ...*colblk.KeySchema) KeySchemas {
 
 // ReaderOptions holds the parameters needed for reading an sstable.
 type ReaderOptions struct {
-	// LoadBlockSema, if set, is used to limit the number of blocks that can be
-	// loaded (i.e. read from the filesystem) in parallel. Each load acquires one
-	// unit from the semaphore for the duration of the read.
-	LoadBlockSema *fifo.Semaphore
+	block.ReaderOptions
 
 	// User properties specified in this map will not be added to sst.Properties.UserProperties.
 	DeniedUserProperties map[string]struct{}
@@ -110,28 +106,8 @@ type ReaderOptions struct {
 	// policies that are not in this map will be ignored.
 	Filters map[string]FilterPolicy
 
-	// Logger is an optional logger and tracer.
-	LoggerAndTracer base.LoggerAndTracer
-
 	// FilterMetricsTracker is optionally used to track filter metrics.
 	FilterMetricsTracker *FilterMetricsTracker
-
-	// internal options can only be used from within the pebble package.
-	internal sstableinternal.ReaderOptions
-}
-
-// SetInternal sets the internal reader options. Note that even though this
-// method is public, a caller outside the pebble package can't construct a value
-// to pass to it.
-func (o *ReaderOptions) SetInternal(internalOpts sstableinternal.ReaderOptions) {
-	o.internal = internalOpts
-}
-
-// SetInternalCacheOpts sets the internal cache options. Note that even though
-// this method is public, a caller outside the pebble package can't construct a
-// value to pass to it.
-func (o *ReaderOptions) SetInternalCacheOpts(cacheOpts sstableinternal.CacheOptions) {
-	o.internal.CacheOpts = cacheOpts
 }
 
 func (o ReaderOptions) ensureDefaults() ReaderOptions {
diff --git a/sstable/reader.go b/sstable/reader.go
index 1c06e046..028b2598 100644
--- a/sstable/reader.go
+++ b/sstable/reader.go
@@ -815,11 +815,11 @@ func NewReader(ctx context.Context, f objstorage.Readable, o ReaderOptions) (*Re
 		f, objstorage.ReadBeforeForNewReader, &preallocRH)
 	defer rh.Close()
 
-	footer, err := readFooter(ctx, f, rh, o.LoggerAndTracer, o.internal.CacheOpts.FileNum)
+	footer, err := readFooter(ctx, f, rh, o.LoggerAndTracer, o.CacheOpts.FileNum)
 	if err != nil {
 		return nil, errors.CombineErrors(err, f.Close())
 	}
-	r.blockReader.Init(f, o.internal.CacheOpts, o.LoadBlockSema, o.LoggerAndTracer, footer.checksum)
+	r.blockReader.Init(f, o.ReaderOptions, footer.checksum)
 	r.tableFormat = footer.format
 	r.indexBH = footer.indexBH
 	r.metaindexBH = footer.metaindexBH
diff --git a/tool/db.go b/tool/db.go
index ee234817..b7664435 100644
--- a/tool/db.go
+++ b/tool/db.go
@@ -962,11 +962,9 @@ func (d *dbT) addProps(
 	opts := d.opts.MakeReaderOptions()
 	opts.Mergers = d.mergers
 	opts.Comparers = d.comparers
-	opts.SetInternal(sstableinternal.ReaderOptions{
-		CacheOpts: sstableinternal.CacheOptions{
-			FileNum: m.FileBacking.DiskFileNum,
-		},
-	})
+	opts.ReaderOptions.CacheOpts = sstableinternal.CacheOptions{
+		FileNum: m.FileBacking.DiskFileNum,
+	}
 	r, err := sstable.NewReader(ctx, f, opts)
 	if err != nil {
 		_ = f.Close()
diff --git a/tool/find.go b/tool/find.go
index 3db24292..85574471 100644
--- a/tool/find.go
+++ b/tool/find.go
@@ -442,12 +442,10 @@ func (f *findT) searchTables(stdout io.Writer, searchKey []byte, refs []findRef)
 			opts := f.opts.MakeReaderOptions()
 			opts.Comparers = f.comparers
 			opts.Mergers = f.mergers
-			opts.SetInternal(sstableinternal.ReaderOptions{
-				CacheOpts: sstableinternal.CacheOptions{
-					Cache:   cache,
-					FileNum: fl.DiskFileNum,
-				},
-			})
+			opts.CacheOpts = sstableinternal.CacheOptions{
+				Cache:   cache,
+				FileNum: fl.DiskFileNum,
+			}
 			readable, err := sstable.NewSimpleReadable(tf)
 			if err != nil {
 				return err
diff --git a/tool/sstable.go b/tool/sstable.go
index 76b09f72..fa99824e 100644
--- a/tool/sstable.go
+++ b/tool/sstable.go
@@ -153,11 +153,7 @@ func (s *sstableT) newReader(f vfs.File) (*sstable.Reader, error) {
 	o.Mergers = s.mergers
 	c := pebble.NewCache(128 << 20 /* 128 MB */)
 	defer c.Unref()
-	o.SetInternal(sstableinternal.ReaderOptions{
-		CacheOpts: sstableinternal.CacheOptions{
-			Cache: c,
-		},
-	})
+	o.CacheOpts = sstableinternal.CacheOptions{Cache: c}
 	return sstable.NewReader(context.Background(), readable, o)
 }
 


Test patch:
diff --git a/cockroachkvs/cockroachkvs_bench_test.go b/cockroachkvs/cockroachkvs_bench_test.go
index e7099c16..e3fdb4f3 100644
--- a/cockroachkvs/cockroachkvs_bench_test.go
+++ b/cockroachkvs/cockroachkvs_bench_test.go
@@ -100,14 +100,16 @@ func benchmarkRandSeekInSST(
 	defer c.Unref()
 	ctx := context.Background()
 	readerOpts := sstable.ReaderOptions{
+		ReaderOptions: block.ReaderOptions{
+			CacheOpts: sstableinternal.CacheOptions{
+				Cache:   c,
+				CacheID: c.NewID(),
+				FileNum: 1,
+			},
+		},
 		Comparer:   writerOpts.Comparer,
 		KeySchemas: sstable.MakeKeySchemas(&KeySchema),
 	}
-	readerOpts.SetInternalCacheOpts(sstableinternal.CacheOptions{
-		Cache:   c,
-		CacheID: c.NewID(),
-		FileNum: 1,
-	})
 	reader, err := sstable.NewReader(ctx, obj, readerOpts)
 	require.NoError(b, err)
 	defer reader.Close()
diff --git a/data_test.go b/data_test.go
index c43aff8e..7f179602 100644
--- a/data_test.go
+++ b/data_test.go
@@ -1177,11 +1177,11 @@ func runSSTablePropertiesCmd(t *testing.T, td *datadriven.TestData, d *DB) strin
 	// TODO(bananabrick): cacheOpts is used to set the file number on a Reader,
 	// and virtual sstables expect this file number to be set. Split out the
 	// opts into fileNum opts, and cache opts.
-	readerOpts.SetInternalCacheOpts(sstableinternal.CacheOptions{
+	readerOpts.CacheOpts = sstableinternal.CacheOptions{
 		Cache:   d.opts.Cache,
 		CacheID: 0,
 		FileNum: backingFileNum,
-	})
+	}
 	r, err := sstable.NewReader(context.Background(), readable, readerOpts)
 	if err != nil {
 		return err.Error()
diff --git a/level_checker_test.go b/level_checker_test.go
index 73927fad..6961c7a9 100644
--- a/level_checker_test.go
+++ b/level_checker_test.go
@@ -241,7 +241,7 @@ func TestCheckLevelsCornerCases(t *testing.T) {
 					Comparer:   testkeys.Comparer,
 					KeySchemas: sstable.KeySchemas{writerOpts.KeySchema.Name: writerOpts.KeySchema},
 				}
-				readerOpts.SetInternalCacheOpts(sstableinternal.CacheOptions{FileNum: base.DiskFileNum(fileNum - 1)})
+				readerOpts.CacheOpts = sstableinternal.CacheOptions{FileNum: base.DiskFileNum(fileNum - 1)}
 				r, err := sstable.NewReader(context.Background(), readable, readerOpts)
 				if err != nil {
 					return err.Error()
diff --git a/level_iter_test.go b/level_iter_test.go
index 6a728214..0d3f1d76 100644
--- a/level_iter_test.go
+++ b/level_iter_test.go
@@ -543,9 +543,7 @@ func buildLevelIterTables(
 	c := NewCache(128 << 20 /* 128MB */)
 	defer c.Unref()
 	opts := sstable.ReaderOptions{Comparer: DefaultComparer}
-	opts.SetInternalCacheOpts(sstableinternal.CacheOptions{
-		Cache: c,
-	})
+	opts.CacheOpts = sstableinternal.CacheOptions{Cache: c}
 	readers := make([]*sstable.Reader, len(files))
 	for i := range files {
 		f, err := mem.Open(fmt.Sprintf("bench%d", i))
diff --git a/merging_iter_test.go b/merging_iter_test.go
index b287f6ec..afd766af 100644
--- a/merging_iter_test.go
+++ b/merging_iter_test.go
@@ -372,9 +372,7 @@ func buildMergingIterTables(
 	defer c.Unref()
 
 	var opts sstable.ReaderOptions
-	opts.SetInternalCacheOpts(sstableinternal.CacheOptions{
-		Cache: c,
-	})
+	opts.CacheOpts = sstableinternal.CacheOptions{Cache: c}
 
 	readers := make([]*sstable.Reader, len(files))
 	for i := range files {
@@ -617,9 +615,7 @@ func buildLevelsForMergingIterSeqSeek(
 	defer c.Unref()
 
 	opts := sstable.ReaderOptions{Comparer: DefaultComparer}
-	opts.SetInternalCacheOpts(sstableinternal.CacheOptions{
-		Cache: c,
-	})
+	opts.CacheOpts = sstableinternal.CacheOptions{Cache: c}
 
 	if writeBloomFilters {
 		opts.Filters = make(map[string]FilterPolicy)
diff --git a/sstable/blob/blob_test.go b/sstable/blob/blob_test.go
index c6c4f44a..3003ae47 100644
--- a/sstable/blob/blob_test.go
+++ b/sstable/blob/blob_test.go
@@ -58,10 +58,11 @@ func TestBlobWriter(t *testing.T) {
 			fmt.Fprintf(&buf, "  FileLen: %d\n", stats.FileLen)
 			return buf.String()
 		case "open":
-			r, err := NewFileReader(context.Background(), obj)
+			r, err := NewFileReader(context.Background(), obj, FileReaderOptions{})
 			if err != nil {
 				t.Fatal(err)
 			}
+			defer r.Close()
 			fmt.Fprintf(&buf, "TableFormat: %s\n", r.footer.format)
 			fmt.Fprintf(&buf, "ChecksumType: %s\n", r.footer.checksum)
 			fmt.Fprintf(&buf, "IndexHandle: %s\n", r.footer.indexHandle.String())
diff --git a/sstable/copier_test.go b/sstable/copier_test.go
index f5b7ce37..628470d5 100644
--- a/sstable/copier_test.go
+++ b/sstable/copier_test.go
@@ -14,6 +14,7 @@ import (
 	"github.com/cockroachdb/datadriven"
 	"github.com/cockroachdb/pebble/internal/base"
 	"github.com/cockroachdb/pebble/internal/cache"
+	"github.com/cockroachdb/pebble/internal/sstableinternal"
 	"github.com/cockroachdb/pebble/internal/testkeys"
 	"github.com/cockroachdb/pebble/objstorage/objstorageprovider"
 	"github.com/cockroachdb/pebble/sstable/block"
@@ -107,12 +108,17 @@ func TestCopySpan(t *testing.T) {
 				}
 			}
 			rOpts := ReaderOptions{
+				ReaderOptions: block.ReaderOptions{
+					CacheOpts: sstableinternal.CacheOptions{
+						Cache:   blockCache,
+						CacheID: cacheID,
+						FileNum: base.DiskFileNum(fileNameToNum[d.CmdArgs[0].Key]),
+					},
+				},
 				Comparer:   testkeys.Comparer,
 				KeySchemas: KeySchemas{keySchema.Name: &keySchema},
 			}
-			rOpts.internal.CacheOpts.Cache = blockCache
-			rOpts.internal.CacheOpts.CacheID = cacheID
-			rOpts.internal.CacheOpts.FileNum = base.DiskFileNum(fileNameToNum[d.CmdArgs[0].Key])
+
 			r, err := NewReader(context.TODO(), readable, rOpts)
 			defer r.Close()
 			if err != nil {
@@ -156,12 +162,16 @@ func TestCopySpan(t *testing.T) {
 				return err.Error()
 			}
 			rOpts := ReaderOptions{
+				ReaderOptions: block.ReaderOptions{
+					CacheOpts: sstableinternal.CacheOptions{
+						Cache:   blockCache,
+						CacheID: cacheID,
+						FileNum: base.DiskFileNum(fileNameToNum[d.CmdArgs[0].Key]),
+					},
+				},
 				Comparer:   testkeys.Comparer,
 				KeySchemas: KeySchemas{keySchema.Name: &keySchema},
 			}
-			rOpts.internal.CacheOpts.Cache = blockCache
-			rOpts.internal.CacheOpts.CacheID = cacheID
-			rOpts.internal.CacheOpts.FileNum = base.DiskFileNum(fileNameToNum[inputFile])
 			r, err := NewReader(context.TODO(), readable, rOpts)
 			if err != nil {
 				return err.Error()
diff --git a/sstable/data_test.go b/sstable/data_test.go
index 5c9addf9..3ef693af 100644
--- a/sstable/data_test.go
+++ b/sstable/data_test.go
@@ -179,9 +179,7 @@ func openReader(obj *objstorage.MemObj, writerOpts *WriterOptions, cacheSize int
 	if cacheSize > 0 {
 		c := cache.New(int64(cacheSize))
 		defer c.Unref()
-		readerOpts.SetInternal(sstableinternal.ReaderOptions{
-			CacheOpts: sstableinternal.CacheOptions{Cache: c},
-		})
+		readerOpts.CacheOpts = sstableinternal.CacheOptions{Cache: c}
 	}
 	r, err := NewMemReader(obj.Data(), readerOpts)
 	if err != nil {
diff --git a/sstable/reader_test.go b/sstable/reader_test.go
index bd156848..b3a7314f 100644
--- a/sstable/reader_test.go
+++ b/sstable/reader_test.go
@@ -1751,7 +1751,7 @@ func buildTestTableWithProvider(
 	c := cache.New(128 << 20)
 	defer c.Unref()
 	r, err := NewReader(context.Background(), f1, ReaderOptions{
-		internal: sstableinternal.ReaderOptions{
+		ReaderOptions: block.ReaderOptions{
 			CacheOpts: sstableinternal.CacheOptions{
 				Cache: c,
 			},
@@ -1794,7 +1794,7 @@ func buildBenchmarkTable(
 	c := cache.New(128 << 20)
 	defer c.Unref()
 	r, err := newReader(f1, ReaderOptions{
-		internal: sstableinternal.ReaderOptions{
+		ReaderOptions: block.ReaderOptions{
 			CacheOpts: sstableinternal.CacheOptions{
 				Cache: c,
 			},
@@ -2081,7 +2081,7 @@ func BenchmarkIteratorScanManyVersions(b *testing.B) {
 		require.NoError(b, err)
 		r, err := newReader(f0, ReaderOptions{
 			Comparer: testkeys.Comparer,
-			internal: sstableinternal.ReaderOptions{
+			ReaderOptions: block.ReaderOptions{
 				CacheOpts: sstableinternal.CacheOptions{
 					Cache: c,
 				},
@@ -2201,7 +2201,7 @@ func BenchmarkIteratorScanNextPrefix(b *testing.B) {
 		require.NoError(b, err)
 		r, err = newReader(f0, ReaderOptions{
 			Comparer: testkeys.Comparer,
-			internal: sstableinternal.ReaderOptions{
+			ReaderOptions: block.ReaderOptions{
 				CacheOpts: sstableinternal.CacheOptions{
 					Cache: c,
 				},
@@ -2363,7 +2363,7 @@ func BenchmarkIteratorScanObsolete(b *testing.B) {
 		require.NoError(b, err)
 		r, err := newReader(f0, ReaderOptions{
 			Comparer: testkeys.Comparer,
-			internal: sstableinternal.ReaderOptions{
+			ReaderOptions: block.ReaderOptions{
 				CacheOpts: sstableinternal.CacheOptions{
 					Cache: c,
 				},
diff --git a/sstable/writer_test.go b/sstable/writer_test.go
index fc924d7a..a5897c6c 100644
--- a/sstable/writer_test.go
+++ b/sstable/writer_test.go
@@ -708,10 +708,8 @@ func TestWriterClearCache(t *testing.T) {
 	defer cacheOpts.Cache.Unref()
 
 	readerOpts := ReaderOptions{
-		Comparer: testkeys.Comparer,
-		internal: sstableinternal.ReaderOptions{
-			CacheOpts: cacheOpts,
-		},
+		ReaderOptions: block.ReaderOptions{CacheOpts: cacheOpts},
+		Comparer:      testkeys.Comparer,
 	}
 
 	writerOpts := WriterOptions{