[BugFix] Fix MV rewrite serving stale results after Iceberg rollback_to_snapshot (backport #75924)#76170
Merged
Merged
Conversation
Contributor
Author
|
Cherry-pick of 86051d9 has failed: To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally |
Contributor
Author
|
@mergify[bot]: Backport conflict, please reslove the conflict and resubmit the pr |
Signed-off-by: tqqq <dirtysalt1987@gmail.com>
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.
Why I'm doing:
By default, StarRocks only rewrites a query against an async materialized view (MV) if every base table is unchanged since the MV's last refresh.
mv_rewrite_staleness_secondrelaxes that: it lets the optimizer still use the MV as long as the elapsed time between the base tables' most recent refresh/modification and the MV's own last refresh is within the configured budget.isStalenessSatisfied()computedmvStaleness = (baseTableRefreshTimestamp - mvRefreshTimestamp) / 1000and only rejected the MV as fresh when that value exceededmv_rewrite_staleness_second. It never checked for the base table's refresh timestamp going backwards relative to the MV's own last refresh.For Iceberg base tables,
IcebergPartitionTraits.maxPartitionRefreshTs()returns the rawcurrentSnapshot().timestampMillis(). This is normally monotonically increasing, but Iceberg'srollback_to_snapshot/rollback_to_timestampprocedures can move a table's current snapshot - and therefore its timestamp - backwards in time. When that happens,mvStalenessgoes negative, which always satisfiesmvStaleness > staleness_budgetas false, so the MV is treated as fresh and used for rewrite even though the underlying data has been rolled back to a different state than what the MV last captured.Concretely,
getUpdatedPartitionNamesOfExternalTable()/getUpdatedPartitionNamesOfOlapTable()short-circuit to "no partitions need checking" wheneverisStalenessSatisfied()returns true, so the bug skips the actual per-partition staleness comparison entirely.What I'm doing:
Reject the MV outright whenever
mvStaleness < 0instead of falling through to the budget comparison, so a rolled-back base table forces the planner to fall through to the normal per-partition comparison instead of blindly trusting a stale MV.This only affects MVs that explicitly set
mv_rewrite_staleness_second(default is0, i.e. disabled) and, in practice, is only reachable through Iceberg'srollback_to_snapshot/rollback_to_timestampprocedures, since other connectors'maxPartitionRefreshTs()implementations don't have a documented operation that moves their timestamp backwards.Fixes #71977
What type of PR is this:
Does this PR entail a change in behavior?
If yes, please specify the type of change:
Checklist:
Bugfix cherry-pick branch check:
Test plan
MaterializedViewTest#testIsStalenessSatisfiedWithNegativeStalenessand#testIsStalenessSatisfiedWithPositiveStalenessWithinBudget, exercising the new negative-staleness branch and confirming the existing positive-staleness-within-budget path is unaffected.mvn -pl fe-grammar,fe-parser,fe-core -am test-compileclean.mvn -pl fe-core checkstyle:checkclean.Tests run: 2, Failures: 0, Errors: 0.This is an automatic backport of pull request [BugFix] Fix MV rewrite serving stale results after Iceberg rollback_to_snapshot #75924 done by Mergify.