Conversation
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary:
On a file-cache miss, CachedRemoteFileReader currently reads the requested data
from remote storage and then appends and finalizes the corresponding local cache
blocks on the query thread. The remote read is required to satisfy the query, but
the subsequent local writes are best-effort cache population. Coupling the two
makes local filesystem latency and backpressure part of foreground scan latency.
Moving only the append call to a background thread is not sufficient. Concurrent
readers may fetch the same missing range, FileBlock downloader ownership has
thread-affine cleanup semantics, and cache clear/remove can invalidate queued
work. Warm-up, prefetch, dry-run, and other explicit cache-population callers also
require synchronous completion semantics.
This change introduces an opt-in asynchronous write path with the following
architecture:
1. BlockFileCache provides a read-only probe API that reports downloaded,
downloading, empty, and missing ranges without creating cache cells or taking
downloader ownership.
2. Each cache instance owns an InflightWriteBufferIndex. It publishes remote-read
buffers with insert-if-absent semantics so later readers can reuse bytes that
have already been fetched and are awaiting persistence.
3. Each cache disk owns an AsyncCacheWriteService with a bounded MPMC queue,
tracked-buffer accounting, dynamically resizable workers, task-age protection,
and an explicit shutdown protocol.
4. The ordinary read path combines inflight buffers and downloaded cache blocks,
reads the remaining middle range from remote storage once, copies all required
bytes into the caller buffer, and submits background tasks only for true cache
misses.
5. Workers revalidate the cache write epoch and current block state before writing.
Conditional index removal and epoch changes prevent stale callbacks or queued
tasks from deleting newer entries or recreating data after cache invalidation.
Queue rejection, tracked-buffer pressure, or asynchronous persistence failure does
not fail a remote read that has already produced the requested data. The inflight
entry is rolled back and the operation falls back to best-effort cache behavior.
Explicit cache-population paths continue to use the synchronous implementation.
The feature is disabled by default and can be switched online. Worker counts,
pending-task limits, batch size, and task-age thresholds are validated
through configuration. New bvars and runtime-profile counters expose submissions,
inflight reuse, probe results, rejections, failures, queue depth, buffer memory,
and write latency.
The asynchronous reader implementation is isolated in
cached_remote_file_reader_async_write.cpp. The top-level read function only
orchestrates planning, covered-range materialization, a single remote middle read,
and task submission; the detailed steps are kept in cohesive helper functions.
### Release note
Add an opt-in asynchronous file-cache write path controlled by
`enable_async_file_cache_write`. It is disabled by default.
### Check List (For Author)
- Test:
- [x] Regression test
- Docker cloud suite `test_async_file_cache_write`: 1 suite passed
- [x] Unit Test
- BE ASAN targeted tests: 26 cases from 4 suites passed
- [x] Build
- `./build.sh --be --fe --cloud -j100`
- `./build.sh --be -j100`
- [x] Code style
- `build-support/check-format.sh`
- clang-tidy was intentionally not run for this change
- Behavior changed:
- [x] Yes. When explicitly enabled, ordinary file-cache misses return after
the caller buffer is complete and persist missing cache blocks in background
workers. The default and explicit cache-population behavior are unchanged.
- Does this need documentation:
- [x] No. The feature is experimental and disabled by default.
### 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
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: The async file cache write path exposed only an aggregate pending count and a limited set of outcome counters. When throughput degraded, operators could not distinguish producer backpressure, MPMC queue buildup, unavailable workers, inflight-index lock contention, BlockFileCache metadata contention, append/finalize latency, or watchdog and stale-epoch drops.
Add exact atomically maintained queue, active-task, running-worker, configured-capacity, and active-stage gauges. Add reason-specific rejection and watchdog counters, submitted and persisted byte/block throughput, and latency recorders for submission, allocation, queue wait, worker processing, get-or-set, append, finalize, probing, read-plan construction, and write submission. Instrument inflight shard lock wait and hold time, and route probe/get-or-set locking through the existing BlockFileCache lock-wait metric.
Keep queue monitoring passive and exact instead of adding a sampling thread. Remove the unused AsyncCacheWriteService::stats snapshot API and use the service state and metrics directly in tests. Do not expose an inflight index metadata memory estimate because it can be mistaken for total payload memory; async_cache_write_buffer_memory_bytes remains the payload-memory metric.
Extend the BE unit tests to verify live queue and stage gauges, metric counters and latency samples, lock instrumentation, and the concurrent MPMC growth, rejection, scale-up, and drain flow.
### Release note
Add monitoring metrics for async file cache write queue pressure, worker activity, stage latency, throughput, rejection causes, and lock contention.
### Check List (For Author)
- Test: Unit Test
- ./run-be-ut.sh -j100 --run --filter=AsyncCacheWriteServiceTest.*:InflightWriteBufferIndexTest.*:BlockFileCacheTest.Probe*:AsyncCachedRemoteFileReaderTest.* (29 tests passed under ASAN_UT)
- Behavior changed: Yes. Adds observability only; async cache read and write semantics are unchanged.
- Does this need documentation: No
### 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 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)
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: File writers allocate cache cells at the normal full block size before the final upload buffer size is known. The final partial block is shrunk to its actual byte count only during FileBlock::finalize(). An async cache read racing with that interval builds a logical tail block ending at file EOF, but BlockFileCache::probe required the cached right boundary to match it exactly. The mismatch aborted the BE in probe; the async read plan and materialization path also carried the same exact-range assumption.
Allow the final short probe slot to be covered by a larger preallocated cache block while preserving exact-size assertions for complete slots. Keep the async reader stricter by allowing the larger boundary only for the logical block that ends at the real file EOF. Add a focused probe unit test that reproduced the original fatal assertion and an async CachedRemoteFileReader test that exercises the complete EOF read flow. The reproducer aborted before the fix and both tests pass after it.
### Release note
Fix a BE crash when an async file-cache read races with preallocation of a partial final file block.
### Check List (For Author)
- Test: Unit Test
- ./run-be-ut.sh --run --filter=BlockFileCacheTest.ProbeAcceptsPreallocatedBlockCoveringFileTail:AsyncCachedRemoteFileReaderTest.preallocated_cache_block_can_cover_the_short_file_tail -j100 (ASAN, 2 tests passed)
- Behavior changed: Yes. Async cache reads now accept a full-size preallocated cache block that covers the short EOF block.
- 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#65905 Problem Summary: SegmentWriter::finalize allocated a cache holder over the segment index range and changed every intersecting block to INDEX. The range can start inside a file-cache block and conflict with canonical async-read probe ranges or cross S3 multipart write boundaries. The previous workaround aligned all FileCacheAllocatorBuilder allocations and added a SegmentWriter alignment test, but that changes the behavior of every writer-side allocation to compensate for a holder whose only purpose is cache-type reclassification. Follow the narrower solution from apache#65905: remove the post-finalize holder allocation and change_cache_type call. Segment cache blocks now retain the type selected by the file writer. Remove the allocator-alignment workaround, its accessor and comments, and the temporary synchronization hook and end-to-end alignment test. Keep the strict probe contract and the independent file-tail and downloader-ownership fixes. ### Release note None ### Check List (For Author) - Test: Unit Test - ./run-be-ut.sh --run --filter=CloudFileCacheWriteIndexOnlyTest.* -j100 (3 tests passed) - build-support/check-format.sh - Behavior changed: Yes. Segment cache blocks are no longer reclassified to INDEX after SegmentWriter finalization. - Does this need documentation: No
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: The bounded async file-cache writer rejected newly downloaded blocks whenever every pending slot was occupied. After the caller rolled back the new inflight entry, adjacent reads of the same block could issue duplicate remote reads. Replace the per-producer MPMC queue with a mutex-protected global FIFO, add an opt-in drop_oldest policy that replaces only the oldest queued task, and finalize victims outside the queue lock while preserving pending, active, and inflight ownership. Keep reject_new as the default and add runtime validation, metrics, and benchmark coverage.
### Release note
Adds `async_file_cache_write_queue_full_policy` with `reject_new` (default) and `drop_oldest`.
### Check List (For Author)
- Test: Unit Test
- ASAN BE unit tests for async cache write service, inflight index, block file cache, and cached remote reader
- ASAN BE and async file cache write microbenchmark build
- Async file cache write microbenchmark functional smoke test for both policies
- BE clang-format and check-format
- Behavior changed: Yes. Operators can opt into replacing the oldest queued async cache write when the bounded queue is full; the default remains reject-new.
- Does this need documentation: No
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: The locked FIFO async file-cache writer supports both reject_new and drop_oldest, but the initial default retained reject_new. Make drop_oldest the production and service-option default so newly downloaded blocks remain available to adjacent reads under queue saturation, while keeping reject_new available through the dynamic configuration for rollback and A/B comparison.
### Release note
`async_file_cache_write_queue_full_policy` now defaults to `drop_oldest`. Set it to `reject_new` to restore the previous admission behavior.
### Check List (For Author)
- Test: No need to test (default-selection-only change; both policy implementations and runtime switching are covered by existing unit tests)
- BE clang-format and check-format
- Behavior changed: Yes. Full async write queues now replace the oldest queued task by default.
- Does this need documentation: No
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: Async file-cache write admission exposed both reject_new and drop_oldest even though retaining the newest queued block is now the intended behavior. Remove the selectable policy and its dynamic configuration so a full queue always replaces the oldest queued task. Preserve bounded admission by rejecting only when every pending task is already active or a runtime limit reduction leaves pending above the new limit. Move the standalone async-write benchmark out of the HTTP microbenchmark directory.
### Release note
Remove `async_file_cache_write_queue_full_policy`. A full async file-cache write queue now always replaces its oldest queued task; submissions are rejected only when no queued victim exists or pending remains above a reduced runtime limit.
### Check List (For Author)
- Test: Unit Test
- `./run-be-ut.sh --run --filter='AsyncCacheWriteServiceTest.*:AsyncCachedRemoteFileReaderTest.drop_oldest_keeps_new_block_for_adjacent_page_and_drops_old_victim' -j50` (22 tests passed, Release)
- `./build.sh --be -j100` (Release)
- `./build-support/clang-format.sh`
- `./build-support/check-format.sh`
- Behavior changed: Yes. Full async write queues always preserve the newest accepted task by replacing the oldest queued task.
- Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: The async file-cache writer bounded pending work by task count, which did not express its actual memory ownership, while its queue-age watchdog could discard accepted writes. Bound queued plus active work by fixed cache-block buffer capacity, preserve FIFO drop-oldest admission for queued tasks, remove watchdog expiry, and use write_size as the valid prefix for the short physical EOF block. Add explicit read-plan contracts and invariants so aligned block partitioning, source classification, remote slicing, inflight publication, and task creation retain the same index and range semantics. Keep the microbenchmark wrapper in its source directory without changing build.sh or installing the wrapper. ### Release note The async file-cache writer now uses async_file_cache_write_max_pending_bytes_per_disk, defaulting to 256 MiB per cache disk, instead of a pending-task count. Accepted writes are no longer discarded based on queue age. ### Check List (For Author) - Test: Unit Test - ./build.sh --be -j100 - ./run-be-ut.sh --run --filter=AsyncCacheWriteServiceTest.*:AsyncCachedRemoteFileReaderTest.*:BlockFileCacheTest.async_write_* -j 100 (37 tests passed) - Behavior changed: Yes. Pending async writes are bounded by buffer-capacity bytes; a full queue evicts its oldest queued task; watchdog age drops are removed. - Does this need documentation: Yes. The existing PR description and companion async-write design and observability documents need the new config, metric, and EOF-tail contracts.
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: Async write admission modeled replacement and each backpressure reason as separate submit results. In particular, lowering the mutable pending-memory limit below the current pending bytes rejected new submissions even when an oldest queued task could be replaced, which diverged from the FIFO drop-oldest policy and made try_submit harder to follow. Collapse admission outcomes to accepted, backpressure, and enqueue failure. Both a full service and a temporarily over-limit service now replace the oldest queued fixed-size task without increasing pending bytes, and return backpressure only when no queued victim exists or one task exceeds the limit. ### Release note After the async file-cache write pending-memory limit is lowered at runtime, new writes continue by replacing the oldest queued task while queued work exists. Active tasks are never evicted. ### Check List (For Author) - Test: Unit Test - ./build.sh --be -j100 - ./run-be-ut.sh --run --filter=AsyncCacheWriteServiceTest.* -j 100 (22 tests passed) - Behavior changed: Yes. Runtime pending-memory limit decreases use the same FIFO drop-oldest admission path as a full service. - Does this need documentation: Yes. The async-write design and observability documentation should describe the runtime limit-decrease behavior and aggregate backpressure metrics.
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: Async file-cache write testing needs query-side cache population to remain enabled while load and compaction outputs do not pre-populate file cache. Add a default-enabled BE switch that can disable cache population from normal S3 file uploads and packed small-file writes without changing object storage writes.
### Release note
None
### Check List (For Author)
- Test: Unit Test
- ./run-be-ut.sh --run --filter=S3FileWriterTest.DisableFileCacheWriteFromS3FileWriter:PackedFileManagerTest.DisableFileCacheWriteFromS3FileWriter -j100
- Behavior changed: No. The new switch defaults to enabled, preserving existing behavior.
- 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 branch retained unrelated changes to build.sh and the existing file-cache microbenchmark README. Restore both files to the PR merge-base content so the branch remains scoped to async file-cache write behavior. ### Release note None ### Check List (For Author) - Test: No need to test. Both files are restored exactly to the PR merge-base and have no final PR diff. - Behavior changed: No - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: Async cache write admission retained an artificial enqueue-failure path, duplicate queued-task state, and always-on invariant checks in hot paths. The batch-size option did not batch queue transfers or writes; it only delayed worker resize and shutdown checks. Simplify admission to direct locked FIFO insertion, derive queue size from the deque, retain conservation checks as debug assertions, and remove the ineffective batch-size configuration while preserving one active task per worker and FIFO drop-oldest behavior. ### Release note Remove the ineffective `async_file_cache_write_batch_size` backend configuration. Async cache write workers continue to process one task at a time. ### Check List (For Author) - Test: Unit Test - `./build.sh --be -j100` - `./run-be-ut.sh --run --filter='AsyncCacheWriteServiceTest.*:InflightWriteBufferIndexTest.*:AsyncCachedRemoteFileReaderTest.*' -j100` (36 tests passed) - `build-support/check-format.sh` - Behavior changed: Yes. Remove the ineffective async cache write batch-size option; FIFO admission and drop-oldest semantics are unchanged. - Does this need documentation: Yes. Async-write design and observability documentation should no longer describe a batch-size setting.
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: Async cache write workers used numeric worker IDs both as identity and resize control state. Each loop compared its ID with a global target count, while start, resize, and shutdown separately maintained a scheduled-bit vector and condition variable. Encapsulate submission, resize stop requests, and completion waiting in a Worker object. The service now resizes an owned worker collection directly, while task processing, one-task-at-a-time execution, FIFO admission, and drop-oldest behavior remain unchanged. ### Release note None ### Check List (For Author) - Test: Unit Test - `./build.sh --be -j100` - `./run-be-ut.sh --run --filter=AsyncCacheWriteServiceTest.* -j100` (21 tests passed) - `build-support/check-format.sh` - Behavior changed: No. Worker lifecycle management is encapsulated without changing queue admission, task concurrency, resize, or shutdown behavior. - 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 change moved assignment of a new runtime configuration value ahead of validation in the generic UPDATE_FIELD macro. This changes shared configuration behavior outside the feature scope. Restore the existing macro ordering so the async write implementation does not modify the generic runtime configuration framework. ### Release note None ### Check List (For Author) - Test: Unit Test - ./build.sh --be -j100 - ./run-be-ut.sh --run --filter=AsyncCacheWriteServiceTest.MutableConfigUpdatesServicesExplicitly -j100 - ./run-be-ut.sh --run --filter=ConfigTest.UpdateConfigs:ConfigOnUpdateTest.* -j100 - build-support/check-format.sh - Behavior changed: No. This restores the pre-existing generic runtime configuration update behavior. - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: The pending, queued, and active byte fields are maintained independently from their task-count counterparts, but their ownership relationship was not documented next to the state. Clarify that pending covers queued plus active ownership and that byte state remains the authoritative directly maintained input for memory admission, including the fixed-capacity EOF buffer contract. ### Release note None ### Check List (For Author) - Test: No need to test. Comment-only change. - clang-format --dry-run --Werror be/src/io/cache/async_cache_write_service.h - git diff --check - Behavior changed: No - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#65658 Problem Summary: The inflight write-buffer index exposed an ambiguously named size gauge but did not expose the buffer capacity retained by its entries. Rename the entry gauge to count, maintain a separate byte gauge across insertion, epoch replacement, stale removal, and conditional removal, and remove redundant queue invariant and post-shutdown terminal checks. ### Release note Rename `inflight_write_buffer_index_size` to `inflight_write_buffer_index_count` and add `inflight_write_buffer_index_buffer_bytes`. ### Check List (For Author) - Test: Unit Test / Manual test - `./build.sh --be --file-cache-microbench -j100` - Focused async cache write, inflight index, and cached reader BE unit tests with `./run-be-ut.sh ... -j100` - Async file cache write microbenchmark all-mode smoke run - Targeted `clang-format --dry-run --Werror` and `git diff --check` - Behavior changed: Yes. The inflight index size metric is renamed and a retained-buffer byte metric is added; async write lifecycle behavior is unchanged. - 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 queue used a fixed 256 MiB pending-memory limit per cache disk, so its default did not scale on large-memory BEs. Raise the default absolute per-disk limit to 512 MiB and support -1 as an automatic mode that selects max(512 MiB, 1% of the BE memory limit). Positive values remain exact per-disk byte limits, and cache creation and runtime updates share the same resolver. ### Release note async_file_cache_write_max_pending_bytes_per_disk now defaults to 512 MiB. Set it to -1 to use max(512 MiB, 1% of the BE memory limit) for each cache disk. ### Check List (For Author) - Test: Unit Test - ./run-be-ut.sh --run --filter=AsyncCacheWriteConfigTest.ResolveMaxPendingBytesPerDisk:AsyncCacheWriteServiceTest.MutableConfigUpdatesServicesExplicitly -j100 - Behavior changed: Yes. The default per-disk limit is 512 MiB, and -1 enables automatic sizing. - Does this need documentation: No
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 29200 ms |
Contributor
TPC-DS: Total hot run time: 170244 ms |
Contributor
ClickBench: Total hot run time: 23.93 s |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)