liaoxin01 opened a new pull request, #67971:
URL: https://github.com/apache/doris/pull/67971
### What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary:
`BlockFileCacheTtlMgr` decides whether to promote a tablet's cached blocks
into the TTL queue from `was_zero_ttl`, which is true only when `_ttl_info_map`
holds no entry for the tablet. The entry is erased only when the tablet's TTL
is zero, so that flag really answers *"has this BE ever seen a non-zero TTL
here"*, not *"are the blocks currently outside the TTL queue"*.
The expiration check demotes blocks back to `NORMAL` but leaves the non-zero
TTL in the map. So once a TTL expired, the promotion path was closed for that
tablet for good:
```
TTL = 600
-> tablet is older than 600s
-> expiration check converts the TTL blocks back to NORMAL
-> _ttl_info_map still holds ttl = 600
-> ALTER TABLE ... SET ("file_cache_ttl_seconds" = "30758400")
-> was_zero_ttl is false, so the NORMAL -> TTL conversion is skipped
-> the blocks stay NORMAL, and no later round ever brings them back
```
FE, Meta Service and `_ttl_info_map` all hold the new TTL, while
`information_schema.file_cache_info` keeps reporting `type = normal` for those
tablets no matter how many background rounds go by. The blocks are then evicted
under the normal policy instead of getting the retention the TTL cache is
supposed to give them, and `SHOW CREATE TABLE`, Meta Service and
`file_cache_info` disagree about whether TTL applies. There is no warning in
the BE log because the conversion is never attempted.
### Release note
Fixed cached blocks staying in the normal queue when a table's expired
`file_cache_ttl_seconds` was extended to a value that has not expired.
### What is changed and how it works?
**Record the applied state instead of inferring it.** `TtlInfo` gains
`blocks_are_ttl`, the cache type last applied to the tablet's blocks, and
`is_ttl_active()` derives the type the blocks should have from `ttl`,
`tablet_ctime` and the current time. Conversions run only on a transition
between the two, in both directions:
| tablet state | applied | wanted | action |
|---|---|---|---|
| 0 -> non-zero, not expired | false | true | promote |
| **expired non-zero -> longer non-zero** | **false** | **true** | **promote
(was missing)** |
| non-zero, not expired -> expired | true | false | demote |
| expired -> still expired | false | false | none |
| non-zero -> 0 | true | false | demote, then stop tracking |
**Serialize per tablet.** Both background threads now funnel through
`reconcile_tablet_blocks()`, guarded by a striped lock and re-reading the
tablet's state after taking it. The expiration check previously decided from a
snapshot of `_ttl_info_map` that could be a full gc interval old, so it could
demote blocks the update thread had just promoted under a newly extended TTL;
the map then held the new TTL, neither thread saw a transition, and the blocks
stayed `NORMAL` permanently. That also made the "set TTL to 0, wait, set the
target TTL" workaround unreliable. The snapshot is now only a candidate list.
Two further problems fall out of the same change:
- The block scan no longer runs while `_ttl_info_mutex` is held. It walks
the meta store and takes the cache lock once per block, which on a tablet with
a few thousand cached blocks blocked every other user of the map for the
duration.
- An expired tablet is scanned once rather than on every gc round, because
`blocks_are_ttl` records that the demotion already happened.
The decision is deliberately a state comparison, not a comparison of TTL
values: a tablet whose TTL is rewritten to another still-valid value keeps its
blocks where they are. Deployments that rewrite this property on a schedule
would otherwise rescan the whole tablet on every change.
The low-frequency reconcile for tablets with no TTL info is preserved, so
#65434's skip of per-round scans for ordinary non-TTL tablets still holds.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
New cases in `BlockFileCacheTtlMgrTest`:
- `ExpiredTtlExtendedMovesBlocksBackToTtl` - the scenario above
- `ExtendedTtlThatIsStillExpiredKeepsBlocksNormal` - guards against
over-correcting
- `TtlExtensionWinsOverConcurrentExpirationScan` - stalls a demotion scan,
extends the TTL underneath it, asserts the blocks end up as TTL
- `RewritingTtlToAnotherValidValueDoesNotRescanBlocks` - no rescan when the
value moves but the state does not
```bash
DORIS_TOOLCHAIN=clang DISABLE_BE_JAVA_EXTENSIONS=ON
ENABLE_INJECTION_POINT=ON ENABLE_CACHE_LOCK_DEBUG=0 ENABLE_PCH=0 sh
run-be-ut.sh --run --filter='BlockFileCacheTtlMgrTest.*'
```
- [ ] This is a refactor/code format and no logic has been changed.
- Behavior changed:
- [ ] No.
- [x] Yes. A tablet whose expired TTL is extended to one that has not
expired now has its cached blocks converted back to the TTL queue. Blocks are
no longer rescanned on every gc round once a tablet has expired.
- Does this need documentation?
- [x] No.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should
merge into -->
--
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]
