Extend FS buffer prefetch to async multi-buffer path#14552
Open
eniac1024 wants to merge 6 commits intofacebook:mainfrom
Open
Extend FS buffer prefetch to async multi-buffer path#14552eniac1024 wants to merge 6 commits intofacebook:mainfrom
eniac1024 wants to merge 6 commits intofacebook:mainfrom
Conversation
✅ clang-tidy: No findings on changed linesCompleted in 179.7s. |
added 2 commits
April 12, 2026 22:59
52651ee to
c01cbe7
Compare
c01cbe7 to
14ac8df
Compare
02b544d to
3946458
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
RocksDB's
FilePrefetchBuffersupports an optimization where the filesystem provides its own buffer (fs_scratch), avoiding an extra memory copy. However, this optimization was restricted to the synchronous single-buffer path (num_buffers_ == 1). The async multi-buffer prefetch path — used for scan-ahead with 2+ buffers — always allocated its own buffers and copied data, missing the zero-copy benefit.Solution
Remove the
num_buffers_ == 1gate fromUseFSBuffer()and plumbuse_fs_bufferthrough the entire async pipeline:ReadAsync: setsscratch = nullptrto signal the FS to provide its own buffer; sync fallback routes toFSBufferDirectReadPrefetchAsyncCallback: adopts FS-provided buffer viaSetBuffer()instead of updating pre-allocated buffer sizeHandleOverlappingAsyncData: handles the single-buffer-remaining case for FS buffers by polling async reads and copying partial data tooverlap_buf_; passesuse_fs_bufferthrough the multi-buffer pathPrefetchAsync/PrefetchRemBuffers: computeuse_fs_bufferonce and thread it through allReadAheadSizeTuning,PrepareBufferForRead, andReadAsynccallsTests are updated to exercise FS buffer mode in async paths, with two new tests (
PrefetchAsyncWithFSBuffer,SequentialReadWithFSBuffer) validating correctness.file/file_prefetch_buffer.hRemoved num_buffers_ == 1 restriction from UseFSBuffer()
Added use_fs_buffer parameter to ReadAsync() and PrefetchRemBuffers()
Updated comment on UseFSBuffer() to remove stale "async not handled" note
file/file_prefetch_buffer.ccReadAsync: sets scratch = nullptr when use_fs_buffer; sync fallback routes to FSBufferDirectRead
PrefetchAsyncCallback: added fs_scratch branch adopting FS buffer via SetBuffer() with offset_/initial_end_offset_ update
HandleOverlappingAsyncData: added single-buffer FS path — polls async read if in progress, copies partial data to overlap_buf_; multi-buffer path computes and passes use_fs_buffer instead of hardcoded false
PrefetchInternal: added num_buffers_ == 1 guard on HandleOverlappingSyncData; replaced redundant UseFSBuffer(reader) call with cached local
PrefetchAsync: computes use_fs_buffer once and threads it through all ReadAheadSizeTuning, PrepareBufferForRead, and ReadAsync calls
PrefetchRemBuffers: accepts and threads use_fs_buffer to ReadAheadSizeTuning and ReadAsync
file/prefetch_test.ccSetUp always uses BufferReuseFS (previously skipped for async)
Added ReadAsync override in mock FS with fs_scratch allocation (scratch == nullptr check) and io_handle/del_fn set to nullptr
Updated FSBufferPrefetchStatsInternals with separate use_fs_buffer assertion branches for async path
Refactored FSBufferPrefetchUnalignedReads — unified sync/async assertions where FS buffer alignment = 1 makes them identical
New test: PrefetchAsyncWithFSBuffer — async prefetch at non-zero offset, num_buffers = 2
New test: SequentialReadWithFSBuffer — two consecutive sequential reads with both sync and async paths
db/db_io_failure_test.ccAdded ReadAsync override in CorruptionFS with fs_scratch allocation for FS buffer mode
IterReadCorruptionRetry: skips fs_buffer + async_io combination (read ordering changes cause corruption to hit wrong read)