yujun777 commented on code in PR #68180:
URL: https://github.com/apache/doris/pull/68180#discussion_r4069294942
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/MTMV.java:
##########
@@ -423,6 +437,10 @@ public void alterMvProperties(AlterMTMV alterMTMV, boolean
isReplay) {
}
}
}
+ if (containsSyncWindow && ivmInfo != null && ivmInfo.isEnableIvm()
Review Comment:
Fixed in 253120b64b5. Both false triggers are real, and so is the cost.
The comparison is now on the effective window rather than on the stored
values, and only a change that can bring a base partition back into the
maintained set requires the rebuild:
```java
public static boolean partitionSyncWindowWidens(Map<String, String>
oldWindowProperties,
Map<String, String> newWindowProperties) {
MTMVPartitionSyncConfig oldWindow =
MTMVRelatedPartitionDescSyncLimitGenerator
.generateMTMVPartitionSyncConfigByProperties(oldWindowProperties);
if (oldWindow.getSyncLimit() <= 0) {
return false;
}
MTMVPartitionSyncConfig newWindow =
MTMVRelatedPartitionDescSyncLimitGenerator
.generateMTMVPartitionSyncConfigByProperties(newWindowProperties);
if (newWindow.getSyncLimit() <= 0) {
return true;
}
return !oldWindow.getTimeUnit().equals(newWindow.getTimeUnit())
|| !oldWindow.getDateFormat().equals(newWindow.getDateFormat())
|| newWindow.getSyncLimit() > oldWindow.getSyncLimit();
}
```
A window that starts applying brings nothing back, so `oldWindow` being
inactive answers false, and a narrower one answers false as well. A unit or
date format change while active counts as one that may widen, because two
windows in different units are not comparable without a clock -- and `now()` is
deliberately kept out of this decision: a comparison that lands on the wrong
side of a unit boundary would be wrong in the unsafe direction, a missed
rebuild rather than an extra one. Restating the same window compares equal.
On the parts you left open:
- A narrowing needs no generation bump here, and I did not add one.
`schemaChangeVersion` is not a silent discard: a task whose captured version no
longer matches fails at its next `persistIvmBaselineGuard`,
`validateIvmRefreshStart` or `releaseIvmBaselineRebuild` ("Base table metadata
changed before IVM baseline refresh"). For a narrowing that trades a failed
refresh for nothing: what an in-flight task publishes is rows for partitions
the next partition sync drops, with their IVM offsets, and the rows themselves
still match the base table they were read from.
- The three tests that set the window no longer carry the barrier release
added in the previous round: a window that starts applying records no barrier
now, so their assertions are carried by the selection they are about rather
than by the ALTER.
Coverage:
`IvmBaselineRebuildTest.testOnlyAWiderSyncWindowRequiresCompleteBaselineRebuild`
-- a unit set while no limit is in effect (nothing), a window that starts
applying (nothing), the same window restated (nothing), a narrower one
(nothing), a wider one (rebuild), a removed one (rebuild). Against the previous
comparison it fails on the first of those with `expected: <false> but was:
<true>`, which is your absent -> unit-only example.
Verification on this head: `IvmBaselineRebuildTest` 37/37,
`MTMVPropertyUtilTest` 13/13, `MTMVTest` 23/23, `MTMVPartitionUtilTest` 18/18,
`checkstyle:check` clean, and `mtmv_p0/ivm` test_ivm_partition_sync_limit,
test_ivm_partition_sync_limit_with_window, test_ivm_baseline_marker_scope,
test_ivm_partition_baseline_rebuild,
test_ivm_partition_baseline_rebuild_dup_keys,
test_ivm_drop_referenced_column_baseline_rebuild,
test_ivm_partition_drop_live_delta, test_ivm_replace_table_reconcile,
test_ivm_replace_stream_cleanup and test_ivm_drop_column_fallback_reason green
on a local cluster built from it.
--
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]
