github-actions[bot] commented on code in PR #60850:
URL: https://github.com/apache/doris/pull/60850#discussion_r3231894612
##########
be/src/storage/index/ann/ann_index_reader.cpp:
##########
@@ -117,57 +135,97 @@ Status AnnIndexReader::query(io::IOContext* io_ctx,
AnnTopNParam* param, AnnInde
const float* query_vec = param->query_value;
const int limit = static_cast<int>(param->limit);
IndexSearchResult index_search_result;
- if (_index_type == AnnIndexType::HNSW) {
- HNSWSearchParameters hnsw_search_params;
- hnsw_search_params.roaring = param->roaring;
- hnsw_search_params.rows_of_segment = param->rows_of_segment;
- hnsw_search_params.io_ctx = io_ctx;
- hnsw_search_params.ef_search = param->_user_params.hnsw_ef_search;
- hnsw_search_params.check_relative_distance =
- param->_user_params.hnsw_check_relative_distance;
- hnsw_search_params.bounded_queue =
param->_user_params.hnsw_bounded_queue;
- RETURN_IF_ERROR(_vector_index->ann_topn_search(query_vec, limit,
hnsw_search_params,
-
index_search_result));
- // Accumulate detailed engine timings
-
stats->engine_search_ns.update(index_search_result.engine_search_ns);
-
stats->engine_convert_ns.update(index_search_result.engine_convert_ns);
-
stats->engine_prepare_ns.update(index_search_result.engine_prepare_ns);
- } else if (_index_type == AnnIndexType::IVF || _index_type ==
AnnIndexType::IVF_ON_DISK) {
- IVFSearchParameters ivf_search_params;
- ivf_search_params.roaring = param->roaring;
- ivf_search_params.rows_of_segment = param->rows_of_segment;
- ivf_search_params.io_ctx = io_ctx;
- ivf_search_params.nprobe = param->_user_params.ivf_nprobe;
- RETURN_IF_ERROR(_vector_index->ann_topn_search(query_vec, limit,
ivf_search_params,
-
index_search_result));
- // Accumulate detailed engine timings
-
stats->engine_search_ns.update(index_search_result.engine_search_ns);
-
stats->engine_convert_ns.update(index_search_result.engine_convert_ns);
-
stats->engine_prepare_ns.update(index_search_result.engine_prepare_ns);
- if (_index_type == AnnIndexType::IVF_ON_DISK) {
- stats->ivf_on_disk_cache_hit_cnt.update(
- index_search_result.ivf_on_disk_cache_hit_cnt);
- stats->ivf_on_disk_cache_miss_cnt.update(
- index_search_result.ivf_on_disk_cache_miss_cnt);
-
DorisMetrics::instance()->ann_ivf_on_disk_cache_hit_cnt->increment(
- index_search_result.ivf_on_disk_cache_hit_cnt);
-
DorisMetrics::instance()->ann_ivf_on_disk_cache_miss_cnt->increment(
- index_search_result.ivf_on_disk_cache_miss_cnt);
+
+ {
+ AnnIndexResultCache* topn_result_cache =
Review Comment:
This new call uses `ExecEnv::GetInstance()`, but this translation unit only
has the forward declaration from `runtime_state.h`; it does not include
`runtime/exec_env.h`, which declares the static method. This will fail to
compile with an incomplete `ExecEnv` type once this file is built. Please add
the proper `#include "runtime/exec_env.h"` before using `ExecEnv` here.
##########
be/test/storage/index/ann/ann_index_result_cache_test.cpp:
##########
@@ -0,0 +1,769 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "storage/index/ann/ann_index_result_cache/ann_index_result_cache.h"
+
+#include <gtest/gtest.h>
+
+#include <memory>
+#include <roaring/roaring.hh>
+#include <vector>
+
+#include
"storage/index/ann/ann_index_result_cache/ann_index_result_cache_handle.h"
+#include "storage/index/ann/ann_search_params.h"
+#include "storage/index/ann/vector_search_utils.h"
+
+namespace doris::segment_v2 {
+
+// ============================================================================
+// Helper to build AnnTopNParam for cache key tests
+// ============================================================================
+static AnnTopNParam make_topn_param(const std::vector<float>& vec, size_t
limit,
+ roaring::Roaring* bitmap, size_t
rows_of_segment,
+ VectorSearchUserParams user_params = {}) {
+ AnnTopNParam p {
+ .query_value = vec.data(),
+ .query_value_size = vec.size(),
+ .limit = limit,
+ ._user_params = user_params,
+ .roaring = bitmap,
+ .rows_of_segment = rows_of_segment,
+ };
+ return p;
+}
+
+// ============================================================================
+// TopN Cache Key Tests
+// ============================================================================
+class TopnCacheKeyTest : public ::testing::Test {
+protected:
+ void SetUp() override {
+ cache_ = std::make_unique<AnnIndexResultCache>(/*capacity=*/64 * 1024
* 1024,
+ /*shards=*/1);
Review Comment:
`AnnIndexResultCache(size_t, uint32_t)` is declared in the private section
of `ann_index_result_cache.h`, so this test cannot instantiate it with
`std::make_unique`. The same pattern appears in the other fixtures in this
file, so the new BE unit test target will fail to compile. Please expose a
test-only factory/friend the fixture, or make the constructor accessible in a
controlled way for tests.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
