bobhan1 opened a new pull request, #65840:
URL: https://github.com/apache/doris/pull/65840
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary:
`FileCacheAllocatorBuilder` previously forwarded the caller's exact byte
range to
`BlockFileCache::get_or_set`. This is unsafe for metadata writers because
their ranges do not
necessarily begin at a file-cache block boundary. For example, a real
`SegmentWriter::finalize`
in the new test asks to classify index bytes beginning at offset 113. Before
the fix, the cache
contains an `EMPTY` `INDEX` cell `[113, 757]`, while normal cache
partitioning for the same file
uses the canonical one-megabyte slot `[0, 1048575]`.
Once another reader or writer addresses the same cache key using canonical
block boundaries, the
same logical block can be represented by incompatible ranges. Strict range
checks then fail rather
than allowing two conflicting cache layouts to coexist.
This PR fixes the invariant at the shared writer-allocation boundary:
1. `SegmentWriter::finalize` writes the segment indexes and footer, then
allocates a temporary
holder to classify the index-covered cache block as `INDEX`.
2. `FileCacheAllocatorBuilder` treats the requested bytes as `[offset,
offset + size)` and expands
both ends to the owning `BlockFileCache` instance's canonical block
boundaries.
3. Metadata classification and asynchronous S3/HDFS data persistence
therefore create or reuse
compatible cells for the same remote file.
4. When the last block contains less than a full cache block, the existing
`FileBlock::finalize`
behavior still shrinks the downloaded block to the actual EOF.
The alignment is centralized in `FileCacheAllocatorBuilder` instead of
weakening reader/probe
checks or adding a `SegmentWriter` special case. It consequently covers
every current writer-side
allocation call site. The implementation reads the block size from the
actual cache instance and
checks both range additions for overflow.
The commit history is intentionally split:
- `[test](be) Reproduce unaligned segment file cache allocation` adds a real
SegmentWriter E2E
BEUT and a test-only observation point. On unmodified master it fails with
`[113, 757]` versus
`[0, 1048575]`.
- `[fix](be) Align file cache writer allocations to block boundaries`
implements the shared
allocator fix.
### Release note
Fix incompatible file-cache block boundaries created by unaligned writer
allocations.
### Check List (For Author)
- Test
- [ ] Regression test
- [x] Unit Test
- Pre-fix reproducer run with `-j100`: failed as expected with
`[113, 757]`.
- Post-fix
`SegmentWriterFileCacheAlignmentTest.FinalizeCreatesCanonicalCacheBlocksBeforeAsyncUploadCompletes`
with `-j100`: passed 1/1.
- Post-fix `CloudFileCacheWriteIndexOnlyTest.*` with `-j100`: passed
3/3.
- `build-support/check-format.sh`: passed.
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- Behavior changed:
- [ ] No.
- [x] Yes. File-cache writer allocations are expanded to the owning
cache's canonical block
boundaries before cells are created or reused.
- Does this need documentation?
- [x] No.
- [ ] Yes.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
--
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]
