[fix](cloud) Set file cache TTL on cloud schema change output - #67454
Open
liaoxin01 wants to merge 1 commit into
Open
[fix](cloud) Set file cache TTL on cloud schema change output#67454liaoxin01 wants to merge 1 commit into
liaoxin01 wants to merge 1 commit into
Conversation
`CloudSchemaChangeJob::_convert_historical_rowsets()` builds its `RowsetWriterContext` with `write_file_cache` but never sets `file_cache_ttl_sec`, so for a table with `file_cache_ttl_seconds` the schema change output is written into the NORMAL/INDEX cache queues instead of the TTL queue. The load path (`CloudRowsetBuilder::init()`) and the compaction path (`CloudCompactionMixin::construct_output_rowset_writer()`) both set it. Two consequences: - The schema change output cannot be retained by the table TTL on the cluster that runs the job, and can be evicted early by NORMAL/INDEX LRU. - Every warm-up path on the destination cluster (event driven, sync rowset and warm-up job) derives the expiration from `tablet_meta->ttl_seconds()` unconditionally, so the same rowsets land in the TTL queue there. Source and destination end up with the same cached bytes but different queues. This is what `test_warm_up_cluster_event_schema_change` catches: after a schema change the `ttl_cache_size` of the destination cluster exceeds the source by exactly the size of the schema change output. Take the TTL from the new tablet, which is also the tablet the warm-up paths read it from.
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
gavinchou
approved these changes
Sep 2, 2026
Contributor
Author
|
run buildall |
Contributor
TPC-H: Total hot run time: 16944 ms |
Contributor
TPC-DS: Total hot run time: 81971 ms |
Contributor
ClickBench: Total hot run time: 14.53 s |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
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
3 participants
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.
What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
CloudSchemaChangeJob::_convert_historical_rowsets()builds itsRowsetWriterContextwithwrite_file_cachebut never setsfile_cache_ttl_sec. For a table withfile_cache_ttl_seconds, the schema change output is therefore written into the NORMAL/INDEX file cache queues instead of the TTL queue. The other two cloud write paths both set it:CloudRowsetBuilder::init()—context.file_cache_ttl_sec = _tablet->ttl_seconds();CloudCompactionMixin::construct_output_rowset_writer()—ctx.file_cache_ttl_sec = _tablet->ttl_seconds();Two consequences:
tablet_meta->ttl_seconds()unconditionally — event driven (CloudInternalService::warm_up_rowset), sync rowset (CloudTablet::add_rowsets) and warm-up job (CloudWarmUpManager) — so the very same rowsets land in the TTL queue there. Source and destination end up with the same total cached bytes but different queue attribution.This is what
test_warm_up_cluster_event_schema_changecatches. After the schema change,checkTTLCacheSizeSumEqual()fails because the destinationttl_cache_sizeexceeds the source by exactly the size of the schema change output:489694 - 229588 = 260106 = 148362 (data) + 111744 (index), which is exactly what the schema change job wrote (its log showsinput_hit_rate=1, should_cache=1, so the output was cached — just in the wrong queue).The fix takes the TTL from the new tablet, which is the same tablet the warm-up paths read it from.
Release note
None
Check List (For Author)
Covered by the existing
test_warm_up_cluster_event_schema_change, which fails before this change and passes after it.Behavior changed:
file_cache_ttl_seconds, the cloud schema change output is now cached in the TTL queue instead of the NORMAL/INDEX queues, matching the load and compaction paths.Does this need documentation?
Check List (For Reviewer who merge this PR)