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]

Reply via email to