feat(core): add a block cache for the reads the prefetched ranges do not cover - #272
feat(core): add a block cache for the reads the prefetched ranges do not cover#272lucasfang wants to merge 2 commits into
Conversation
lucasfang
changed the title
| std::unique_ptr<FileBlockCache> block_cache_; | ||
| // Statistics of the Read() requests issued to the cache, aggregated over | ||
| // all streams sharing this cache. | ||
| std::atomic<uint64_t> read_count_{0}; |
There was a problem hiding this comment.
TODO: metrics refactor to a struct
| std::unordered_map<uint64_t, std::shared_ptr<Block>> blocks_; | ||
| // Bytes held by blocks_, guarded by mutex_ and bounded by capacity_. | ||
| uint64_t cached_bytes_ = 0; | ||
| std::atomic<uint64_t> hits_{0}; |
There was a problem hiding this comment.
TODO: metrics refactor to a struct
| std::atomic<uint64_t> hits_{0}; | ||
| std::atomic<uint64_t> hit_bytes_{0}; | ||
| std::atomic<uint64_t> fetches_{0}; | ||
| std::atomic<uint64_t> fetch_bytes_{0}; |
There was a problem hiding this comment.
A broader question: at the moment there are many layers of metrics across different levels of the read path — ParquetMetrics, PrefetchMetrics, ReadAheadCache metrics, FileBlockCache metrics. Have you looked into whether the meanings of these metrics overlap or conflict with each other, and whether the metric collection is accurate?
| /// instead of replacing a block. That keeps every dispatched fetch reachable | ||
| /// through its block, so Release() and the destructor can wait for the fetches | ||
| /// still writing into the block buffers. | ||
| class PAIMON_EXPORT FileBlockCache { |
There was a problem hiding this comment.
Please do a self-test and analysis of whether there are any lifecycle issues between bytes and pool inside ReadAheadCache and FileBlockCache, especially in asynchronous S3/OSS scenarios. Please do not use GetDefaultPool(), since it is a global static pool and may hide lifecycle problems even if they exist.
| /// | ||
| /// Blocks are aligned to the END of the file: block 0 is | ||
| /// [file_size - block_size, file_size). The metadata of a parquet/orc file lives | ||
| /// in its tail and arrow reads exactly the last 64 KiB as the footer, so an |
There was a problem hiding this comment.
arrow reads exactly the last 64 KiB as the footer
means parquet or just arrow? what is the behavior or any bad case in orc and other format?
2 participants
Purpose
Linked issue: close #273
ReadAheadCachecaches exactly the ranges registered throughInit(). A read that no registered range covers is reported as a miss and handed back to the caller, which reads it from the underlying stream — uncached, so the next reader asking for the same bytes pays for it again. This leaves every read outside the current prefetch plan uncached: the metadata reads a reader must issue before it can compute the ranges it wants to prefetch, the reads beyond a narrowed plan, and the reads repeated after aReset()with a new range set. Since oneReadAheadCacheis shared by all the readersPrefetchFileBatchReaderImplcreates for a data file and outlives a round of registered ranges, those bytes are fetched once per reader and once per round, each time a separate full-latency request.This PR adds a
FileBlockCacheas a second level below the registered-range lookup: when no registered range covers a read, the block cache serves it at a fixed block granularity instead of letting it fall through to the stream. It is a general mechanism over the whole file — any reader, any offset, any round — with the metadata reads that precede a prefetch plan as its most visible beneficiary.Changes:
src/paimon/common/utils/file_block_cache.h/.cpp(new): a cache of fixed-size blocks of one file. A read is served out of the single block containing it, so the first read of a region fetches its whole block and every later read landing in that block is served from memory. A block is published together with its promise before its fetch is dispatched, so concurrent readers of the same block wait for that single fetch (shared_future) instead of issuing their own; the wait and the copy happen outside the lock. Blocks are keyed by index counted from the end of the file (block 0 is[file_size - block_size, file_size)), which keeps them disjoint by construction, keeps every block inside the file, and makes the 64 KiB footer read of arrow cover exactly one block instead of straddling two. Blocks are never evicted — once the capacity is reachedRead()declines instead of replacing a block — which keeps every dispatched fetch reachable through its block soRelease()and the destructor can wait for the fetches still writing into the block buffers.Read()returnsbooland is transparent by contract: it declines, leavingdestuntouched so the caller reads the bytes itself, when one block cannot serve the range, when the capacity is exhausted, or when the block fetch failed. A block fetch reads more than the caller asked for, so its failure must not become the caller's error; the failed block is left in place with its failed future and is not fetched again.src/paimon/common/utils/read_ahead_cache.h/.cpp: the constructor takes thefile_sizethe blocks are indexed against and creates the block cache when the size is known and both block-cache options are non-zero.Read()consults the block cache when no registered range covers the request, before counting a miss.Reset()now clears only the prefetch state and zeroes the block counters, keeping the blocks (they cache the file, not a round of registered ranges) via the new privateReleasePrefetchBuffers();ReleaseBuffers()releases the blocks as well, since the file is not read again.CollectMetrics()publishes the block counters and folds the block fetches into the io counters.include/paimon/utils/prefetch_cache_config.h:CacheConfiggainsGetBlockSize/SetBlockSize(default64 * 1024, matching arrow'skDefaultFooterReadSize) andGetBlockCacheLimit/SetBlockCacheLimit(default1024 * 1024, per file). Zero for either disables the block cache.src/paimon/common/reader/prefetch_file_batch_reader_impl.cpp: passes the data file size to the cache; a non-positive size means unknown and leaves the block cache off.src/paimon/testing/utils/gated_async_input_stream.h(new): anInputStreamwrapper that holds theReadAsynccallbacks untilReleaseAll(), so tests can observe a cache while its fetches are in flight.src/paimon/CMakeLists.txt: registers the new source and test file.New metrics:
read-ahead-cache.block.{hits,hit-bytes}for the reads served by a block andread-ahead-cache.block.{fetches,fetch-bytes}for the fetches issued for the blocks themselves. A read is counted as a hit, a block hit or a miss, soread.count = read.hits + block.hits + read.misses. The block fetches also go to the underlying stream and are therefore included inread-ahead-cache.io.{count,bytes}, so comparing them with the block hit bytes shows how many bytes the block granularity added on top of the requested ones.Tests
src/paimon/common/utils/file_block_cache_test.cpp(new, 9 cases):TestServesRepeatedTailReads,TestClampsLowestBlockAtFileStart,TestSingleFlightForConcurrentReads(two readers of the same block, one fetch),TestExhaustedCapacityDeclinesNewBlocks,TestDeclinesStraddlingAndOversizedReads,TestDisabledByZeroCapacityOrBlockSize,TestResetCountersKeepsBlocksAndReleaseDropsThem,TestFetchErrorDeclinesTheReads(a failed block is declined without being fetched again, the other blocks keep working),TestFileSizeLargerThanTheFileOnlyLosesTheLastBlock(a file size larger than the physical file only stops one block from being cached and never fails a read).src/paimon/common/utils/read_ahead_cache_test.cpp: addsTestBlockCacheServesUncoveredReads,TestBlockCacheDeclinedReadIsAMiss,TestBlockCacheSurvivesReset,TestBlockCacheDisabled, and covers the new metrics.src/paimon/format/parquet/parquet_file_batch_reader_test.cpp: addsTestBlockCacheSharesTailReadsAcrossReaders, an end-to-end check that the parallel readers of one file fetch their common metadata once.src/paimon/common/io/cache_input_stream_test.cpp: adapted to the new constructor, with the file size left unknown so the block cache stays off and the miss fallback ofCacheInputStreamis still exercised.cmake --build build --target unittest→ 28/28 ctest suites passed (134 s).paimon-common-test --gtest_filter='TestFileBlockCache.*:TestReadAheadCache.*:CacheInputStreamTest.*'→ 37/37 passed.paimon-parquet-format-test202/202,paimon-orc-format-test117/117,paimon-read-inte-test294/294,paimon-read-inte-with-index-test68/68,paimon-scan-and-read-inte-test490/490 passed.git diff --checkclean andpre-commit run --files <changed files>all passed.API and Format
Public API:
include/paimon/utils/prefetch_cache_config.hadds fourCacheConfigaccessors (Get/SetBlockSize,Get/SetBlockCacheLimit) with defaults, so existing callers are source and behavior compatible.ReadAheadCacheisPAIMON_EXPORTed but internal to the read path; its constructor gains afile_sizeparameter and every in-tree caller is updated.FileBlockCacheis not part of the public headers.Storage format and protocol: unchanged. This only affects how the read path fetches bytes, not what is written or read.
Documentation
No new table option or user-facing configuration key. The behavior change is internal to the read-ahead cache and the new metrics are documented in
ReadAheadCacheMetrics, next to the existing ones.Generative AI tooling
Generated-by: Qoder