Repository navigation
[refactor](filecache) ttl management refactoring - #57922
Merged
Merged
Conversation
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
16 tasks
freemandealer
force-pushed
the
ttl-management-rebased
branch
from
November 19, 2025 16:42
105eaea to
1db6407
Compare
freemandealer
marked this pull request as ready for review
November 19, 2025 16:45
freemandealer
requested review from
dataroaring and
gavinchou
as code owners
November 19, 2025 16:45
Member
Author
|
run buildall |
freemandealer
force-pushed
the
ttl-management-rebased
branch
from
November 19, 2025 16:58
a2a9195 to
eace969
Compare
Member
Author
|
run buildall |
TPC-H: Total hot run time: 34150 ms |
TPC-DS: Total hot run time: 184937 ms |
ClickBench: Total hot run time: 27.51 s |
gavinchou
reviewed
Nov 20, 2025
freemandealer
force-pushed
the
ttl-management-rebased
branch
from
November 20, 2025 07:22
39af956 to
d4fbd2a
Compare
Contributor
|
run buildall |
TPC-H: Total hot run time: 34175 ms |
TPC-DS: Total hot run time: 185168 ms |
ClickBench: Total hot run time: 27.41 s |
gavinchou
requested changes
Nov 28, 2025
freemandealer
force-pushed
the
ttl-management-rebased
branch
from
November 28, 2025 11:46
6d89920 to
4c85cfb
Compare
Member
Author
|
run buildall |
TPC-H: Total hot run time: 34221 ms |
TPC-DS: Total hot run time: 182313 ms |
ClickBench: Total hot run time: 27.34 s |
Member
Author
|
run nonConcurrent |
Member
Author
|
run external |
Member
Author
|
run beut |
gavinchou
reviewed
Dec 1, 2025
gavinchou
reviewed
Dec 1, 2025
gavinchou
reviewed
Dec 1, 2025
Member
Author
|
run buildall |
TPC-H: Total hot run time: 36347 ms |
TPC-DS: Total hot run time: 178677 ms |
ClickBench: Total hot run time: 27.28 s |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Member
Author
|
run nonConcurrent |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
gavinchou
approved these changes
Dec 18, 2025
github-actions
Bot
added
the
approved
Contributor
|
PR approved by at least one committer and no changes requested. |
Contributor
|
PR approved by anyone and no changes requested. |
0AyanamiRei
approved these changes
Dec 18, 2025
16 tasks
dataroaring
pushed a commit
that referenced
this pull request
Feb 27, 2026
…r due to race condition in `CloudTabletMgr::get_tablet` (#60832) ### What problem does this PR solve? Issue Number: close #xxx Related PR: #57922 Fix a race condition in `CloudTabletMgr::get_tablet()` introduced by #57922 (commit `0918952c70`) that causes tablets to permanently disappear from `_tablet_map`, making them invisible to the compaction scheduler. This leads to tables accumulating hundreds of rowsets without any compaction under high-frequency import. ## Root Cause Commit `0918952c70` refactored `get_tablet()` by moving `_cache->insert()` and `_tablet_map->put()` from inside the `SingleFlight` lambda to outside it. This introduced a race condition: 1. When N concurrent `get_tablet()` calls arrive for the same `tablet_id`, `SingleFlight` executes the load lambda only once (by the "leader"), but all N callers receive a `shared_ptr` pointing to the same `CloudTablet` object. 2. After `SingleFlight::load()` returns, all N callers independently execute: ```cpp _cache->insert(key, value, ...) // each creates a competing LRU cache entry _tablet_map->put(tablet) // each inserts into tablet_map ``` 3. **Each `_cache->insert()` evicts the previous entry for the same key. The evicted `Value`'s destructor calls `_tablet_map.erase(tablet.get())`**. The safety check `it->second.get() == tablet` was designed to prevent erasing a newer tablet object — but here **all callers share the same raw pointer from `SingleFlight`**, so the check always passes, and the erase succeeds. 4. After the last caller's old cache handle is released (when its returned shared_ptr goes out of scope), the destructor erases the entry from _tablet_map. 5. Crucially, subsequent `get_tablet()` calls find the tablet in the LRU cache (cache hit path), which never touches `_tablet_map`. So the tablet is permanently invisible to `_tablet_map` and can never re-enter it. 6. **The compaction scheduler uses `get_weak_tablets()` which iterates `_tablet_map`, so it never sees these tablets and never schedules compaction for them.** ## Before (original correct code, prior to #57922): ```cpp auto load_tablet = [this, &key, ...](int64_t tablet_id) { // load from meta service... // Cache insert + tablet_map put INSIDE lambda — only leader executes auto* handle = _cache->insert(key, value.release(), ...); _tablet_map->put(std::move(tablet)); return ret; }; s_singleflight_load_tablet.load(tablet_id, std::move(load_tablet)); ``` ## After #57922 (buggy code): ```cpp auto load_tablet = [this, ...](int64_t tablet_id) { // load from meta service... return tablet; // just return raw tablet }; auto result = s_singleflight_load_tablet.load(tablet_id, std::move(load_tablet)); // Cache insert + tablet_map put OUTSIDE lambda — ALL concurrent callers execute _cache->insert(key, value.release(), ...); _tablet_map->put(std::move(tablet)); ``` ## Fix Move `_cache->insert()` and `_tablet_map->put()` back inside the `SingleFlight` lambda, ensuring only the leader caller performs cache insertion and `_tablet_map` registration. This restores the invariant that a single `get_tablet()` cache miss produces exactly one LRU cache entry and one `_tablet_map` entry, eliminating the race condition. ### Release note None ### Check List (For Author) - Test <!-- At least one of them must be included. --> - [ ] Regression test - [ ] Unit Test - [ ] Manual test (add detailed scripts or steps below) - [ ] No need to test or manual test. Explain why: - [ ] This is a refactor/code format and no logic has been changed. - [ ] Previous test can cover this change. - [ ] No code files have been changed. - [ ] Other reason <!-- Add your reason? --> - Behavior changed: - [ ] No. - [ ] Yes. <!-- Explain the behavior change --> - Does this need documentation? - [ ] No. - [ ] Yes. <!-- Add document PR link here. eg: apache/doris-website#1214 --> ### 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 -->
16 tasks
yiguolei
pushed a commit
that referenced
this pull request
Mar 21, 2026
…ation (#61581) pick the following PRs/Commits: - #57072 -> 30406d6 - #57783 -> 4cbae8a - #57922 -> 0918952 - #58082 -> 9f9a3e4 - #59259 -> 9f2c6ed - #59269 -> e010d8d - #59311 -> d336eed - #59314 -> f7aa74e - #59645 -> c4c3d14 - #60252 -> b1585e1 - #60269 -> 26f44b5 - #59645 -> c4c3d14 * #60252 -> b1585e1 * #60269 -> 26f44b5 * #59269 -> e010d8d ### What problem does this PR solve? Issue Number: close #xxx Related PR: #xxx Problem Summary: ### Release note None ### Check List (For Author) - Test <!-- At least one of them must be included. --> - [ ] Regression test - [ ] Unit Test - [ ] Manual test (add detailed scripts or steps below) - [ ] No need to test or manual test. Explain why: - [ ] This is a refactor/code format and no logic has been changed. - [ ] Previous test can cover this change. - [ ] No code files have been changed. - [ ] Other reason <!-- Add your reason? --> - Behavior changed: - [ ] No. - [ ] Yes. <!-- Explain the behavior change --> - Does this need documentation? - [ ] No. - [ ] Yes. <!-- Add document PR link here. eg: apache/doris-website#1214 --> ### 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 --> --------- Signed-off-by: zhengyu <zhangzhengyu@selectdb.com> Signed-off-by: freemandealer <freeman.zhang1992@gmail.com>
bobhan1
added a commit
to bobhan1/doris
that referenced
this pull request
Mar 23, 2026
…r due to race condition in `CloudTabletMgr::get_tablet` (apache#60832) ### What problem does this PR solve? Issue Number: close #xxx Related PR: apache#57922 Fix a race condition in `CloudTabletMgr::get_tablet()` introduced by apache#57922 (commit `0918952c70`) that causes tablets to permanently disappear from `_tablet_map`, making them invisible to the compaction scheduler. This leads to tables accumulating hundreds of rowsets without any compaction under high-frequency import. ## Root Cause Commit `0918952c70` refactored `get_tablet()` by moving `_cache->insert()` and `_tablet_map->put()` from inside the `SingleFlight` lambda to outside it. This introduced a race condition: 1. When N concurrent `get_tablet()` calls arrive for the same `tablet_id`, `SingleFlight` executes the load lambda only once (by the "leader"), but all N callers receive a `shared_ptr` pointing to the same `CloudTablet` object. 2. After `SingleFlight::load()` returns, all N callers independently execute: ```cpp _cache->insert(key, value, ...) // each creates a competing LRU cache entry _tablet_map->put(tablet) // each inserts into tablet_map ``` 3. **Each `_cache->insert()` evicts the previous entry for the same key. The evicted `Value`'s destructor calls `_tablet_map.erase(tablet.get())`**. The safety check `it->second.get() == tablet` was designed to prevent erasing a newer tablet object — but here **all callers share the same raw pointer from `SingleFlight`**, so the check always passes, and the erase succeeds. 4. After the last caller's old cache handle is released (when its returned shared_ptr goes out of scope), the destructor erases the entry from _tablet_map. 5. Crucially, subsequent `get_tablet()` calls find the tablet in the LRU cache (cache hit path), which never touches `_tablet_map`. So the tablet is permanently invisible to `_tablet_map` and can never re-enter it. 6. **The compaction scheduler uses `get_weak_tablets()` which iterates `_tablet_map`, so it never sees these tablets and never schedules compaction for them.** ## Before (original correct code, prior to apache#57922): ```cpp auto load_tablet = [this, &key, ...](int64_t tablet_id) { // load from meta service... // Cache insert + tablet_map put INSIDE lambda — only leader executes auto* handle = _cache->insert(key, value.release(), ...); _tablet_map->put(std::move(tablet)); return ret; }; s_singleflight_load_tablet.load(tablet_id, std::move(load_tablet)); ``` ## After apache#57922 (buggy code): ```cpp auto load_tablet = [this, ...](int64_t tablet_id) { // load from meta service... return tablet; // just return raw tablet }; auto result = s_singleflight_load_tablet.load(tablet_id, std::move(load_tablet)); // Cache insert + tablet_map put OUTSIDE lambda — ALL concurrent callers execute _cache->insert(key, value.release(), ...); _tablet_map->put(std::move(tablet)); ``` ## Fix Move `_cache->insert()` and `_tablet_map->put()` back inside the `SingleFlight` lambda, ensuring only the leader caller performs cache insertion and `_tablet_map` registration. This restores the invariant that a single `get_tablet()` cache miss produces exactly one LRU cache entry and one `_tablet_map` entry, eliminating the race condition. ### Release note None ### Check List (For Author) - Test <!-- At least one of them must be included. --> - [ ] Regression test - [ ] Unit Test - [ ] Manual test (add detailed scripts or steps below) - [ ] No need to test or manual test. Explain why: - [ ] This is a refactor/code format and no logic has been changed. - [ ] Previous test can cover this change. - [ ] No code files have been changed. - [ ] Other reason <!-- Add your reason? --> - Behavior changed: - [ ] No. - [ ] Yes. <!-- Explain the behavior change --> - Does this need documentation? - [ ] No. - [ ] Yes. <!-- Add document PR link here. eg: apache/doris-website#1214 --> ### 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 -->
bobhan1
added a commit
to bobhan1/doris
that referenced
this pull request
Mar 23, 2026
…r due to race condition in `CloudTabletMgr::get_tablet` (apache#60832) Issue Number: close #xxx Related PR: apache#57922 Fix a race condition in `CloudTabletMgr::get_tablet()` introduced by disappear from `_tablet_map`, making them invisible to the compaction scheduler. This leads to tables accumulating hundreds of rowsets without any compaction under high-frequency import. ## Root Cause Commit `0918952c70` refactored `get_tablet()` by moving `_cache->insert()` and `_tablet_map->put()` from inside the `SingleFlight` lambda to outside it. This introduced a race condition: 1. When N concurrent `get_tablet()` calls arrive for the same `tablet_id`, `SingleFlight` executes the load lambda only once (by the "leader"), but all N callers receive a `shared_ptr` pointing to the same `CloudTablet` object. 2. After `SingleFlight::load()` returns, all N callers independently execute: ```cpp _cache->insert(key, value, ...) // each creates a competing LRU cache entry _tablet_map->put(tablet) // each inserts into tablet_map ``` 3. **Each `_cache->insert()` evicts the previous entry for the same key. The evicted `Value`'s destructor calls `_tablet_map.erase(tablet.get())`**. The safety check `it->second.get() == tablet` was designed to prevent erasing a newer tablet object — but here **all callers share the same raw pointer from `SingleFlight`**, so the check always passes, and the erase succeeds. 4. After the last caller's old cache handle is released (when its returned shared_ptr goes out of scope), the destructor erases the entry from _tablet_map. 5. Crucially, subsequent `get_tablet()` calls find the tablet in the LRU cache (cache hit path), which never touches `_tablet_map`. So the tablet is permanently invisible to `_tablet_map` and can never re-enter it. 6. **The compaction scheduler uses `get_weak_tablets()` which iterates `_tablet_map`, so it never sees these tablets and never schedules compaction for them.** ## Before (original correct code, prior to apache#57922): ```cpp auto load_tablet = [this, &key, ...](int64_t tablet_id) { // load from meta service... // Cache insert + tablet_map put INSIDE lambda — only leader executes auto* handle = _cache->insert(key, value.release(), ...); _tablet_map->put(std::move(tablet)); return ret; }; s_singleflight_load_tablet.load(tablet_id, std::move(load_tablet)); ``` ## After apache#57922 (buggy code): ```cpp auto load_tablet = [this, ...](int64_t tablet_id) { // load from meta service... return tablet; // just return raw tablet }; auto result = s_singleflight_load_tablet.load(tablet_id, std::move(load_tablet)); // Cache insert + tablet_map put OUTSIDE lambda — ALL concurrent callers execute _cache->insert(key, value.release(), ...); _tablet_map->put(std::move(tablet)); ``` ## Fix Move `_cache->insert()` and `_tablet_map->put()` back inside the `SingleFlight` lambda, ensuring only the leader caller performs cache insertion and `_tablet_map` registration. This restores the invariant that a single `get_tablet()` cache miss produces exactly one LRU cache entry and one `_tablet_map` entry, eliminating the race condition. None - Test <!-- At least one of them must be included. --> - [ ] Regression test - [ ] Unit Test - [ ] Manual test (add detailed scripts or steps below) - [ ] No need to test or manual test. Explain why: - [ ] This is a refactor/code format and no logic has been changed. - [ ] Previous test can cover this change. - [ ] No code files have been changed. - [ ] Other reason <!-- Add your reason? --> - Behavior changed: - [ ] No. - [ ] Yes. <!-- Explain the behavior change --> - Does this need documentation? - [ ] No. - [ ] Yes. <!-- Add document PR link here. eg: apache/doris-website#1214 --> - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label <!-- Add branch pick label that this PR should merge into -->
This was referenced Sep 16, 2026
liaoxin01
added a commit
to liaoxin01/doris
that referenced
this pull request
Sep 16, 2026
### What problem does this PR solve? Related PR: apache#57922 Problem Summary: apache#57922 moved file cache TTL management from block granularity to tablet granularity. As part of that change every path stopped computing an expiration timestamp and started passing the raw ttl_seconds instead: expiration_time = tablet_meta->ttl_seconds(); The receiving side was never updated. KeyMeta::expiration_time still documents itself as an absolute time, and that value is what gets persisted into the block meta store and compared by the cache consistency check. Those fields now hold a duration such as 3600 rather than a timestamp, so the only information left in them is "non-zero means this is a TTL block". Meanwhile the expiration decision itself moved into BlockFileCacheTtlMgr, which sweeps by tablet_ctime + ttl < now, anchored at the tablet creation time. Two consequences: 1. Blocks are created as TTL blocks regardless of whether the tablet is still within its TTL window. Once a tablet is past creation_time + ttl_seconds, the load, compaction and query paths keep putting blocks into the TTL queue and the background sweep keeps pulling them straight back out. Each conversion writes the block meta store and takes the cache lock to move the block between LRU queues, and the loop never terminates for the rest of the tablet's life. 2. The two writer paths disagreed with each other. The packed file path in RowsetWriterContext still computed an absolute newest_write_timestamp + ttl, while the regular file writer path passed the relative ttl_seconds, so segment files and packed small files of the same rowset were stamped with different kinds of value. check_file_cache_consistency reports that as EXPIRATION_TIME_INCONSISTENT. This PR keeps the tablet creation time as the anchor and gives that deadline a single definition, TabletMeta::file_cache_ttl_expiration_time(), which returns the absolute creation_time + ttl_seconds, or 0 when the tablet has no TTL or is already past the deadline. The load, compaction, schema change, query and warm up paths all stamp the blocks they create with that value, so a block's recorded expiration time now agrees with the sweep that acts on it, and a tablet past its deadline has its blocks created as NORMAL directly instead of cycling through the TTL queue. BlockFileCacheTtlMgr itself is left untouched. apache#67971 has since routed both directions through reconcile_tablet_blocks(), which already derives its target cache type from TtlInfo::is_ttl_active(), so an expired tablet is never promoted and this PR does not need to guard that separately. The existing regression tests could not catch this: they all create a table and load into it immediately, so the tablet creation time and the data write time coincide and the bug is invisible. ### Release note Fixed the file cache TTL expiration time being computed inconsistently across the load, compaction, query and warm up paths. Data belonging to a tablet that is past its TTL deadline is now written directly into the normal cache queue instead of repeatedly entering and leaving the TTL queue. ### Check List (For Author) - Test: Unit Test, Regression test. Both are added in this PR but have NOT been executed locally yet, so they still need to pass in CI. - Added TabletMetaTest.FileCacheTtlExpirationTime, covering no TTL, unknown creation time, a live tablet, an expired tablet, the exact deadline, and a ttl large enough to overflow. - Added regression-test/suites/cloud_p0/cache/ttl/test_ttl_expired_tablet.groovy, which lets a tablet pass its deadline before loading anything, then asserts every block lands in the normal queue and that ttl_cache_size stays at 0 across repeated samples, i.e. no promote/demote churn. - Adjusted alter_ttl_seconds.groovy. It uses ttl=5 and the load itself outlives the deadline, so its intermediate "wait until all blocks are ttl" assertion no longer holds by design. Its remaining assertions are unchanged and are now stronger. - All modified translation units were verified to compile, and clang-format reports no changes for the modified files. - Behavior changed: Yes. Blocks of a tablet past its TTL deadline are now created as NORMAL directly, rather than created as TTL and later demoted by the background sweep. The deadline itself is unchanged: it remains the tablet creation time plus file_cache_ttl_seconds. - Does this need documentation: No
liaoxin01
added a commit
to liaoxin01/doris
that referenced
this pull request
Sep 16, 2026
### What problem does this PR solve? Related PR: apache#57922 Problem Summary: apache#57922 moved file cache TTL management from block granularity to tablet granularity. As part of that change every path stopped computing an expiration timestamp and started passing the raw ttl_seconds instead: expiration_time = tablet_meta->ttl_seconds(); The receiving side was never updated. KeyMeta::expiration_time still documents itself as an absolute time, and that value is what gets persisted into the block meta store and compared by the cache consistency check. Those fields now hold a duration such as 3600 rather than a timestamp, so the only information left in them is "non-zero means this is a TTL block". Meanwhile the expiration decision itself moved into BlockFileCacheTtlMgr, which sweeps by tablet_ctime + ttl < now, anchored at the tablet creation time. Two consequences: 1. Blocks are created as TTL blocks regardless of whether the tablet is still within its TTL window. Once a tablet is past creation_time + ttl_seconds, the load, compaction and query paths keep putting blocks into the TTL queue and the background sweep keeps pulling them straight back out. Each conversion writes the block meta store and takes the cache lock to move the block between LRU queues, and the loop never terminates for the rest of the tablet's life. 2. The two writer paths disagreed with each other. The packed file path in RowsetWriterContext still computed an absolute newest_write_timestamp + ttl, while the regular file writer path passed the relative ttl_seconds, so segment files and packed small files of the same rowset were stamped with different kinds of value. check_file_cache_consistency reports that as EXPIRATION_TIME_INCONSISTENT. This PR keeps the tablet creation time as the anchor and gives that deadline a single definition, TabletMeta::file_cache_ttl_expiration_time(), which returns the absolute creation_time + ttl_seconds, or 0 when the tablet has no TTL or is already past the deadline. The load, compaction, schema change, query and warm up paths all stamp the blocks they create with that value, so a block's recorded expiration time now agrees with the sweep that acts on it, and a tablet past its deadline has its blocks created as NORMAL directly instead of cycling through the TTL queue. The score-runtime read in OlapScanner::_prepare_impl() is part of that set: it passed the raw ttl_seconds into IOContext::expiration_time as a positional argument, so inverted index reads for scoring persisted the duration too. BlockFileCacheTtlMgr itself is left untouched. apache#67971 has since routed both directions through reconcile_tablet_blocks(), which already derives its target cache type from TtlInfo::is_ttl_active(), so an expired tablet is never promoted and this PR does not need to guard that separately. The existing regression tests could not catch this: they all create a table and load into it immediately, so the tablet creation time and the data write time coincide and the bug is invisible. ### Release note Fixed the file cache TTL expiration time being computed inconsistently across the load, compaction, query and warm up paths. Data belonging to a tablet that is past its TTL deadline is now written directly into the normal cache queue instead of repeatedly entering and leaving the TTL queue. ### Check List (For Author) - Test: Unit Test, Regression test. Both are added in this PR but have NOT been executed locally yet, so they still need to pass in CI. - Added TabletMetaTest.FileCacheTtlExpirationTime, covering no TTL, unknown creation time, a live tablet, an expired tablet, the exact deadline, and a ttl large enough to overflow. - Added regression-test/suites/cloud_p0/cache/ttl/test_ttl_expired_tablet.groovy. It runs with both TTL background threads turned down to a 10 minute interval, which is what gives it teeth: the sweep would otherwise repair the cache type within a second and the assertions would hold whatever the write and read paths did. With the sweep out of the way the observed cache type is purely the one chosen at admission. It covers a load into an already expired tablet, a read of that tablet on a deliberately emptied cache (so the query path admits the blocks rather than hitting them), and a tablet still inside its window that must still get TTL blocks. Each case also asserts that blocks were actually cached, so it cannot pass vacuously. - Adjusted alter_ttl_seconds.groovy. It uses ttl=5 and the load itself outlives the deadline, so its intermediate "wait until all blocks are ttl" assertion no longer holds by design. Its remaining assertions are unchanged and are now stronger. - All modified translation units were verified to compile, and clang-format reports no changes for the modified files. - Behavior changed: Yes. Blocks of a tablet past its TTL deadline are now created as NORMAL directly, rather than created as TTL and later demoted by the background sweep. The deadline itself is unchanged: it remains the tablet creation time plus file_cache_ttl_seconds. - Does this need documentation: No
liaoxin01
added a commit
that referenced
this pull request
Sep 17, 2026
…68090) ### What problem does this PR solve? Related PR: #57922 Problem Summary: #57922 moved file cache TTL management from block granularity to tablet granularity. As part of that change every path stopped computing an expiration timestamp and started passing the raw ttl_seconds instead: expiration_time = tablet_meta->ttl_seconds(); The receiving side was never updated. KeyMeta::expiration_time still documents itself as an absolute time, and that value is what gets persisted into the block meta store and compared by the cache consistency check. Those fields now hold a duration such as 3600 rather than a timestamp, so the only information left in them is "non-zero means this is a TTL block". Meanwhile the expiration decision itself moved into BlockFileCacheTtlMgr, which sweeps by tablet_ctime + ttl < now, anchored at the tablet creation time. Two consequences: 1. Blocks are created as TTL blocks regardless of whether the tablet is still within its TTL window. Once a tablet is past creation_time + ttl_seconds, the load, compaction and query paths keep putting blocks into the TTL queue and the background sweep keeps pulling them straight back out. Each conversion writes the block meta store and takes the cache lock to move the block between LRU queues, and the loop never terminates for the rest of the tablet's life. 2. The two writer paths disagreed with each other. The packed file path in RowsetWriterContext still computed an absolute newest_write_timestamp + ttl, while the regular file writer path passed the relative ttl_seconds, so segment files and packed small files of the same rowset were stamped with different kinds of value. check_file_cache_consistency reports that as EXPIRATION_TIME_INCONSISTENT. This PR keeps the tablet creation time as the anchor and gives that deadline a single definition, TabletMeta::file_cache_ttl_expiration_time(), which returns the absolute creation_time + ttl_seconds, or 0 when the tablet has no TTL or is already past the deadline. The load, compaction, schema change, query and warm up paths all stamp the blocks they create with that value, so a block's recorded expiration time now agrees with the sweep that acts on it, and a tablet past its deadline has its blocks created as NORMAL directly instead of cycling through the TTL queue. BlockFileCacheTtlMgr additionally no longer promotes the blocks of an already expired tablet on the edge where it first sees that tablet, which previously caused one full promote/demote pass per TTL tablet after every BE restart. The existing regression tests could not catch this: they all create a table and load into it immediately, so the tablet creation time and the data write time coincide and the bug is invisible. ### Release note Fixed the file cache TTL expiration time being computed inconsistently across the load, compaction, query and warm up paths. Data belonging to a tablet that is past its TTL deadline is now written directly into the normal cache queue instead of repeatedly entering and leaving the TTL queue.
liaoxin01
added a commit
to liaoxin01/doris
that referenced
this pull request
Sep 21, 2026
…pache#68090) ### What problem does this PR solve? Related PR: apache#57922 Problem Summary: apache#57922 moved file cache TTL management from block granularity to tablet granularity. As part of that change every path stopped computing an expiration timestamp and started passing the raw ttl_seconds instead: expiration_time = tablet_meta->ttl_seconds(); The receiving side was never updated. KeyMeta::expiration_time still documents itself as an absolute time, and that value is what gets persisted into the block meta store and compared by the cache consistency check. Those fields now hold a duration such as 3600 rather than a timestamp, so the only information left in them is "non-zero means this is a TTL block". Meanwhile the expiration decision itself moved into BlockFileCacheTtlMgr, which sweeps by tablet_ctime + ttl < now, anchored at the tablet creation time. Two consequences: 1. Blocks are created as TTL blocks regardless of whether the tablet is still within its TTL window. Once a tablet is past creation_time + ttl_seconds, the load, compaction and query paths keep putting blocks into the TTL queue and the background sweep keeps pulling them straight back out. Each conversion writes the block meta store and takes the cache lock to move the block between LRU queues, and the loop never terminates for the rest of the tablet's life. 2. The two writer paths disagreed with each other. The packed file path in RowsetWriterContext still computed an absolute newest_write_timestamp + ttl, while the regular file writer path passed the relative ttl_seconds, so segment files and packed small files of the same rowset were stamped with different kinds of value. check_file_cache_consistency reports that as EXPIRATION_TIME_INCONSISTENT. This PR keeps the tablet creation time as the anchor and gives that deadline a single definition, TabletMeta::file_cache_ttl_expiration_time(), which returns the absolute creation_time + ttl_seconds, or 0 when the tablet has no TTL or is already past the deadline. The load, compaction, schema change, query and warm up paths all stamp the blocks they create with that value, so a block's recorded expiration time now agrees with the sweep that acts on it, and a tablet past its deadline has its blocks created as NORMAL directly instead of cycling through the TTL queue. BlockFileCacheTtlMgr additionally no longer promotes the blocks of an already expired tablet on the edge where it first sees that tablet, which previously caused one full promote/demote pass per TTL tablet after every BE restart. The existing regression tests could not catch this: they all create a table and load into it immediately, so the tablet creation time and the data write time coincide and the bug is invisible. ### Release note Fixed the file cache TTL expiration time being computed inconsistently across the load, compaction, query and warm up paths. Data belonging to a tablet that is past its TTL deadline is now written directly into the normal cache queue instead of repeatedly entering and leaving the TTL queue.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This commit refactors the file cache TTL management by:
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
Release note
None
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)