diff --git a/CMakeLists.txt b/CMakeLists.txt index 99de33262..9b581274b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -136,7 +136,27 @@ endif() include(cmake/utils/platform_check.cmake) include(cmake/utils/compile_flags.cmake) -find_package(milvus-common REQUIRED) +# milvus-common can be provided two ways: +# - standalone knowhere build: Conan supplies it and find_package() defines the +# imported target milvus-common::milvus-common. +# - embedded in Milvus: Milvus adds milvus-common via add_subdirectory() BEFORE +# knowhere, creating a plain in-tree `milvus-common` target. In that case +# find_package() would fail (no installed config file), so reuse the existing +# target and alias it to the namespaced name the rest of the tree links to. +if(NOT TARGET milvus-common::milvus-common) + if(TARGET milvus-common) + add_library(milvus-common::milvus-common ALIAS milvus-common) + # In-tree milvus-common only uses directory-scoped include_directories() and + # does not export PUBLIC/INTERFACE include dirs, so linking the alias alone + # does not make filemanager/FileManager.h visible to knowhere. Milvus sets + # MILVUS_COMMON_INCLUDE_DIR when it FetchContent-populates milvus-common. + if(DEFINED MILVUS_COMMON_INCLUDE_DIR) + include_directories(${MILVUS_COMMON_INCLUDE_DIR}) + endif() + else() + find_package(milvus-common REQUIRED) + endif() +endif() include(cmake/libs/libhnsw.cmake) include(cmake/libs/libfaiss.cmake) diff --git a/cmake/libs/libfaiss.cmake b/cmake/libs/libfaiss.cmake index 056b530e6..173f970b9 100644 --- a/cmake/libs/libfaiss.cmake +++ b/cmake/libs/libfaiss.cmake @@ -355,9 +355,18 @@ if(APPLE) find_package(LAPACK REQUIRED) find_package(BLAS REQUIRED) else() - find_package(OpenBLAS CONFIG REQUIRED) - set(BLAS_LIBRARIES OpenBLAS::OpenBLAS) - set(LAPACK_LIBRARIES OpenBLAS::OpenBLAS) + # Prefer OpenBLAS's CMake config package when present, but fall back to + # module-mode BLAS/LAPACK discovery so builds work on environments that ship + # libopenblas without OpenBLASConfig.cmake. + find_package(OpenBLAS CONFIG QUIET) + if(OpenBLAS_FOUND) + set(BLAS_LIBRARIES OpenBLAS::OpenBLAS) + set(LAPACK_LIBRARIES OpenBLAS::OpenBLAS) + else() + set(BLA_VENDOR OpenBLAS) + find_package(BLAS REQUIRED) + find_package(LAPACK REQUIRED) + endif() endif() find_package(xxHash REQUIRED) diff --git a/src/common/tracer.cc b/src/common/tracer.cc index 53b912087..98f5a58ca 100644 --- a/src/common/tracer.cc +++ b/src/common/tracer.cc @@ -122,9 +122,10 @@ StartSpan(const std::string& name, TraceContext* ctx) { return noop_trace_provider->GetTracer("noop")->StartSpan("noop"); } - opts.parent = trace::SpanContext(trace::TraceId({ctx->traceID, trace::TraceId::kSize}), - trace::SpanId({ctx->spanID, trace::SpanId::kSize}), - trace::TraceFlags(ctx->traceFlags), true); + opts.parent = trace::SpanContext( + trace::TraceId(nostd::span{ctx->traceID, trace::TraceId::kSize}), + trace::SpanId(nostd::span{ctx->spanID, trace::SpanId::kSize}), + trace::TraceFlags(ctx->traceFlags), true); } return GetTracer()->StartSpan(name, opts); } diff --git a/src/index/hnsw/base_hnsw_config.h b/src/index/hnsw/base_hnsw_config.h index 47bd8605a..d18c67a0e 100644 --- a/src/index/hnsw/base_hnsw_config.h +++ b/src/index/hnsw/base_hnsw_config.h @@ -33,6 +33,9 @@ class BaseHnswConfig : public BaseConfig { CFG_INT overview_levels; CFG_BOOL disable_fallback_brute_force; // default is false, means we will use fallback brute force when hnsw search // does not get enough topk results + CFG_BOOL force_brute_force; // default is false, when true the search bypasses graph traversal and + // scans the index storage exhaustively (e.g. quantized codes for + // HNSW_SQ/PQ), optionally refined when the index was built with refine. KNOWHERE_DECLARE_CONFIG(BaseHnswConfig) { KNOWHERE_CONFIG_DECLARE_FIELD(M).description("hnsw M").set_default(30).set_range(2, 2048).for_train(); KNOWHERE_CONFIG_DECLARE_FIELD(efConstruction) @@ -56,6 +59,11 @@ class BaseHnswConfig : public BaseConfig { .description("disable fallback brute force") .set_default(false) .for_search(); + KNOWHERE_CONFIG_DECLARE_FIELD(force_brute_force) + .description("force exhaustive brute force search instead of graph traversal") + .set_default(false) + .for_search() + .for_range_search(); } Status diff --git a/src/index/hnsw/faiss_hnsw.cc b/src/index/hnsw/faiss_hnsw.cc index d7f48ddb4..be7ecc340 100644 --- a/src/index/hnsw/faiss_hnsw.cc +++ b/src/index/hnsw/faiss_hnsw.cc @@ -1466,6 +1466,12 @@ class BaseFaissRegularIndexHNSWNode : public BaseFaissRegularIndexNode { // wait for the completion WaitAllSuccess(futs); + + if (hnsw_cfg.force_brute_force.value_or(false)) { + LOG_KNOWHERE_INFO_ << "force_brute_force: completed exhaustive search for " << rows + << " queries (k=" << k << ", ntotal=" << indexes[index_id]->ntotal + << ", used_bf_wrapper=" << whether_bf_search.value_or(false) << ")"; + } } catch (const std::exception& e) { LOG_KNOWHERE_WARNING_ << "faiss inner error: " << e.what(); return expected::Err(Status::faiss_inner_error, e.what()); diff --git a/src/index/hnsw/impl/IndexConditionalWrapper.cc b/src/index/hnsw/impl/IndexConditionalWrapper.cc index e543d5dd9..bea048fd2 100644 --- a/src/index/hnsw/impl/IndexConditionalWrapper.cc +++ b/src/index/hnsw/impl/IndexConditionalWrapper.cc @@ -39,6 +39,14 @@ WhetherPerformBruteForceSearch(const faiss::Index* index, const BaseConfig& cfg, return std::nullopt; } + // an explicit user request to force brute force overrides the heuristics below + const auto* hnsw_cfg = dynamic_cast(&cfg); + if (hnsw_cfg != nullptr && hnsw_cfg->force_brute_force.value_or(false)) { + LOG_KNOWHERE_INFO_ << "force_brute_force enabled: bypassing HNSW graph traversal for exhaustive search (k=" + << cfg.k.value() << ", ntotal=" << index->ntotal << ")"; + return true; + } + // decide const auto k = cfg.k.value(); @@ -72,6 +80,14 @@ WhetherPerformBruteForceRangeSearch(const faiss::Index* index, const FaissHnswCo return std::nullopt; } + // an explicit user request to force brute force overrides the heuristics below + if (cfg.force_brute_force.has_value() && cfg.force_brute_force.value_or(false)) { + LOG_KNOWHERE_INFO_ + << "force_brute_force enabled: bypassing HNSW graph traversal for exhaustive range search (ef=" + << cfg.ef.value() << ", ntotal=" << index->ntotal << ")"; + return true; + } + // decide const auto ef = cfg.ef.value(); diff --git a/tests/ut/test_faiss_hnsw.cc b/tests/ut/test_faiss_hnsw.cc index bed2a043a..07e469904 100644 --- a/tests/ut/test_faiss_hnsw.cc +++ b/tests/ut/test_faiss_hnsw.cc @@ -2550,3 +2550,83 @@ TEST_CASE("HNSW RangeSearch BF path with empty bitset", "[faiss_hnsw][range_sear REQUIRE(hnsw_total > 0); } } + +// Verifies the `force_brute_force` search param bypasses HNSW graph traversal +// and performs an exhaustive scan of the index storage. For a flat HNSW the +// scan is exact, so results must match a FLAT (IDMAP) oracle exactly. For a +// quantized HNSW (HNSW_SQ) the scan runs over the quantized codes, so recall +// stays high relative to the fp32 oracle. The flag overrides the topk/filter +// heuristics in WhetherPerformBruteForceSearch, so it triggers even for a +// small topk that would otherwise take the regular graph path. +TEST_CASE("HNSW force_brute_force forces exhaustive search", "[faiss_hnsw][brute_force]") { + const int32_t nb = 2000; + const int32_t dim = 32; + const int32_t nq = 10; + const int32_t topk = 20; + const std::string metric = knowhere::metric::L2; + + const auto version = knowhere::Version::GetCurrentVersion().VersionNumber(); + + auto train_ds = GenDataSet(nb, dim, /*seed=*/42); + auto query_ds = GenDataSet(nq, dim, /*seed=*/43); + + knowhere::Json base_conf; + base_conf[knowhere::meta::METRIC_TYPE] = metric; + base_conf[knowhere::meta::DIM] = dim; + base_conf[knowhere::meta::ROWS] = nb; + base_conf[knowhere::meta::TOPK] = topk; + + // FLAT oracle: exact fp32 ground truth. + auto flat_index = knowhere::IndexFactory::Instance() + .Create(knowhere::IndexEnum::INDEX_FAISS_IDMAP, version) + .value(); + REQUIRE(flat_index.Build(train_ds, base_conf) == knowhere::Status::success); + auto golden = flat_index.Search(query_ds, base_conf, nullptr); + REQUIRE(golden.has_value()); + + // Build an HNSW index of the given type and return recall vs. the FLAT oracle. + // `ef == topk` (the minimum allowed) keeps graph-search recall imperfect so + // that forcing brute force is observably different. + auto build_and_recall = [&](const std::string& index_type, const knowhere::Json& extra_conf, bool force_bf) { + knowhere::Json conf = base_conf; + conf[knowhere::meta::INDEX_TYPE] = index_type; + conf[knowhere::indexparam::HNSW_M] = 8; + conf[knowhere::indexparam::EFCONSTRUCTION] = 100; + conf[knowhere::indexparam::EF] = topk; + for (auto it = extra_conf.begin(); it != extra_conf.end(); ++it) { + conf[it.key()] = it.value(); + } + if (force_bf) { + conf["force_brute_force"] = true; + } + auto index = knowhere::IndexFactory::Instance().Create(index_type, version).value(); + REQUIRE(index.Build(train_ds, conf) == knowhere::Status::success); + auto res = index.Search(query_ds, conf, nullptr); + REQUIRE(res.has_value()); + return GetKNNRecall(*golden.value(), *res.value()); + }; + + SECTION("flat HNSW: forced brute force is exact") { + const float recall_graph = + build_and_recall(knowhere::IndexEnum::INDEX_HNSW, knowhere::Json::object(), /*force_bf=*/false); + const float recall_forced = + build_and_recall(knowhere::IndexEnum::INDEX_HNSW, knowhere::Json::object(), /*force_bf=*/true); + INFO("graph recall=" << recall_graph << " forced recall=" << recall_forced); + + // exhaustive exact scan over the flat storage matches the FLAT oracle + REQUIRE(recall_forced == 1.0f); + // forcing brute force can only help relative to graph traversal + REQUIRE(recall_forced >= recall_graph); + } + + SECTION("quantized HNSW_SQ: forced brute force scans all codes") { + knowhere::Json sq_conf; + sq_conf[knowhere::indexparam::SQ_TYPE] = "SQ8"; + const float recall_forced = build_and_recall(knowhere::IndexEnum::INDEX_HNSW_SQ, sq_conf, /*force_bf=*/true); + INFO("forced recall=" << recall_forced); + + // exhaustive scan over the quantized codes stays high-recall vs the + // fp32 oracle (SQ8 quantization error is tiny for this data). + REQUIRE(recall_forced >= 0.9f); + } +} diff --git a/tests/ut/test_tracer.cc b/tests/ut/test_tracer.cc index 0c3cb46dd..01ffc843d 100644 --- a/tests/ut/test_tracer.cc +++ b/tests/ut/test_tracer.cc @@ -111,8 +111,11 @@ TEST_CASE("Test Tracer span", "Span test") { auto span = StartSpan("test", ctx.get()); auto spanCtx = span->GetContext(); - REQUIRE(spanCtx.trace_id() == trace::TraceId({ctx->traceID, trace::TraceId::kSize})); - // REQUIRE(spanCtx.span_id() == trace::SpanId({ctx->spanID, trace::SpanId::kSize})); + REQUIRE(spanCtx.trace_id() == trace::TraceId(opentelemetry::nostd::span{ + ctx->traceID, trace::TraceId::kSize})); + // REQUIRE(spanCtx.span_id() == + // trace::SpanId(opentelemetry::nostd::span{ + // ctx->spanID, trace::SpanId::kSize})); REQUIRE(spanCtx.trace_flags() == trace::TraceFlags(ctx->traceFlags)); auto trace_id_hex = BytesToHexStr(ctx->traceID, TraceId::kSize);