[feature](file cache) Add asynchronous file cache writes - #65658
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: The phase-one asynchronous file-cache reader built detailed coverage runs, maintained multiple cursors, and materialized individual holes inside a read even though most read_at requests span only one or two cache blocks. This made the query-side orchestration difficult to review and maintain without providing meaningful value for the common case. Replace that logic with one aligned inflight lookup, one read-only cache probe, and a simple per-block source plan. The reader still gives inflight buffers priority and still distinguishes downloaded, downloading, and missing cache blocks. Downloading blocks outside the remote span retain their wait behavior. When real misses exist, the reader takes the first through last miss as one remote range, intentionally rereads any cache or inflight blocks inside that range, and submits background writes only for the blocks that were actual misses. A cache-side race falls back to one full aligned remote read. This preserves caller-buffer completeness, inflight deduplication, existing-block reads, cache wait semantics, non-blocking write submission, and backpressure rollback while substantially reducing the amount of control flow in CachedRemoteFileReader::_read_async_write_path and its helpers. ### Release note None ### Check List (For Author) - Test: - Unit Test: six targeted BlockFileCacheTest cases passed under ASAN, covering inflight reuse, DOWNLOADING wait, cached sides, one remote middle span, real-miss-only submission, backpressure rollback, and per-read mode selection - Build: ./build.sh --be -j100 passed - Style check: build-support/check-format.sh and git diff --check passed - Behavior changed: No. This refactor preserves the phase-one asynchronous cache-write behavior while simplifying how the read range is assembled. - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: The asynchronous cache read planner called BlockFileCache::probe before consulting the inflight write-buffer index. BlockFileCache::probe acquires the cache mutex, so a request already covered entirely by inflight buffers still contended on BlockFileCache even though it needed no cache metadata. Build the aligned block list and perform the batch inflight lookup first. If every requested block is covered for the current write epoch, return the plan immediately and materialize the caller buffer directly from inflight memory. If any block is not covered, retain the existing mixed-source behavior by issuing one whole-range read-only cache probe and classifying only the non-inflight blocks as downloaded, downloading, or remote misses. Make the probe result optional in the read plan so ownership matches the conditional probe. Extend the inflight reuse unit test to hold the BlockFileCache mutex during the second read; the read must still complete, directly proving that the full-inflight fast path does not enter BlockFileCache::probe. ### Release note None ### Check List (For Author) - Test: - Unit Test: six targeted BlockFileCacheTest cases passed under ASAN, including full inflight coverage while the BlockFileCache mutex is held, partial cache coverage, downloading waits, middle-span reads, backpressure rollback, and per-read mode selection - Build: ./build.sh --be -j100 passed - Style check: build-support/check-format.sh and git diff --check passed - Behavior changed: Yes. Reads fully covered by current-epoch inflight buffers no longer call BlockFileCache::probe or acquire its cache mutex; partial inflight coverage still probes and combines existing cache blocks. - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: AsyncCacheWriteService previously used a follow_global_config flag to switch between fixed test options and direct reads of mutable BE configuration. That made queue admission, batching, and watchdog behavior depend on global state that was not visible in the service interface. It also split online updates across two mechanisms: worker-count changes were forwarded by FileCacheFactory, while the remaining settings were read implicitly from worker and submission paths. Make configuration ownership explicit. A newly initialized BlockFileCache constructs a complete per-disk options snapshot, and FileCacheFactory registers update callbacks for all five mutable async-write settings. Each callback captures one complete configuration snapshot and forwards it through FileCacheFactory::update_async_write_options to AsyncCacheWriteService::update_options. The service validates the snapshot, applies the requested worker count, and atomically publishes immutable queue, batch, and watchdog settings. Submission and worker paths now consume service-owned snapshots and no longer include or reference common/config.h. Update unit tests to configure isolated services through the explicit interface, and add coverage proving that config::set_config propagates every mutable setting through the factory into an initialized per-disk service. ### Release note None ### Check List (For Author) - Test: - Unit Test: `./run-be-ut.sh --run --filter=AsyncCacheWriteServiceTest.*:BlockFileCacheTest.async_write_backpressure_rolls_back_inflight_entry -j 100` passed all 11 tests under ASAN - Build: `./build.sh --be -j100` passed - Style check: `build-support/check-format.sh` and `git diff --check` passed - Behavior changed: No. Online mutable settings keep their existing behavior but are propagated through explicit update interfaces. - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: The phase-one asynchronous file-cache write service used two persistent workers per cache disk. The synchronous path it replaces persisted cache blocks directly on scanner threads, so its effective per-disk write concurrency could scale with the external scanner concurrency, whose default per-context upper bound is 16, and could grow further across concurrent query contexts. A two-worker default therefore serialized writeback far more aggressively than the former path and could fill the bounded pending queue during ordinary scan fan-out. Increase the default to 16 workers per cache disk. Keep one MPMC queue and let each worker dequeue, revalidate, claim the FileBlock downloader, and write the block in the same thread. Splitting consumption and persistence into separate pools would add a full-task handoff without an independent processing stage, and claiming a downloader before that handoff would violate FileBlock's thread-bound ownership contract. Each worker now uses its own ConsumerToken so concurrent consumers maintain independent producer-stream cursors instead of rescanning streams for every task. Avoid creating 16 persistent per-disk worker loops while asynchronous writeback is disabled. The cache still constructs the service state and inflight index, but starts workers only when the feature is enabled. A false-to-true online configuration update explicitly starts all initialized services through the factory interface, while mutable service options continue to flow through the explicit factory/service update API. Service readiness is published only after all configured worker loops have been accepted, so query threads reject best-effort submissions instead of enqueueing work to an unready service. Add deterministic coverage for eight workers consuming distinct tasks concurrently, disabled-service rejection, online enablement, and the existing runtime resize, shutdown, watchdog, inflight cleanup, and reader backpressure rollback behavior. ### Release note Increase the default asynchronous file-cache write concurrency from 2 to 16 workers per cache disk. Worker threads are created only after asynchronous file-cache writeback is enabled. ### Check List (For Author) - Test: Unit Test - `./build.sh --be -j100` - `./run-be-ut.sh --run --filter=AsyncCacheWriteServiceTest.*:BlockFileCacheTest.async_write_backpressure_rolls_back_inflight_entry -j 100` (12 tests passed) - `build-support/check-format.sh` - `git diff --check` - Behavior changed: Yes. The default per-disk asynchronous write concurrency is 16, and disabled services no longer keep worker loops resident. - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: The phase-one async file-cache write implementation had strong happy-path coverage, but several correctness boundaries were not exercised through the complete component interactions. Missing coverage included cache-file disappearance and whole-key self-heal cleanup, wait timeout fallback, direct-read prefix preservation, final concurrent publication deduplication, tracked-buffer allocation failure, external-table cache reuse, worker ownership of existing or deleting cells, remove/write epoch races, runtime worker growth and shrink, and complete propagation of the new profile and synchronous cache-population semantics. Add compact scenario-oriented BE unit tests that drive the real reader, cache, inflight index, async service, worker, removal, downloader, and index-preload paths. The tests verify both returned data and persistent cache state, including metadata and physical file deletion. Narrow test-only sync points make allocation failures and race windows deterministic without changing normal runtime behavior. ### Release note None ### Check List (For Author) - Test: Unit Test - 51 related ASAN BE unit tests passed with run-be-ut.sh and -j100 - 7 focused changed-path tests passed with run-be-ut.sh and -j100 - BE build passed with build.sh --be -j100 - build-support/check-format.sh passed - Behavior changed: No; only test coverage and deterministic test injection points are added - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: The async cache-write planner queried one block-aligned range, but `BlockFileCache::probe` returned a `FileBlocksHolder` of cache hits plus an independent gap list. The planner then had to scan all hits and gaps for every logical read block even though the read-plan blocks and probe slots use the same block boundaries. This obscured the alignment invariant and introduced unnecessary nested matching logic on the query read path. Change the probe contract to return one ordered nullable `FileBlock` pointer per aligned input block. A non-null slot is asserted to have the exact corresponding range, except that the final block may end at EOF, while a null slot directly represents a cache miss. The planner now preserves its inflight-first fast path and joins probe results to plan blocks by index; materialization also reads the matching slot directly. Remove `FileBlocksHolder` and the independent gaps from `FileBlocksProbeResult`. Preserve the existing deferred cleanup semantics for EMPTY and deleting cache blocks through a shared cache-user reference release helper rather than embedding a holder in the probe result. Update focused and end-to-end tests for hit/miss slots, a short final block, retained block states, self-heal cleanup, and an aligned direct-cache prefix followed by an async-written suffix. ### Release note None ### Check List (For Author) - Test: Unit Test - 51 related ASAN BE unit tests passed with `run-be-ut.sh` and `-j100` - 2 focused probe/direct-prefix ASAN BE unit tests passed with `run-be-ut.sh` and `-j100` - `build-support/check-format.sh` passed - Behavior changed: No; this simplifies an internal probe/planning contract without changing user-visible cache semantics - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: Existing async file-cache write tests covered a single pending-limit rejection and runtime worker resizing independently, but they did not exercise the complete dynamic backpressure lifecycle. Without that coverage, regressions could allow the MPMC backlog to exceed its bound, lose submissions under producer concurrency, fail to expose sustained rejection at capacity, or leave accepted work stranded after consumers are scaled up. Add one deterministic service-level BE unit test that stalls the initial worker before cache mutation and drives four producers in controlled waves. The test verifies that the actual queued backlog grows through 4, 8, 12, and 16 tasks, that pending count includes the blocked active task, and that a subsequent 48-task producer burst is rejected without changing the bounded queue or accepted-task count. After producers stop, the test increases worker concurrency from one to four and batch size from one to four, then releases the artificial write delay. It samples the MPMC backlog independently from pending count, verifies an intermediate lower watermark and an empty queue, and confirms that all accepted tasks finalize with pending count returning to zero. The synchronization point makes both phases deterministic without adding a production-only observation API. ### Release note None ### Check List (For Author) - Test: Unit Test - New dynamic MPMC backpressure ASAN BE unit test passed with run-be-ut.sh and -j100 - All 14 AsyncCacheWriteServiceTest ASAN BE unit tests passed with run-be-ut.sh and -j100 - build-support/check-format.sh passed - Behavior changed: No; this adds deterministic test coverage only - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: The async file cache write settings were mixed into the broader block file cache configuration section, while the inflight write buffer index settings did not carry the feature name. This made the feature difficult to locate as one configuration group and made name-based filtering incomplete. Move all async file cache write declarations, definitions, and validators into a dedicated contiguous section. Keep the primary enable_async_file_cache_write switch unchanged, rename only the inflight index enable and shard-count settings with the async_file_cache_write prefix, and update runtime consumers plus BE and regression test configuration. Defaults, mutability, validation, and runtime behavior remain unchanged. ### Release note None ### Check List (For Author) - Test: Unit Test - ./run-be-ut.sh --run --filter=AsyncCachedRemoteFileReaderTest.*:BlockFileCacheTest.*async_write*:BlockFileCacheTest.cache_write_mode_is_resolved_for_each_read_context:AsyncCacheWriteServiceTest.* -j100 (26 tests passed) - build-support/check-format.sh - Behavior changed: No - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: The dynamic async-write backpressure test created four producer threads during queue growth, but each producer submitted only one task per fill wave. That exercised simultaneous entry only weakly and did not model sustained concurrent production before backpressure. Start every producer in a wave through a barrier and let each producer submit four consecutive tasks. The test now observes deterministic queue growth through 16, 32, 48, and 64 queued tasks, verifies a subsequent 128-task concurrent burst is rejected at the pending limit, then confirms the enlarged and accelerated consumer side drains the queue to zero. ### Release note None ### Check List (For Author) - Test: Unit Test - ./run-be-ut.sh --run --filter=AsyncCachedRemoteFileReaderTest.*:BlockFileCacheTest.*async_write*:BlockFileCacheTest.cache_write_mode_is_resolved_for_each_read_context:AsyncCacheWriteServiceTest.* -j100 (26 tests passed) - build-support/check-format.sh - Behavior changed: No - Does this need documentation: No
|
/review |
There was a problem hiding this comment.
Request changes: the async-write design is broadly implemented and the happy-path coverage is substantial, but nine blocking issues remain.
- Reader correctness: legal noncanonical/restored cache cells can abort
probe(), and a live peer-cache enable transition can make the async path abort the BE with a null remote buffer. - Concurrency and lifecycle: accepted-but-unstarted persistent worker loops can deadlock shrink; invalidation can enqueue a generation-blind delete that removes a newer refill.
- Configuration and errors: runtime/startup validation does not validate a coherent incoming option set; concurrent callbacks can apply stale snapshots; online enablement reports success after service-start failure.
- Admission: async handoff drops both
file_cache_query_limit_bytesand the lifetime needed to enforcefile_cache_query_limit_percent. - Performance: the default idle workers poll every millisecond, around 16,000 timed wakeups per second per cache disk, while the current notification protocol is not safe for a naive indefinite wait.
Critical checkpoint conclusions:
- Goal and test proof: ordinary aligned misses do return promptly and persistence is handed to a bounded service. Unit/regression coverage is broad, but it omits the negative and concurrency cases identified inline.
- Scope, parallel paths, and compatibility: downloader, index-loader, prefetch, dry-run, warm-up, and stable peer paths retain synchronous completion. No FE/BE wire, transaction, EditLog, MoW, or visible-version contract changes were found. Existing writer/restored cache layouts are not compatible with the new exact-boundary probe.
- Concurrency, lifecycle, configuration, and errors: shutdown/destruction order, thread context, tracker ownership, lock order, and pointer-conditional inflight cleanup are sound. The worker-start/shrink, generation deletion, validation, snapshot ordering, and enablement failures remain blocking.
- Data and persistence: source data is immutable/remote and no transactional persistence boundary changes. The generation race can delete a newer derived cache file/meta and force self-heal, but does not corrupt the remote source.
- Observability and performance: new query/service metrics are propagated through merge/diff/reporting and scanner detection. The 1 ms idle wake protocol is the remaining observability/performance concern.
- Tests and validation: static review only, as the review contract forbids builds and source changes.
.worktree_initializedandthirdparty/installedare also absent. Existing review threads/comments were empty, and all three round-two reviewers returnedNO_NEW_VALUABLE_FINDINGSagainst this exact nine-comment set. - User focus: no additional focus was supplied; the whole PR was reviewed.
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: The existing file-cache microbenchmark either includes object-store and network latency through CachedRemoteFileReader or stops at BlockFileCache::get_or_set. It cannot isolate phase-1 asynchronous writeback, distinguish caller return time from background drain, or expose queue saturation and inflight-index contention. Add a standalone Release microbenchmark beside the existing tool. It combines a deterministic in-memory remote reader with a real filesystem-backed BlockFileCache and covers three layers: synchronous versus asynchronous cold-miss reader latency with complete-range persistence verification; producer admission, bounded MPMC queue behavior, worker scaling, backpressure, and real get_or_set/append/finalize persistence; and sharded miss/hit versus single-hot-key InflightWriteBufferIndex contention. Each case emits machine-readable latency percentiles, throughput, accepted/rejected/persisted counts, and pending/queued/inflight high-water marks. Benchmark data defaults to output/ so it remains untracked and uses the larger workspace disk. ### Release note None ### Check List (For Author) - Test: Manual test - ./build.sh --be --file-cache-microbench -j100 - Existing file_cache_get_or_set benchmark with 1 and 32 threads - Full async benchmark in all mode with 16 producers and worker counts 1,4,16 - build-support/check-format.sh - git diff --check - Behavior changed: No (benchmark tooling only) - Does this need documentation: No (tool README updated)
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: The asynchronous file-cache write microbenchmark previously emitted only one sample per case and did not establish the storage baseline of the cache filesystem. These short concurrent cases are sensitive to scheduler activity, page-cache state, filesystem metadata, and background writeback, so a single number can hide material variance and make worker-scaling conclusions unreliable. Run every selected reader, service, and inflight-index case five times by default and add the one-based repetition to each machine-readable RESULT line. Add an installed runner that automatically detects fio, measures direct 1 MiB sequential QD1 and random QD16 writes on the same filesystem, then starts the benchmark. The runner uses a unique sibling directory under the selected cache path, unlinks fio data, and keeps direct I/O out of the page cache. The benchmark now rejects non-empty cache paths instead of recursively clearing them, and suppresses INFO logging so merged stdout and stderr cannot corrupt RESULT records. Expand the tool README with the component flow, coverage and non-goals of each group, default workload, field semantics, fio controls, cache-path ownership, repetition methodology, and interpretation guidance. Median is the primary value and the observed min-to-max range is retained. Release experiment on /dev/nvme11n1 ext4, 1 MiB blocks, 64 KiB caller reads, 16 producers, 128 reader operations, 256 service attempts, and five repetitions: - fio direct baseline: sequential QD1 was 2513 MiB/s with 161 us p95 completion latency; random QD16 was 3106 MiB/s with 10.552 ms p95 completion latency. - Reader foreground throughput in ops/s, median [min, max]: sync 5645 [5124, 7649], async 6739 [4739, 8298]. Median average latency was 1459 us for sync and 912 us for async. The median throughput improved 19.4% and median average latency fell 37.5%, while the overlapping ranges show why repeated samples are required. - Verified service completion in MiB/s, median [min, max]: 1 worker 798 [730, 968], 4 workers 1562 [1260, 1751], and 16 workers 7825 [5255, 13203]. Median drain time fell from 0.300 s to 0.153 s and 0.014 s. These are buffered append/finalize completions without fsync and are not durable-media throughput. - Backpressure accepted 76 [64, 101] and rejected 180 [155, 192] tasks, with peak pending fixed at the configured limit of 64, peak queued fixed at 48, and peak inflight 65 [65, 67]. Every accepted task was verified as persisted. - Inflight lookup throughput in ops/s, median [min, max]: sharded miss 5.531M [4.028M, 6.621M], sharded hit 4.198M [3.270M, 6.036M], and hot-key hit 1.104M [1.090M, 1.268M]. - All 45 RESULT records were complete and parseable. All reader ranges and all accepted service tasks passed final BlockFileCache coverage verification. ### Release note None ### Check List (For Author) - Test: Manual test - ./build.sh --be --file-cache-microbench -j100 (Release) - ./output/be/bin/run-async-file-cache-write-microbench.sh --benchmark_mode=all --cache_path=./output/async_file_cache_write_microbench_repeat_5_clean --producer_threads=16 --reader_workers=16 --worker_counts=1,4,16 --repetitions=5 - Non-empty cache-path rejection with sentinel preservation - build-support/clang-format.sh - build-support/check-format.sh - bash -n and shellcheck for the runner - git diff --check - Behavior changed: No (benchmark tooling only) - Does this need documentation: No (tool README updated)
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: The asynchronous file-cache write microbenchmark previously emitted only one sample per case and did not establish the storage baseline of the cache filesystem. These short concurrent cases are sensitive to scheduler activity, page-cache state, filesystem metadata, and background writeback, so a single number can hide material variance and make worker-scaling conclusions unreliable. Run every selected reader, service, and inflight-index case five times by default and add the one-based repetition to each machine-readable RESULT line. Add an installed runner that can measure direct 1 MiB sequential QD1 and random QD16 writes on the same filesystem before starting the benchmark. The fio behavior is explicit and does not make fio a mandatory dependency: | RUN_FIO | fio available | Behavior | | --- | --- | --- | | auto (default) | Yes | Run both disk baselines, then run the cache benchmark | | auto (default) | No | Print DISK_BASELINE skipped and continue directly with the cache benchmark | | 1 | No | Fail because the caller explicitly required fio | | 0 | Any | Skip fio and run the cache benchmark | The runner uses a unique sibling directory under the selected cache path, unlinks fio data, and keeps direct I/O out of the page cache. The benchmark rejects non-empty cache paths instead of recursively clearing them, and suppresses INFO logging so merged stdout and stderr cannot corrupt RESULT records. Expand the tool README with the component flow, coverage and non-goals of each group, default workload, field semantics, fio controls, cache-path ownership, repetition methodology, and interpretation guidance. Median is the primary value and the observed minimum and maximum are retained. Release experiment configuration: /dev/nvme11n1 ext4, 1 MiB blocks, 64 KiB caller reads, 16 producers, 128 reader operations, 256 service attempts, and five repetitions. fio direct-I/O baseline: | Workload | Bandwidth | p95 completion latency | | --- | ---: | ---: | | 1 MiB sequential write, QD1 | 2513 MiB/s | 161 us | | 1 MiB random write, QD16 | 3106 MiB/s | 10.552 ms | CachedRemoteFileReader foreground results: | Write mode | Median ops/s | Minimum ops/s | Maximum ops/s | Median average latency | | --- | ---: | ---: | ---: | ---: | | Synchronous | 5645 | 5124 | 7649 | 1459 us | | Asynchronous | 6739 | 4739 | 8298 | 912 us | The asynchronous median was 19.4% higher in throughput and 37.5% lower in average latency. The overlapping ranges are retained because they show why a single run is insufficient. AsyncCacheWriteService verified completion results: | Workers | Median MiB/s | Minimum MiB/s | Maximum MiB/s | Median drain time | | ---: | ---: | ---: | ---: | ---: | | 1 | 798 | 730 | 968 | 0.300 s | | 4 | 1562 | 1260 | 1751 | 0.153 s | | 16 | 7825 | 5255 | 13203 | 0.014 s | These values measure buffered append and finalize completion without fsync. They are not durable-media throughput and are not directly comparable with the direct-I/O fio baseline. Bounded backpressure results: | Metric | Median | Minimum | Maximum | | --- | ---: | ---: | ---: | | Accepted tasks | 76 | 64 | 101 | | Rejected tasks | 180 | 155 | 192 | | Peak pending | 64 | 64 | 64 | | Peak queued | 48 | 48 | 48 | | Peak inflight | 65 | 65 | 67 | Every accepted task was verified as persisted, and peak pending stayed at the configured limit. InflightWriteBufferIndex lookup results: | Workload | Median ops/s | Minimum ops/s | Maximum ops/s | Median average latency | | --- | ---: | ---: | ---: | ---: | | Sharded miss | 5.531M | 4.028M | 6.621M | 2.688 us | | Sharded hit | 4.198M | 3.270M | 6.036M | 3.171 us | | Hot-key hit | 1.104M | 1.090M | 1.268M | 13.701 us | All 45 RESULT records were complete and parseable. All reader ranges and all accepted service tasks passed final BlockFileCache coverage verification. ### Release note None ### Check List (For Author) - Test: Manual test - ./build.sh --be --file-cache-microbench -j100 (Release) - ./output/be/bin/run-async-file-cache-write-microbench.sh --benchmark_mode=all --cache_path=./output/async_file_cache_write_microbench_repeat_5_clean --producer_threads=16 --reader_workers=16 --worker_counts=1,4,16 --repetitions=5 - Non-empty cache-path rejection with sentinel preservation - build-support/clang-format.sh - build-support/check-format.sh - bash -n and shellcheck for the runner - git diff --check - Behavior changed: No (benchmark tooling only) - Does this need documentation: No (tool README updated)
6792d8e to
733ee93
Compare
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: A systematic review after the file-tail crash found that FileBlocksProbeResult reused FileBlocksHolder cleanup semantics. Destroying a read-only probe result on the same thread as an independently owned downloader therefore called complete_unlocked(), reset a valid DOWNLOADING block to EMPTY, and cleared its downloader. A focused BEUT reproduced that state transition before the fix. Give holder and probe references explicit cleanup roles: holders still complete downloader ownership acquired through get_or_set(), while probes only retain blocks and perform the existing deferred EMPTY/deleting-cell cleanup. Also stop re-reading the mutable FileBlock range after probe() has validated slot coverage under the cache mutex; a concurrent file writer may shrink a preallocated EOF block during finalize(), so the async reader now consistently uses its immutable logical plan range for cache offsets and diagnostics. The review added concise end-to-end coverage for a DOWNLOADING preallocated tail that finalizes while a reader waits, mixed existing-cache and inflight coverage, and operation with the optional inflight index disabled. The fixture now resets the process-wide FD cache together with FileCacheFactory because its key omits the per-test cache path; without that isolation, newly added cases exposed stale descriptors from earlier cases. ### Release note Fix async file-cache read races involving read-only probe lifetime and concurrent finalization of a preallocated file-tail block. ### Check List (For Author) - Test: Unit Test - Pre-fix reproduction: BlockFileCacheTest.ProbeResultDoesNotCompleteDownloaderOwnedByCaller failed because the block became EMPTY and its downloader was cleared - Targeted ASAN BEUT: 6 focused probe/EOF/cache-inflight/external-table cases passed with -j100 - Relevant ASAN BEUT sweep: 58 of 60 passed and exposed two cross-case FDCache isolation failures; after the isolation fix, the complete affected AsyncCachedRemoteFileReaderTest suite passed 9 of 9 with -j100, while the other 51 relevant tests had already passed in the sweep - build-support/clang-format.sh, build-support/check-format.sh, and git diff --check passed - Behavior changed: Yes. Read-only probes no longer complete downloader ownership, and async reads remain valid while a preallocated EOF block is finalized and shrunk. - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: A systematic review after the file-tail crash found that FileBlocksProbeResult reused FileBlocksHolder cleanup semantics. Destroying a read-only probe result on the same thread as an independently owned downloader therefore called complete_unlocked(), reset a valid DOWNLOADING block to EMPTY, and cleared its downloader. A focused BEUT reproduced that state transition before the fix. Give holder and probe references explicit cleanup roles: holders still complete downloader ownership acquired through get_or_set(), while probes only retain blocks and perform the existing deferred EMPTY/deleting-cell cleanup. Also stop re-reading the mutable FileBlock range after probe() has validated slot coverage under the cache mutex; a concurrent file writer may shrink a preallocated EOF block during finalize(), so the async reader now consistently uses its immutable logical plan range for cache offsets and diagnostics. The review added concise end-to-end coverage for a DOWNLOADING preallocated tail that finalizes while a reader waits, mixed existing-cache and inflight coverage, and operation with the optional inflight index disabled. The fixture now resets the process-wide FD cache together with FileCacheFactory because its key omits the per-test cache path; without that isolation, newly added cases exposed stale descriptors from earlier cases. ### Release note Fix async file-cache read races involving read-only probe lifetime and concurrent finalization of a preallocated file-tail block. ### Check List (For Author) - Test: Unit Test - Pre-fix reproduction: BlockFileCacheTest.ProbeResultDoesNotCompleteDownloaderOwnedByCaller failed because the block became EMPTY and its downloader was cleared - Targeted ASAN BEUT: 6 focused probe/EOF/cache-inflight/external-table cases passed with -j100 - Relevant ASAN BEUT sweep: 58 of 60 passed and exposed two cross-case FDCache isolation failures; after the isolation fix, the complete affected AsyncCachedRemoteFileReaderTest suite passed 9 of 9 with -j100, while the other 51 relevant tests had already passed in the sweep - build-support/clang-format.sh, build-support/check-format.sh, and git diff --check passed - Behavior changed: Yes. Read-only probes no longer complete downloader ownership, and async reads remain valid while a preallocated EOF block is finalized and shrunk. - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: Enabling asynchronous file-cache writes could abort BE during cloud compaction or page reads when BlockFileCache::probe encountered an existing cache cell whose range did not match the logical async-read slot. The probe treated exact range alignment as an invariant, but generic get_or_set callers such as segment index-cache writers can legitimately create cells at arbitrary offsets and sizes. A cell beginning inside a slot triggered the left-boundary fatal check, while a cell beginning at the slot but ending early could trigger the adjacent right-boundary check. Change the read-only probe to look up each logical slot by its exact start offset. Return only an exact slot-sized block, while retaining support for a full-size preallocated block covering the final short file tail. Treat all other valid cache layouts as misses so the async reader falls back to one remote read without crashing, and continue probing later aligned slots independently. Add a low-level probe test covering both incompatible boundary shapes and preservation of a later aligned hit. Add an async CachedRemoteFileReader test proving an unaligned cached fragment falls back to the remote aligned range, returns correct data, and submits persistence without aborting. ### Release note Fix a BE crash when asynchronous file-cache reads encounter valid cache blocks whose ranges do not align with async probe slots. ### Check List (For Author) - Test: Unit Test - Pre-fix BEUT reproduced the fatal left-boundary check. - 23 related probe and asynchronous reader tests passed with -j100. - 2 final focused boundary and reader tests passed with -j100 after extending adjacent coverage. - build-support/check-format.sh passed. - Behavior changed: Yes. Incompatible existing cache cells are treated as probe misses and read from remote instead of aborting BE. - Does this need documentation: No. This is an internal correctness fix.
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: SegmentWriter::finalize closes an S3 segment asynchronously and classifies its index bytes through FileCacheAllocatorBuilder. The index range starts at the actual index offset, which is generally not aligned to the file-cache block size. While that holder is alive, a concurrent asynchronous cache reader can probe the same file using canonical block slots. The writer-created cell then starts inside the first probe slot and triggers the strict BlockFileCache::probe left-boundary check. The end-to-end reproduction produced an EMPTY INDEX cell at [113, 757] and aborted at the same file_block range assertion reported by cloud_p0. The previous fix made probe treat incompatible existing cells as misses. Revert that behavior and restore the strict probe invariant. Instead, expand every FileCacheAllocatorBuilder request outward to the owning BlockFileCache block boundaries before get_or_set. This keeps metadata-only SegmentWriter allocations, S3 data-buffer allocations, and read-only probe slots on one canonical partition. File writers can still shrink the final downloaded block to the real EOF during finalize. Add an end-to-end BEUT that constructs a real SegmentWriter, appends a block, executes SegmentWriter::finalize, pauses the asynchronous cache upload, and probes while the index holder is still alive. It checks the unaligned index input, aligned EMPTY INDEX block, successful strict probe, and the final aligned DOWNLOADED block after upload completion. ### Release note Fix a BE crash when asynchronous file-cache reads race with unaligned SegmentWriter index-cache allocation. ### Check List (For Author) - Test: Unit Test - Before the fix, the new SegmentWriter BEUT reproduced the exact left-boundary fatal check with cache range [113, 757]. - The new end-to-end SegmentWriter file-cache alignment BEUT passed with -j100. - 18 related BlockFileCache probe, asynchronous CachedRemoteFileReader, and cloud file-cache tests passed with -j100. - build-support/check-format.sh passed. - Behavior changed: Yes. FileCacheAllocatorBuilder now expands writer allocations to canonical cache block boundaries, and BlockFileCache::probe retains its strict aligned-slot contract. - Does this need documentation: No. This is an internal correctness fix.
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: Enabling asynchronous file-cache writes could abort BE during cloud compaction or page reads when BlockFileCache::probe encountered an existing cache cell whose range did not match the logical async-read slot. The probe treated exact range alignment as an invariant, but generic get_or_set callers such as segment index-cache writers can legitimately create cells at arbitrary offsets and sizes. A cell beginning inside a slot triggered the left-boundary fatal check, while a cell beginning at the slot but ending early could trigger the adjacent right-boundary check. Change the read-only probe to look up each logical slot by its exact start offset. Return only an exact slot-sized block, while retaining support for a full-size preallocated block covering the final short file tail. Treat all other valid cache layouts as misses so the async reader falls back to one remote read without crashing, and continue probing later aligned slots independently. Add a low-level probe test covering both incompatible boundary shapes and preservation of a later aligned hit. Add an async CachedRemoteFileReader test proving an unaligned cached fragment falls back to the remote aligned range, returns correct data, and submits persistence without aborting. ### Release note Fix a BE crash when asynchronous file-cache reads encounter valid cache blocks whose ranges do not align with async probe slots. ### Check List (For Author) - Test: Unit Test - Pre-fix BEUT reproduced the fatal left-boundary check. - 23 related probe and asynchronous reader tests passed with -j100. - 2 final focused boundary and reader tests passed with -j100 after extending adjacent coverage. - build-support/check-format.sh passed. - Behavior changed: Yes. Incompatible existing cache cells are treated as probe misses and read from remote instead of aborting BE. - Does this need documentation: No. This is an internal correctness fix.
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: SegmentWriter::finalize closes an S3 segment asynchronously and classifies its index bytes through FileCacheAllocatorBuilder. The index range starts at the actual index offset, which is generally not aligned to the file-cache block size. While that holder is alive, a concurrent asynchronous cache reader can probe the same file using canonical block slots. The writer-created cell then starts inside the first probe slot and triggers the strict BlockFileCache::probe left-boundary check. The end-to-end reproduction produced an EMPTY INDEX cell at [113, 757] and aborted at the same file_block range assertion reported by cloud_p0. The previous fix made probe treat incompatible existing cells as misses. Revert that behavior and restore the strict probe invariant. Instead, expand every FileCacheAllocatorBuilder request outward to the owning BlockFileCache block boundaries before get_or_set. This keeps metadata-only SegmentWriter allocations, S3 data-buffer allocations, and read-only probe slots on one canonical partition. File writers can still shrink the final downloaded block to the real EOF during finalize. Add an end-to-end BEUT that constructs a real SegmentWriter, appends a block, executes SegmentWriter::finalize, pauses the asynchronous cache upload, and probes while the index holder is still alive. It checks the unaligned index input, aligned EMPTY INDEX block, successful strict probe, and the final aligned DOWNLOADED block after upload completion. ### Release note Fix a BE crash when asynchronous file-cache reads race with unaligned SegmentWriter index-cache allocation. ### Check List (For Author) - Test: Unit Test - Before the fix, the new SegmentWriter BEUT reproduced the exact left-boundary fatal check with cache range [113, 757]. - The new end-to-end SegmentWriter file-cache alignment BEUT passed with -j100. - 18 related BlockFileCache probe, asynchronous CachedRemoteFileReader, and cloud file-cache tests passed with -j100. - build-support/check-format.sh passed. - Behavior changed: Yes. FileCacheAllocatorBuilder now expands writer allocations to canonical cache block boundaries, and BlockFileCache::probe retains its strict aligned-slot contract. - Does this need documentation: No. This is an internal correctness fix.
d07c9b3 to
3ec1e5d
Compare
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: The phase-one asynchronous file-cache reader built detailed coverage runs, maintained multiple cursors, and materialized individual holes inside a read even though most read_at requests span only one or two cache blocks. This made the query-side orchestration difficult to review and maintain without providing meaningful value for the common case. Replace that logic with one aligned inflight lookup, one read-only cache probe, and a simple per-block source plan. The reader still gives inflight buffers priority and still distinguishes downloaded, downloading, and missing cache blocks. Downloading blocks outside the remote span retain their wait behavior. When real misses exist, the reader takes the first through last miss as one remote range, intentionally rereads any cache or inflight blocks inside that range, and submits background writes only for the blocks that were actual misses. A cache-side race falls back to one full aligned remote read. This preserves caller-buffer completeness, inflight deduplication, existing-block reads, cache wait semantics, non-blocking write submission, and backpressure rollback while substantially reducing the amount of control flow in CachedRemoteFileReader::_read_async_write_path and its helpers. ### Release note None ### Check List (For Author) - Test: - Unit Test: six targeted BlockFileCacheTest cases passed under ASAN, covering inflight reuse, DOWNLOADING wait, cached sides, one remote middle span, real-miss-only submission, backpressure rollback, and per-read mode selection - Build: ./build.sh --be -j100 passed - Style check: build-support/check-format.sh and git diff --check passed - Behavior changed: No. This refactor preserves the phase-one asynchronous cache-write behavior while simplifying how the read range is assembled. - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: The asynchronous cache read planner called BlockFileCache::probe before consulting the inflight write-buffer index. BlockFileCache::probe acquires the cache mutex, so a request already covered entirely by inflight buffers still contended on BlockFileCache even though it needed no cache metadata. Build the aligned block list and perform the batch inflight lookup first. If every requested block is covered for the current write epoch, return the plan immediately and materialize the caller buffer directly from inflight memory. If any block is not covered, retain the existing mixed-source behavior by issuing one whole-range read-only cache probe and classifying only the non-inflight blocks as downloaded, downloading, or remote misses. Make the probe result optional in the read plan so ownership matches the conditional probe. Extend the inflight reuse unit test to hold the BlockFileCache mutex during the second read; the read must still complete, directly proving that the full-inflight fast path does not enter BlockFileCache::probe. ### Release note None ### Check List (For Author) - Test: - Unit Test: six targeted BlockFileCacheTest cases passed under ASAN, including full inflight coverage while the BlockFileCache mutex is held, partial cache coverage, downloading waits, middle-span reads, backpressure rollback, and per-read mode selection - Build: ./build.sh --be -j100 passed - Style check: build-support/check-format.sh and git diff --check passed - Behavior changed: Yes. Reads fully covered by current-epoch inflight buffers no longer call BlockFileCache::probe or acquire its cache mutex; partial inflight coverage still probes and combines existing cache blocks. - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: AsyncCacheWriteService previously used a follow_global_config flag to switch between fixed test options and direct reads of mutable BE configuration. That made queue admission, batching, and watchdog behavior depend on global state that was not visible in the service interface. It also split online updates across two mechanisms: worker-count changes were forwarded by FileCacheFactory, while the remaining settings were read implicitly from worker and submission paths. Make configuration ownership explicit. A newly initialized BlockFileCache constructs a complete per-disk options snapshot, and FileCacheFactory registers update callbacks for all five mutable async-write settings. Each callback captures one complete configuration snapshot and forwards it through FileCacheFactory::update_async_write_options to AsyncCacheWriteService::update_options. The service validates the snapshot, applies the requested worker count, and atomically publishes immutable queue, batch, and watchdog settings. Submission and worker paths now consume service-owned snapshots and no longer include or reference common/config.h. Update unit tests to configure isolated services through the explicit interface, and add coverage proving that config::set_config propagates every mutable setting through the factory into an initialized per-disk service. ### Release note None ### Check List (For Author) - Test: - Unit Test: `./run-be-ut.sh --run --filter=AsyncCacheWriteServiceTest.*:BlockFileCacheTest.async_write_backpressure_rolls_back_inflight_entry -j 100` passed all 11 tests under ASAN - Build: `./build.sh --be -j100` passed - Style check: `build-support/check-format.sh` and `git diff --check` passed - Behavior changed: No. Online mutable settings keep their existing behavior but are propagated through explicit update interfaces. - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: The phase-one asynchronous file-cache write service used two persistent workers per cache disk. The synchronous path it replaces persisted cache blocks directly on scanner threads, so its effective per-disk write concurrency could scale with the external scanner concurrency, whose default per-context upper bound is 16, and could grow further across concurrent query contexts. A two-worker default therefore serialized writeback far more aggressively than the former path and could fill the bounded pending queue during ordinary scan fan-out. Increase the default to 16 workers per cache disk. Keep one MPMC queue and let each worker dequeue, revalidate, claim the FileBlock downloader, and write the block in the same thread. Splitting consumption and persistence into separate pools would add a full-task handoff without an independent processing stage, and claiming a downloader before that handoff would violate FileBlock's thread-bound ownership contract. Each worker now uses its own ConsumerToken so concurrent consumers maintain independent producer-stream cursors instead of rescanning streams for every task. Avoid creating 16 persistent per-disk worker loops while asynchronous writeback is disabled. The cache still constructs the service state and inflight index, but starts workers only when the feature is enabled. A false-to-true online configuration update explicitly starts all initialized services through the factory interface, while mutable service options continue to flow through the explicit factory/service update API. Service readiness is published only after all configured worker loops have been accepted, so query threads reject best-effort submissions instead of enqueueing work to an unready service. Add deterministic coverage for eight workers consuming distinct tasks concurrently, disabled-service rejection, online enablement, and the existing runtime resize, shutdown, watchdog, inflight cleanup, and reader backpressure rollback behavior. ### Release note Increase the default asynchronous file-cache write concurrency from 2 to 16 workers per cache disk. Worker threads are created only after asynchronous file-cache writeback is enabled. ### Check List (For Author) - Test: Unit Test - `./build.sh --be -j100` - `./run-be-ut.sh --run --filter=AsyncCacheWriteServiceTest.*:BlockFileCacheTest.async_write_backpressure_rolls_back_inflight_entry -j 100` (12 tests passed) - `build-support/check-format.sh` - `git diff --check` - Behavior changed: Yes. The default per-disk asynchronous write concurrency is 16, and disabled services no longer keep worker loops resident. - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: Async cache write workers wait on a condition variable whose predicate includes the service shutdown flag and each worker stop flag. Shutdown and worker resize previously changed those flags without holding the queue mutex, so a notification could occur after a worker observed a false predicate but before it entered the wait. With an idle queue there may be no later notification, causing shutdown or runtime worker shrink to block indefinitely while joining the worker. Publish shutdown and resize stop requests while holding the same queue mutex used by the wait predicate, then notify workers after releasing it. Keep the flags atomic for existing checks outside the queue lock. Add deterministic synchronization-point tests that hold the worker at the wait boundary and verify both lifecycle paths serialize the predicate transition through the queue mutex. ### Release note None ### Check List (For Author) - Test: Unit Test - ./build.sh --be -j100 - Focused AsyncCacheWriteService lost-wakeup unit tests (2 tests passed) - build-support/clang-format.sh - build-support/check-format.sh - git diff --check - Behavior changed: Yes. Async cache write worker shutdown and runtime shrink can no longer lose their wakeup while workers are idle. - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: Async cache write service shutdown and runtime worker resize maintained separate worker exit protocols. Each worker interpreted both its own stop request and service-wide shutdown state, while shutdown relied on workers to detect when all accepted work had drained. This duplicated lifecycle state and required separate synchronization paths and tests. Make the service own shutdown draining explicitly. Shutdown now stops admission, waits registered submitters, waits accepted pending work to drain, and then uses the same worker stop-and-join helper as resize. Workers only react to their own stop request. Rename the lifecycle mutex to match its start, resize, and shutdown scope, remove the redundant service shutdown flag, and consolidate lost-wakeup coverage around the common stop path. ### Release note None ### Check List (For Author) - Test: Unit Test - ./build.sh --be -j100 - ./run-be-ut.sh --run lifecycle filters -j100 (5 tests passed) - ./run-be-ut.sh --run ShutdownDrainsAcceptedTask -j100 (1 test passed after the final assertion) - build-support/clang-format.sh - build-support/check-format.sh - git diff --check - Behavior changed: No. Shutdown still rejects new submissions, waits registered submitters, drains accepted tasks, and joins workers; runtime resize still synchronously retires selected workers. - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: AsyncCacheWriteService mixed queue and worker state transitions with task-owned cleanup decisions, cache-admission context conversion, and dozens of individual bvar registrations and updates. This obscured terminal task transitions and made focused tests depend directly on individual metric objects. Move optional owner cleanup behind AsyncCacheWriteTask::finalize(), centralize task contract helpers and cache-admission context conversion, and isolate bvar ownership and event bookkeeping in a private Metrics component with a read-only test snapshot. Rename internal methods around queued-to-active, persistence, and active-to-completed transitions. Keep admission, locked FIFO drop-oldest, epoch validation, callback timing, byte and count accounting, and worker lifecycle semantics unchanged. ### Release note None ### Check List (For Author) - Test: Unit Test - ./build.sh --be -j100 - ./run-be-ut.sh --run --filter=AsyncCacheWriteServiceTest.*:AsyncCachedRemoteFileReaderTest.* -j100 (34 tests passed) - build-support/check-format.sh - git diff --cached --check - Behavior changed: No. This only reorganizes async write task, metrics, and internal transition responsibilities without changing queueing, persistence, cleanup, or lifecycle behavior. - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: The per-disk asynchronous File Cache writer was named as a service even though it is an internal manager, exposed many detailed bvars per cache disk with ambiguous unit suffixes, did not document why explicit prefetch must remain synchronous, and allowed an exception from one write task to escape the long-lived worker path. Rename the component and its APIs to AsyncCacheWriteManager, keep only core per-disk capacity and outcome bvars exposed with explicit count or bytes suffixes, retain detailed diagnostics as unexposed internal statistics, document the prefetch completion contract, and contain task exceptions at the worker boundary so later tasks and shutdown can continue. Update the microbenchmark, profile names, regression metric lookups, and focused unit tests accordingly. ### Release note Async File Cache write bvar names now use explicit count and bytes suffixes, and non-core per-disk bvars are no longer exposed. ### Check List (For Author) - Test: Unit Test - ./build.sh --be -j100 - ./run-be-ut.sh -j100 --run --filter='AsyncCacheWriteConfigTest.*:AsyncCacheWriteManagerTest.*:InflightWriteBufferIndexTest.*:AsyncCachedRemoteFileReaderTest.*:FileCacheProfileReporterTest.*:BlockFileCacheTest.Probe*:BlockFileCacheTest.async_write*:BlockFileCacheTest.cache_write_mode_is_resolved_for_each_read_context' (58 tests passed) - build-support/check-format.sh - git diff --cached --check - Behavior changed: Yes. Task exceptions no longer terminate async write workers, and the exposed bvar surface and metric names are updated. - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: Clarify that CachedRemoteFileReader::prefetch_range is a fire-and-forget operation whose callers do not explicitly wait for completion. The synchronous cache-write override preserves the existing coordination contract in which concurrent readers wait on a DOWNLOADING cache block until the prefetch task finishes populating it. ### Release note None ### Check List (For Author) - Test: No need to test (comment-only change)\n- Behavior changed: No\n- Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: The asynchronous file-cache write pending limit was configured independently for every cache disk, so the BE-wide accepted queued and active task ownership grew with the number of cache instances. Cache keys are hash-sharded across the successfully initialized instances, making one BE-wide budget with equal per-instance shares a clearer and more stable product contract. Rename the setting to async_file_cache_write_max_pending_bytes, make its default a 1 GiB BE-wide total, and resolve -1 to max(1 GiB, 1% of the BE memory limit) before splitting. Refresh all manager shares after cache creation or reload and after online worker or pending-limit updates. ### Release note The asynchronous file-cache write pending ownership setting is now async_file_cache_write_max_pending_bytes. It defaults to a 1 GiB BE-wide total and is divided equally among initialized cache instances. A value of -1 selects max(1 GiB, 1% of the BE memory limit) as the BE-wide total. ### Check List (For Author) - Test: Unit Test - ./build.sh --be -j100 - ./run-be-ut.sh --run --filter=AsyncCacheWriteConfigTest.*:AsyncCacheWriteManagerTest.* -j100 (26 tests passed) - build-support/check-format.sh - Behavior changed: Yes. The pending ownership configuration is now BE-wide and is split equally across initialized cache instances. - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: HDFS file writers split writer-side cache allocations at hdfs_write_batch_buffer_size_mb boundaries. When that byte size is not divisible by file_cache_each_block_size, a non-final HDFS batch can leave a short cache cell and shift subsequent cell boundaries away from the canonical reader layout. Asynchronous cache reads probe canonical slots, so this valid configuration could violate the probe coverage invariant and terminate BE. Validate the HDFS batch size together with the existing file-cache startup checks. Require the batch size in bytes to be positive, at least one cache block, and exactly divisible by file_cache_each_block_size. Add a focused unit test for valid aligned values and the 640 KiB block / 1 MiB HDFS batch mismatch. ### Release note When file cache is enabled, hdfs_write_batch_buffer_size_mb in bytes must be an integer multiple of file_cache_each_block_size. ### Check List (For Author) - Test: Unit Test - ./run-be-ut.sh --run --filter=HdfsFileSystemTest.RejectsCacheBlockSizeNotDividingHdfsBatch -j100 - ./build.sh --be -j100 - build-support/check-format.sh - Behavior changed: Yes. BE startup now rejects HDFS write batch sizes that cannot be partitioned into canonical file-cache blocks. - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: An asynchronous read could re-read the mutable peer policy after mode resolution and enter a peer path that cannot provide the contiguous retained buffer required by async cache writes. Tracked buffer allocation also did not enable the Doris catchable allocator scope, and a persistent Worker task could be accepted after an OS-thread creation failure while another pool thread remained alive, leaving resize or shutdown waiting forever. Pin asynchronous calls to direct remote storage with a private bypass_peer_read context, convert Doris allocator failures to Status, and reserve backing pool threads before submitting long-running Worker tasks. ### Release note Asynchronous file-cache writes do not support peer-cache reads and pin selected asynchronous calls to direct remote storage. ### Check List (For Author) - Test: Unit Test - ./build.sh --be -j100 - ./run-be-ut.sh --run with 7 focused async manager and cached-reader cases -j100 - clang-format --dry-run --Werror on the six changed files - Behavior changed: Yes. Async mode now remains on direct remote storage for the complete call and Worker growth fails before publishing an unbacked persistent task. - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: Runtime set_config invoked registered validators before assigning the parsed proposed value. DEFINE_Validator closures read the global field, so they validated the old value and could allow an invalid new worker or pending-byte setting into the live configuration. Assign the proposed value before validation and restore the old value on failure. Also isolate the asynchronous file-cache Docker regression from upload-side S3 writer cache population so its cold-read assertions measure only reader-driven cache writes. ### Release note Runtime configuration updates now reject invalid proposed values without changing the active value. ### Check List (For Author) - Test: Unit Test - ./build.sh --be -j100 - ./run-be-ut.sh --run --filter=ConfigValidatorTest.Validator:AsyncCacheWriteManagerTest.MutableConfigUpdatesManagersExplicitly -j 100 (the active async test passed; ConfigValidatorTest is excluded by be/test/CMakeLists.txt) - ./run-be-ut.sh --run --filter=ConfigTest.UpdateConfigs:ConfigOnUpdateTest.* -j 100 - build-support/check-format.sh - Docker regression was not rerun because the existing output cluster state was preserved - Behavior changed: Yes. Invalid mutable configuration values are validated before they can remain active. - Does this need documentation: No
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: The async file cache write path exposed low-value per-disk state, diagnostic outcome counters, persisted-byte counters, and middle-span byte counters as named bvars. This expands the scrape surface for every cache instance without adding essential operational signals. Keep test-relevant diagnostic counters unnamed, remove redundant persisted counters whose behavior is already verified by reading the cached block, and retain only the core per-disk and global bvars.
### Release note
Remove low-value async file cache write bvar metrics.
### Check List (For Author)
- Test: Build and format check
- Manual test: ./build.sh --be -j100; build-support/clang-format.sh; build-support/check-format.sh
- Behavior changed: Yes. Only the exposed observability surface changes; async write behavior is unchanged.
- Does this need documentation: No
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: The async file cache write path still exposed redundant pending and inflight entry-count metrics per cache instance, while the global async read path exposed detailed counters and latency recorders that are better represented by query-level statistics. Remove these named bvars and their collection overhead, retaining five operational metrics per cache instance and only the four cache-probe metrics globally. Query-level ReadStatistics and RuntimeProfile counters remain unchanged.
### Release note
Further reduce the public async file cache write bvar surface.
### Check List (For Author)
- Test: Build and format check
- Manual test: ./build.sh --be -j100; build-support/clang-format.sh; build-support/check-format.sh
- Behavior changed: Yes. Only the exposed observability surface changes; async read and write behavior is unchanged.
- Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: The asynchronous file-cache write pending-ownership limit defaulted to a fixed 1 GiB BE-wide total even though the configuration already supports an automatic mode. Change the default to -1 so it resolves to max(1 GiB, 1% of the BE memory limit) before being split across initialized cache instances. ### Release note The default value of async_file_cache_write_max_pending_bytes is now -1, selecting max(1 GiB, 1% of the BE memory limit). ### Check List (For Author) - Test: No need to test (the existing unit tests cover the -1 resolution path; compilation and tests were not run for this default-only change) - Behavior changed: Yes (the default pending-ownership limit now scales automatically for BE memory limits above 100 GiB) - Does this need documentation: No (the pull request description is updated with the new default)
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: Write-epoch token construction and map publication happened while holding the registry shard mutex. A shared ownership or map allocation failure could destroy the candidate token during stack unwinding, re-enter the same shard through the token destructor, and deadlock. Construct complete ownership outside the lock, recheck for a concurrent winner before publication, and keep candidate destruction outside the locked scope. ### Release note None ### Check List (For Author) - Test: Unit Test (added allocation-unwind and concurrent publication coverage; batch execution pending) - Behavior changed: Yes (allocation failures propagate without re-entering the held shard mutex) - Does this need documentation: No
### What problem does this PR solve? Issue Number: close #xxx Related PR: apache#65658 Problem Summary: Async write submission registered an active submitter and checked the accepting flag through independent atomic operations. A concurrent shutdown could therefore stop accepting and observe no active submitter while a submission had observed the old accepting state but had not registered yet, allowing that task to be queued after workers were stopped. Serialize the accepting check, submitter registration, and queue admission with the queue mutex so shutdown either rejects an unregistered submission or waits for an admitted submission to finish its queue-external finalization. ### Release note None ### Check List (For Author) - Test: Unit Test (updated shutdown/admission race coverage; execution deferred until the review-fix batch is complete) - Behavior changed: Yes (shutdown now has a single ordering point with async write admission) - Does this need documentation: No
### What problem does this PR solve? Issue Number: close #xxx Related PR: apache#65658 Problem Summary: Async writes require fixed cache-block-capacity buffers, but the physical EOF task may contain a shorter valid prefix. If a full-capacity EMPTY cache cell had already been preallocated at that EOF offset, the persistence path classified it as a partial overlap and skipped the valid tail permanently. Treat only a same-offset full-capacity cell as the short EOF task container, append the valid prefix, and let FileBlock::finalize shrink the cell to the persisted size. Preserve the existing skip behavior for every other partial overlap. ### Release note None ### Check List (For Author) - Test: Unit Test (extended the preallocated short EOF coverage with a cache-only reread; execution deferred until the review-fix batch is complete) - Behavior changed: Yes (a preallocated physical EOF cell is now persisted instead of skipped) - Does this need documentation: No
### What problem does this PR solve? Issue Number: close #xxx Related PR: apache#65658 Problem Summary: Apply the repository clang-format v16 rules to the async write review fixes after completing the shared build and unit-test validation. This commit contains formatting changes only and keeps the already-published functional commits intact. ### Release note None ### Check List (For Author) - Test: No need to test (format-only; the BE build and 39 focused unit tests passed before this non-semantic formatting) - Behavior changed: No - Does this need documentation: No
|
run buildall |
1 similar comment
|
run buildall |
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: `BlockFileCache` exposes its asynchronous write manager and inflight index through pointers, but its widely included header also included both complete implementation headers. This grew the transitive include closure of `exec/pipeline/dependency.h` from the master baseline of 357 project headers to 359, causing the configure-time build-hygiene gate to fail. Forward-declare the pointer-owned types, move the `BlockFileCache` destructor out of line so unique_ptr destruction and manager shutdown see complete definitions only in the implementation, and add direct includes to implementation and test files that use the complete types. Also forward-declare the async write epoch used only in a cached-reader private function declaration. The dependency closure returns to 357 without changing runtime behavior. ### Release note None ### Check List (For Author) - Test: - Build: `./build.sh --be -j100` - Build hygiene: `build-support/check-build-hygiene.sh` - Code style: `build-support/clang-format.sh` and `build-support/check-format.sh` - Unit Test: Not run (requested to skip) - Static analysis: Not run (requested to skip) - Behavior changed: No - Does this need documentation: No
|
run buildall |
|
/review |
|
Codex automated review failed and did not complete. Error: All Codex review accounts are usage-limited; earliest retry is 2026-08-20T03:35:00Z. Please trigger /review again after that time. |
TPC-H: Total hot run time: 17419 ms |
TPC-DS: Total hot run time: 83291 ms |
ClickBench: Total hot run time: 14.8 s |
|
run cloud_p0 |
|
/review |
There was a problem hiding this comment.
Request changes: one new P1 startup-compatibility regression remains in the opt-in asynchronous file-cache write feature.
Review completion: converged after three complete rounds. The final normal configuration/lifecycle pass, normal reader/data/test pass, and separate adversarial risk pass all returned NO_NEW_VALUABLE_FINDINGS. MAIN-001 was independently revalidated; every other candidate was dismissed with code evidence or duplicate-fenced against live threads and landed review bodies.
Critical checkpoint conclusions:
- Goal, scope, focus, clarity, and proof: the cohesive 48-file change implements opt-in query-side asynchronous cache persistence with bounded per-disk workers, inflight reuse, dynamic options, metrics, unit tests, a cloud regression, and a benchmark. Happy-path proof is broad, but the unconditional startup gate violates opt-in compatibility and lacks a feature-disabled upgrade test. No additional user focus was supplied.
- Concurrency, locks, and lifecycle: query submitters, persistent workers, config threads, concurrent readers, invalidation, and cleanup were traced through lifecycle, queue, shard, cache, and block locks plus stop/epoch atomics. Manager, worker, buffer, inflight, epoch, probe, block, and reader ownership were checked through start, resize, invalidate, exceptions, shutdown, and teardown. No new distinct deadlock or lifecycle issue survived the existing live-thread fences.
- Configuration and compatibility: enable/disable, worker/pending limits, validators, callbacks, per-disk option fan-out, and HDFS/S3 writer sizing were checked. The attached P1 makes a previously valid 640 KiB cache-block/default-writer configuration startup-fatal on upgrade while the dependent feature is disabled. Existing runtime-validation, partial-application, stale-snapshot, enablement, and noncanonical-layout defects were not duplicated. There is no FE/BE wire, Thrift, EditLog, transaction-visibility, or durable user-data format change.
- Parallel paths and special conditions: Doris/external, direct/inflight/probe/remote, peer, EOF, downloader, prefetch, segment-index, warmup/dry-run, S3, HDFS, and packed paths were traced. Completion-sensitive population callers remain synchronous. Dormant
UNALIGNEDandNO_WRITEstates have no current production producer. - Tests and results: manager/index/probe/reader/writer tests, the cloud regression/output, and benchmark plumbing were inspected, including negative, EOF, rejection, exception, invalidation, resize, shutdown, and mixed-read cases. The feature-disabled startup-compatibility case is missing. No build or test command was run because the authoritative review prompt requires static review only.
- Observability, persistence, and performance: profiles, metrics, bvars, logs, bounded admission, worker pools, and inflight deduplication were reviewed. The changed persistence is expendable BE cache state rather than database transaction or user-data durability state. All other surviving observability, accounting, direct-map, wakeup, benchmark, append/finalize, invalidation, and cleanup concerns already have live review owners.
- FE/BE variables and other checks: all new settings and reader contexts are BE-local, with no cross-process propagation path. All 48 authoritative changed files matched the bundle and were swept; every candidate was accepted, dismissed with evidence, or deduplicated.
What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary:
On a file-cache miss,
CachedRemoteFileReadercurrently performs two different kinds of work on the query thread:The first operation is on the critical path. The second is best-effort cache population, but local filesystem latency and backpressure are still charged to foreground scan latency because the two operations are coupled.
This PR implements phase 1 of asynchronous file-cache writes. For ordinary reads, the query thread returns after the caller's buffer is complete and hands eligible cache-miss blocks to per-disk background workers. The feature is opt-in and disabled by default. Explicit cache-population operations, including warm-up and prefetch/download paths, retain synchronous completion semantics.
Simply moving
append()to another thread is unsafe. The design also has to preserve cache-block ownership, avoid duplicate queued writes, survive clear/remove races, bound background work, and keep existing warm-up and dry-run behavior. This PR introduces dedicated components for those responsibilities instead of transferring existingFileBlockobjects across threads.Goals
Non-goals of this phase
file_cache_query_limit_bytesorfile_cache_query_limit_percent; deployments that depend on either query-scoped admission limit must keep asynchronous file-cache writes disabled.Performance validation
A storage-compute separated PL1 benchmark was run on commit
b586dd1f429with one 32-core, 123 GiB BE, two PL1 file-cache disks, 16 background write workers per disk, andenable_adaptive_scan=false. Each round restarted the BE with an empty file cache. The workload was a full scan ofssb.lineorder: about 6 billion rows and 17 columns, with 135.8 GB reported bySHOW DATA.Pending task ownership versus the active working set
At 64 target scanners, the estimated active buffer working set was
16 instances x 4 scanners x 17 columns x 1 MiB = 1,088 MiB, or about 544 MiB per cache disk.With the pending ownership limit below the working set, older queued buffers can be evicted before later scans reuse them or before persistence completes, causing repeated remote reads. Once the pending ownership limit covers the working set, reuse improves and foreground scans no longer wait for local persistence. At 1,024 MiB per disk, query time fell from 171 s to 90 s, a 47.4% reduction. Increasing the limit to 2,048 MiB per disk improved the result by only one additional second. The synchronous baseline already drove one cache disk to 94.4% utilization, confirming that local cache writes were the bottleneck in this workload.
Scanner scaling after working-set coverage
The pending ownership limit was then increased with scanner concurrency so that each run continued to cover its estimated working set. Remote I/O stayed within 6.2% of the synchronous cold-cache baseline.
As peak scanner concurrency increased from 63 to 128, query time fell from 90 s to 56 s and network receive throughput increased, while combined cache-disk write throughput remained approximately 635 MiB/s. This shows that asynchronous persistence removed the saturated local disks from the foreground critical path for this workload. Gains began to narrow at 128 scanners, indicating that the next bottleneck was shifting toward the network, remote storage, or CPU.
These results are workload-specific rather than a universal speedup claim. The benefit requires both a foreground bottleneck in local file-cache writes and enough effective accepted-task ownership on each cache instance to cover the active scanner working set. The tables report the effective per-instance shares used by the benchmark. With the current configuration,
async_file_cache_write_max_pending_bytesis one BE-wide total divided equally among successfully initialized cache instances; reproducing a two-disk row therefore requires a BE-wide total twice the listed per-instance value. This remains an ownership limit, not an aggregate hard limit on all live async buffer memory.Architecture
The query thread owns read planning and result assembly. It performs one whole-range inflight lookup and, unless inflight buffers cover every aligned block, at most one read-only probe for the complete aligned request. It then represents each aligned block with a small source classification. Background workers own cache-cell acquisition and persistence. The two sides exchange immutable, reference-counted buffers through an inflight index and a per-disk manager whose admission is bounded by accepted queued and active task ownership.
flowchart LR Caller[Scan / query caller] Reader[CachedRemoteFileReader] Remote[Direct remote FileReader] subgraph CacheDisk[One file-cache disk] Probe[BlockFileCache read-only probe] Index[InflightWriteBufferIndex] Manager[AsyncCacheWriteManager] Queue[Locked FIFO deque with byte admission] Workers[Resizable write workers] Storage[BlockFileCache and local storage] end Metrics[Runtime profile, per-disk bvars, and global probe bvars] Caller -->|read request| Reader Reader -->|lookup completed remote buffers| Index Reader -->|observe cache state without ownership| Probe Reader -->|one remaining middle-range read| Remote Remote -->|aligned bytes| Reader Reader -->|complete caller buffer| Caller Reader -->|publish true MISS buffers| Index Reader -->|submit without disk IO wait| Manager Manager --> Queue Queue --> Workers Workers -->|revalidate, acquire, append, finalize| Storage Workers -->|conditional cleanup| Index Storage -.->|clear/remove advances write epoch| Manager Reader -.-> Metrics Index -.-> Metrics Manager -.-> MetricsComponent responsibilities
CachedRemoteFileReaderBlockFileCache::probeFileBlockdownloader ownership. LRU touch happens only after a block is actually consumed.InflightWriteBufferIndexAsyncCacheWriteManagerdeque, queued/active/pending count and byte accounting, fixed drop-oldest admission, tracked-buffer accounting, resizable workers, shutdown, and manager-level metrics.Important flows
1. Ordinary read with mixed coverage
Each aligned block is classified in this order:
InflightWriteBufferIndex;The lookup covers the complete aligned request. A full inflight hit skips the file-cache probe; otherwise one probe covers the same whole range, so the planner does not build or repeatedly probe individual uncovered runs. It identifies the first and last real cache miss, materializes only inflight/cache blocks outside those boundaries, and retains the existing wait behavior for a
DOWNLOADINGblock outside the remote span. If misses remain, everything from the first through the last miss is fetched with one aligned direct-remote read. Existing cache, inflight, or downloading blocks inside that span are intentionally covered by the same remote result instead of being filled or waited one by one. Only the blocks that were real misses are published and submitted for asynchronous persistence.sequenceDiagram participant C as Caller participant R as CachedRemoteFileReader participant I as Inflight index participant F as BlockFileCache participant O as Direct remote storage participant S as Async write manager participant W as Worker C->>R: read_at(offset, size) R->>I: lookup aligned blocks for current epoch I-->>R: reusable pending buffers R->>F: at most one read-only probe for the aligned request F-->>R: downloaded / downloading / miss coverage R->>R: materialize blocks outside the first-to-last miss span alt all requested bytes are covered R-->>C: return completed caller buffer else a middle range remains R->>O: one aligned middle-range read O-->>R: remote bytes R->>R: copy requested overlap into caller buffer loop each true MISS block in the remote span R->>I: insert completed block buffer if absent alt another owner already published it I-->>R: existing entry, skip duplicate write else this reader owns persistence R->>S: try_submit without waiting alt fits pending byte limit S-->>R: accepted at FIFO tail else full with a queued victim S-->>R: accepted and oldest queued task evicted S->>I: conditional victim cleanup outside queue lock else task exceeds byte limit, no queued victim, or manager stopped S-->>R: rejected R->>I: conditional rollback end end end R-->>C: return completed caller buffer S->>W: dequeue in background W->>F: revalidate epoch and current block state W->>F: append and finalize eligible empty blocks W->>I: conditional completion cleanup end2. Later reader reuses a pending buffer
After the first remote read publishes a buffer and before its worker finishes persistence, a later reader can copy the requested bytes directly from the inflight entry. It does not wait for the local file to be finalized and does not submit another write task for that block.
This index deduplicates pending cache writes and bridges the interval between remote-read completion and local persistence. It intentionally does not claim to deduplicate remote requests that began before publication.
3. Pending-ownership admission and asynchronous failure
Each cache instance's share of the BE-wide pending-byte limit accounts the full buffer-capacity bytes owned by accepted queued and active tasks. The manager protects a FIFO
dequeand all queued/active/pending ownership transitions with one mutex, maintaining bothpending_count = queued_count + active_countandpending_bytes = queued_bytes + active_bytes.This is an admission and ownership-accounting limit, not a hard upper bound on every live async buffer allocation. A buffer is allocated before
try_submit(). After a replacement or worker completion, the task can also remain live while its callback removes the inflight entry and while other shared references are released, even though it no longer contributes topending_bytes. The manager memory tracker and inflight-index buffer gauges expose those live allocations separately.Production submits one task per aligned cache block. Every task allocates exactly
file_cache_each_block_sizebytes;write_sizeis the valid prefix and may be shorter only for the physical EOF block. Admission and byte accounting use the full buffer capacity, including for that short tail block. Therefore a configured remainder smaller than one cache block is intentionally unusable, and replacing one queued task with another preserves pending bytes exactly.pending_bytes + task_buffer_bytesfits, the new task is appended to the FIFO tail and increases queued and pending count/byte state.Evicted-task metrics, callback execution, conditional inflight-index removal, and buffer release happen outside the queue mutex. An evicted task is best-effort cache population that never reaches a worker; it does not affect the query that already received its remote bytes.
Likewise, an append/finalize failure is logged and counted by the worker; it does not retroactively fail a query whose remote read succeeded. A future read simply observes another cache miss and may retry population.
4. Clear/remove and stale-task handling
Cache invalidation advances the manager's write epoch. Both inflight lookup/replacement and worker processing compare their entry or task against the current epoch. Workers check the epoch before acquiring cache blocks and again while processing them. This closes the window where old queued work could recreate blocks after a clear/remove operation.
Completion cleanup uses pointer-conditional removal. Therefore, even if a newer generation has published an entry for the same block offset, an older task cannot erase it.
5. Mode selection and compatibility
CacheWriteModeis resolved for every read so an online configuration switch affects existing readers. Ordinary reads may select the asynchronous path when the feature is enabled. Peer-cache reads are not supported by the asynchronous path: a read that observes peer mode during resolution selects synchronous cache write. Once asynchronous mode has been selected, the reader copies the call'sIOContextand setsbypass_peer_readon that private copy, pinning direct remote storage for the rest of the call even if the online peer-read policy changes; the caller's context is not mutated. Dry-run, warm-up, segment-index population, downloader, and explicit prefetch paths also select or override synchronous mode because their completion semantics depend on the cache being populated before returning.The default remains synchronous because
enable_async_file_cache_writedefaults tofalse.The asynchronous path currently does not support
file_cache_query_limit_bytesorfile_cache_query_limit_percent. Background tasks do not retain the query-ownedRemoteScanCacheWriteLimiteror the query-context-holder lifetime required by those admission policies, so their limits are not guaranteed for asynchronous persistence. Keep asynchronous file-cache writes disabled when either query-level limit is required.6. Worker lifecycle
Each
BlockFileCachecreates its inflight index and async-write manager state during initialization. When asynchronous writeback is disabled, the manager does not create persistent worker loops. Enabling the feature online explicitly starts all initialized per-disk managers.Workers pop one task at a time from the FIFO head under the same queue mutex used by producers and atomically move it from queued to active. There is no batch-size setting or bulk dequeue. When an active task finishes, active and pending count/byte state is decremented under the queue mutex; lifecycle metrics and the task callback run after releasing it.
Each worker object owns its stop request and completion signal. Worker count can be resized online without exposing numeric worker IDs as lifecycle state. Before submitting long-running Worker tasks, the manager raises the pool's minimum thread count to the configured worker count, so an OS-thread creation failure is returned before any new Worker task can be accepted and stranded in the pool queue. Shutdown first stops new submitters, waits for already registered submitters, drains accepted tasks, joins workers, and only then destroys state referenced by task-finalization callbacks. There is no second persistence pool or cross-thread
FileBlockownership transfer.Configuration
enable_async_file_cache_writefalseasync_file_cache_write_workers_per_disk16async_file_cache_write_max_pending_bytes-1(automatic)-1resolves tomax(1 GiB, 1% of the BE memory limit)before the total is split equally across successfully initialized cache instances. Supports online update.enable_async_file_cache_write_inflight_write_buffer_indextrueasync_file_cache_write_inflight_write_buffer_index_shard_count64async_file_cache_write_max_pending_bytesis one BE-wide total. After cache initialization,FileCacheFactorydivides it by the number of successfully initialized cache instances and applies the same floor-divided share to every per-disk manager. Cache keys are hash-sharded across those instances, so this keeps the aggregate ownership budget stable as the cache-disk count changes; integer-division and sub-cache-block remainders are intentionally unused. The value is an upper bound on accepted queued + active task ownership, not a reservation or a hard upper bound on all live async buffer memory. Task buffers are allocated lazily before admission, and finalizing or externally referenced buffers can outlive pending ownership. The queue-full behavior is fixed to FIFO drop-oldest and has no separate policy configuration. Accepted tasks are not discarded by an age watchdog.Observability
Query runtime profiles expose asynchronous submissions, rejections, buffer-allocation failures, stale-epoch drops, and inflight hits/misses.
Each
BlockFileCacheinstance exposes five bvars prefixed by its cache base path:async_cache_write_running_worker_count: currently running persistence workers;async_cache_write_buffer_memory_bytes: all liveAsyncCacheWriteBuffercapacity charged to the manager memory tracker;async_cache_write_submitted_task_count: accepted asynchronous write tasks;async_cache_write_evicted_oldest_task_count: queued tasks evicted by fixed FIFO drop-oldest admission;inflight_write_buffer_index_buffer_bytes: buffer capacity currently retained by inflight-index entries.The BE exposes four global cache-probe bvars:
cached_remote_file_reader_probe_count: whole-range cache probes performed while building asynchronous read plans;cached_remote_file_reader_probe_hit_downloaded_count: probed logical blocks already downloaded;cached_remote_file_reader_probe_hit_downloading_count: probed logical blocks currently downloading;cached_remote_file_reader_probe_miss_count: probed logical blocks classified as real misses.Detailed queue and pending accounting, rejection-reason breakdowns, operation latencies, write-epoch details, state-based skips, failures, and inflight-index operation breakdowns are not exposed as named bvars. Query-relevant outcomes remain available through runtime profiles, while focused tests inspect manager diagnostics directly without adding scrape series.
async_cache_write_buffer_memory_bytesis the more complete live-memory signal: task buffers are allocated before admission and can outlive pending ownership while callbacks and other shared references release them.inflight_write_buffer_index_buffer_bytesreports only capacity retained by index entries, so the two gauges can differ transiently and when the index is disabled.Tests
Latest bvar-surface reduction:
./build.sh --be -j100,build-support/clang-format.sh, andbuild-support/check-format.shpassed. Focused unit tests were not rerun because the final commits only removed metric exposure and collection; the behavioral tests below predate that observability-only cleanup.Latest review fixes: 7/7 targeted ASAN BE unit tests passed.
Current locked-FIFO/drop-oldest manager: 21/21
AsyncCacheWriteManagerTestcases passed after the worker-lifecycle encapsulation.Current one-task-at-a-time queue and read integration: 36/36 focused manager, inflight-index, and async-reader tests passed after removing the ineffective batch-size path.
Pending-ownership-bounded admission and read/task contracts: 37/37 focused manager and reader tests passed.
write_size;Latest BE-wide pending-limit update: 26/26 focused manager and configuration BE unit tests passed.
-1automatic sizing.Latest read-planning refactor: 6/6 targeted ASAN
BlockFileCacheTestcases passed.DOWNLOADINGwait outside the remote span;Docker cloud regression: 1/1 suite passed.
Build and style:
./build.sh --be --fe --cloud -j100./build.sh --be -j100./build.sh --be --file-cache-microbench -j100and an all-mode microbenchmark smoke run completed without hangingbuild-support/check-format.shRelease note
Add an opt-in asynchronous file-cache write path controlled by
enable_async_file_cache_write. Each cache disk uses a locked FIFO whose accepted queued + active task buffer ownership is byte-bounded; when the effective per-instance limit is reached, a new fixed-block task replaces the oldest queued task, while active tasks are never evicted. This is not a hard upper bound on every live async buffer allocation.async_file_cache_write_max_pending_bytesdefaults to-1, which resolves to a BE-widemax(1 GiB, 1% of the BE memory limit); the resolved total is split equally across successfully initialized cache instances. The asynchronous path does not support peer-cache reads,file_cache_query_limit_bytes, orfile_cache_query_limit_percent. The feature remains disabled by default.Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)