yujun777 commented on code in PR #68390:
URL: https://github.com/apache/doris/pull/68390#discussion_r4083014743
##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVRelationManager.java:
##########
@@ -486,6 +484,15 @@ private void processBaseTableChange(BaseTableInfo
baseTableInfo, String msgPrefi
if (checkIvmQueryUsable) {
invalidateIvmBaselineIfQueryUnusable(baseTableInfo, mtmv);
}
+ if (renamed && mtmv instanceof MTMV && ((MTMV) mtmv).isIvm()) {
+ // A rename leaves every column alone, and the failure it does
cause -- the MV query still
+ // spells the old name -- is reported by the refresh itself:
it resolves the base tables from
+ // the query (MTMVTask#run) before it looks at anything else,
so the state is not what makes
+ // that failure visible. What the state does to an IVM MV is
make the next refresh rebuild the
+ // whole MV (MTMVTask#buildAttempts), which a rename back
would have it repeat for nothing. A
+ // non-IVM MV keeps the state it has always got, which is what
its own refresh reads.
+ continue;
Review Comment:
Superseded: the re-key is dropped rather than patched. Renaming a base table
invalidates the MVs that read it again, as it did before this PR, because the
move could not be made durable. The reasoning is in the replies to the three
findings from the later round on this file.
`IvmBaselineRebuildTest#testRenameTableMarksBaselineRebuild` and
`#testRenameTableBackStillRequiresAWholeMvRefresh` pin both directions, with
the non-IVM case next to them.
##########
fe/fe-core/src/main/java/org/apache/doris/job/extensions/mtmv/MTMVTask.java:
##########
@@ -715,13 +664,36 @@ private AttemptResultType
executeIvmAttempt(MTMVRefreshContext refreshContext,
+ "Continuing with COMPLETE refresh.", mtmv.getName(),
getTaskId());
return AttemptResultType.FALLBACK_TO_COMPLETE;
}
+ // The partitions the criterion says must be rebuilt rather than
caught up: the delta path can only
+ // append, so a partition it treated as current would record that in
its epoch while its rows still
+ // come from before the change. Rebuilt first, with the partition
executor, because that is the
+ // full recomputation they need -- and only in this task's batches, so
a change that arrives while
+ // it runs leaves them dirty for the next round instead of being
swallowed.
+ Set<String> dirtyPartitions = mtmv.getDirtyPartitions();
+ Map<String, MTMVRefreshPartitionSnapshot> rebuiltSnapshots =
Maps.newHashMap();
+ if (!dirtyPartitions.isEmpty()) {
+ LOG.info("Rebuilding {} invalidated MV partitions before the
incremental refresh, mv={}, taskId={}",
+ dirtyPartitions.size(), mtmv.getName(), getTaskId());
+ List<String> toRebuild = Lists.newArrayList(dirtyPartitions);
+ toRebuild.sort(Comparator.naturalOrder());
+ this.needRefreshPartitions = toRebuild;
+ this.refreshMode = generateRefreshMode(toRebuild);
+ executePartitionBasedRefresh(refreshContext,
RefreshMode.PARTITIONS, ctx);
+ rebuiltSnapshots.putAll(partitionSnapshots);
+ recordRebuiltPartitions(request, dirtyPartitions.size());
Review Comment:
Implemented. The count is taken from the partitions whose batches committed,
not from the plan: the rebuild records it in a `finally` around the committed
groups, and the whole-MV escalation records its own count only once the attempt
has returned, so a refresh that fails before its first commit reports nothing.
--
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]
