Explain why RandomAccessFileReader* is not passed into FilePrefetchBu…

…ffer constructor (#13159)

Summary:
In #13118 (comment), we decided to make a separate follow-up PR that refactors `FilePrefetchBuffer` to determine `use_fs_buffer` once at construction time.

The change would have involved passing in the `RandomAccessFileReader*` directly to the constructor, and using that to determine `use_fs_buffer`. This would avoid repeatedly calling `UseFSBuffer(RandomAccessFileReader* reader)` during the actual prefetch requests.

I started working on this refactoring change but ran into issues with these 2 files, which used `GetOrCreatePrefetchBuffer`
- https://github.com/facebook/rocksdb/blob/main/db/compaction/compaction_iterator.cc
- https://github.com/facebook/rocksdb/blob/main/db/merge_helper.cc

As I explained in the added code comments, sometimes the `RandomAccessFileReader*` is not available when we construct the `FilePrefetchBuffer`, so although it is not the most elegant, I think right now it makes sense to pass in the `reader` into the `Prefetch` / `PrefetchAsync` / `TryReadFromCache` calls. Maybe there is a workaround but I don't think the refactor would be worth it.

Pull Request resolved: #13159

Test Plan: N/A (comments)

Reviewed By: anand1976

Differential Revision: D66473731

Pulled By: archang19

fbshipit-source-id: ce3473694c2cd82513da1a76ad5995afa5bc9cfa

file/file_prefetch_buffer.h

@@ -300,6 +300,14 @@ class FilePrefetchBuffer {
   // offset                : the file offset to start reading from.
   // n                     : the number of bytes to read.
   //
+  // Note: Why do we pass in the RandomAccessFileReader* for every single call
+  // to Prefetch/PrefetchAsync/TryReadFromCache? Why can't we just pass it in at
+  // construction time?
+  // Although the RandomAccessFileReader* is often available when creating
+  // the FilePrefetchBuffer, this is not true for BlobDB (see
+  // BlobSource::GetBlob). The file reader gets retrieved or created inside
+  // BlobFileCache::GetBlobFileReader, after we have already allocated a new
+  // FilePrefetchBuffer.
   Status Prefetch(const IOOptions& opts, RandomAccessFileReader* reader,
                   uint64_t offset, size_t n);
 
@@ -489,6 +497,10 @@ class FilePrefetchBuffer {
   // Whether we reuse the file system provided buffer
   // Until we also handle the async read case, only enable this optimization
   // for the synchronous case when num_buffers_ = 1.
+  // Note: Although it would be more convenient if we could determine
+  // whether we want to reuse the file system buffer at construction time,
+  // this would not work in all cases, because not all clients (BlobDB in
+  // particular) have a RandomAccessFileReader* available at construction time.
   bool UseFSBuffer(RandomAccessFileReader* reader) {
     return reader->file() != nullptr && !reader->use_direct_io() &&
            fs_ != nullptr &&