This is an automated email from the ASF dual-hosted git repository.
airborne12 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 76cbb6bf790 [fix](be) Read instead of recursing when the ANN IVF list
cache is absent (#67024)
76cbb6bf790 is described below
commit 76cbb6bf7908b0d89833e22ec47d0e6246c7daf0
Author: Jack <[email protected]>
AuthorDate: Mon Aug 24 14:35:14 2026 +0800
[fix](be) Read instead of recursing when the ANN IVF list cache is absent
(#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
---
be/src/storage/index/ann/faiss_ann_index.cpp | 24 ++++++-
.../storage/index/ann/faiss_vector_index_test.cpp | 76 ++++++++++++++++++++++
2 files changed, 99 insertions(+), 1 deletion(-)
diff --git a/be/src/storage/index/ann/faiss_ann_index.cpp
b/be/src/storage/index/ann/faiss_ann_index.cpp
index f143b7ec6a5..821bba40397 100644
--- a/be/src/storage/index/ann/faiss_ann_index.cpp
+++ b/be/src/storage/index/ann/faiss_ann_index.cpp
@@ -368,7 +368,18 @@ struct CachedRandomAccessReader :
faiss::RandomAccessReader {
auto* cache = AnnIndexIVFListCache::instance();
if (!cache) {
- return RandomAccessReader::borrow(offset, nbytes);
+ // Read it ourselves. NOT RandomAccessReader::borrow(): faiss
documents
+ // its default as "allocates a buffer and calls read_at()", and
read_at()
+ // below is implemented by calling borrow() -- so delegating to
the base
+ // is unbounded mutual recursion, never a fallback. No cache means
no
+ // cache accounting either, hence a plain owning buffer rather
than a
+ // DataPage; it lives only until the caller drops the ref.
+ std::vector<uint8_t> bytes(nbytes);
+ {
+ std::lock_guard<std::mutex> lock(_io_mutex);
+ _read_clucene(offset, reinterpret_cast<char*>(bytes.data()),
nbytes);
+ }
+ return std::make_unique<OwnedReadRef>(std::move(bytes));
}
AnnIndexIVFListCache::CacheKey key(_cache_key_prefix, _file_size,
@@ -408,6 +419,17 @@ struct CachedRandomAccessReader :
faiss::RandomAccessReader {
private:
// ---- ReadRef that pins a cache entry ----
+ // Owns the bytes it serves. Used when there is no list cache to pin a
page in.
+ struct OwnedReadRef : faiss::ReadRef {
+ explicit OwnedReadRef(std::vector<uint8_t> bytes) :
_bytes(std::move(bytes)) {
+ data_ = _bytes.data();
+ size_ = _bytes.size();
+ }
+
+ private:
+ std::vector<uint8_t> _bytes;
+ };
+
struct PinnedReadRef : faiss::ReadRef {
explicit PinnedReadRef(PageCacheHandle handle, const uint8_t* ptr,
size_t len)
: _handle(std::move(handle)) {
diff --git a/be/test/storage/index/ann/faiss_vector_index_test.cpp
b/be/test/storage/index/ann/faiss_vector_index_test.cpp
index d60d83ecb17..370cd33e0c1 100644
--- a/be/test/storage/index/ann/faiss_vector_index_test.cpp
+++ b/be/test/storage/index/ann/faiss_vector_index_test.cpp
@@ -25,6 +25,7 @@
#include <barrier>
#include <chrono>
#include <cstddef>
+#include <future>
#include <limits>
#include <memory>
#include <random>
@@ -1465,6 +1466,81 @@ TEST_F(VectorSearchTest, IVFOnDiskSaveLoadAndSearch) {
EXPECT_GT(result.ivf_on_disk_cache_miss_cnt, 0);
}
+// Searching WITHOUT a list cache has to read, not recurse.
+//
+// faiss documents RandomAccessReader::borrow()'s default as "allocates a
buffer
+// and calls read_at()", and CachedRandomAccessReader::read_at() is
implemented by
+// calling borrow(). Falling back to the base borrow() when
+// AnnIndexIVFListCache::instance() is null is therefore unbounded mutual
+// recursion. Every other IVF-on-disk test above installs the cache first,
which
+// is exactly why nothing has ever hit it.
+//
+// exec_env_init installs the cache unconditionally at BE startup, so a
running BE
+// does not reach this today. It is a trap for any unit test that does not
know to
+// install one -- it does not fail, it never returns -- and for any change that
+// makes the cache optional.
+//
+// The search runs on a worker with a deadline because before the fix it does
not
+// return at all: called inline it would hang the whole suite instead of
failing
+// this one case. Everything the worker touches is shared-owned so a hung
thread
+// cannot outlive its arguments.
+TEST_F(VectorSearchTest, IVFOnDiskSearchWithoutTheListCacheStillReads) {
+ ASSERT_EQ(AnnIndexIVFListCache::instance(), nullptr)
+ << "this case is about the no-cache path, but something installed
a cache";
+
+ auto builder = std::make_unique<FaissVectorIndex>();
+ FaissBuildParameter params;
+ params.dim = 32;
+ params.ivf_nlist = 4;
+ params.quantizer = FaissBuildParameter::Quantizer::FLAT;
+ params.index_type = FaissBuildParameter::IndexType::IVF_ON_DISK;
+ builder->build(params);
+
+ const int num_vectors = 200;
+ std::vector<float> vectors;
+ vectors.reserve(static_cast<size_t>(num_vectors) * params.dim);
+ for (int i = 0; i < num_vectors; i++) {
+ auto tmp = vector_search_utils::generate_random_vector(params.dim);
+ vectors.insert(vectors.end(), tmp.begin(), tmp.end());
+ }
+ ASSERT_TRUE(builder->train(num_vectors, vectors.data()).ok());
+ ASSERT_TRUE(builder->add(num_vectors, vectors.data()).ok());
+
+ auto dir = std::make_shared<lucene::store::RAMDirectory>();
+ ASSERT_TRUE(builder->save(dir.get()).ok());
+
+ auto index = std::make_shared<FaissVectorIndex>();
+ index->set_type(AnnIndexType::IVF_ON_DISK);
+ index->set_ivfdata_cache_key_prefix("ut_no_list_cache");
+ ASSERT_TRUE(index->load(dir.get()).ok());
+
+ auto query_vec = std::make_shared<std::vector<float>>(
+ vector_search_utils::generate_random_vector(params.dim));
+ auto roaring = std::make_shared<roaring::Roaring>();
+ for (int i = 0; i < num_vectors; ++i) {
+ roaring->add(i);
+ }
+ auto result = std::make_shared<IndexSearchResult>();
+
+ auto done = std::make_shared<std::promise<Status>>();
+ auto ready = done->get_future();
+ std::thread worker([index, dir, query_vec, roaring, result, done,
num_vectors] {
+ IVFSearchParameters search_params;
+ search_params.nprobe = 4;
+ search_params.roaring = roaring.get();
+ search_params.rows_of_segment = num_vectors;
+ done->set_value(index->ann_topn_search(query_vec->data(), 10,
search_params, *result));
+ });
+ const bool returned = ready.wait_for(std::chrono::seconds(60)) ==
std::future_status::ready;
+ if (!returned) {
+ worker.detach();
+ FAIL() << "the search never returned: borrow() recurses when there is
no list cache";
+ }
+ worker.join();
+ ASSERT_TRUE(ready.get().ok());
+ EXPECT_GT(result->roaring->cardinality(), 0U);
+}
+
// All threads share a single FaissVectorIndex (and thus a single
// CachedRandomAccessReader with its _io_mutex). On the first round
// (cold cache) threads contend on the same _io_mutex; the first thread
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
