From 2bc4319eeab54f92b02859905d78486c15a8f191 Mon Sep 17 00:00:00 2001
From: Radu Berinde <radu@cockroachlabs.com>
Date: Tue, 16 May 2023 12:13:50 -0700
Subject: [PATCH] objstorage: remove genericFileReadable

Remove the `genericFileReadable` implementation; since we moved
`Prealloc` to be a method on `File`, the regular implementation should
always work. Using the same implementation for all cases helps testing
- e.g. we can test readahead logic with files in MemFS.
---
 objstorage/objstorageprovider/vfs.go          |  8 +--
 objstorage/objstorageprovider/vfs_readable.go | 61 ++-----------------
 2 files changed, 5 insertions(+), 64 deletions(-)

diff --git a/objstorage/objstorageprovider/vfs.go b/objstorage/objstorageprovider/vfs.go
index 1debbc192c..5b2025179a 100644
--- a/objstorage/objstorageprovider/vfs.go
+++ b/objstorage/objstorageprovider/vfs.go
@@ -31,13 +31,7 @@ func (p *provider) vfsOpenForReading(
 		}
 		return nil, err
 	}
-	// TODO(radu): we use the existence of the file descriptor as an indication
-	// that the File might support Prefetch and SequentialReadsOption. We should
-	// replace this with a cleaner way to obtain the capabilities of the FS / File.
-	if fd := file.Fd(); fd != vfs.InvalidFd {
-		return newFileReadable(file, p.st.FS, filename)
-	}
-	return newGenericFileReadable(file)
+	return newFileReadable(file, p.st.FS, filename)
 }
 
 func (p *provider) vfsCreate(
diff --git a/objstorage/objstorageprovider/vfs_readable.go b/objstorage/objstorageprovider/vfs_readable.go
index af7a4e41eb..25cea57d5f 100644
--- a/objstorage/objstorageprovider/vfs_readable.go
+++ b/objstorage/objstorageprovider/vfs_readable.go
@@ -15,8 +15,10 @@ import (
 	"github.com/cockroachdb/pebble/vfs"
 )
 
-// fileReadable implements objstorage.Readable on top of a vfs.File that
-// supports Prefetch and/or on an FS that supports SequentialReadsOption.
+// fileReadable implements objstorage.Readable on top of a vfs.File.
+//
+// The implementation might use Prealloc and might reopen the file with
+// SequentialReadsOption.
 type fileReadable struct {
 	file vfs.File
 	size int64
@@ -163,61 +165,6 @@ func (rh *vfsReadHandle) RecordCacheHit(_ context.Context, offset, size int64) {
 	rh.rs.recordCacheHit(offset, size)
 }
 
-// genericFileReadable implements objstorage.Readable on top of any vfs.File.
-// This implementation does not support read-ahead.
-type genericFileReadable struct {
-	file vfs.File
-	size int64
-
-	rh objstorage.NoopReadHandle
-}
-
-var _ objstorage.Readable = (*genericFileReadable)(nil)
-
-func newGenericFileReadable(file vfs.File) (*genericFileReadable, error) {
-	info, err := file.Stat()
-	if err != nil {
-		return nil, err
-	}
-	r := &genericFileReadable{
-		file: file,
-		size: info.Size(),
-	}
-	r.rh = objstorage.MakeNoopReadHandle(r)
-	invariants.SetFinalizer(r, func(obj interface{}) {
-		if obj.(*genericFileReadable).file != nil {
-			fmt.Fprintf(os.Stderr, "Readable was not closed")
-			os.Exit(1)
-		}
-	})
-	return r, nil
-}
-
-// ReadAt is part of the objstorage.Readable interface.
-func (r *genericFileReadable) ReadAt(_ context.Context, p []byte, off int64) error {
-	n, err := r.file.ReadAt(p, off)
-	if invariants.Enabled && err == nil && n != len(p) {
-		panic("short read")
-	}
-	return err
-}
-
-// Close is part of the objstorage.Readable interface.
-func (r *genericFileReadable) Close() error {
-	defer func() { r.file = nil }()
-	return r.file.Close()
-}
-
-// Size is part of the objstorage.Readable interface.
-func (r *genericFileReadable) Size() int64 {
-	return r.size
-}
-
-// NewReadHandle is part of the objstorage.Readable interface.
-func (r *genericFileReadable) NewReadHandle(_ context.Context) objstorage.ReadHandle {
-	return &r.rh
-}
-
 // TestingCheckMaxReadahead returns true if the ReadHandle has switched to
 // OS-level read-ahead.
 func TestingCheckMaxReadahead(rh objstorage.ReadHandle) bool {