enhance: adapt segcore to knowhere iterator error-code interface (#51017)

This commit is contained in:
foxspy
2026-07-18 10:44:30 +08:00
committed by GitHub
parent 5def5ced4a
commit a4437e7daa
7 changed files with 91 additions and 39 deletions
+29 -7
View File
@@ -28,6 +28,7 @@
#include <NamedType/named_type.hpp>
#include "common/BitsetView.h"
#include "common/EasyAssert.h"
#include "common/FieldMeta.h"
#include "common/ArrayOffsets.h"
#include "common/OffsetMapping.h"
@@ -178,10 +179,18 @@ class ChunkMergeIterator : public VectorIterator {
if (!heap_.empty()) {
auto top = heap_.top();
heap_.pop();
if (iterators_[top->GetIteratorIdx()]->HasNext()) {
auto origin_pair = iterators_[top->GetIteratorIdx()]->Next();
auto& iter = iterators_[top->GetIteratorIdx()];
auto has_next = iter->HasNext();
AssertInfo(has_next.has_value(),
"knowhere iterator HasNext failed: {}",
has_next.what());
if (has_next.value()) {
auto origin_pair = iter->Next();
AssertInfo(origin_pair.has_value(),
"knowhere iterator Next failed: {}",
origin_pair.what());
auto off_dis_pair = std::make_shared<OffsetDisPair>(
origin_pair, top->GetIteratorIdx());
origin_pair.value(), top->GetIteratorIdx());
heap_.push(off_dis_pair);
}
auto result = top->GetOffDis();
@@ -205,12 +214,25 @@ class ChunkMergeIterator : public VectorIterator {
void
seal() {
sealed = true;
int idx = 0;
for (auto& iter : iterators_) {
if (iter->HasNext()) {
// idx must track each chunk's position in iterators_, because Next()
// refills from iterators_[OffsetDisPair::GetIteratorIdx()]. Incrementing
// only for non-empty iterators would misalign the index once a leading
// iterator is empty (e.g. emptied by a bitset filter), stamping later
// chunks with the wrong iterator and silently dropping their remaining
// results from the merged top-k.
for (int idx = 0; idx < static_cast<int>(iterators_.size()); ++idx) {
auto& iter = iterators_[idx];
auto has_next = iter->HasNext();
AssertInfo(has_next.has_value(),
"knowhere iterator HasNext failed: {}",
has_next.what());
if (has_next.value()) {
auto origin_pair = iter->Next();
AssertInfo(origin_pair.has_value(),
"knowhere iterator Next failed: {}",
origin_pair.what());
auto off_dis_pair =
std::make_shared<OffsetDisPair>(origin_pair, idx++);
std::make_shared<OffsetDisPair>(origin_pair.value(), idx);
heap_.push(off_dis_pair);
}
}
+7 -5
View File
@@ -83,13 +83,15 @@ struct EmptyEmbListState {
class DiskEmptyVectorIterator : public knowhere::IndexNode::iterator {
public:
std::pair<int64_t, float>
Next() override {
throw std::runtime_error("empty vector iterator has no next result");
knowhere::expected<std::pair<int64_t, float>>
Next() noexcept override {
return knowhere::expected<std::pair<int64_t, float>>::Err(
knowhere::Status::knowhere_inner_error,
"empty vector iterator has no next result");
}
bool
HasNext() override {
knowhere::expected<bool>
HasNext() noexcept override {
return false;
}
};
+7 -5
View File
@@ -91,13 +91,15 @@ struct EmptyEmbListState {
class EmptyVectorIterator : public knowhere::IndexNode::iterator {
public:
std::pair<int64_t, float>
Next() override {
throw std::runtime_error("empty vector iterator has no next result");
knowhere::expected<std::pair<int64_t, float>>
Next() noexcept override {
return knowhere::expected<std::pair<int64_t, float>>::Err(
knowhere::Status::knowhere_inner_error,
"empty vector iterator has no next result");
}
bool
HasNext() override {
knowhere::expected<bool>
HasNext() noexcept override {
return false;
}
};
@@ -278,8 +278,18 @@ CachedSearchIterator::GetNextValidResult(
const std::optional<float>& radius,
const std::optional<float>& range_filter) {
auto& iterator = iterators_[iterator_idx];
while (iterator->HasNext()) {
auto result = ConvertIteratorResult(iterator->Next());
while (true) {
auto has_next = iterator->HasNext();
AssertInfo(has_next.has_value(),
"knowhere iterator HasNext failed: {}",
has_next.what());
if (!has_next.value()) {
break;
}
auto next = iterator->Next();
AssertInfo(
next.has_value(), "knowhere iterator Next failed: {}", next.what());
auto result = ConvertIteratorResult(next.value());
if (IsValid(result, last_bound, radius, range_filter)) {
return result;
}
@@ -342,8 +352,19 @@ CachedSearchIterator::GetBatchedNextResults(size_t query_idx,
if (num_chunks_ == 1) {
auto& iterator = iterators_[query_idx];
while (iterator->HasNext() && rst.size() < batch_size_) {
auto result = ConvertIteratorResult(iterator->Next());
while (rst.size() < batch_size_) {
auto has_next = iterator->HasNext();
AssertInfo(has_next.has_value(),
"knowhere iterator HasNext failed: {}",
has_next.what());
if (!has_next.value()) {
break;
}
auto next = iterator->Next();
AssertInfo(next.has_value(),
"knowhere iterator Next failed: {}",
next.what());
auto result = ConvertIteratorResult(next.value());
if (IsValid(result, last_bound, radius, range_filter)) {
rst.emplace_back(result);
}
@@ -184,8 +184,8 @@ class TestSparseFloatSearchBruteForce : public ::testing::Test {
auto q = *(query.get() + i);
auto last_dis = std::numeric_limits<float>::max();
// we should see strict decreasing distances for brute force iterator.
while (it->HasNext()) {
auto [offset, dis] = it->Next();
while (it->HasNext().value()) {
auto [offset, dis] = it->Next().value();
ASSERT_LE(dis, last_dis);
last_dis = dis;
ASSERT_FLOAT_EQ(dis, base[offset].dot(q));
+4 -1
View File
@@ -14,7 +14,10 @@
# Update KNOWHERE_VERSION for the first occurrence
milvus_add_pkg_config("knowhere")
set_property(DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} PROPERTY INCLUDE_DIRECTORIES "")
set( KNOWHERE_VERSION 49507ef3 )
# zilliztech/knowhere#1699 (iterator error-code interface) that this segcore
# adaptation depends on has landed on knowhere main and is included in the
# v3.0.6 release tag, so pin to the tagged release instead of a raw commit.
set( KNOWHERE_VERSION v3.0.6 )
set( GIT_REPOSITORY "https://github.com/zilliztech/knowhere.git")
message(STATUS "Knowhere repo: ${GIT_REPOSITORY}")
+17 -15
View File
@@ -535,9 +535,9 @@ TEST(Indexing, Iterator) {
ASSERT_TRUE(kw_iterators.has_value());
ASSERT_EQ(kw_iterators.value().size(), 1);
auto iterator = kw_iterators.value()[0];
ASSERT_TRUE(iterator->HasNext());
while (iterator->HasNext()) {
auto [off, dis] = iterator->Next();
ASSERT_TRUE(iterator->HasNext().value());
while (iterator->HasNext().value()) {
auto [off, dis] = iterator->Next().value();
ASSERT_TRUE(off >= 0);
ASSERT_TRUE(dis >= 0);
}
@@ -940,8 +940,8 @@ TEST(Indexing, HnswEmbListBuildAllNullNullableFromBinlog) {
iterator_dataset, iterator_conf, nullptr);
ASSERT_TRUE(iterators.has_value()) << iterators.what();
ASSERT_EQ(iterators.value().size(), 2);
EXPECT_FALSE(iterators.value()[0]->HasNext());
EXPECT_FALSE(iterators.value()[1]->HasNext());
EXPECT_FALSE(iterators.value()[0]->HasNext().value());
EXPECT_FALSE(iterators.value()[1]->HasNext().value());
std::vector<float> emb_list_iterator_queries(3 * dim, 0.1F);
auto emb_list_iterator_dataset =
@@ -955,9 +955,10 @@ TEST(Indexing, HnswEmbListBuildAllNullNullableFromBinlog) {
emb_list_iterator_dataset, iterator_conf, nullptr);
ASSERT_TRUE(emb_list_iterators.has_value()) << emb_list_iterators.what();
ASSERT_EQ(emb_list_iterators.value().size(), 4);
EXPECT_TRUE(std::all_of(emb_list_iterators.value().begin(),
emb_list_iterators.value().end(),
[](const auto& iter) { return !iter->HasNext(); }));
EXPECT_TRUE(
std::all_of(emb_list_iterators.value().begin(),
emb_list_iterators.value().end(),
[](const auto& iter) { return !iter->HasNext().value(); }));
milvus::SearchInfo iterator_search_info;
iterator_search_info.topk_ = 3;
@@ -1117,8 +1118,8 @@ TEST(Indexing, HnswEmbListBuildAllValidEmptyListsFromBinlog) {
iterator_dataset, iterator_conf, nullptr);
ASSERT_TRUE(iterators.has_value()) << iterators.what();
ASSERT_EQ(iterators.value().size(), 2);
EXPECT_FALSE(iterators.value()[0]->HasNext());
EXPECT_FALSE(iterators.value()[1]->HasNext());
EXPECT_FALSE(iterators.value()[0]->HasNext().value());
EXPECT_FALSE(iterators.value()[1]->HasNext().value());
std::vector<int64_t> ids = {0, 3, 7};
auto ids_ds = GenIdsDataset(ids.size(), ids.data());
@@ -1143,9 +1144,10 @@ TEST(Indexing, HnswEmbListBuildAllValidEmptyListsFromBinlog) {
emb_list_iterator_dataset, emb_list_iterator_conf, nullptr);
ASSERT_TRUE(emb_list_iterators.has_value()) << emb_list_iterators.what();
ASSERT_EQ(emb_list_iterators.value().size(), 4);
EXPECT_TRUE(std::all_of(emb_list_iterators.value().begin(),
emb_list_iterators.value().end(),
[](const auto& iter) { return !iter->HasNext(); }));
EXPECT_TRUE(
std::all_of(emb_list_iterators.value().begin(),
emb_list_iterators.value().end(),
[](const auto& iter) { return !iter->HasNext().value(); }));
std::vector<float> query(dim, 0.1F);
auto xq_dataset = knowhere::GenDataSet(1, dim, query.data());
@@ -1834,8 +1836,8 @@ TEST(Indexing, DiskAnnEmbListBuildAllNullNullableFromBinlog) {
iterator_dataset, iterator_conf, nullptr);
ASSERT_TRUE(iterators.has_value()) << iterators.what();
ASSERT_EQ(iterators.value().size(), 2);
EXPECT_FALSE(iterators.value()[0]->HasNext());
EXPECT_FALSE(iterators.value()[1]->HasNext());
EXPECT_FALSE(iterators.value()[0]->HasNext().value());
EXPECT_FALSE(iterators.value()[1]->HasNext().value());
milvus::SearchInfo search_info;
search_info.topk_ = 3;