github-actions[bot] commented on code in PR #38518:
URL: https://github.com/apache/doris/pull/38518#discussion_r1696395354
##########
be/test/io/cache/block_file_cache_test.cpp:
##########
@@ -4319,4 +4318,259 @@
}
}
+TEST_F(BlockFileCacheTest, change_cache_type1) {
+ if (fs::exists(cache_base_path)) {
+ fs::remove_all(cache_base_path);
+ }
+ fs::create_directories(cache_base_path);
+ auto sp = SyncPoint::get_instance();
+ sp->set_call_back("FileBlock::change_cache_type", [](auto&& args) {
+ *try_any_cast<Status*>(args[0]) = Status::IOError("inject io error");
+ });
+ sp->enable_processing();
+ TUniqueId query_id;
+ query_id.hi = 1;
+ query_id.lo = 1;
+ io::FileCacheSettings settings;
+ settings.query_queue_size = 30;
+ settings.query_queue_elements = 5;
+ settings.capacity = 30;
+ settings.max_file_block_size = 30;
+ settings.max_query_cache_size = 30;
+ io::CacheContext context;
+ context.cache_type = io::FileCacheType::TTL;
+ context.query_id = query_id;
+ int64_t cur_time = UnixSeconds();
+ context.expiration_time = cur_time + 120;
+ int64_t modify_time = cur_time + 5;
+ auto key1 = io::BlockFileCache::hash("key1");
+ io::BlockFileCache cache(cache_base_path, settings);
+ ASSERT_TRUE(cache.initialize());
+ for (int i = 0; i < 100; i++) {
+ if (cache.get_lazy_open_success()) {
+ break;
+ };
+ std::this_thread::sleep_for(std::chrono::milliseconds(1));
+ }
+ {
+ auto holder = cache.get_or_set(key1, 50, 10, context); /// Add range
[50, 59]
+ auto segments = fromHolder(holder);
+ ASSERT_EQ(segments.size(), 1);
+ assert_range(1, segments[0], io::FileBlock::Range(50, 59),
io::FileBlock::State::EMPTY);
+ ASSERT_TRUE(segments[0]->get_or_set_downloader() ==
io::FileBlock::get_caller_id());
+ download(segments[0]);
+ assert_range(1, segments[0], io::FileBlock::Range(50, 59),
+ io::FileBlock::State::DOWNLOADED);
+ EXPECT_EQ(segments[0]->cache_type(), io::FileCacheType::TTL);
+ EXPECT_EQ(segments[0]->expiration_time(), context.expiration_time);
+ }
+ context.cache_type = io::FileCacheType::NORMAL;
+ context.expiration_time = 0;
+ {
+ auto holder = cache.get_or_set(key1, 50, 10, context); /// Add range
[50, 59]
+ auto segments = fromHolder(holder);
+ ASSERT_EQ(segments.size(), 1);
+ assert_range(1, segments[0], io::FileBlock::Range(50, 59),
+ io::FileBlock::State::DOWNLOADED);
+ EXPECT_EQ(segments[0]->cache_type(), io::FileCacheType::TTL);
+ EXPECT_EQ(segments[0]->expiration_time(), 0);
+ }
+ sp->clear_call_back("FileBlock::change_cache_type");
+ context.cache_type = io::FileCacheType::TTL;
+ context.expiration_time = modify_time;
+ {
+ auto holder = cache.get_or_set(key1, 50, 10, context); /// Add range
[50, 59]
+ auto segments = fromHolder(holder);
+ ASSERT_EQ(segments.size(), 1);
+ assert_range(1, segments[0], io::FileBlock::Range(50, 59),
+ io::FileBlock::State::DOWNLOADED);
+ EXPECT_EQ(segments[0]->cache_type(), io::FileCacheType::TTL);
+ EXPECT_EQ(segments[0]->expiration_time(), modify_time);
+ }
+ if (fs::exists(cache_base_path)) {
+ fs::remove_all(cache_base_path);
+ }
+}
+
+TEST_F(BlockFileCacheTest, change_cache_type2) {
Review Comment:
warning: function 'TEST_F' exceeds recommended size/complexity thresholds
[readability-function-size]
```cpp
^
```
<details>
<summary>Additional context</summary>
**be/test/io/cache/block_file_cache_test.cpp:4394:** 86 lines including
whitespace and comments (threshold 80)
```cpp
^
```
</details>
##########
be/test/io/cache/block_file_cache_test.cpp:
##########
@@ -4319,4 +4318,259 @@ TEST_F(BlockFileCacheTest, reset_capacity) {
}
}
+TEST_F(BlockFileCacheTest, change_cache_type1) {
+ if (fs::exists(cache_base_path)) {
+ fs::remove_all(cache_base_path);
+ }
+ fs::create_directories(cache_base_path);
+ auto sp = SyncPoint::get_instance();
Review Comment:
warning: 'auto sp' can be declared as 'auto *sp' [readability-qualified-auto]
```suggestion
h);auto *
```
##########
be/test/io/cache/block_file_cache_test.cpp:
##########
@@ -4319,4 +4318,259 @@
}
}
+TEST_F(BlockFileCacheTest, change_cache_type1) {
+ if (fs::exists(cache_base_path)) {
+ fs::remove_all(cache_base_path);
+ }
+ fs::create_directories(cache_base_path);
+ auto sp = SyncPoint::get_instance();
+ sp->set_call_back("FileBlock::change_cache_type", [](auto&& args) {
+ *try_any_cast<Status*>(args[0]) = Status::IOError("inject io error");
+ });
+ sp->enable_processing();
+ TUniqueId query_id;
+ query_id.hi = 1;
+ query_id.lo = 1;
+ io::FileCacheSettings settings;
+ settings.query_queue_size = 30;
+ settings.query_queue_elements = 5;
+ settings.capacity = 30;
+ settings.max_file_block_size = 30;
+ settings.max_query_cache_size = 30;
+ io::CacheContext context;
+ context.cache_type = io::FileCacheType::TTL;
+ context.query_id = query_id;
+ int64_t cur_time = UnixSeconds();
+ context.expiration_time = cur_time + 120;
+ int64_t modify_time = cur_time + 5;
+ auto key1 = io::BlockFileCache::hash("key1");
+ io::BlockFileCache cache(cache_base_path, settings);
+ ASSERT_TRUE(cache.initialize());
+ for (int i = 0; i < 100; i++) {
+ if (cache.get_lazy_open_success()) {
+ break;
+ };
+ std::this_thread::sleep_for(std::chrono::milliseconds(1));
+ }
+ {
+ auto holder = cache.get_or_set(key1, 50, 10, context); /// Add range
[50, 59]
+ auto segments = fromHolder(holder);
+ ASSERT_EQ(segments.size(), 1);
+ assert_range(1, segments[0], io::FileBlock::Range(50, 59),
io::FileBlock::State::EMPTY);
+ ASSERT_TRUE(segments[0]->get_or_set_downloader() ==
io::FileBlock::get_caller_id());
+ download(segments[0]);
+ assert_range(1, segments[0], io::FileBlock::Range(50, 59),
+ io::FileBlock::State::DOWNLOADED);
+ EXPECT_EQ(segments[0]->cache_type(), io::FileCacheType::TTL);
+ EXPECT_EQ(segments[0]->expiration_time(), context.expiration_time);
+ }
+ context.cache_type = io::FileCacheType::NORMAL;
+ context.expiration_time = 0;
+ {
+ auto holder = cache.get_or_set(key1, 50, 10, context); /// Add range
[50, 59]
+ auto segments = fromHolder(holder);
+ ASSERT_EQ(segments.size(), 1);
+ assert_range(1, segments[0], io::FileBlock::Range(50, 59),
+ io::FileBlock::State::DOWNLOADED);
+ EXPECT_EQ(segments[0]->cache_type(), io::FileCacheType::TTL);
+ EXPECT_EQ(segments[0]->expiration_time(), 0);
+ }
+ sp->clear_call_back("FileBlock::change_cache_type");
+ context.cache_type = io::FileCacheType::TTL;
+ context.expiration_time = modify_time;
+ {
+ auto holder = cache.get_or_set(key1, 50, 10, context); /// Add range
[50, 59]
+ auto segments = fromHolder(holder);
+ ASSERT_EQ(segments.size(), 1);
+ assert_range(1, segments[0], io::FileBlock::Range(50, 59),
+ io::FileBlock::State::DOWNLOADED);
+ EXPECT_EQ(segments[0]->cache_type(), io::FileCacheType::TTL);
+ EXPECT_EQ(segments[0]->expiration_time(), modify_time);
+ }
+ if (fs::exists(cache_base_path)) {
+ fs::remove_all(cache_base_path);
+ }
+}
+
+TEST_F(BlockFileCacheTest, change_cache_type2) {
+ if (fs::exists(cache_base_path)) {
+ fs::remove_all(cache_base_path);
+ }
+ fs::create_directories(cache_base_path);
+ auto sp = SyncPoint::get_instance();
Review Comment:
warning: 'auto sp' can be declared as 'auto *sp' [readability-qualified-auto]
```suggestion
h);auto *
```
--
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: commits-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
