Skip to content

Commit c71514d

Browse files
freemandealerfreemandealer
authored
[enhancement](cloud) clarify codes and make TTL expiration work after abnormal cache type transition (#40226)
current TTL embeds the expiration time and type into filename and path. Maintaining both is buggy for lack of atomicity. I simplify this by using only expiration time to infer the type so that we need only expiration time. Signed-off-by: freemandealer <[email protected]>
1 parent 5eb3c60 commit c71514d

12 files changed

+420
-217
lines changed

‎be/src/common/config.cpp

+2
Original file line numberDiff line numberDiff line change
@@ -997,6 +997,8 @@ DEFINE_mInt64(file_cache_ttl_valid_check_interval_second, "0"); // zero for not
997997
// If true, evict the ttl cache using LRU when full.
998998
// Otherwise, only expiration can evict ttl and new data won't add to cache when full.
999999
DEFINE_Bool(enable_ttl_cache_evict_using_lru, "true");
1000+
// rename ttl filename to new format during read, with some performance cost
1001+
DEFINE_mBool(translate_to_new_ttl_format_during_read, "false");
10001002

10011003
DEFINE_mInt32(index_cache_entry_stay_time_after_lookup_s, "1800");
10021004
DEFINE_mInt32(inverted_index_cache_stale_sweep_time_sec, "600");

‎be/src/common/config.h

+2
Original file line numberDiff line numberDiff line change
@@ -1046,6 +1046,8 @@ DECLARE_mInt64(file_cache_ttl_valid_check_interval_second);
10461046
// If true, evict the ttl cache using LRU when full.
10471047
// Otherwise, only expiration can evict ttl and new data won't add to cache when full.
10481048
DECLARE_Bool(enable_ttl_cache_evict_using_lru);
1049+
// rename ttl filename to new format during read, with some performance cost
1050+
DECLARE_Bool(translate_to_new_ttl_format_during_read);
10491051

10501052
// inverted index searcher cache
10511053
// cache entry stay time after lookup

‎be/src/io/cache/block_file_cache.cpp

+32-17
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ FileBlocks BlockFileCache::get_impl(const UInt128Wrapper& hash, const CacheConte
261261
/// find list [block1, ..., blockN] of blocks which intersect with given range.
262262
auto it = _files.find(hash);
263263
if (it == _files.end()) {
264-
if (_lazy_open_done) {
264+
if (_async_open_done) {
265265
return {};
266266
}
267267
FileCacheKey key;
@@ -285,11 +285,10 @@ FileBlocks BlockFileCache::get_impl(const UInt128Wrapper& hash, const CacheConte
285285
if (!st.ok()) {
286286
LOG_WARNING("Failed to change key meta").error(st);
287287
}
288-
}
289-
for (auto& [_, cell] : file_blocks) {
288+
290289
FileCacheType origin_type = cell.file_block->cache_type();
291290
if (origin_type == FileCacheType::TTL) continue;
292-
Status st = cell.file_block->change_cache_type_by_mgr(FileCacheType::TTL);
291+
st = cell.file_block->change_cache_type_between_ttl_and_others(FileCacheType::TTL);
293292
if (st.ok()) {
294293
auto& queue = get_queue(origin_type);
295294
queue.remove(cell.queue_iterator.value(), cache_lock);
@@ -309,6 +308,7 @@ FileBlocks BlockFileCache::get_impl(const UInt128Wrapper& hash, const CacheConte
309308
_time_to_key.insert(std::make_pair(context.expiration_time, hash));
310309
}
311310
if (auto iter = _key_to_time.find(hash);
311+
// TODO(zhengyu): Why the hell the type is NORMAL while context set expiration_time?
312312
(context.cache_type == FileCacheType::NORMAL || context.cache_type == FileCacheType::TTL) &&
313313
iter != _key_to_time.end() && iter->second != context.expiration_time) {
314314
// remove from _time_to_key
@@ -330,7 +330,8 @@ FileBlocks BlockFileCache::get_impl(const UInt128Wrapper& hash, const CacheConte
330330
for (auto& [_, cell] : file_blocks) {
331331
auto cache_type = cell.file_block->cache_type();
332332
if (cache_type != FileCacheType::TTL) continue;
333-
auto st = cell.file_block->change_cache_type_by_mgr(FileCacheType::NORMAL);
333+
auto st = cell.file_block->change_cache_type_between_ttl_and_others(
334+
FileCacheType::NORMAL);
334335
if (st.ok()) {
335336
if (config::enable_ttl_cache_evict_using_lru) {
336337
auto& ttl_queue = get_queue(FileCacheType::TTL);
@@ -699,9 +700,9 @@ BlockFileCache::FileBlockCell* BlockFileCache::add_cell(const UInt128Wrapper& ha
699700
FileBlockCell cell(std::make_shared<FileBlock>(key, size, this, state), cache_lock);
700701
Status st;
701702
if (context.expiration_time == 0 && context.cache_type == FileCacheType::TTL) {
702-
st = cell.file_block->change_cache_type_by_mgr(FileCacheType::NORMAL);
703+
st = cell.file_block->change_cache_type_between_ttl_and_others(FileCacheType::NORMAL);
703704
} else if (context.cache_type != FileCacheType::TTL && context.expiration_time != 0) {
704-
st = cell.file_block->change_cache_type_by_mgr(FileCacheType::TTL);
705+
st = cell.file_block->change_cache_type_between_ttl_and_others(FileCacheType::TTL);
705706
}
706707
if (!st.ok()) {
707708
LOG(WARNING) << "Cannot change cache type. expiration_time=" << context.expiration_time
@@ -912,8 +913,8 @@ bool BlockFileCache::try_reserve_for_ttl(size_t size, std::lock_guard<std::mutex
912913
bool BlockFileCache::try_reserve(const UInt128Wrapper& hash, const CacheContext& context,
913914
size_t offset, size_t size,
914915
std::lock_guard<std::mutex>& cache_lock) {
915-
if (!_lazy_open_done) {
916-
return try_reserve_for_lazy_load(size, cache_lock);
916+
if (!_async_open_done) {
917+
return try_reserve_during_async_load(size, cache_lock);
917918
}
918919

919920
// use this strategy in scenarios where there is insufficient disk capacity or insufficient number of inodes remaining
@@ -1022,10 +1023,10 @@ bool BlockFileCache::remove_if_ttl_file_unlock(const UInt128Wrapper& file_key, b
10221023
LOG_WARNING("Failed to update expiration time to 0").error(st);
10231024
}
10241025
}
1025-
}
1026-
for (auto& [_, cell] : _files[file_key]) {
1026+
10271027
if (cell.file_block->cache_type() == FileCacheType::NORMAL) continue;
1028-
auto st = cell.file_block->change_cache_type_by_mgr(FileCacheType::NORMAL);
1028+
auto st = cell.file_block->change_cache_type_between_ttl_and_others(
1029+
FileCacheType::NORMAL);
10291030
if (st.ok()) {
10301031
if (config::enable_ttl_cache_evict_using_lru) {
10311032
ttl_queue.remove(cell.queue_iterator.value(), cache_lock);
@@ -1396,6 +1397,21 @@ std::string BlockFileCache::dump_structure_unlocked(const UInt128Wrapper& hash,
13961397
return result.str();
13971398
}
13981399

1400+
std::string BlockFileCache::dump_single_cache_type(const UInt128Wrapper& hash, size_t offset) {
1401+
std::lock_guard cache_lock(_mutex);
1402+
return dump_single_cache_type_unlocked(hash, offset, cache_lock);
1403+
}
1404+
1405+
std::string BlockFileCache::dump_single_cache_type_unlocked(const UInt128Wrapper& hash,
1406+
size_t offset,
1407+
std::lock_guard<std::mutex>&) {
1408+
std::stringstream result;
1409+
const auto& cells_by_offset = _files[hash];
1410+
const auto& cell = cells_by_offset.find(offset);
1411+
1412+
return cache_type_to_string(cell->second.file_block->cache_type());
1413+
}
1414+
13991415
void BlockFileCache::change_cache_type(const UInt128Wrapper& hash, size_t offset,
14001416
FileCacheType new_type,
14011417
std::lock_guard<std::mutex>& cache_lock) {
@@ -1621,11 +1637,10 @@ void BlockFileCache::modify_expiration_time(const UInt128Wrapper& hash,
16211637
if (!st.ok()) {
16221638
LOG_WARNING("").error(st);
16231639
}
1624-
}
1625-
for (auto& [_, cell] : iter->second) {
1640+
16261641
FileCacheType origin_type = cell.file_block->cache_type();
16271642
if (origin_type == FileCacheType::TTL) continue;
1628-
auto st = cell.file_block->change_cache_type_by_mgr(FileCacheType::TTL);
1643+
st = cell.file_block->change_cache_type_between_ttl_and_others(FileCacheType::TTL);
16291644
if (st.ok()) {
16301645
auto& queue = get_queue(origin_type);
16311646
queue.remove(cell.queue_iterator.value(), cache_lock);
@@ -1672,8 +1687,8 @@ BlockFileCache::get_hot_blocks_meta(const UInt128Wrapper& hash) const {
16721687
return blocks_meta;
16731688
}
16741689

1675-
bool BlockFileCache::try_reserve_for_lazy_load(size_t size,
1676-
std::lock_guard<std::mutex>& cache_lock) {
1690+
bool BlockFileCache::try_reserve_during_async_load(size_t size,
1691+
std::lock_guard<std::mutex>& cache_lock) {
16771692
size_t removed_size = 0;
16781693
size_t normal_queue_size = _normal_queue.get_capacity(cache_lock);
16791694
size_t disposable_queue_size = _disposable_queue.get_capacity(cache_lock);

‎be/src/io/cache/block_file_cache.h

+8-4
Original file line numberDiff line numberDiff line change
@@ -104,8 +104,9 @@ class BlockFileCache {
104104
std::string reset_capacity(size_t new_capacity);
105105

106106
std::map<size_t, FileBlockSPtr> get_blocks_by_key(const UInt128Wrapper& hash);
107-
/// For debug.
107+
/// For debug and UT
108108
std::string dump_structure(const UInt128Wrapper& hash);
109+
std::string dump_single_cache_type(const UInt128Wrapper& hash, size_t offset);
109110

110111
[[nodiscard]] size_t get_used_cache_size(FileCacheType type) const;
111112

@@ -130,7 +131,7 @@ class BlockFileCache {
130131
[[nodiscard]] std::vector<std::tuple<size_t, size_t, FileCacheType, uint64_t>>
131132
get_hot_blocks_meta(const UInt128Wrapper& hash) const;
132133

133-
[[nodiscard]] bool get_lazy_open_success() const { return _lazy_open_done; }
134+
[[nodiscard]] bool get_async_open_success() const { return _async_open_done; }
134135

135136
BlockFileCache& operator=(const BlockFileCache&) = delete;
136137
BlockFileCache(const BlockFileCache&) = delete;
@@ -338,7 +339,7 @@ class BlockFileCache {
338339
const CacheContext& context, size_t offset, size_t size,
339340
std::lock_guard<std::mutex>& cache_lock);
340341

341-
bool try_reserve_for_lazy_load(size_t size, std::lock_guard<std::mutex>& cache_lock);
342+
bool try_reserve_during_async_load(size_t size, std::lock_guard<std::mutex>& cache_lock);
342343

343344
std::vector<FileCacheType> get_other_cache_type(FileCacheType cur_cache_type);
344345

@@ -358,6 +359,9 @@ class BlockFileCache {
358359
std::string dump_structure_unlocked(const UInt128Wrapper& hash,
359360
std::lock_guard<std::mutex>& cache_lock);
360361

362+
std::string dump_single_cache_type_unlocked(const UInt128Wrapper& hash, size_t offset,
363+
std::lock_guard<std::mutex>& cache_lock);
364+
361365
void fill_holes_with_empty_file_blocks(FileBlocks& file_blocks, const UInt128Wrapper& hash,
362366
const CacheContext& context,
363367
const FileBlock::Range& range,
@@ -413,7 +417,7 @@ class BlockFileCache {
413417
std::mutex _close_mtx;
414418
std::condition_variable _close_cv;
415419
std::thread _cache_background_thread;
416-
std::atomic_bool _lazy_open_done {false};
420+
std::atomic_bool _async_open_done {false};
417421
bool _async_clear_file_cache {false};
418422
// disk space or inode is less than the specified value
419423
bool _disk_resource_limit_mode {false};

‎be/src/io/cache/file_block.cpp

+23-17
Original file line numberDiff line numberDiff line change
@@ -161,32 +161,41 @@ Status FileBlock::read(Slice buffer, size_t read_offset) {
161161
return _mgr->_storage->read(_key, read_offset, buffer);
162162
}
163163

164-
Status FileBlock::change_cache_type_by_mgr(FileCacheType new_type) {
164+
Status FileBlock::change_cache_type_between_ttl_and_others(FileCacheType new_type) {
165165
std::lock_guard block_lock(_mutex);
166166
DCHECK(new_type != _key.meta.type);
167-
if (_download_state == State::DOWNLOADED) {
168-
KeyMeta new_meta;
169-
new_meta.expiration_time = _key.meta.expiration_time;
170-
new_meta.type = new_type;
171-
auto st = _mgr->_storage->change_key_meta(_key, new_meta);
172-
TEST_SYNC_POINT_CALLBACK("FileBlock::change_cache_type", &st);
173-
if (!st.ok()) return st;
167+
bool expr = (new_type == FileCacheType::TTL || _key.meta.type == FileCacheType::TTL);
168+
if (!expr) {
169+
LOG(WARNING) << "none of the cache type is TTL"
170+
<< ", hash: " << _key.hash.to_string() << ", offset: " << _key.offset
171+
<< ", new type: " << BlockFileCache::cache_type_to_string(new_type)
172+
<< ", old type: " << BlockFileCache::cache_type_to_string(_key.meta.type);
174173
}
174+
DCHECK(expr);
175+
176+
// change cache type between TTL to others don't need to rename the filename suffix
175177
_key.meta.type = new_type;
176178
return Status::OK();
177179
}
178180

179-
Status FileBlock::change_cache_type_self(FileCacheType new_type) {
181+
Status FileBlock::change_cache_type_between_normal_and_index(FileCacheType new_type) {
180182
std::lock_guard cache_lock(_mgr->_mutex);
181183
std::lock_guard block_lock(_mutex);
184+
bool expr = (new_type != FileCacheType::TTL && _key.meta.type != FileCacheType::TTL);
185+
if (!expr) {
186+
LOG(WARNING) << "one of the cache type is TTL"
187+
<< ", hash: " << _key.hash.to_string() << ", offset: " << _key.offset
188+
<< ", new type: " << BlockFileCache::cache_type_to_string(new_type)
189+
<< ", old type: " << BlockFileCache::cache_type_to_string(_key.meta.type);
190+
}
191+
DCHECK(expr);
182192
if (_key.meta.type == FileCacheType::TTL || new_type == _key.meta.type) {
183193
return Status::OK();
184194
}
185195
if (_download_state == State::DOWNLOADED) {
186-
KeyMeta new_meta;
187-
new_meta.expiration_time = _key.meta.expiration_time;
188-
new_meta.type = new_type;
189-
RETURN_IF_ERROR(_mgr->_storage->change_key_meta(_key, new_meta));
196+
Status st;
197+
TEST_SYNC_POINT_CALLBACK("FileBlock::change_cache_type", &st);
198+
RETURN_IF_ERROR(_mgr->_storage->change_key_meta_type(_key, new_type));
190199
}
191200
_mgr->change_cache_type(_key.hash, _block_range.left, new_type, cache_lock);
192201
_key.meta.type = new_type;
@@ -196,10 +205,7 @@ Status FileBlock::change_cache_type_self(FileCacheType new_type) {
196205
Status FileBlock::update_expiration_time(uint64_t expiration_time) {
197206
std::lock_guard block_lock(_mutex);
198207
if (_download_state == State::DOWNLOADED) {
199-
KeyMeta new_meta;
200-
new_meta.expiration_time = expiration_time;
201-
new_meta.type = _key.meta.type;
202-
auto st = _mgr->_storage->change_key_meta(_key, new_meta);
208+
auto st = _mgr->_storage->change_key_meta_expiration(_key, expiration_time);
203209
if (!st.ok() && !st.is<ErrorCode::NOT_FOUND>()) {
204210
return st;
205211
}

‎be/src/io/cache/file_block.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -115,9 +115,9 @@ class FileBlock {
115115

116116
std::string get_info_for_log() const;
117117

118-
[[nodiscard]] Status change_cache_type_by_mgr(FileCacheType new_type);
118+
[[nodiscard]] Status change_cache_type_between_ttl_and_others(FileCacheType new_type);
119119

120-
[[nodiscard]] Status change_cache_type_self(FileCacheType new_type);
120+
[[nodiscard]] Status change_cache_type_between_normal_and_index(FileCacheType new_type);
121121

122122
[[nodiscard]] Status update_expiration_time(uint64_t expiration_time);
123123

‎be/src/io/cache/file_cache_storage.h

+3-1
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,9 @@ class FileCacheStorage {
4040
// remove the block
4141
virtual Status remove(const FileCacheKey& key) = 0;
4242
// change the block meta
43-
virtual Status change_key_meta(const FileCacheKey& key, const KeyMeta& new_meta) = 0;
43+
virtual Status change_key_meta_type(const FileCacheKey& key, const FileCacheType type) = 0;
44+
virtual Status change_key_meta_expiration(const FileCacheKey& key,
45+
const uint64_t expiration) = 0;
4446
// use when lazy load cache
4547
virtual void load_blocks_directly_unlocked(BlockFileCache* _mgr, const FileCacheKey& key,
4648
std::lock_guard<std::mutex>& cache_lock) {}

0 commit comments

Comments
 (0)