This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-4.1
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-4.1 by this push:
new 05fafcc07ab branch-4.1: [fix](be) Read instead of recursing when the
ANN IVF list cache is absent #67024 (#67057)
05fafcc07ab is described below
commit 05fafcc07ab32c77f07f973bfd755d0a25799f85
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Tue Aug 25 09:47:41 2026 +0800
branch-4.1: [fix](be) Read instead of recursing when the ANN IVF list cache
is absent #67024 (#67057)
Cherry-picked from #67024
Co-authored-by: Jack <[email protected]>
---
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 2c4e17d6f3f..34a78dc4c56 100644
--- a/be/src/storage/index/ann/faiss_ann_index.cpp
+++ b/be/src/storage/index/ann/faiss_ann_index.cpp
@@ -345,7 +345,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,
@@ -385,6 +396,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 60c89951cb5..282d3463ce9 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>
@@ -1461,6 +1462,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]
