Skip to content

branch-4.1: [fix](be) Read instead of recursing when the ANN IVF list cache is absent #67024 - #67057

Merged
yiguolei merged 1 commit into
branch-4.1from
auto-pick-67024-branch-4.1
Aug 25, 2026
Merged

branch-4.1: [fix](be) Read instead of recursing when the ANN IVF list cache is absent #67024#67057
yiguolei merged 1 commit into
branch-4.1from
auto-pick-67024-branch-4.1

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

Cherry-picked from #67024

…sent (#67024)
### What problem does this PR solve?
Issue Number: None
Related PR: #61160
Problem Summary: An IVF-on-disk search with no `AnnIndexIVFListCache`
installed never returns — it recurses until the stack is exhausted.
**Reproduction**: build an `IVF_ON_DISK` index, save it, load it, and
search it without calling `AnnIndexIVFListCache::create_global_cache()`
first. Under ASAN the process dies with `AddressSanitizer:
stack-overflow`; on a large stack it simply spins.
**Root cause**: `CachedRandomAccessReader::borrow()` falls back to
`RandomAccessReader::borrow()` when `AnnIndexIVFListCache::instance()`
is null. faiss documents that base implementation as *"allocates a
buffer and calls read_at()"*, and `CachedRandomAccessReader::read_at()`
is overridden to call `borrow()` — so the two call each other without
bound:
```
CachedRandomAccessReader::borrow (no cache)
-> faiss::RandomAccessReader::borrow (base: allocate + read_at)
-> CachedRandomAccessReader::read_at (override: calls borrow)
-> CachedRandomAccessReader::borrow
-> ...
```
The branch reads like a fallback but cannot work at all.
**Reachability, stated plainly**: `exec_env_init` installs the cache
unconditionally at BE startup, so a running BE does not hit this today.
What it does hit is unit tests. Every existing IVF-on-disk case in
`faiss_vector_index_test.cpp` opens with
`AnnIndexIVFListCache::create_global_cache(...)`, and a new test that
does not know to do so gets a stack overflow rather than a diagnosable
failure — which is how this was found, while writing an unrelated ANN
container test. It also becomes live the moment the cache is made
optional or its lifetime is shortened.
**Fix**: the no-cache path reads the region itself under `_io_mutex` and
returns an owning `ReadRef`. No cache means no cache accounting, so it
uses a plain buffer rather than a `DataPage`; the buffer lives exactly
as long as the ref the caller holds.
### Release note
None
@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@hello-stephen

Copy link
Copy Markdown
Contributor

run buildall

@yiguolei

Copy link
Copy Markdown
Contributor

skip buildall

@hello-stephen

Copy link
Copy Markdown
Contributor

BE UT Coverage Report

Increment line coverage 100.00% (10/10) 🎉

Increment coverage report
Complete coverage report

CategoryCoverage
Function Coverage59.30% (25584/43144)
Line Coverage43.81% (261403/596630)
Region Coverage39.76% (207054/520706)
Branch Coverage41.24% (95492/231537)

@hello-stephen

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 100.00% (10/10) 🎉

Increment coverage report
Complete coverage report

CategoryCoverage
Function Coverage74.08% (31141/42036)
Line Coverage58.29% (345182/592137)
Region Coverage55.05% (287335/521984)
Branch Coverage55.98% (129455/231271)

@yiguolei
yiguolei merged commit 05fafcc into branch-4.1Aug 25, 2026
33 of 35 checks passed
@github-actions

Copy link
Copy Markdown
ContributorAuthor

PR approved by anyone and no changes requested.

@github-actionsgithub-actionsBot added reviewed approved Indicates a PR has been approved by one committer. labels Aug 25, 2026
@github-actions

Copy link
Copy Markdown
ContributorAuthor

PR approved by at least one committer and no changes requested.

@morrySnow
morrySnow deleted the auto-pick-67024-branch-4.1 branch August 31, 2026 07:11
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approvedIndicates a PR has been approved by one committer.reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@hello-stephen@yiguolei@airborne12