yujun777 commented on code in PR #68390:
URL: https://github.com/apache/doris/pull/68390#discussion_r4089235305
##########
fe/fe-core/src/main/java/org/apache/doris/job/extensions/mtmv/MTMVTask.java:
##########
@@ -510,9 +549,44 @@ && hasUnusableIvmStream()) {
mtmv.getName(), getTaskId());
return Lists.newArrayList(RefreshAttemptType.COMPLETE);
}
+ // Every partition either needs a rebuild or was never filled, and at
least one needs a rebuild:
+ // COMPLETE then does nothing the per-partition routing would not, in
one read of the MV.
+ if (!request.explicitPartitions &&
attempts.contains(RefreshAttemptType.IVM)
+ && shouldEscalateToComplete()) {
+ LOG.info("Every MV partition needs a rebuild or has no data yet,
mv={}, taskId={}. "
+ + "Continuing with COMPLETE refresh.", mtmv.getName(),
getTaskId());
+ return Lists.newArrayList(RefreshAttemptType.COMPLETE);
+ }
return attempts;
}
+ /**
+ * Notes that this refresh rebuilds partitions the request did not ask to
rebuild, which is what a
+ * strict INCREMENTAL request cannot tell from its result otherwise: it
reports the count, and a request
+ * that asked for a complete refresh reports nothing because rebuilding
everything is what it asked for.
+ */
+ private void recordRebuiltPartitions(RefreshRequest request, int
rebuiltPartitions) {
Review Comment:
Fixed: `recordRebuiltPartitions` returns early when the MV has no IVM
baseline. `MTMVTaskTest#testANonIvmRefreshReportsNoRebuiltPartitions` pins it
-- a plain MV on an AUTO request records nothing.
##########
fe/fe-core/src/main/java/org/apache/doris/job/extensions/mtmv/MTMVTask.java:
##########
@@ -732,6 +780,10 @@ private AttemptResultType
executeIvmAttempt(MTMVRefreshContext refreshContext,
}
try {
syncPartitionsIfNeeded(ctx, tableIfs);
+ // The retry can add a partition that did not exist at the
first alignment. It has to get
+ // its entry before the retried refresh reads a base table, or
an invalidation arriving
+ // in between would have nothing to land on for rows this task
is about to write.
+ mtmv.alignPartitionStates(mtmv.getPartitionNames());
Review Comment:
Fixed: the retry reads the states again after the alignment and adopts what
it created -- the new partition joins the dirty set, so the incremental attempt
leaves it out instead of recording a delta's capture as it being caught up, and
it gets the entry the batches are clamped against.
It is not rebuilt in this task, and the comment says why: the partition the
alignment created holds no rows yet, so what it needs is a build rather than a
rebuild, and leaving it dirty is what the next refresh's rebuild gives it.
Rebuilding it here would need a second rebuild phase inside the retry and would
make `IvmRebuiltPartitions` count over a task-wide accumulator.
`MTMVTaskTest#testRetryAdoptsThePartitionsAlignmentCreated` covers the
adoption.
--
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]