deardeng opened a new pull request, #69071:
URL: https://github.com/apache/doris/pull/69071
Related PR: #65658
Problem Summary:
The async file cache write path added in #65658 did not support peer reads.
`_resolve_cache_write_mode` forced SYNC_WRITE whenever peer read was enabled,
and `_read_async_write_path` set `bypass_peer_read` on a copy of the IOContext.
In a cloud deployment with peer read enabled, a cache miss therefore always
took the sync path, writing and finalizing cache blocks on the query thread.
The existing peer chain is zero-copy: the server reads cache files into an
IOBuf, the client cuts the attachment, and the sync path writes it with
`append_iobuf` (writev). Routing peer data through the async path must not add
a flattening copy.
1. Peer chain takes block ranges
`PeerFileCacheReader::fetch_blocks` and the peer dispatch helpers
(`execute_peer_read`, `run_peer_race`, `_execute_sequential_peer_read`,
`_execute_remote_read`, `_execute_winner_race`) took
`std::vector<FileBlockSPtr>` but only read `block->range()`. The async path has
no `FileBlock` for its misses, so the chain now takes
`std::vector<FileBlock::Range>`. The sync caller converts its empty blocks
before dispatch. A peer bthread that loses the race no longer holds
`FileBlockSPtr`s.
2. Async write buffers can hold an IOBuf
`AsyncCacheWriteManager::wrap_iobuf(capacity, payload)` wraps one cache
block of peer payload without copying. It reports `capacity` as `size()`, so
pending-byte accounting, the fixed task buffer size check and EOF short-block
detection are unchanged. brpc allocates IOBuf blocks with malloc, outside any
Doris MemTracker, so the wrapper charges the payload length to the async-write
tracker on construction and releases it on destruction. `copy_to()` serves
inflight readers and `append_to()` serves worker persistence through
`FileBlock::append_iobuf`. Allocated buffers behave as before.
3. Async path reads from peers
- Mode resolution no longer depends on peer read, and the async path no
longer forces `bypass_peer_read`.
- A peer fails the whole RPC when it misses any requested block, so peer
is only tried when every block in the remote span is a real cache miss. A span
that rereads local hits (miss-hit-miss, or the whole-range fallback after a
side race) goes straight to remote storage.
- On a peer win, chunks are copied to the user result from their IOBufs,
regrouped by reference into one IOBuf per span block and wrapped as task
buffers. The inflight index and the write worker use the received IOBuf blocks
directly; the copy into the user buffer is the only copy. Storage results keep
the existing tracked-buffer copy.
- Peer bytes copied to the user are accounted as `peer_bytes`.
4. Profile timers
Remote IO is already reported as `PeerIOUseTimer` / `RemoteIOUseTimer`.
Add `AsyncCopyToUserTimer` (copying the remote/peer span into the user buffer)
and `AsyncSubmitWriteTimer` (`_submit_async_write_tasks`) to the file cache
profile, so a peer-miss read on the async path can be broken down and compared
with the sync path's `WriteCacheIOUseTimer`. Both are included in the file
cache group total. No metric or bvar is added.
The async file cache write path (`enable_async_file_cache_write`) now reads
missing blocks from peer BEs when `enable_cache_read_from_peer` is enabled,
instead of falling back to synchronous cache writes. Add AsyncCopyToUserTimer
and AsyncSubmitWriteTimer to the file cache profile.
- Test: Unit Test
- AsyncCacheWriteManagerTest: a multi-segment IOBuf payload is readable
across segment boundaries, persisted intact, and its tracker charge returns to
baseline; a short IOBuf payload persists the EOF block; an IOBuf task evicted
by drop-oldest is finalized on the submitting thread and releases its tracker
charge.
- CachedRemoteFileReaderAsyncPeerTest (peer injected by debug point,
sequential path): peer data is returned and persisted from the IOBuf payload
(second read is local and peer-sourced); reordered attachments; a miss-hit-miss
span bypasses peer; peer NOT_FOUND, peer error and an incomplete peer response
fall back to remote storage and still submit async writes; profile timers are
recorded on a peer miss and stay zero on a local read.
- CrossCGAsyncWinnerRaceTest (default peer/S3 winner race): peer wins
and its payload is persisted; S3 wins when peer fails; S3 wins while a slow
peer is still in flight.
- FileCacheProfileReporterTest covers the new profile fields.
- cache_write_mode_is_resolved_for_each_read_context now expects
ASYNC_WRITE with peer read enabled;
async_write_keeps_direct_remote_after_peer_policy_change is removed because the
behavior it covered no longer exists. Existing fetch_blocks tests pass ranges.
- CachedRemoteFileReaderPeerTest.*,
CachedRemoteFileReaderAsyncPeerTest.*, CrossCGWinnerRaceTest.*,
CrossCGAsyncWinnerRaceTest.*, AsyncCachedRemoteFileReaderTest.*,
AsyncCacheWriteManagerTest.*, InflightWriteBufferIndexTest.*,
FileCacheProfileReporterTest.*,
BlockFileCacheTest.cache_write_mode_is_resolved_for_each_read_context (113
tests, ASAN) all pass
- Behavior changed: Yes. With both async cache write and peer read enabled,
cache misses use the async write path and may be served by peers.
- Does this need documentation: No
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [ ] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [ ] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should
merge into -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
