[fix](filecache) avoid BE crash when finalize misses local cache writer#62389
Merged
hello-stephen merged 1 commit intoMay 12, 2026
Conversation
FSFileCacheStorage::finalize() relied on DCHECK to assume the writer entry was always present in _key_to_writer. In release builds a missing entry could still fall through, dereference end(), and crash BE. Return InternalError when finalize cannot find the writer instead of touching an invalid iterator. The existing FileBlock::set_downloaded() failure path already aborts the pending cache write and resets the block state, so the failure becomes recoverable and diagnosable. Add a BE unit test that covers the missing-writer finalize path.
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Member
Author
|
run buildall |
gavinchou
approved these changes
Apr 11, 2026
Contributor
|
PR approved by at least one committer and no changes requested. |
github-actions
Bot
added
the
approved
Contributor
|
PR approved by anyone and no changes requested. |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Member
Author
|
run buildall |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Member
Author
|
run buildall |
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
1 similar comment
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
|
skip buildall |
github-actions Bot
pushed a commit
that referenced
this pull request
May 12, 2026
…er (#62389) ## Problem `FSFileCacheStorage::finalize()` relied on `DCHECK` to assume the writer entry always existed in `_key_to_writer`. In release builds a missing writer entry could still fall through, dereference `end()`, and crash BE. ## Root Cause When `(hash, offset)` is missing from `_key_to_writer`, the current finalize path moves and erases through an invalid iterator. The target branch already uses the newer `finalize(key, size)` signature, so this PR ports the hardening change to that path. ## Solution - return `InternalError` immediately when finalize cannot find the writer entry - include hash, offset, cache type, and expiration time in the error message for diagnosis - keep the normal finalize path unchanged when the writer exists - rely on the existing `FileBlock::set_downloaded()` cleanup path to abort the pending cache write and reset block state ## Tests - `BlockFileCacheTest.fs_file_cache_storage_finalize_missing_writer_returns_error` - `DORIS_TOOLCHAIN=clang DISABLE_BE_JAVA_EXTENSIONS=ON ENABLE_INJECTION_POINT=ON ENABLE_CACHE_LOCK_DEBUG=0 ENABLE_PCH=0 sh run-be-ut.sh --clean --run --filter=BlockFileCacheTest.fs_file_cache_storage_finalize_missing_writer_returns_error -j 39` - Source patch: `selectdb/selectdb-core#8321`
github-actions Bot
pushed a commit
that referenced
this pull request
May 12, 2026
…er (#62389) ## Problem `FSFileCacheStorage::finalize()` relied on `DCHECK` to assume the writer entry always existed in `_key_to_writer`. In release builds a missing writer entry could still fall through, dereference `end()`, and crash BE. ## Root Cause When `(hash, offset)` is missing from `_key_to_writer`, the current finalize path moves and erases through an invalid iterator. The target branch already uses the newer `finalize(key, size)` signature, so this PR ports the hardening change to that path. ## Solution - return `InternalError` immediately when finalize cannot find the writer entry - include hash, offset, cache type, and expiration time in the error message for diagnosis - keep the normal finalize path unchanged when the writer exists - rely on the existing `FileBlock::set_downloaded()` cleanup path to abort the pending cache write and reset block state ## Tests - `BlockFileCacheTest.fs_file_cache_storage_finalize_missing_writer_returns_error` - `DORIS_TOOLCHAIN=clang DISABLE_BE_JAVA_EXTENSIONS=ON ENABLE_INJECTION_POINT=ON ENABLE_CACHE_LOCK_DEBUG=0 ENABLE_PCH=0 sh run-be-ut.sh --clean --run --filter=BlockFileCacheTest.fs_file_cache_storage_finalize_missing_writer_returns_error -j 39` - Source patch: `selectdb/selectdb-core#8321`
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
5 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.
Problem
FSFileCacheStorage::finalize()relied onDCHECKto assume the writer entry always existed in_key_to_writer. In release builds a missing writer entry could still fall through, dereferenceend(), and crash BE.Root Cause
When
(hash, offset)is missing from_key_to_writer, the current finalize path moves and erases through an invalid iterator. The target branch already uses the newerfinalize(key, size)signature, so this PR ports the hardening change to that path.Solution
InternalErrorimmediately when finalize cannot find the writer entryFileBlock::set_downloaded()cleanup path to abort the pending cache write and reset block stateTests
BlockFileCacheTest.fs_file_cache_storage_finalize_missing_writer_returns_errorDORIS_TOOLCHAIN=clang DISABLE_BE_JAVA_EXTENSIONS=ON ENABLE_INJECTION_POINT=ON ENABLE_CACHE_LOCK_DEBUG=0 ENABLE_PCH=0 sh run-be-ut.sh --clean --run --filter=BlockFileCacheTest.fs_file_cache_storage_finalize_missing_writer_returns_error -j 39selectdb/selectdb-core#8321