github-actions[bot] commented on code in PR #16003: URL: https://github.com/apache/doris/pull/16003#discussion_r1071937146
########## be/src/olap/rowset/segment_v2/inverted_index_cache.h: ########## @@ -0,0 +1,167 @@ +// 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. + +#pragma once + +#include <CLucene.h> + +#include <iostream> +#include <map> +#include <memory> +#include <mutex> +#include <vector> + +#include "io/fs/file_system.h" +#include "olap/lru_cache.h" +#include "runtime/memory/mem_tracker.h" +#include "util/time.h" + +namespace doris { + +namespace segment_v2 { +using IndexSearcherPtr = std::shared_ptr<lucene::search::IndexSearcher>; + +class InvertedIndexCacheHandle; + +class InvertedIndexSearcherCache { +public: + // The cache key of index_searcher lru cache + struct CacheKey { + CacheKey(std::string index_file_path) : index_file_path(index_file_path) {} + std::string index_file_path; + }; + + // The cache value of index_searcher lru cache. + // Holding a opened index_searcher. + struct CacheValue { + // Save the last visit time of this cache entry. + // Use atomic because it may be modified by multi threads. + std::atomic<int64_t> last_visit_time = 0; + IndexSearcherPtr index_searcher; + size_t size = 0; + }; + + // Create global instance of this class. + // "capacity" is the capacity of lru cache. + static void create_global_instance(size_t capacity, uint32_t num_shards = 16); + + // Return global instance. + // Client should call create_global_cache before. + static InvertedIndexSearcherCache* instance() { return _s_instance; } + + static IndexSearcherPtr build_index_searcher(const io::FileSystemSPtr& fs, + const std::string& index_dir, + const std::string& file_name); + + InvertedIndexSearcherCache(size_t capacity, uint32_t num_shards); + + Status get_index_searcher(const io::FileSystemSPtr& fs, const std::string& index_dir, + const std::string& file_name, InvertedIndexCacheHandle* cache_handle, + bool use_cache = true); + + // function `insert` called after inverted index writer close + Status insert(const io::FileSystemSPtr& fs, const std::string& index_dir, + const std::string& file_name); + + // function `erase` called after compaction remove segment + Status erase(const std::string& index_file_path); + +private: + InvertedIndexSearcherCache(); + + // Lookup the given index_searcher in the cache. + // If the index_searcher is found, the cache entry will be written into handle. + // Return true if entry is found, otherwise return false. + bool _lookup(const InvertedIndexSearcherCache::CacheKey& key, InvertedIndexCacheHandle* handle); + + // Insert a cache entry by key. + // And the cache entry will be returned in handle. + // This function is thread-safe. + Cache::Handle* _insert(const InvertedIndexSearcherCache::CacheKey& key, CacheValue* value); + +private: + static InvertedIndexSearcherCache* _s_instance; + // A LRU cache to cache all opened index_searcher + std::unique_ptr<Cache> _cache = nullptr; + std::unique_ptr<MemTracker> _mem_tracker = nullptr; +}; + +using IndexCacheValuePtr = std::unique_ptr<InvertedIndexSearcherCache::CacheValue>; + +// A handle for a index_searcher from index_searcher lru cache. +// The handle can ensure that the index_searcher is valid +// and will not be closed while the holder of the handle is accessing the index_searcher. +// The handle will automatically release the cache entry when it is destroyed. +// So the caller need to make sure the handle is valid in lifecycle. +class InvertedIndexCacheHandle { +public: + InvertedIndexCacheHandle() {} + InvertedIndexCacheHandle(Cache* cache, Cache::Handle* handle) + : _cache(cache), _handle(handle) {} + + ~InvertedIndexCacheHandle() { + if (_handle != nullptr) { + CHECK(_cache != nullptr); + CHECK(!owned); + // only after get_index_searcher call this destructor will + // add `config::index_cache_entry_stay_time_after_lookup_s` on last_visit_time, + // this is to extend the retention time of the entries hit by lookup. + ((InvertedIndexSearcherCache::CacheValue*)_cache->value(_handle))->last_visit_time = + UnixMillis() + config::index_cache_entry_stay_time_after_lookup_s * 1000; + _cache->release(_handle); + } + } + + InvertedIndexCacheHandle(InvertedIndexCacheHandle&& other) noexcept { + std::swap(_cache, other._cache); + std::swap(_handle, other._handle); + this->owned = other.owned; + this->index_searcher = std::move(other.index_searcher); + } + + InvertedIndexCacheHandle& operator=(InvertedIndexCacheHandle&& other) noexcept { + std::swap(_cache, other._cache); + std::swap(_handle, other._handle); + this->owned = other.owned; + this->index_searcher = std::move(other.index_searcher); + return *this; + } + + IndexSearcherPtr get_index_searcher() { + if (owned) { + return index_searcher; + } else { + return ((InvertedIndexSearcherCache::CacheValue*)_cache->value(_handle)) + ->index_searcher; + } + } + +public: Review Comment: warning: redundant access specifier has the same accessibility as the previous access specifier [readability-redundant-access-specifiers] ```suggestion ``` **be/src/olap/rowset/segment_v2/inverted_index_cache.h:110:** previously declared here ```cpp public: ^ ``` ########## be/test/olap/rowset/segment_v2/inverted_index_searcher_cache_test.cpp: ########## @@ -0,0 +1,199 @@ +// 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 "olap/rowset/segment_v2/inverted_index_cache.h" + +#include <gtest/gtest.h> + +#include "common/logging.h" +#include "io/fs/local_file_system.h" +#include "util/file_utils.h" +#include "util/time.h" + +#define protected public +#define private public + +#ifndef BE_TEST +#define BE_TEST +#endif + +namespace doris { +namespace segment_v2 { + +auto fs = io::global_local_filesystem(); + +class InvertedIndexSearcherCacheTest : public testing::Test { +public: + // there is set 1 shard in ShardedLRUCache + // And the LRUHandle size is about 100B. So the cache size should big enough + // to run the UT. + static const int kCacheSize = 1000; + const std::string kTestDir = "./ut_dir/invertedInvertedIndexSearcherCache::instance()_test"; + + void SetUp() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + EXPECT_TRUE(FileUtils::create_dir(kTestDir).ok()); + } + void TearDown() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + } +}; + + +TEST_F(InvertedIndexSearcherCacheTest, insert_lookup) { + InvertedIndexSearcherCache* index_searcher_cache = new InvertedIndexSearcherCache(kCacheSize, 1); + std::string file_name_1 = "test_1.idx"; + std::string file_name_2 = "test_2.idx"; + + //insert searcher + Status status = Status::OK(); + status = index_searcher_cache->insert(fs, kTestDir, file_name_1); + EXPECT_EQ(Status::OK(), status); + + status = index_searcher_cache->insert(fs, kTestDir, file_name_2); + EXPECT_EQ(Status::OK(), status); + + // lookup after insert + { + // case 1: lookup exist entry + InvertedIndexCacheHandle inverted_index_cache_handle; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_1, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_1 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); Review Comment: warning: '_handle' is a private member of 'doris::segment_v2::InvertedIndexCacheHandle' [clang-diagnostic-error] ```cpp (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); ^ ``` **be/src/olap/rowset/segment_v2/inverted_index_cache.h:159:** declared private here ```cpp Cache::Handle* _handle = nullptr; ^ ``` ########## be/test/olap/rowset/segment_v2/inverted_index_searcher_cache_test.cpp: ########## @@ -0,0 +1,199 @@ +// 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 "olap/rowset/segment_v2/inverted_index_cache.h" + +#include <gtest/gtest.h> + +#include "common/logging.h" +#include "io/fs/local_file_system.h" +#include "util/file_utils.h" +#include "util/time.h" + +#define protected public +#define private public + +#ifndef BE_TEST +#define BE_TEST +#endif + +namespace doris { +namespace segment_v2 { + +auto fs = io::global_local_filesystem(); + +class InvertedIndexSearcherCacheTest : public testing::Test { +public: + // there is set 1 shard in ShardedLRUCache + // And the LRUHandle size is about 100B. So the cache size should big enough + // to run the UT. + static const int kCacheSize = 1000; + const std::string kTestDir = "./ut_dir/invertedInvertedIndexSearcherCache::instance()_test"; + + void SetUp() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + EXPECT_TRUE(FileUtils::create_dir(kTestDir).ok()); + } + void TearDown() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + } +}; + + +TEST_F(InvertedIndexSearcherCacheTest, insert_lookup) { + InvertedIndexSearcherCache* index_searcher_cache = new InvertedIndexSearcherCache(kCacheSize, 1); + std::string file_name_1 = "test_1.idx"; + std::string file_name_2 = "test_2.idx"; + + //insert searcher + Status status = Status::OK(); + status = index_searcher_cache->insert(fs, kTestDir, file_name_1); + EXPECT_EQ(Status::OK(), status); + + status = index_searcher_cache->insert(fs, kTestDir, file_name_2); + EXPECT_EQ(Status::OK(), status); + + // lookup after insert + { + // case 1: lookup exist entry + InvertedIndexCacheHandle inverted_index_cache_handle; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_1, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_1 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); + EXPECT_GE(UnixMillis(), cache_value_1->last_visit_time); + + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_2, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_2 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); Review Comment: warning: '_cache' is a private member of 'doris::segment_v2::InvertedIndexCacheHandle' [clang-diagnostic-error] ```cpp (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); ^ ``` **be/src/olap/rowset/segment_v2/inverted_index_cache.h:158:** declared private here ```cpp Cache* _cache = nullptr; ^ ``` ########## be/test/olap/rowset/segment_v2/inverted_index_searcher_cache_test.cpp: ########## @@ -0,0 +1,199 @@ +// 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 "olap/rowset/segment_v2/inverted_index_cache.h" + +#include <gtest/gtest.h> + +#include "common/logging.h" +#include "io/fs/local_file_system.h" +#include "util/file_utils.h" +#include "util/time.h" + +#define protected public +#define private public + +#ifndef BE_TEST +#define BE_TEST +#endif + +namespace doris { +namespace segment_v2 { + +auto fs = io::global_local_filesystem(); + +class InvertedIndexSearcherCacheTest : public testing::Test { +public: + // there is set 1 shard in ShardedLRUCache + // And the LRUHandle size is about 100B. So the cache size should big enough + // to run the UT. + static const int kCacheSize = 1000; + const std::string kTestDir = "./ut_dir/invertedInvertedIndexSearcherCache::instance()_test"; + + void SetUp() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + EXPECT_TRUE(FileUtils::create_dir(kTestDir).ok()); + } + void TearDown() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + } +}; + + +TEST_F(InvertedIndexSearcherCacheTest, insert_lookup) { + InvertedIndexSearcherCache* index_searcher_cache = new InvertedIndexSearcherCache(kCacheSize, 1); + std::string file_name_1 = "test_1.idx"; + std::string file_name_2 = "test_2.idx"; + + //insert searcher + Status status = Status::OK(); + status = index_searcher_cache->insert(fs, kTestDir, file_name_1); + EXPECT_EQ(Status::OK(), status); + + status = index_searcher_cache->insert(fs, kTestDir, file_name_2); + EXPECT_EQ(Status::OK(), status); + + // lookup after insert + { + // case 1: lookup exist entry + InvertedIndexCacheHandle inverted_index_cache_handle; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_1, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_1 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); + EXPECT_GE(UnixMillis(), cache_value_1->last_visit_time); + + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_2, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_2 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); + EXPECT_GE(UnixMillis(), cache_value_2->last_visit_time); + } + + { + // case 2: lookup not exist entry + std::string file_name_not_exist_1 = "test_3.idx"; + std::string file_name_not_exist_2 = "test_4.idx"; + // use cache + { + InvertedIndexCacheHandle inverted_index_cache_handle_1; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_1, &inverted_index_cache_handle_1); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_1.owned); + } + + // lookup again + { + InvertedIndexCacheHandle inverted_index_cache_handle_1; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_1, &inverted_index_cache_handle_1); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_1.owned); + + auto cache_value_use_cache = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle_1._cache)->value(inverted_index_cache_handle_1._handle); + EXPECT_LT(UnixMillis(), cache_value_use_cache->last_visit_time); + } + + // not use cache + InvertedIndexCacheHandle inverted_index_cache_handle_2; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_2, &inverted_index_cache_handle_2, false); + EXPECT_EQ(Status::OK(), status); + EXPECT_TRUE(inverted_index_cache_handle_2.owned); + EXPECT_EQ(nullptr, inverted_index_cache_handle_2._cache); Review Comment: warning: '_cache' is a private member of 'doris::segment_v2::InvertedIndexCacheHandle' [clang-diagnostic-error] ```cpp EXPECT_EQ(nullptr, inverted_index_cache_handle_2._cache); ^ ``` **be/src/olap/rowset/segment_v2/inverted_index_cache.h:158:** declared private here ```cpp Cache* _cache = nullptr; ^ ``` ########## be/test/olap/rowset/segment_v2/inverted_index_searcher_cache_test.cpp: ########## @@ -0,0 +1,199 @@ +// 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 "olap/rowset/segment_v2/inverted_index_cache.h" + +#include <gtest/gtest.h> + +#include "common/logging.h" +#include "io/fs/local_file_system.h" +#include "util/file_utils.h" +#include "util/time.h" + +#define protected public +#define private public + +#ifndef BE_TEST +#define BE_TEST +#endif + +namespace doris { +namespace segment_v2 { + +auto fs = io::global_local_filesystem(); + +class InvertedIndexSearcherCacheTest : public testing::Test { +public: + // there is set 1 shard in ShardedLRUCache + // And the LRUHandle size is about 100B. So the cache size should big enough + // to run the UT. + static const int kCacheSize = 1000; + const std::string kTestDir = "./ut_dir/invertedInvertedIndexSearcherCache::instance()_test"; + + void SetUp() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + EXPECT_TRUE(FileUtils::create_dir(kTestDir).ok()); + } + void TearDown() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + } +}; + + +TEST_F(InvertedIndexSearcherCacheTest, insert_lookup) { + InvertedIndexSearcherCache* index_searcher_cache = new InvertedIndexSearcherCache(kCacheSize, 1); + std::string file_name_1 = "test_1.idx"; + std::string file_name_2 = "test_2.idx"; + + //insert searcher + Status status = Status::OK(); + status = index_searcher_cache->insert(fs, kTestDir, file_name_1); + EXPECT_EQ(Status::OK(), status); + + status = index_searcher_cache->insert(fs, kTestDir, file_name_2); + EXPECT_EQ(Status::OK(), status); + + // lookup after insert + { + // case 1: lookup exist entry + InvertedIndexCacheHandle inverted_index_cache_handle; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_1, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_1 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); + EXPECT_GE(UnixMillis(), cache_value_1->last_visit_time); + + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_2, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_2 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); Review Comment: warning: '_handle' is a private member of 'doris::segment_v2::InvertedIndexCacheHandle' [clang-diagnostic-error] ```cpp (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); ^ ``` **be/src/olap/rowset/segment_v2/inverted_index_cache.h:159:** declared private here ```cpp Cache::Handle* _handle = nullptr; ^ ``` ########## be/test/olap/rowset/segment_v2/inverted_index_searcher_cache_test.cpp: ########## @@ -0,0 +1,199 @@ +// 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 "olap/rowset/segment_v2/inverted_index_cache.h" + +#include <gtest/gtest.h> + +#include "common/logging.h" +#include "io/fs/local_file_system.h" +#include "util/file_utils.h" +#include "util/time.h" + +#define protected public +#define private public + +#ifndef BE_TEST +#define BE_TEST +#endif + +namespace doris { +namespace segment_v2 { + +auto fs = io::global_local_filesystem(); + +class InvertedIndexSearcherCacheTest : public testing::Test { +public: + // there is set 1 shard in ShardedLRUCache + // And the LRUHandle size is about 100B. So the cache size should big enough + // to run the UT. + static const int kCacheSize = 1000; + const std::string kTestDir = "./ut_dir/invertedInvertedIndexSearcherCache::instance()_test"; + + void SetUp() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + EXPECT_TRUE(FileUtils::create_dir(kTestDir).ok()); + } + void TearDown() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + } +}; + + +TEST_F(InvertedIndexSearcherCacheTest, insert_lookup) { + InvertedIndexSearcherCache* index_searcher_cache = new InvertedIndexSearcherCache(kCacheSize, 1); + std::string file_name_1 = "test_1.idx"; + std::string file_name_2 = "test_2.idx"; + + //insert searcher + Status status = Status::OK(); + status = index_searcher_cache->insert(fs, kTestDir, file_name_1); + EXPECT_EQ(Status::OK(), status); + + status = index_searcher_cache->insert(fs, kTestDir, file_name_2); + EXPECT_EQ(Status::OK(), status); + + // lookup after insert + { + // case 1: lookup exist entry + InvertedIndexCacheHandle inverted_index_cache_handle; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_1, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_1 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); + EXPECT_GE(UnixMillis(), cache_value_1->last_visit_time); + + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_2, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_2 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); + EXPECT_GE(UnixMillis(), cache_value_2->last_visit_time); + } + + { + // case 2: lookup not exist entry + std::string file_name_not_exist_1 = "test_3.idx"; + std::string file_name_not_exist_2 = "test_4.idx"; + // use cache + { + InvertedIndexCacheHandle inverted_index_cache_handle_1; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_1, &inverted_index_cache_handle_1); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_1.owned); + } + + // lookup again + { + InvertedIndexCacheHandle inverted_index_cache_handle_1; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_1, &inverted_index_cache_handle_1); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_1.owned); + + auto cache_value_use_cache = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle_1._cache)->value(inverted_index_cache_handle_1._handle); + EXPECT_LT(UnixMillis(), cache_value_use_cache->last_visit_time); + } + + // not use cache + InvertedIndexCacheHandle inverted_index_cache_handle_2; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_2, &inverted_index_cache_handle_2, false); + EXPECT_EQ(Status::OK(), status); + EXPECT_TRUE(inverted_index_cache_handle_2.owned); + EXPECT_EQ(nullptr, inverted_index_cache_handle_2._cache); + EXPECT_EQ(nullptr, inverted_index_cache_handle_2._handle); + + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_2, &inverted_index_cache_handle_2); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_2.owned); + auto cache_value_use_cache_2 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle_2._cache)->value(inverted_index_cache_handle_2._handle); + EXPECT_EQ(0, cache_value_use_cache_2->last_visit_time); + } + + delete index_searcher_cache; +} + +TEST_F(InvertedIndexSearcherCacheTest, evict) { + InvertedIndexSearcherCache* index_searcher_cache = new InvertedIndexSearcherCache(kCacheSize, 1); + // no need evict + std::string file_name_1 = "test_1.idx"; + InvertedIndexSearcherCache::CacheKey key_1(file_name_1); + IndexCacheValuePtr cache_value_1 = std::make_unique<InvertedIndexSearcherCache::CacheValue>(); + cache_value_1->size = 200; + cache_value_1->index_searcher = nullptr; + cache_value_1->last_visit_time = 10; + index_searcher_cache->_cache->release(index_searcher_cache->_insert(key_1, cache_value_1.release())); + + // should evict {key_1, cache_value_1} + std::string file_name_2 = "test_2.idx"; + InvertedIndexSearcherCache::CacheKey key_2(file_name_2); + IndexCacheValuePtr cache_value_2 = std::make_unique<InvertedIndexSearcherCache::CacheValue>(); + cache_value_2->size = 800; + cache_value_2->index_searcher = nullptr; + cache_value_2->last_visit_time = 20; + index_searcher_cache->_cache->release(index_searcher_cache->_insert(key_2, cache_value_2.release())); Review Comment: warning: '_cache' is a private member of 'doris::segment_v2::InvertedIndexSearcherCache' [clang-diagnostic-error] ```cpp index_searcher_cache->_cache->release(index_searcher_cache->_insert(key_2, cache_value_2.release())); ^ ``` **be/src/olap/rowset/segment_v2/inverted_index_cache.h:98:** declared private here ```cpp std::unique_ptr<Cache> _cache = nullptr; ^ ``` ########## be/test/olap/rowset/segment_v2/inverted_index_searcher_cache_test.cpp: ########## @@ -0,0 +1,199 @@ +// 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 "olap/rowset/segment_v2/inverted_index_cache.h" + +#include <gtest/gtest.h> + +#include "common/logging.h" +#include "io/fs/local_file_system.h" +#include "util/file_utils.h" +#include "util/time.h" + +#define protected public +#define private public + +#ifndef BE_TEST +#define BE_TEST +#endif + +namespace doris { +namespace segment_v2 { + +auto fs = io::global_local_filesystem(); + +class InvertedIndexSearcherCacheTest : public testing::Test { +public: + // there is set 1 shard in ShardedLRUCache + // And the LRUHandle size is about 100B. So the cache size should big enough + // to run the UT. + static const int kCacheSize = 1000; + const std::string kTestDir = "./ut_dir/invertedInvertedIndexSearcherCache::instance()_test"; + + void SetUp() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + EXPECT_TRUE(FileUtils::create_dir(kTestDir).ok()); + } + void TearDown() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + } +}; + + +TEST_F(InvertedIndexSearcherCacheTest, insert_lookup) { + InvertedIndexSearcherCache* index_searcher_cache = new InvertedIndexSearcherCache(kCacheSize, 1); + std::string file_name_1 = "test_1.idx"; + std::string file_name_2 = "test_2.idx"; + + //insert searcher + Status status = Status::OK(); + status = index_searcher_cache->insert(fs, kTestDir, file_name_1); + EXPECT_EQ(Status::OK(), status); + + status = index_searcher_cache->insert(fs, kTestDir, file_name_2); + EXPECT_EQ(Status::OK(), status); + + // lookup after insert + { + // case 1: lookup exist entry + InvertedIndexCacheHandle inverted_index_cache_handle; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_1, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_1 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); + EXPECT_GE(UnixMillis(), cache_value_1->last_visit_time); + + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_2, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_2 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); + EXPECT_GE(UnixMillis(), cache_value_2->last_visit_time); + } + + { + // case 2: lookup not exist entry + std::string file_name_not_exist_1 = "test_3.idx"; + std::string file_name_not_exist_2 = "test_4.idx"; + // use cache + { + InvertedIndexCacheHandle inverted_index_cache_handle_1; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_1, &inverted_index_cache_handle_1); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_1.owned); + } + + // lookup again + { + InvertedIndexCacheHandle inverted_index_cache_handle_1; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_1, &inverted_index_cache_handle_1); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_1.owned); + + auto cache_value_use_cache = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle_1._cache)->value(inverted_index_cache_handle_1._handle); + EXPECT_LT(UnixMillis(), cache_value_use_cache->last_visit_time); + } + + // not use cache + InvertedIndexCacheHandle inverted_index_cache_handle_2; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_2, &inverted_index_cache_handle_2, false); + EXPECT_EQ(Status::OK(), status); + EXPECT_TRUE(inverted_index_cache_handle_2.owned); + EXPECT_EQ(nullptr, inverted_index_cache_handle_2._cache); + EXPECT_EQ(nullptr, inverted_index_cache_handle_2._handle); + + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_2, &inverted_index_cache_handle_2); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_2.owned); + auto cache_value_use_cache_2 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle_2._cache)->value(inverted_index_cache_handle_2._handle); + EXPECT_EQ(0, cache_value_use_cache_2->last_visit_time); + } + + delete index_searcher_cache; +} + +TEST_F(InvertedIndexSearcherCacheTest, evict) { + InvertedIndexSearcherCache* index_searcher_cache = new InvertedIndexSearcherCache(kCacheSize, 1); + // no need evict + std::string file_name_1 = "test_1.idx"; + InvertedIndexSearcherCache::CacheKey key_1(file_name_1); + IndexCacheValuePtr cache_value_1 = std::make_unique<InvertedIndexSearcherCache::CacheValue>(); + cache_value_1->size = 200; + cache_value_1->index_searcher = nullptr; + cache_value_1->last_visit_time = 10; + index_searcher_cache->_cache->release(index_searcher_cache->_insert(key_1, cache_value_1.release())); + + // should evict {key_1, cache_value_1} + std::string file_name_2 = "test_2.idx"; + InvertedIndexSearcherCache::CacheKey key_2(file_name_2); + IndexCacheValuePtr cache_value_2 = std::make_unique<InvertedIndexSearcherCache::CacheValue>(); + cache_value_2->size = 800; + cache_value_2->index_searcher = nullptr; + cache_value_2->last_visit_time = 20; + index_searcher_cache->_cache->release(index_searcher_cache->_insert(key_2, cache_value_2.release())); + { + InvertedIndexCacheHandle cache_handle; + // lookup key_1 + EXPECT_FALSE(index_searcher_cache->_lookup(key_1, &cache_handle)); Review Comment: warning: '_lookup' is a private member of 'doris::segment_v2::InvertedIndexSearcherCache' [clang-diagnostic-error] ```cpp EXPECT_FALSE(index_searcher_cache->_lookup(key_1, &cache_handle)); ^ ``` **be/src/olap/rowset/segment_v2/inverted_index_cache.h:88:** declared private here ```cpp bool _lookup(const InvertedIndexSearcherCache::CacheKey& key, InvertedIndexCacheHandle* handle); ^ ``` ########## be/test/olap/rowset/segment_v2/inverted_index_searcher_cache_test.cpp: ########## @@ -0,0 +1,199 @@ +// 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 "olap/rowset/segment_v2/inverted_index_cache.h" + +#include <gtest/gtest.h> + +#include "common/logging.h" +#include "io/fs/local_file_system.h" +#include "util/file_utils.h" +#include "util/time.h" + +#define protected public +#define private public + +#ifndef BE_TEST +#define BE_TEST +#endif + +namespace doris { +namespace segment_v2 { + +auto fs = io::global_local_filesystem(); + +class InvertedIndexSearcherCacheTest : public testing::Test { +public: + // there is set 1 shard in ShardedLRUCache + // And the LRUHandle size is about 100B. So the cache size should big enough + // to run the UT. + static const int kCacheSize = 1000; + const std::string kTestDir = "./ut_dir/invertedInvertedIndexSearcherCache::instance()_test"; + + void SetUp() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + EXPECT_TRUE(FileUtils::create_dir(kTestDir).ok()); + } + void TearDown() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + } +}; + + +TEST_F(InvertedIndexSearcherCacheTest, insert_lookup) { + InvertedIndexSearcherCache* index_searcher_cache = new InvertedIndexSearcherCache(kCacheSize, 1); + std::string file_name_1 = "test_1.idx"; + std::string file_name_2 = "test_2.idx"; + + //insert searcher + Status status = Status::OK(); + status = index_searcher_cache->insert(fs, kTestDir, file_name_1); + EXPECT_EQ(Status::OK(), status); + + status = index_searcher_cache->insert(fs, kTestDir, file_name_2); + EXPECT_EQ(Status::OK(), status); + + // lookup after insert + { + // case 1: lookup exist entry + InvertedIndexCacheHandle inverted_index_cache_handle; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_1, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_1 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); + EXPECT_GE(UnixMillis(), cache_value_1->last_visit_time); + + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_2, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_2 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); + EXPECT_GE(UnixMillis(), cache_value_2->last_visit_time); + } + + { + // case 2: lookup not exist entry + std::string file_name_not_exist_1 = "test_3.idx"; + std::string file_name_not_exist_2 = "test_4.idx"; + // use cache + { + InvertedIndexCacheHandle inverted_index_cache_handle_1; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_1, &inverted_index_cache_handle_1); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_1.owned); + } + + // lookup again + { + InvertedIndexCacheHandle inverted_index_cache_handle_1; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_1, &inverted_index_cache_handle_1); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_1.owned); + + auto cache_value_use_cache = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle_1._cache)->value(inverted_index_cache_handle_1._handle); + EXPECT_LT(UnixMillis(), cache_value_use_cache->last_visit_time); + } + + // not use cache + InvertedIndexCacheHandle inverted_index_cache_handle_2; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_2, &inverted_index_cache_handle_2, false); + EXPECT_EQ(Status::OK(), status); + EXPECT_TRUE(inverted_index_cache_handle_2.owned); + EXPECT_EQ(nullptr, inverted_index_cache_handle_2._cache); + EXPECT_EQ(nullptr, inverted_index_cache_handle_2._handle); + + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_2, &inverted_index_cache_handle_2); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_2.owned); + auto cache_value_use_cache_2 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle_2._cache)->value(inverted_index_cache_handle_2._handle); + EXPECT_EQ(0, cache_value_use_cache_2->last_visit_time); + } + + delete index_searcher_cache; +} + +TEST_F(InvertedIndexSearcherCacheTest, evict) { + InvertedIndexSearcherCache* index_searcher_cache = new InvertedIndexSearcherCache(kCacheSize, 1); + // no need evict + std::string file_name_1 = "test_1.idx"; + InvertedIndexSearcherCache::CacheKey key_1(file_name_1); + IndexCacheValuePtr cache_value_1 = std::make_unique<InvertedIndexSearcherCache::CacheValue>(); + cache_value_1->size = 200; + cache_value_1->index_searcher = nullptr; + cache_value_1->last_visit_time = 10; + index_searcher_cache->_cache->release(index_searcher_cache->_insert(key_1, cache_value_1.release())); + + // should evict {key_1, cache_value_1} + std::string file_name_2 = "test_2.idx"; + InvertedIndexSearcherCache::CacheKey key_2(file_name_2); + IndexCacheValuePtr cache_value_2 = std::make_unique<InvertedIndexSearcherCache::CacheValue>(); + cache_value_2->size = 800; + cache_value_2->index_searcher = nullptr; + cache_value_2->last_visit_time = 20; + index_searcher_cache->_cache->release(index_searcher_cache->_insert(key_2, cache_value_2.release())); + { + InvertedIndexCacheHandle cache_handle; + // lookup key_1 + EXPECT_FALSE(index_searcher_cache->_lookup(key_1, &cache_handle)); + // lookup key_2 + EXPECT_TRUE(index_searcher_cache->_lookup(key_2, &cache_handle)); + } + + // should evict {key_2, cache_value_2} + std::string file_name_3 = "test_3.idx"; + InvertedIndexSearcherCache::CacheKey key_3(file_name_3); + IndexCacheValuePtr cache_value_3 = std::make_unique<InvertedIndexSearcherCache::CacheValue>(); + cache_value_3->size = 400; + cache_value_3->index_searcher = nullptr; + cache_value_3->last_visit_time = 30; + index_searcher_cache->_cache->release(index_searcher_cache->_insert(key_3, cache_value_3.release())); Review Comment: warning: '_insert' is a private member of 'doris::segment_v2::InvertedIndexSearcherCache' [clang-diagnostic-error] ```cpp index_searcher_cache->_cache->release(index_searcher_cache->_insert(key_3, cache_value_3.release())); ^ ``` **be/src/olap/rowset/segment_v2/inverted_index_cache.h:93:** declared private here ```cpp Cache::Handle* _insert(const InvertedIndexSearcherCache::CacheKey& key, CacheValue* value); ^ ``` ########## be/test/olap/rowset/segment_v2/inverted_index_searcher_cache_test.cpp: ########## @@ -0,0 +1,199 @@ +// 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 "olap/rowset/segment_v2/inverted_index_cache.h" + +#include <gtest/gtest.h> + +#include "common/logging.h" +#include "io/fs/local_file_system.h" +#include "util/file_utils.h" +#include "util/time.h" + +#define protected public +#define private public + +#ifndef BE_TEST +#define BE_TEST +#endif + +namespace doris { +namespace segment_v2 { + +auto fs = io::global_local_filesystem(); + +class InvertedIndexSearcherCacheTest : public testing::Test { +public: + // there is set 1 shard in ShardedLRUCache + // And the LRUHandle size is about 100B. So the cache size should big enough + // to run the UT. + static const int kCacheSize = 1000; + const std::string kTestDir = "./ut_dir/invertedInvertedIndexSearcherCache::instance()_test"; + + void SetUp() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + EXPECT_TRUE(FileUtils::create_dir(kTestDir).ok()); + } + void TearDown() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + } +}; + + +TEST_F(InvertedIndexSearcherCacheTest, insert_lookup) { + InvertedIndexSearcherCache* index_searcher_cache = new InvertedIndexSearcherCache(kCacheSize, 1); + std::string file_name_1 = "test_1.idx"; + std::string file_name_2 = "test_2.idx"; + + //insert searcher + Status status = Status::OK(); + status = index_searcher_cache->insert(fs, kTestDir, file_name_1); + EXPECT_EQ(Status::OK(), status); + + status = index_searcher_cache->insert(fs, kTestDir, file_name_2); + EXPECT_EQ(Status::OK(), status); + + // lookup after insert + { + // case 1: lookup exist entry + InvertedIndexCacheHandle inverted_index_cache_handle; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_1, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_1 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); + EXPECT_GE(UnixMillis(), cache_value_1->last_visit_time); + + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_2, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_2 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); + EXPECT_GE(UnixMillis(), cache_value_2->last_visit_time); + } + + { + // case 2: lookup not exist entry + std::string file_name_not_exist_1 = "test_3.idx"; + std::string file_name_not_exist_2 = "test_4.idx"; + // use cache + { + InvertedIndexCacheHandle inverted_index_cache_handle_1; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_1, &inverted_index_cache_handle_1); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_1.owned); + } + + // lookup again + { + InvertedIndexCacheHandle inverted_index_cache_handle_1; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_1, &inverted_index_cache_handle_1); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_1.owned); + + auto cache_value_use_cache = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle_1._cache)->value(inverted_index_cache_handle_1._handle); Review Comment: warning: '_cache' is a private member of 'doris::segment_v2::InvertedIndexCacheHandle' [clang-diagnostic-error] ```cpp (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle_1._cache)->value(inverted_index_cache_handle_1._handle); ^ ``` **be/src/olap/rowset/segment_v2/inverted_index_cache.h:158:** declared private here ```cpp Cache* _cache = nullptr; ^ ``` ########## be/test/olap/rowset/segment_v2/inverted_index_searcher_cache_test.cpp: ########## @@ -0,0 +1,199 @@ +// 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 "olap/rowset/segment_v2/inverted_index_cache.h" + +#include <gtest/gtest.h> + +#include "common/logging.h" +#include "io/fs/local_file_system.h" +#include "util/file_utils.h" +#include "util/time.h" + +#define protected public +#define private public + +#ifndef BE_TEST +#define BE_TEST +#endif + +namespace doris { +namespace segment_v2 { + +auto fs = io::global_local_filesystem(); + +class InvertedIndexSearcherCacheTest : public testing::Test { +public: + // there is set 1 shard in ShardedLRUCache + // And the LRUHandle size is about 100B. So the cache size should big enough + // to run the UT. + static const int kCacheSize = 1000; + const std::string kTestDir = "./ut_dir/invertedInvertedIndexSearcherCache::instance()_test"; + + void SetUp() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + EXPECT_TRUE(FileUtils::create_dir(kTestDir).ok()); + } + void TearDown() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + } +}; + + +TEST_F(InvertedIndexSearcherCacheTest, insert_lookup) { + InvertedIndexSearcherCache* index_searcher_cache = new InvertedIndexSearcherCache(kCacheSize, 1); + std::string file_name_1 = "test_1.idx"; + std::string file_name_2 = "test_2.idx"; + + //insert searcher + Status status = Status::OK(); + status = index_searcher_cache->insert(fs, kTestDir, file_name_1); + EXPECT_EQ(Status::OK(), status); + + status = index_searcher_cache->insert(fs, kTestDir, file_name_2); + EXPECT_EQ(Status::OK(), status); + + // lookup after insert + { + // case 1: lookup exist entry + InvertedIndexCacheHandle inverted_index_cache_handle; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_1, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_1 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); + EXPECT_GE(UnixMillis(), cache_value_1->last_visit_time); + + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_2, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_2 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); + EXPECT_GE(UnixMillis(), cache_value_2->last_visit_time); + } + + { + // case 2: lookup not exist entry + std::string file_name_not_exist_1 = "test_3.idx"; + std::string file_name_not_exist_2 = "test_4.idx"; + // use cache + { + InvertedIndexCacheHandle inverted_index_cache_handle_1; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_1, &inverted_index_cache_handle_1); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_1.owned); + } + + // lookup again + { + InvertedIndexCacheHandle inverted_index_cache_handle_1; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_1, &inverted_index_cache_handle_1); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_1.owned); + + auto cache_value_use_cache = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle_1._cache)->value(inverted_index_cache_handle_1._handle); + EXPECT_LT(UnixMillis(), cache_value_use_cache->last_visit_time); + } + + // not use cache + InvertedIndexCacheHandle inverted_index_cache_handle_2; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_2, &inverted_index_cache_handle_2, false); + EXPECT_EQ(Status::OK(), status); + EXPECT_TRUE(inverted_index_cache_handle_2.owned); + EXPECT_EQ(nullptr, inverted_index_cache_handle_2._cache); + EXPECT_EQ(nullptr, inverted_index_cache_handle_2._handle); + + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_2, &inverted_index_cache_handle_2); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_2.owned); + auto cache_value_use_cache_2 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle_2._cache)->value(inverted_index_cache_handle_2._handle); + EXPECT_EQ(0, cache_value_use_cache_2->last_visit_time); + } + + delete index_searcher_cache; +} + +TEST_F(InvertedIndexSearcherCacheTest, evict) { + InvertedIndexSearcherCache* index_searcher_cache = new InvertedIndexSearcherCache(kCacheSize, 1); + // no need evict + std::string file_name_1 = "test_1.idx"; + InvertedIndexSearcherCache::CacheKey key_1(file_name_1); + IndexCacheValuePtr cache_value_1 = std::make_unique<InvertedIndexSearcherCache::CacheValue>(); + cache_value_1->size = 200; + cache_value_1->index_searcher = nullptr; + cache_value_1->last_visit_time = 10; + index_searcher_cache->_cache->release(index_searcher_cache->_insert(key_1, cache_value_1.release())); Review Comment: warning: '_insert' is a private member of 'doris::segment_v2::InvertedIndexSearcherCache' [clang-diagnostic-error] ```cpp index_searcher_cache->_cache->release(index_searcher_cache->_insert(key_1, cache_value_1.release())); ^ ``` **be/src/olap/rowset/segment_v2/inverted_index_cache.h:93:** declared private here ```cpp Cache::Handle* _insert(const InvertedIndexSearcherCache::CacheKey& key, CacheValue* value); ^ ``` ########## be/test/olap/rowset/segment_v2/inverted_index_searcher_cache_test.cpp: ########## @@ -0,0 +1,199 @@ +// 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 "olap/rowset/segment_v2/inverted_index_cache.h" + +#include <gtest/gtest.h> + +#include "common/logging.h" +#include "io/fs/local_file_system.h" +#include "util/file_utils.h" +#include "util/time.h" + +#define protected public +#define private public + +#ifndef BE_TEST +#define BE_TEST +#endif + +namespace doris { +namespace segment_v2 { + +auto fs = io::global_local_filesystem(); + +class InvertedIndexSearcherCacheTest : public testing::Test { +public: + // there is set 1 shard in ShardedLRUCache + // And the LRUHandle size is about 100B. So the cache size should big enough + // to run the UT. + static const int kCacheSize = 1000; + const std::string kTestDir = "./ut_dir/invertedInvertedIndexSearcherCache::instance()_test"; + + void SetUp() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + EXPECT_TRUE(FileUtils::create_dir(kTestDir).ok()); + } + void TearDown() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + } +}; + + +TEST_F(InvertedIndexSearcherCacheTest, insert_lookup) { + InvertedIndexSearcherCache* index_searcher_cache = new InvertedIndexSearcherCache(kCacheSize, 1); + std::string file_name_1 = "test_1.idx"; + std::string file_name_2 = "test_2.idx"; + + //insert searcher + Status status = Status::OK(); + status = index_searcher_cache->insert(fs, kTestDir, file_name_1); + EXPECT_EQ(Status::OK(), status); + + status = index_searcher_cache->insert(fs, kTestDir, file_name_2); + EXPECT_EQ(Status::OK(), status); + + // lookup after insert + { + // case 1: lookup exist entry + InvertedIndexCacheHandle inverted_index_cache_handle; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_1, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_1 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); + EXPECT_GE(UnixMillis(), cache_value_1->last_visit_time); + + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_2, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_2 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); + EXPECT_GE(UnixMillis(), cache_value_2->last_visit_time); + } + + { + // case 2: lookup not exist entry + std::string file_name_not_exist_1 = "test_3.idx"; + std::string file_name_not_exist_2 = "test_4.idx"; + // use cache + { + InvertedIndexCacheHandle inverted_index_cache_handle_1; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_1, &inverted_index_cache_handle_1); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_1.owned); + } + + // lookup again + { + InvertedIndexCacheHandle inverted_index_cache_handle_1; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_1, &inverted_index_cache_handle_1); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_1.owned); + + auto cache_value_use_cache = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle_1._cache)->value(inverted_index_cache_handle_1._handle); + EXPECT_LT(UnixMillis(), cache_value_use_cache->last_visit_time); + } + + // not use cache + InvertedIndexCacheHandle inverted_index_cache_handle_2; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_2, &inverted_index_cache_handle_2, false); + EXPECT_EQ(Status::OK(), status); + EXPECT_TRUE(inverted_index_cache_handle_2.owned); + EXPECT_EQ(nullptr, inverted_index_cache_handle_2._cache); + EXPECT_EQ(nullptr, inverted_index_cache_handle_2._handle); + + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_2, &inverted_index_cache_handle_2); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_2.owned); + auto cache_value_use_cache_2 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle_2._cache)->value(inverted_index_cache_handle_2._handle); Review Comment: warning: '_cache' is a private member of 'doris::segment_v2::InvertedIndexCacheHandle' [clang-diagnostic-error] ```cpp (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle_2._cache)->value(inverted_index_cache_handle_2._handle); ^ ``` **be/src/olap/rowset/segment_v2/inverted_index_cache.h:158:** declared private here ```cpp Cache* _cache = nullptr; ^ ``` ########## be/test/olap/rowset/segment_v2/inverted_index_searcher_cache_test.cpp: ########## @@ -0,0 +1,199 @@ +// 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 "olap/rowset/segment_v2/inverted_index_cache.h" + +#include <gtest/gtest.h> + +#include "common/logging.h" +#include "io/fs/local_file_system.h" +#include "util/file_utils.h" +#include "util/time.h" + +#define protected public +#define private public + +#ifndef BE_TEST +#define BE_TEST +#endif + +namespace doris { +namespace segment_v2 { + +auto fs = io::global_local_filesystem(); + +class InvertedIndexSearcherCacheTest : public testing::Test { +public: + // there is set 1 shard in ShardedLRUCache + // And the LRUHandle size is about 100B. So the cache size should big enough + // to run the UT. + static const int kCacheSize = 1000; + const std::string kTestDir = "./ut_dir/invertedInvertedIndexSearcherCache::instance()_test"; + + void SetUp() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + EXPECT_TRUE(FileUtils::create_dir(kTestDir).ok()); + } + void TearDown() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + } +}; + + +TEST_F(InvertedIndexSearcherCacheTest, insert_lookup) { + InvertedIndexSearcherCache* index_searcher_cache = new InvertedIndexSearcherCache(kCacheSize, 1); + std::string file_name_1 = "test_1.idx"; + std::string file_name_2 = "test_2.idx"; + + //insert searcher + Status status = Status::OK(); + status = index_searcher_cache->insert(fs, kTestDir, file_name_1); + EXPECT_EQ(Status::OK(), status); + + status = index_searcher_cache->insert(fs, kTestDir, file_name_2); + EXPECT_EQ(Status::OK(), status); + + // lookup after insert + { + // case 1: lookup exist entry + InvertedIndexCacheHandle inverted_index_cache_handle; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_1, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_1 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); Review Comment: warning: '_cache' is a private member of 'doris::segment_v2::InvertedIndexCacheHandle' [clang-diagnostic-error] ```cpp (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); ^ ``` **be/src/olap/rowset/segment_v2/inverted_index_cache.h:158:** declared private here ```cpp Cache* _cache = nullptr; ^ ``` ########## be/test/olap/rowset/segment_v2/inverted_index_searcher_cache_test.cpp: ########## @@ -0,0 +1,199 @@ +// 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 "olap/rowset/segment_v2/inverted_index_cache.h" + +#include <gtest/gtest.h> + +#include "common/logging.h" +#include "io/fs/local_file_system.h" +#include "util/file_utils.h" +#include "util/time.h" + +#define protected public +#define private public + +#ifndef BE_TEST +#define BE_TEST +#endif + +namespace doris { +namespace segment_v2 { + +auto fs = io::global_local_filesystem(); + +class InvertedIndexSearcherCacheTest : public testing::Test { +public: + // there is set 1 shard in ShardedLRUCache + // And the LRUHandle size is about 100B. So the cache size should big enough + // to run the UT. + static const int kCacheSize = 1000; + const std::string kTestDir = "./ut_dir/invertedInvertedIndexSearcherCache::instance()_test"; + + void SetUp() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + EXPECT_TRUE(FileUtils::create_dir(kTestDir).ok()); + } + void TearDown() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + } +}; + + +TEST_F(InvertedIndexSearcherCacheTest, insert_lookup) { + InvertedIndexSearcherCache* index_searcher_cache = new InvertedIndexSearcherCache(kCacheSize, 1); + std::string file_name_1 = "test_1.idx"; + std::string file_name_2 = "test_2.idx"; + + //insert searcher + Status status = Status::OK(); + status = index_searcher_cache->insert(fs, kTestDir, file_name_1); + EXPECT_EQ(Status::OK(), status); + + status = index_searcher_cache->insert(fs, kTestDir, file_name_2); + EXPECT_EQ(Status::OK(), status); + + // lookup after insert + { + // case 1: lookup exist entry + InvertedIndexCacheHandle inverted_index_cache_handle; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_1, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_1 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); + EXPECT_GE(UnixMillis(), cache_value_1->last_visit_time); + + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_2, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_2 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); + EXPECT_GE(UnixMillis(), cache_value_2->last_visit_time); + } + + { + // case 2: lookup not exist entry + std::string file_name_not_exist_1 = "test_3.idx"; + std::string file_name_not_exist_2 = "test_4.idx"; + // use cache + { + InvertedIndexCacheHandle inverted_index_cache_handle_1; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_1, &inverted_index_cache_handle_1); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_1.owned); + } + + // lookup again + { + InvertedIndexCacheHandle inverted_index_cache_handle_1; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_1, &inverted_index_cache_handle_1); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_1.owned); + + auto cache_value_use_cache = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle_1._cache)->value(inverted_index_cache_handle_1._handle); + EXPECT_LT(UnixMillis(), cache_value_use_cache->last_visit_time); + } + + // not use cache + InvertedIndexCacheHandle inverted_index_cache_handle_2; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_2, &inverted_index_cache_handle_2, false); + EXPECT_EQ(Status::OK(), status); + EXPECT_TRUE(inverted_index_cache_handle_2.owned); + EXPECT_EQ(nullptr, inverted_index_cache_handle_2._cache); + EXPECT_EQ(nullptr, inverted_index_cache_handle_2._handle); Review Comment: warning: '_handle' is a private member of 'doris::segment_v2::InvertedIndexCacheHandle' [clang-diagnostic-error] ```cpp EXPECT_EQ(nullptr, inverted_index_cache_handle_2._handle); ^ ``` **be/src/olap/rowset/segment_v2/inverted_index_cache.h:159:** declared private here ```cpp Cache::Handle* _handle = nullptr; ^ ``` ########## be/src/olap/lru_cache.h: ########## @@ -300,6 +302,10 @@ class HandleTable { void _resize(); }; +typedef std::priority_queue<std::pair<int64_t, LRUHandle*>, + std::vector<std::pair<int64_t, LRUHandle*>>, + std::greater<std::pair<int64_t, LRUHandle*>>> LRUHandleHeap; Review Comment: warning: use 'using' instead of 'typedef' [modernize-use-using] ```suggestion using LRUHandleHeap = std::priority_queue<std::pair<int64_t, LRUHandle *>, std::vector<std::pair<int64_t, LRUHandle *>>, std::greater<std::pair<int64_t, LRUHandle *>>>; ``` ########## be/test/olap/rowset/segment_v2/inverted_index_searcher_cache_test.cpp: ########## @@ -0,0 +1,199 @@ +// 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 "olap/rowset/segment_v2/inverted_index_cache.h" + +#include <gtest/gtest.h> + +#include "common/logging.h" +#include "io/fs/local_file_system.h" +#include "util/file_utils.h" +#include "util/time.h" + +#define protected public +#define private public + +#ifndef BE_TEST +#define BE_TEST +#endif + +namespace doris { +namespace segment_v2 { + +auto fs = io::global_local_filesystem(); + +class InvertedIndexSearcherCacheTest : public testing::Test { +public: + // there is set 1 shard in ShardedLRUCache + // And the LRUHandle size is about 100B. So the cache size should big enough + // to run the UT. + static const int kCacheSize = 1000; + const std::string kTestDir = "./ut_dir/invertedInvertedIndexSearcherCache::instance()_test"; + + void SetUp() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + EXPECT_TRUE(FileUtils::create_dir(kTestDir).ok()); + } + void TearDown() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + } +}; + + +TEST_F(InvertedIndexSearcherCacheTest, insert_lookup) { + InvertedIndexSearcherCache* index_searcher_cache = new InvertedIndexSearcherCache(kCacheSize, 1); + std::string file_name_1 = "test_1.idx"; + std::string file_name_2 = "test_2.idx"; + + //insert searcher + Status status = Status::OK(); + status = index_searcher_cache->insert(fs, kTestDir, file_name_1); + EXPECT_EQ(Status::OK(), status); + + status = index_searcher_cache->insert(fs, kTestDir, file_name_2); + EXPECT_EQ(Status::OK(), status); + + // lookup after insert + { + // case 1: lookup exist entry + InvertedIndexCacheHandle inverted_index_cache_handle; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_1, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_1 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); + EXPECT_GE(UnixMillis(), cache_value_1->last_visit_time); + + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_2, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_2 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); + EXPECT_GE(UnixMillis(), cache_value_2->last_visit_time); + } + + { + // case 2: lookup not exist entry + std::string file_name_not_exist_1 = "test_3.idx"; + std::string file_name_not_exist_2 = "test_4.idx"; + // use cache + { + InvertedIndexCacheHandle inverted_index_cache_handle_1; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_1, &inverted_index_cache_handle_1); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_1.owned); + } + + // lookup again + { + InvertedIndexCacheHandle inverted_index_cache_handle_1; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_1, &inverted_index_cache_handle_1); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_1.owned); + + auto cache_value_use_cache = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle_1._cache)->value(inverted_index_cache_handle_1._handle); + EXPECT_LT(UnixMillis(), cache_value_use_cache->last_visit_time); + } + + // not use cache + InvertedIndexCacheHandle inverted_index_cache_handle_2; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_2, &inverted_index_cache_handle_2, false); + EXPECT_EQ(Status::OK(), status); + EXPECT_TRUE(inverted_index_cache_handle_2.owned); + EXPECT_EQ(nullptr, inverted_index_cache_handle_2._cache); + EXPECT_EQ(nullptr, inverted_index_cache_handle_2._handle); + + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_2, &inverted_index_cache_handle_2); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_2.owned); + auto cache_value_use_cache_2 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle_2._cache)->value(inverted_index_cache_handle_2._handle); + EXPECT_EQ(0, cache_value_use_cache_2->last_visit_time); + } + + delete index_searcher_cache; +} + +TEST_F(InvertedIndexSearcherCacheTest, evict) { + InvertedIndexSearcherCache* index_searcher_cache = new InvertedIndexSearcherCache(kCacheSize, 1); + // no need evict + std::string file_name_1 = "test_1.idx"; + InvertedIndexSearcherCache::CacheKey key_1(file_name_1); + IndexCacheValuePtr cache_value_1 = std::make_unique<InvertedIndexSearcherCache::CacheValue>(); + cache_value_1->size = 200; + cache_value_1->index_searcher = nullptr; + cache_value_1->last_visit_time = 10; + index_searcher_cache->_cache->release(index_searcher_cache->_insert(key_1, cache_value_1.release())); Review Comment: warning: '_cache' is a private member of 'doris::segment_v2::InvertedIndexSearcherCache' [clang-diagnostic-error] ```cpp index_searcher_cache->_cache->release(index_searcher_cache->_insert(key_1, cache_value_1.release())); ^ ``` **be/src/olap/rowset/segment_v2/inverted_index_cache.h:98:** declared private here ```cpp std::unique_ptr<Cache> _cache = nullptr; ^ ``` ########## be/test/olap/rowset/segment_v2/inverted_index_searcher_cache_test.cpp: ########## @@ -0,0 +1,199 @@ +// 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 "olap/rowset/segment_v2/inverted_index_cache.h" + +#include <gtest/gtest.h> + +#include "common/logging.h" +#include "io/fs/local_file_system.h" +#include "util/file_utils.h" +#include "util/time.h" + +#define protected public +#define private public + +#ifndef BE_TEST +#define BE_TEST +#endif + +namespace doris { +namespace segment_v2 { + +auto fs = io::global_local_filesystem(); + +class InvertedIndexSearcherCacheTest : public testing::Test { +public: + // there is set 1 shard in ShardedLRUCache + // And the LRUHandle size is about 100B. So the cache size should big enough + // to run the UT. + static const int kCacheSize = 1000; + const std::string kTestDir = "./ut_dir/invertedInvertedIndexSearcherCache::instance()_test"; + + void SetUp() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + EXPECT_TRUE(FileUtils::create_dir(kTestDir).ok()); + } + void TearDown() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + } +}; + + +TEST_F(InvertedIndexSearcherCacheTest, insert_lookup) { + InvertedIndexSearcherCache* index_searcher_cache = new InvertedIndexSearcherCache(kCacheSize, 1); + std::string file_name_1 = "test_1.idx"; + std::string file_name_2 = "test_2.idx"; + + //insert searcher + Status status = Status::OK(); + status = index_searcher_cache->insert(fs, kTestDir, file_name_1); + EXPECT_EQ(Status::OK(), status); + + status = index_searcher_cache->insert(fs, kTestDir, file_name_2); + EXPECT_EQ(Status::OK(), status); + + // lookup after insert + { + // case 1: lookup exist entry + InvertedIndexCacheHandle inverted_index_cache_handle; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_1, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_1 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); + EXPECT_GE(UnixMillis(), cache_value_1->last_visit_time); + + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_2, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_2 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); + EXPECT_GE(UnixMillis(), cache_value_2->last_visit_time); + } + + { + // case 2: lookup not exist entry + std::string file_name_not_exist_1 = "test_3.idx"; + std::string file_name_not_exist_2 = "test_4.idx"; + // use cache + { + InvertedIndexCacheHandle inverted_index_cache_handle_1; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_1, &inverted_index_cache_handle_1); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_1.owned); + } + + // lookup again + { + InvertedIndexCacheHandle inverted_index_cache_handle_1; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_1, &inverted_index_cache_handle_1); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_1.owned); + + auto cache_value_use_cache = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle_1._cache)->value(inverted_index_cache_handle_1._handle); Review Comment: warning: '_handle' is a private member of 'doris::segment_v2::InvertedIndexCacheHandle' [clang-diagnostic-error] ```cpp (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle_1._cache)->value(inverted_index_cache_handle_1._handle); ^ ``` **be/src/olap/rowset/segment_v2/inverted_index_cache.h:159:** declared private here ```cpp Cache::Handle* _handle = nullptr; ^ ``` ########## be/test/olap/rowset/segment_v2/inverted_index_searcher_cache_test.cpp: ########## @@ -0,0 +1,199 @@ +// 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 "olap/rowset/segment_v2/inverted_index_cache.h" + +#include <gtest/gtest.h> + +#include "common/logging.h" +#include "io/fs/local_file_system.h" +#include "util/file_utils.h" +#include "util/time.h" + +#define protected public +#define private public + +#ifndef BE_TEST +#define BE_TEST +#endif + +namespace doris { +namespace segment_v2 { + +auto fs = io::global_local_filesystem(); + +class InvertedIndexSearcherCacheTest : public testing::Test { +public: + // there is set 1 shard in ShardedLRUCache + // And the LRUHandle size is about 100B. So the cache size should big enough + // to run the UT. + static const int kCacheSize = 1000; + const std::string kTestDir = "./ut_dir/invertedInvertedIndexSearcherCache::instance()_test"; + + void SetUp() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + EXPECT_TRUE(FileUtils::create_dir(kTestDir).ok()); + } + void TearDown() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + } +}; + + +TEST_F(InvertedIndexSearcherCacheTest, insert_lookup) { + InvertedIndexSearcherCache* index_searcher_cache = new InvertedIndexSearcherCache(kCacheSize, 1); + std::string file_name_1 = "test_1.idx"; + std::string file_name_2 = "test_2.idx"; + + //insert searcher + Status status = Status::OK(); + status = index_searcher_cache->insert(fs, kTestDir, file_name_1); + EXPECT_EQ(Status::OK(), status); + + status = index_searcher_cache->insert(fs, kTestDir, file_name_2); + EXPECT_EQ(Status::OK(), status); + + // lookup after insert + { + // case 1: lookup exist entry + InvertedIndexCacheHandle inverted_index_cache_handle; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_1, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_1 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); + EXPECT_GE(UnixMillis(), cache_value_1->last_visit_time); + + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_2, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_2 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); + EXPECT_GE(UnixMillis(), cache_value_2->last_visit_time); + } + + { + // case 2: lookup not exist entry + std::string file_name_not_exist_1 = "test_3.idx"; + std::string file_name_not_exist_2 = "test_4.idx"; + // use cache + { + InvertedIndexCacheHandle inverted_index_cache_handle_1; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_1, &inverted_index_cache_handle_1); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_1.owned); + } + + // lookup again + { + InvertedIndexCacheHandle inverted_index_cache_handle_1; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_1, &inverted_index_cache_handle_1); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_1.owned); + + auto cache_value_use_cache = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle_1._cache)->value(inverted_index_cache_handle_1._handle); + EXPECT_LT(UnixMillis(), cache_value_use_cache->last_visit_time); + } + + // not use cache + InvertedIndexCacheHandle inverted_index_cache_handle_2; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_2, &inverted_index_cache_handle_2, false); + EXPECT_EQ(Status::OK(), status); + EXPECT_TRUE(inverted_index_cache_handle_2.owned); + EXPECT_EQ(nullptr, inverted_index_cache_handle_2._cache); + EXPECT_EQ(nullptr, inverted_index_cache_handle_2._handle); + + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_2, &inverted_index_cache_handle_2); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_2.owned); + auto cache_value_use_cache_2 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle_2._cache)->value(inverted_index_cache_handle_2._handle); + EXPECT_EQ(0, cache_value_use_cache_2->last_visit_time); + } + + delete index_searcher_cache; +} + +TEST_F(InvertedIndexSearcherCacheTest, evict) { + InvertedIndexSearcherCache* index_searcher_cache = new InvertedIndexSearcherCache(kCacheSize, 1); + // no need evict + std::string file_name_1 = "test_1.idx"; + InvertedIndexSearcherCache::CacheKey key_1(file_name_1); + IndexCacheValuePtr cache_value_1 = std::make_unique<InvertedIndexSearcherCache::CacheValue>(); + cache_value_1->size = 200; + cache_value_1->index_searcher = nullptr; + cache_value_1->last_visit_time = 10; + index_searcher_cache->_cache->release(index_searcher_cache->_insert(key_1, cache_value_1.release())); + + // should evict {key_1, cache_value_1} + std::string file_name_2 = "test_2.idx"; + InvertedIndexSearcherCache::CacheKey key_2(file_name_2); + IndexCacheValuePtr cache_value_2 = std::make_unique<InvertedIndexSearcherCache::CacheValue>(); + cache_value_2->size = 800; + cache_value_2->index_searcher = nullptr; + cache_value_2->last_visit_time = 20; + index_searcher_cache->_cache->release(index_searcher_cache->_insert(key_2, cache_value_2.release())); Review Comment: warning: '_insert' is a private member of 'doris::segment_v2::InvertedIndexSearcherCache' [clang-diagnostic-error] ```cpp index_searcher_cache->_cache->release(index_searcher_cache->_insert(key_2, cache_value_2.release())); ^ ``` **be/src/olap/rowset/segment_v2/inverted_index_cache.h:93:** declared private here ```cpp Cache::Handle* _insert(const InvertedIndexSearcherCache::CacheKey& key, CacheValue* value); ^ ``` ########## be/test/olap/rowset/segment_v2/inverted_index_searcher_cache_test.cpp: ########## @@ -0,0 +1,199 @@ +// 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 "olap/rowset/segment_v2/inverted_index_cache.h" + +#include <gtest/gtest.h> + +#include "common/logging.h" +#include "io/fs/local_file_system.h" +#include "util/file_utils.h" +#include "util/time.h" + +#define protected public +#define private public + +#ifndef BE_TEST +#define BE_TEST +#endif + +namespace doris { +namespace segment_v2 { + +auto fs = io::global_local_filesystem(); + +class InvertedIndexSearcherCacheTest : public testing::Test { +public: + // there is set 1 shard in ShardedLRUCache + // And the LRUHandle size is about 100B. So the cache size should big enough + // to run the UT. + static const int kCacheSize = 1000; + const std::string kTestDir = "./ut_dir/invertedInvertedIndexSearcherCache::instance()_test"; + + void SetUp() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + EXPECT_TRUE(FileUtils::create_dir(kTestDir).ok()); + } + void TearDown() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + } +}; + + +TEST_F(InvertedIndexSearcherCacheTest, insert_lookup) { + InvertedIndexSearcherCache* index_searcher_cache = new InvertedIndexSearcherCache(kCacheSize, 1); + std::string file_name_1 = "test_1.idx"; + std::string file_name_2 = "test_2.idx"; + + //insert searcher + Status status = Status::OK(); + status = index_searcher_cache->insert(fs, kTestDir, file_name_1); + EXPECT_EQ(Status::OK(), status); + + status = index_searcher_cache->insert(fs, kTestDir, file_name_2); + EXPECT_EQ(Status::OK(), status); + + // lookup after insert + { + // case 1: lookup exist entry + InvertedIndexCacheHandle inverted_index_cache_handle; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_1, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_1 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); + EXPECT_GE(UnixMillis(), cache_value_1->last_visit_time); + + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_2, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_2 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); + EXPECT_GE(UnixMillis(), cache_value_2->last_visit_time); + } + + { + // case 2: lookup not exist entry + std::string file_name_not_exist_1 = "test_3.idx"; + std::string file_name_not_exist_2 = "test_4.idx"; + // use cache + { + InvertedIndexCacheHandle inverted_index_cache_handle_1; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_1, &inverted_index_cache_handle_1); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_1.owned); + } + + // lookup again + { + InvertedIndexCacheHandle inverted_index_cache_handle_1; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_1, &inverted_index_cache_handle_1); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_1.owned); + + auto cache_value_use_cache = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle_1._cache)->value(inverted_index_cache_handle_1._handle); + EXPECT_LT(UnixMillis(), cache_value_use_cache->last_visit_time); + } + + // not use cache + InvertedIndexCacheHandle inverted_index_cache_handle_2; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_2, &inverted_index_cache_handle_2, false); + EXPECT_EQ(Status::OK(), status); + EXPECT_TRUE(inverted_index_cache_handle_2.owned); + EXPECT_EQ(nullptr, inverted_index_cache_handle_2._cache); + EXPECT_EQ(nullptr, inverted_index_cache_handle_2._handle); + + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_2, &inverted_index_cache_handle_2); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_2.owned); + auto cache_value_use_cache_2 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle_2._cache)->value(inverted_index_cache_handle_2._handle); Review Comment: warning: '_handle' is a private member of 'doris::segment_v2::InvertedIndexCacheHandle' [clang-diagnostic-error] ```cpp (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle_2._cache)->value(inverted_index_cache_handle_2._handle); ^ ``` **be/src/olap/rowset/segment_v2/inverted_index_cache.h:159:** declared private here ```cpp Cache::Handle* _handle = nullptr; ^ ``` ########## be/test/olap/rowset/segment_v2/inverted_index_searcher_cache_test.cpp: ########## @@ -0,0 +1,199 @@ +// 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 "olap/rowset/segment_v2/inverted_index_cache.h" + +#include <gtest/gtest.h> + +#include "common/logging.h" +#include "io/fs/local_file_system.h" +#include "util/file_utils.h" +#include "util/time.h" + +#define protected public +#define private public + +#ifndef BE_TEST +#define BE_TEST +#endif + +namespace doris { +namespace segment_v2 { + +auto fs = io::global_local_filesystem(); + +class InvertedIndexSearcherCacheTest : public testing::Test { +public: + // there is set 1 shard in ShardedLRUCache + // And the LRUHandle size is about 100B. So the cache size should big enough + // to run the UT. + static const int kCacheSize = 1000; + const std::string kTestDir = "./ut_dir/invertedInvertedIndexSearcherCache::instance()_test"; + + void SetUp() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + EXPECT_TRUE(FileUtils::create_dir(kTestDir).ok()); + } + void TearDown() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + } +}; + + +TEST_F(InvertedIndexSearcherCacheTest, insert_lookup) { + InvertedIndexSearcherCache* index_searcher_cache = new InvertedIndexSearcherCache(kCacheSize, 1); + std::string file_name_1 = "test_1.idx"; + std::string file_name_2 = "test_2.idx"; + + //insert searcher + Status status = Status::OK(); + status = index_searcher_cache->insert(fs, kTestDir, file_name_1); + EXPECT_EQ(Status::OK(), status); + + status = index_searcher_cache->insert(fs, kTestDir, file_name_2); + EXPECT_EQ(Status::OK(), status); + + // lookup after insert + { + // case 1: lookup exist entry + InvertedIndexCacheHandle inverted_index_cache_handle; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_1, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_1 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); + EXPECT_GE(UnixMillis(), cache_value_1->last_visit_time); + + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_2, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_2 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); + EXPECT_GE(UnixMillis(), cache_value_2->last_visit_time); + } + + { + // case 2: lookup not exist entry + std::string file_name_not_exist_1 = "test_3.idx"; + std::string file_name_not_exist_2 = "test_4.idx"; + // use cache + { + InvertedIndexCacheHandle inverted_index_cache_handle_1; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_1, &inverted_index_cache_handle_1); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_1.owned); + } + + // lookup again + { + InvertedIndexCacheHandle inverted_index_cache_handle_1; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_1, &inverted_index_cache_handle_1); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_1.owned); + + auto cache_value_use_cache = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle_1._cache)->value(inverted_index_cache_handle_1._handle); + EXPECT_LT(UnixMillis(), cache_value_use_cache->last_visit_time); + } + + // not use cache + InvertedIndexCacheHandle inverted_index_cache_handle_2; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_2, &inverted_index_cache_handle_2, false); + EXPECT_EQ(Status::OK(), status); + EXPECT_TRUE(inverted_index_cache_handle_2.owned); + EXPECT_EQ(nullptr, inverted_index_cache_handle_2._cache); + EXPECT_EQ(nullptr, inverted_index_cache_handle_2._handle); + + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_2, &inverted_index_cache_handle_2); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_2.owned); + auto cache_value_use_cache_2 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle_2._cache)->value(inverted_index_cache_handle_2._handle); + EXPECT_EQ(0, cache_value_use_cache_2->last_visit_time); + } + + delete index_searcher_cache; +} + +TEST_F(InvertedIndexSearcherCacheTest, evict) { + InvertedIndexSearcherCache* index_searcher_cache = new InvertedIndexSearcherCache(kCacheSize, 1); + // no need evict + std::string file_name_1 = "test_1.idx"; + InvertedIndexSearcherCache::CacheKey key_1(file_name_1); + IndexCacheValuePtr cache_value_1 = std::make_unique<InvertedIndexSearcherCache::CacheValue>(); + cache_value_1->size = 200; + cache_value_1->index_searcher = nullptr; + cache_value_1->last_visit_time = 10; + index_searcher_cache->_cache->release(index_searcher_cache->_insert(key_1, cache_value_1.release())); + + // should evict {key_1, cache_value_1} + std::string file_name_2 = "test_2.idx"; + InvertedIndexSearcherCache::CacheKey key_2(file_name_2); + IndexCacheValuePtr cache_value_2 = std::make_unique<InvertedIndexSearcherCache::CacheValue>(); + cache_value_2->size = 800; + cache_value_2->index_searcher = nullptr; + cache_value_2->last_visit_time = 20; + index_searcher_cache->_cache->release(index_searcher_cache->_insert(key_2, cache_value_2.release())); + { + InvertedIndexCacheHandle cache_handle; + // lookup key_1 + EXPECT_FALSE(index_searcher_cache->_lookup(key_1, &cache_handle)); + // lookup key_2 + EXPECT_TRUE(index_searcher_cache->_lookup(key_2, &cache_handle)); + } + + // should evict {key_2, cache_value_2} + std::string file_name_3 = "test_3.idx"; + InvertedIndexSearcherCache::CacheKey key_3(file_name_3); + IndexCacheValuePtr cache_value_3 = std::make_unique<InvertedIndexSearcherCache::CacheValue>(); + cache_value_3->size = 400; + cache_value_3->index_searcher = nullptr; + cache_value_3->last_visit_time = 30; + index_searcher_cache->_cache->release(index_searcher_cache->_insert(key_3, cache_value_3.release())); + { + InvertedIndexCacheHandle cache_handle; + // lookup key_2 + EXPECT_FALSE(index_searcher_cache->_lookup(key_2, &cache_handle)); Review Comment: warning: '_lookup' is a private member of 'doris::segment_v2::InvertedIndexSearcherCache' [clang-diagnostic-error] ```cpp EXPECT_FALSE(index_searcher_cache->_lookup(key_2, &cache_handle)); ^ ``` **be/src/olap/rowset/segment_v2/inverted_index_cache.h:88:** declared private here ```cpp bool _lookup(const InvertedIndexSearcherCache::CacheKey& key, InvertedIndexCacheHandle* handle); ^ ``` ########## be/test/olap/rowset/segment_v2/inverted_index_searcher_cache_test.cpp: ########## @@ -0,0 +1,199 @@ +// 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 "olap/rowset/segment_v2/inverted_index_cache.h" + +#include <gtest/gtest.h> + +#include "common/logging.h" +#include "io/fs/local_file_system.h" +#include "util/file_utils.h" +#include "util/time.h" + +#define protected public +#define private public + +#ifndef BE_TEST +#define BE_TEST +#endif + +namespace doris { +namespace segment_v2 { + +auto fs = io::global_local_filesystem(); + +class InvertedIndexSearcherCacheTest : public testing::Test { +public: + // there is set 1 shard in ShardedLRUCache + // And the LRUHandle size is about 100B. So the cache size should big enough + // to run the UT. + static const int kCacheSize = 1000; + const std::string kTestDir = "./ut_dir/invertedInvertedIndexSearcherCache::instance()_test"; + + void SetUp() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + EXPECT_TRUE(FileUtils::create_dir(kTestDir).ok()); + } + void TearDown() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + } +}; + + +TEST_F(InvertedIndexSearcherCacheTest, insert_lookup) { + InvertedIndexSearcherCache* index_searcher_cache = new InvertedIndexSearcherCache(kCacheSize, 1); + std::string file_name_1 = "test_1.idx"; + std::string file_name_2 = "test_2.idx"; + + //insert searcher + Status status = Status::OK(); + status = index_searcher_cache->insert(fs, kTestDir, file_name_1); + EXPECT_EQ(Status::OK(), status); + + status = index_searcher_cache->insert(fs, kTestDir, file_name_2); + EXPECT_EQ(Status::OK(), status); + + // lookup after insert + { + // case 1: lookup exist entry + InvertedIndexCacheHandle inverted_index_cache_handle; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_1, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_1 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); + EXPECT_GE(UnixMillis(), cache_value_1->last_visit_time); + + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_2, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_2 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); + EXPECT_GE(UnixMillis(), cache_value_2->last_visit_time); + } + + { + // case 2: lookup not exist entry + std::string file_name_not_exist_1 = "test_3.idx"; + std::string file_name_not_exist_2 = "test_4.idx"; + // use cache + { + InvertedIndexCacheHandle inverted_index_cache_handle_1; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_1, &inverted_index_cache_handle_1); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_1.owned); + } + + // lookup again + { + InvertedIndexCacheHandle inverted_index_cache_handle_1; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_1, &inverted_index_cache_handle_1); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_1.owned); + + auto cache_value_use_cache = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle_1._cache)->value(inverted_index_cache_handle_1._handle); + EXPECT_LT(UnixMillis(), cache_value_use_cache->last_visit_time); + } + + // not use cache + InvertedIndexCacheHandle inverted_index_cache_handle_2; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_2, &inverted_index_cache_handle_2, false); + EXPECT_EQ(Status::OK(), status); + EXPECT_TRUE(inverted_index_cache_handle_2.owned); + EXPECT_EQ(nullptr, inverted_index_cache_handle_2._cache); + EXPECT_EQ(nullptr, inverted_index_cache_handle_2._handle); + + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_2, &inverted_index_cache_handle_2); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_2.owned); + auto cache_value_use_cache_2 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle_2._cache)->value(inverted_index_cache_handle_2._handle); + EXPECT_EQ(0, cache_value_use_cache_2->last_visit_time); + } + + delete index_searcher_cache; +} + +TEST_F(InvertedIndexSearcherCacheTest, evict) { + InvertedIndexSearcherCache* index_searcher_cache = new InvertedIndexSearcherCache(kCacheSize, 1); + // no need evict + std::string file_name_1 = "test_1.idx"; + InvertedIndexSearcherCache::CacheKey key_1(file_name_1); + IndexCacheValuePtr cache_value_1 = std::make_unique<InvertedIndexSearcherCache::CacheValue>(); + cache_value_1->size = 200; + cache_value_1->index_searcher = nullptr; + cache_value_1->last_visit_time = 10; + index_searcher_cache->_cache->release(index_searcher_cache->_insert(key_1, cache_value_1.release())); + + // should evict {key_1, cache_value_1} + std::string file_name_2 = "test_2.idx"; + InvertedIndexSearcherCache::CacheKey key_2(file_name_2); + IndexCacheValuePtr cache_value_2 = std::make_unique<InvertedIndexSearcherCache::CacheValue>(); + cache_value_2->size = 800; + cache_value_2->index_searcher = nullptr; + cache_value_2->last_visit_time = 20; + index_searcher_cache->_cache->release(index_searcher_cache->_insert(key_2, cache_value_2.release())); + { + InvertedIndexCacheHandle cache_handle; + // lookup key_1 + EXPECT_FALSE(index_searcher_cache->_lookup(key_1, &cache_handle)); + // lookup key_2 + EXPECT_TRUE(index_searcher_cache->_lookup(key_2, &cache_handle)); + } + + // should evict {key_2, cache_value_2} + std::string file_name_3 = "test_3.idx"; + InvertedIndexSearcherCache::CacheKey key_3(file_name_3); + IndexCacheValuePtr cache_value_3 = std::make_unique<InvertedIndexSearcherCache::CacheValue>(); + cache_value_3->size = 400; + cache_value_3->index_searcher = nullptr; + cache_value_3->last_visit_time = 30; + index_searcher_cache->_cache->release(index_searcher_cache->_insert(key_3, cache_value_3.release())); Review Comment: warning: '_cache' is a private member of 'doris::segment_v2::InvertedIndexSearcherCache' [clang-diagnostic-error] ```cpp index_searcher_cache->_cache->release(index_searcher_cache->_insert(key_3, cache_value_3.release())); ^ ``` **be/src/olap/rowset/segment_v2/inverted_index_cache.h:98:** declared private here ```cpp std::unique_ptr<Cache> _cache = nullptr; ^ ``` ########## be/test/olap/rowset/segment_v2/inverted_index_searcher_cache_test.cpp: ########## @@ -0,0 +1,199 @@ +// 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 "olap/rowset/segment_v2/inverted_index_cache.h" + +#include <gtest/gtest.h> + +#include "common/logging.h" +#include "io/fs/local_file_system.h" +#include "util/file_utils.h" +#include "util/time.h" + +#define protected public +#define private public + +#ifndef BE_TEST +#define BE_TEST +#endif + +namespace doris { +namespace segment_v2 { + +auto fs = io::global_local_filesystem(); + +class InvertedIndexSearcherCacheTest : public testing::Test { +public: + // there is set 1 shard in ShardedLRUCache + // And the LRUHandle size is about 100B. So the cache size should big enough + // to run the UT. + static const int kCacheSize = 1000; + const std::string kTestDir = "./ut_dir/invertedInvertedIndexSearcherCache::instance()_test"; + + void SetUp() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + EXPECT_TRUE(FileUtils::create_dir(kTestDir).ok()); + } + void TearDown() override { + if (FileUtils::check_exist(kTestDir)) { + EXPECT_TRUE(FileUtils::remove_all(kTestDir).ok()); + } + } +}; + + +TEST_F(InvertedIndexSearcherCacheTest, insert_lookup) { + InvertedIndexSearcherCache* index_searcher_cache = new InvertedIndexSearcherCache(kCacheSize, 1); + std::string file_name_1 = "test_1.idx"; + std::string file_name_2 = "test_2.idx"; + + //insert searcher + Status status = Status::OK(); + status = index_searcher_cache->insert(fs, kTestDir, file_name_1); + EXPECT_EQ(Status::OK(), status); + + status = index_searcher_cache->insert(fs, kTestDir, file_name_2); + EXPECT_EQ(Status::OK(), status); + + // lookup after insert + { + // case 1: lookup exist entry + InvertedIndexCacheHandle inverted_index_cache_handle; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_1, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_1 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); + EXPECT_GE(UnixMillis(), cache_value_1->last_visit_time); + + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_2, &inverted_index_cache_handle); + EXPECT_EQ(Status::OK(), status); + + auto cache_value_2 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle._cache)->value(inverted_index_cache_handle._handle); + EXPECT_GE(UnixMillis(), cache_value_2->last_visit_time); + } + + { + // case 2: lookup not exist entry + std::string file_name_not_exist_1 = "test_3.idx"; + std::string file_name_not_exist_2 = "test_4.idx"; + // use cache + { + InvertedIndexCacheHandle inverted_index_cache_handle_1; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_1, &inverted_index_cache_handle_1); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_1.owned); + } + + // lookup again + { + InvertedIndexCacheHandle inverted_index_cache_handle_1; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_1, &inverted_index_cache_handle_1); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_1.owned); + + auto cache_value_use_cache = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle_1._cache)->value(inverted_index_cache_handle_1._handle); + EXPECT_LT(UnixMillis(), cache_value_use_cache->last_visit_time); + } + + // not use cache + InvertedIndexCacheHandle inverted_index_cache_handle_2; + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_2, &inverted_index_cache_handle_2, false); + EXPECT_EQ(Status::OK(), status); + EXPECT_TRUE(inverted_index_cache_handle_2.owned); + EXPECT_EQ(nullptr, inverted_index_cache_handle_2._cache); + EXPECT_EQ(nullptr, inverted_index_cache_handle_2._handle); + + status = index_searcher_cache->get_index_searcher(fs, kTestDir, file_name_not_exist_2, &inverted_index_cache_handle_2); + EXPECT_EQ(Status::OK(), status); + EXPECT_FALSE(inverted_index_cache_handle_2.owned); + auto cache_value_use_cache_2 = + (InvertedIndexSearcherCache::CacheValue*)(inverted_index_cache_handle_2._cache)->value(inverted_index_cache_handle_2._handle); + EXPECT_EQ(0, cache_value_use_cache_2->last_visit_time); + } + + delete index_searcher_cache; +} + +TEST_F(InvertedIndexSearcherCacheTest, evict) { + InvertedIndexSearcherCache* index_searcher_cache = new InvertedIndexSearcherCache(kCacheSize, 1); + // no need evict + std::string file_name_1 = "test_1.idx"; + InvertedIndexSearcherCache::CacheKey key_1(file_name_1); + IndexCacheValuePtr cache_value_1 = std::make_unique<InvertedIndexSearcherCache::CacheValue>(); + cache_value_1->size = 200; + cache_value_1->index_searcher = nullptr; + cache_value_1->last_visit_time = 10; + index_searcher_cache->_cache->release(index_searcher_cache->_insert(key_1, cache_value_1.release())); + + // should evict {key_1, cache_value_1} + std::string file_name_2 = "test_2.idx"; + InvertedIndexSearcherCache::CacheKey key_2(file_name_2); + IndexCacheValuePtr cache_value_2 = std::make_unique<InvertedIndexSearcherCache::CacheValue>(); + cache_value_2->size = 800; + cache_value_2->index_searcher = nullptr; + cache_value_2->last_visit_time = 20; + index_searcher_cache->_cache->release(index_searcher_cache->_insert(key_2, cache_value_2.release())); + { + InvertedIndexCacheHandle cache_handle; + // lookup key_1 + EXPECT_FALSE(index_searcher_cache->_lookup(key_1, &cache_handle)); + // lookup key_2 + EXPECT_TRUE(index_searcher_cache->_lookup(key_2, &cache_handle)); Review Comment: warning: '_lookup' is a private member of 'doris::segment_v2::InvertedIndexSearcherCache' [clang-diagnostic-error] ```cpp EXPECT_TRUE(index_searcher_cache->_lookup(key_2, &cache_handle)); ^ ``` **be/src/olap/rowset/segment_v2/inverted_index_cache.h:88:** declared private here ```cpp bool _lookup(const InvertedIndexSearcherCache::CacheKey& key, InvertedIndexCacheHandle* handle); ^ ``` -- 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]
