github-actions[bot] commented on code in PR #65429:
URL: https://github.com/apache/doris/pull/65429#discussion_r3552321417
##########
be/src/io/cache/block_file_cache_ttl_mgr.cpp:
##########
@@ -239,8 +241,7 @@ void
BlockFileCacheTtlMgr::run_backgroud_update_ttl_info_map() {
}
} else {
// Remove from TTL map if TTL is 0
- _ttl_info_map.erase(tablet_id);
- need_convert_from_ttl = true;
+ need_convert_from_ttl = _ttl_info_map.erase(tablet_id)
> 0;
Review Comment:
This gate makes the TTL-to-NORMAL cleanup depend on `_ttl_info_map` having
been populated in this manager instance, but existing TTL blocks can be
reloaded independently of that map. For example, RocksDB cache metadata is
loaded with `meta_value.type`, `meta_value.ttl`, and `meta_key.tablet_id`, and
`add_cell()` keeps a loaded block in the TTL queue when the persisted
type/expiration are TTL. After a BE restart, if the tablet meta now reports
`ttl_seconds == 0`, line 244 sees no in-memory map entry and skips the
scan/conversion loop below, leaving those persisted TTL blocks in the TTL
queue. The new test only covers an already-NORMAL block with no prior TTL info;
please keep a path that demotes persisted TTL blocks, or add a regression that
seeds/reloads a TTL block for a tablet whose current TTL is disabled.
##########
be/src/io/cache/block_file_cache_ttl_mgr.cpp:
##########
@@ -239,8 +241,7 @@ void
BlockFileCacheTtlMgr::run_backgroud_update_ttl_info_map() {
}
} else {
// Remove from TTL map if TTL is 0
- _ttl_info_map.erase(tablet_id);
- need_convert_from_ttl = true;
+ need_convert_from_ttl = _ttl_info_map.erase(tablet_id)
> 0;
}
Review Comment:
This gate makes the TTL-to-NORMAL cleanup depend on `_ttl_info_map` having
been populated in this manager instance, but existing TTL blocks can be
reloaded independently of that map. For example, RocksDB cache metadata is
loaded with `meta_value.type`, `meta_value.ttl`, and `meta_key.tablet_id`, and
`add_cell()` keeps a loaded block in the TTL queue when the persisted
type/expiration are TTL. After a BE restart, if the tablet meta now reports
`ttl_seconds == 0`, line 244 sees no in-memory map entry and skips the
scan/conversion loop below, leaving those persisted TTL blocks in the TTL
queue. The new test only covers an already-NORMAL block with no prior TTL info;
please keep a path that demotes persisted TTL blocks, or add a regression that
seeds/reloads a TTL block for a tablet whose current TTL is disabled.
--
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]
