github-actions[bot] commented on code in PR #68180:
URL: https://github.com/apache/doris/pull/68180#discussion_r4068332961
##########
fe/fe-core/src/main/java/org/apache/doris/catalog/MTMV.java:
##########
@@ -682,33 +687,238 @@ public void invalidateIvmBaseline() {
editLogItem.await();
}
- public void invalidateIvmBaseline(BaseTableInfo baseTableInfo, Map<String,
Long> changedPartitions) {
+ /**
+ * Mark the MV partitions that may hold rows read from the changed base
table partitions as needing a
+ * rebuild. When those partitions cannot be determined, the whole MV is
marked instead.
+ */
+ /**
+ * @return whether a barrier was recorded. The caller reports the two
outcomes differently: a change
+ * that no MV partition reads leaves nothing to rebuild and must
not be logged as one.
+ */
+ public boolean invalidateIvmBaseline(BaseTableInfo baseTableInfo,
Map<String, Long> changedPartitions) {
+ // Computed before the MV lock is taken, not inside it: the mapping
reads the partition items of the
+ // MV and of every PCT table, so it takes those tables' locks, and the
MV lock has to stay a leaf
+ // (nothing may be acquired under it) the way the rest of this class
assumes. The selection does not
+ // need to be atomic with the barrier it produces: the barrier is
recorded under the lock below, and
+ // the names it carries are intersected with the live partition names
when they are consumed
+ // (MTMVTask).
+ Optional<Set<String>> affectedMvPartitions =
selectAffectedMvPartitions(baseTableInfo,
+ changedPartitions);
+ if (affectedMvPartitions.isPresent() &&
affectedMvPartitions.get().isEmpty()) {
+ // No MV partition reads any of the changed base partitions, so
this change cannot leave
+ // anything behind here: there is no barrier to persist, and
skipping the version bump
+ // keeps it from discarding the result of a task that is already
running.
+ LOG.debug("No MV partition is affected by changed base partitions,
mv={}, baseTable={}, "
+ + "changedPartitions={}", name, baseTableInfo,
changedPartitions);
+ return false;
+ }
EditLogItem editLogItem;
writeMvLock();
try {
if (ivmInfo == null) {
ivmInfo = new IvmInfo();
}
- if (mvPartitionInfo.getPartitionType() !=
MTMVPartitionType.SELF_MANAGE
- && mvPartitionInfo.getPctInfos().stream()
- .anyMatch(pctInfo ->
pctInfo.getTableInfo().equals(baseTableInfo))) {
- Optional<Set<String>> mvPartitionNames =
refreshSnapshot.getMvPartitionNames(baseTableInfo,
- changedPartitions);
- if (mvPartitionNames.isPresent()) {
-
ivmInfo.addPendingBaselineRebuildPartitions(mvPartitionNames.get());
- } else {
- // Without a snapshot for every changed base partition, a
PARTITIONS rebuild is unsafe.
- ivmInfo.requireCompleteBaselineRebuild();
- }
- } else {
+ if (!affectedMvPartitions.isPresent()) {
+ // A narrower rebuild could leave a partition holding rows of
the changed base partition
+ // untouched, and those rows cannot be repaired later: the
change emitted no row binlog.
ivmInfo.requireCompleteBaselineRebuild();
+ } else {
+
ivmInfo.addPendingBaselineRebuildPartitions(affectedMvPartitions.get());
}
schemaChangeVersion++;
editLogItem = submitIvmInfoChange();
} finally {
writeMvUnlock();
}
editLogItem.await();
+ return true;
+ }
Review Comment:
[P1] Make the sync-limit decision use one synchronized property snapshot.
The two endpoint reads do not bracket a concurrent ALTER: the mapping can be
built while `partition_sync_limit` is active, then the property can be cleared
before the second read, leaving both booleans false even though
`mappedBasePartitions` is window-filtered. With an old partition omitted, `res`
can be empty and `invalidateIvmBaseline` skips the barrier; a concurrent
TRUNCATE/REPLACE then leaves stale rows because no row binlog repairs them.
`mvProperties` is a normal HashMap mutated under `mvRwLock`, but this selector
holds only table read locks. Snapshot/guard the properties and mapping
together, or conservatively rebuild when the property generation changes.
--
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]
