liaoxin01 opened a new pull request, #68345:
URL: https://github.com/apache/doris/pull/68345
### What problem does this PR solve?
Related PR: #49456, #68090
Problem Summary:
An LRU dump entry records only <hash, offset, size>, so when
CacheLRUDumper::restore_queue() rebuilds the TTL queue on startup it has no
expiration time to give the blocks it puts back. It invented one:
// TODO(zhengyu): we haven't persist expiration time yet, use 3h default
ctx.expiration_time = 10800;
KeyMeta::expiration_time is an absolute timestamp, not a duration, so 10800
is
1970-01-01 03:00:00 UTC, and the comment's own plan for correcting it later
was
never carried out. FSFileCacheStorage::handle_already_loaded_block() is the
one
place that sees both the restored cell and the values the meta store holds
for
it, and it throws the latter away, fixing only the block size and the tablet
id.
So every block restored from the ttl dump keeps a 1970 expiration for the
life
of the process, and the same gap leaves a block restored from the normal,
index
or disposable dump stranded in that queue when the meta store says it is TTL.
Nothing reads a block's expiration time to decide whether it is expired -
BlockFileCacheTtlMgr sweeps by tablet_ctime + ttl_seconds - so the cache does
not drop the data. What the stale value does reach:
1. check_file_cache_consistency() compares the manager's cache type and
expiration time against the storage layer's, so every restored TTL block
is
reported as EXPIRATION_TIME_INCONSISTENT, and every block restored into
the
wrong queue as CACHE_TYPE_INCONSISTENT, for as long as the BE runs.
2. change_key_meta_type() persists BlockMeta(type, size,
key.meta.expiration_time).
The first time BlockFileCacheTtlMgr converts a restored block, the real
expiration in the meta store is overwritten with 10800. That survives the
restart: the loader then hands back type=NORMAL with a non-zero
expiration,
which add_cell() takes as a reason to force the block into the TTL queue
again, so the block bounces between queues once per restart and the value
it
was created with is gone for good.
3. The v2 compatibility paths address a file as
<hash>_<expiration_time>/<offset>,
so for a cache that has not finished migrating off the v2 layout the read
fallback in get_or_open_file_reader() looks in a directory that cannot
exist,
and remove() deletes that path instead of the real one and leaks the file.
4. get_hot_blocks_meta() reports the block's expiration time, and
cloud_internal_service passes it to the peer that is warming up, which
then
persists 10800 of its own.
This PR keeps the restore honest about what it knows. restore_queue() still
has
to put a non-zero expiration on a TTL block, because that is what keeps it in
the TTL queue and reproducing the queue is the whole point of the dump, but
it
now parks the block a day ahead instead of in 1970 and marks it, and
handle_already_loaded_block() converges every marked cell onto the cache type
and expiration time the storage layer reports. The mark is what keeps this
narrow: a cell created by a writer, or already converged once, is left alone
rather than being reverted to what the loader's snapshot happened to hold.
The conversion is memory only. Storage is the source of truth being copied
from, so FileBlock::converge_meta_to_storage() deliberately does not go
through
change_cache_type_lock(), which would write the value back.
Also fixes _cur_ttl_size drifting: add_cell() and remove() maintain it from
the
block's cache type, but change_cache_type() did not move it, so any
conversion
left it wrong for the rest of the process. Nothing reads it today, but the
new
code moves blocks between queues and should not add to that.
New bvars file_cache_ttl_converged_block_num / _bytes make the correction
visible.
Behaviour change worth calling out: test_lru_duplicate_queue_entry_restore
asserted that a duplicated ttl dump keeps blocks in the TTL queue even though
the disk says NORMAL. They are now pulled back out, which is the point of the
fix, and the test asserts the new outcome.
### Release note
Fixed the file cache keeping a fabricated expiration time for every block
restored from an LRU dump, which made those blocks permanently inconsistent
with the block meta store and could overwrite their real expiration time
there.
### Check List (For Author)
- Test
- [x] Unit Test
- Behavior changed:
- [x] Yes. A block that the LRU dump restores into one queue while the
block
meta store says it belongs in another is now moved to the queue the
meta
store names, instead of staying where the dump put it.
`test_lru_duplicate_queue_entry_restore` asserted the old outcome and
has
been updated.
- Does this need documentation?
- [x] No.
--
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]
