[BugFix] Fix MV refresh transaction rollback to preserve last successful state#70152
[BugFix] Fix MV refresh transaction rollback to preserve last successful state#70152LiShuMing wants to merge 1 commit intoStarRocks:mainfrom
Conversation
…ful state When an MV refresh fails, it was ending up with 0 rows instead of retaining the last successful refresh state. This happened because partitions were dropped before the insert was executed. The fix defers partition drops until after the insert succeeds: 1. Store partitions to be dropped in deferredDropPartitions 2. Add new partitions during syncAddOrDropPartitions() 3. After insert succeeds, call dropDeferredPartitions() 4. If insert fails, original partitions with data are preserved Changes: - MVPCTRefreshPartitioner: Add abstract dropDeferredPartitions() method - MVPCTRefreshListPartitioner: Implement deferred partition drop - MVPCTRefreshRangePartitioner: Implement deferred partition drop - MVPCTRefreshNonPartitioner: Add no-op implementation - MVPCTBasedRefreshProcessor: Call dropDeferredPartitions() after insert - PCTRefreshListPartitionOlapTest: Add test for the fix
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40092d0328
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| dropPartition(db, mv, mvPartitionName); | ||
| } | ||
| logger.info("The process of synchronizing materialized view delete partitions range [{}]", deletes); | ||
| this.deferredDropPartitions = deletes; |
There was a problem hiding this comment.
This change defers all deletions, but range additions are still executed immediately, so addRangePartitions(...) runs while the old partitions are still present. RangePartitionDiffer.diffRange() computes add/delete entries by partition name, so when a partition boundary changes (common split/merge cases), the add side can target a name/range that still exists and addPartitions will fail due to duplicate/overlapping partition definitions. That turns valid refreshes into failures for range-partitioned MVs.
Useful? React with 👍 / 👎.
| } catch (Exception e) { | ||
| logger.warn("Failed to drop deferred partition {} for mv {}, skip", | ||
| mvPartitionName, mv.getName(), e); |
There was a problem hiding this comment.
Swallowing exceptions here lets the task continue as success even when old partitions were not removed. execProcessExecPlan() always calls updateVersionMeta(...) after dropDeferredPartitions(), so a lock/error during dropPartition can leave stale partitions in place while metadata marks refresh complete, which can produce incorrect query results and prevents retry-based cleanup.
Useful? React with 👍 / 👎.
[Java-Extensions Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
[BE Incremental Coverage Report]✅ pass : 0 / 0 (0%) |
2 participants
When an MV refresh fails, it was ending up with 0 rows instead of retaining the last successful refresh state. This happened because partitions were dropped before the insert was executed.
The fix defers partition drops until after the insert succeeds:
Changes:
Why I'm doing:
What I'm doing:
Fixes #issue
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: