yujun777 commented on code in PR #68390:
URL: https://github.com/apache/doris/pull/68390#discussion_r4083022121
##########
fe/fe-core/src/main/java/org/apache/doris/job/extensions/mtmv/MTMVTask.java:
##########
@@ -567,32 +633,13 @@ private MTMVRefreshContext
buildRefreshContext(List<TableIf> tableIfs) throws An
}
}
- /**
- * Makes the barrier that says "these MV partitions must be rebuilt before
their IVM offsets may be
- * used again" durable. Every caller writes it as soon as it has decided
the partition set and
- * before anything that touches MV data or base table streams, so that a
crash or a rejection can
- * only ever leave a barrier with no rebuild behind it, which merely costs
one extra rebuild, and
- * never a rebuild with no barrier, which silently loses rows.
- */
- private void writeIvmBaselineBarrier(RefreshMode refreshMode) throws
JobException {
- if (mtmv.isIvm()) {
- // Persist the guard before the first baseline data transaction.
- mtmv.persistIvmBaselineGuard(refreshMode,
Sets.newHashSet(needRefreshPartitions),
- mtmvSchemaChangeVersion);
- }
- }
-
private void executeCompleteAttempt(MTMVRefreshContext context,
ConnectContext ctx)
throws JobException, AnalysisException {
this.needRefreshPartitions =
Lists.newArrayList(mtmv.getPartitionNames());
this.refreshMode = generateRefreshMode(needRefreshPartitions);
if (refreshMode == MTMVTaskRefreshMode.NOT_REFRESH) {
return;
}
- // The barrier goes first: a stream this rebuild reconciles carries
the base table's current
- // rows as its initial snapshot, and a later incremental refresh that
consumed it as a delta
- // against data still built from the old baseline would double-count
them.
- writeIvmBaselineBarrier(RefreshMode.COMPLETE);
Review Comment:
Implemented. `MTMVTask#executeCompleteAttempt` marks the partitions for
rebuild, journaled as the whole map, ahead of `reconcileIvmStreams`, so a crash
between the stream create and the rebuild leaves the requirement durable rather
than a populated MV with clean epochs and a stream that reads history as an
append.
##########
fe/fe-core/src/main/java/org/apache/doris/mtmv/MTMVRelationManager.java:
##########
@@ -359,16 +362,48 @@ public void alterTable(BaseTableInfo oldTableInfo,
Optional<BaseTableInfo> newTa
// when replace, need deal two table
if (isReplace) {
// REPLACE TABLE already invalidates the IVM baseline explicitly,
see Alter#processReplaceTable
- processBaseTableChange(newTableInfo.get(), "The base table has
been updated:", false);
+ processBaseTableChange(newTableInfo.get(), "The base table has
been updated:", false, false);
}
- // A RENAME leaves every column alone, and the failure it does cause
-- the MV query still
- // spells the old name -- is already reported by the refresh itself
(MTMVTask#run resolves
- // the base tables from the query before it ever looks at the
baseline). Invalidating here
- // would only leave a stale flag behind: rename the table back and the
query is analyzable
- // again, yet every strict INCREMENTAL refresh would stay rejected
until a COMPLETE one ran.
boolean renamed = !isReplace && newTableInfo.isPresent()
&& !Objects.equals(oldTableInfo.getTableName(),
newTableInfo.get().getTableName());
- processBaseTableChange(oldTableInfo, "The base table has been
updated:", !renamed);
+ // The invalidation runs first, while the dependencies are still
registered under the name the
+ // rename is leaving: moving them first would make this lookup --
which is by the old name -- find
+ // nothing, and the rename would stop invalidating anything at all.
+ processBaseTableChange(oldTableInfo, "The base table has been
updated:", !renamed, renamed);
+ if (renamed) {
+ renameBaseTable(oldTableInfo, newTableInfo.get());
+ }
+ }
+
+ /**
+ * Move a renamed table's entries in the dependency maps to its new name.
+ *
+ * <p>The maps are keyed by {@link BaseTableInfo}, which compares by name,
and an MV keeps the relation
+ * it was created against -- a rename leaves the MV query spelling the old
name, so it no longer
+ * analyzes and the relation is not recomputed. Without this the maps
would keep the old name, and a
+ * metadata-only change to the table under its new name -- a TRUNCATE,
say, which emits no row binlog --
+ * would find no dependent MV to invalidate. Renaming the table back then
restores an analyzable query
+ * whose MV still holds the rows that change removed, and nothing names
the partition that would have
+ * to be rebuilt. Moving the entries is what a rename needs instead of the
invalidation it used to
+ * carry: a rename changes no rows, so there is nothing to rebuild, only a
lookup that has to keep
+ * working.
+ */
+ private void renameBaseTable(BaseTableInfo oldTableInfo, BaseTableInfo
newTableInfo) {
Review Comment:
Answered by dropping the re-key: a rename invalidates again, so there is no
derived map to keep in step with `MTMV.relation` across a replay or a cache
reconstruction. The reply on the "close the gaps" finding below has the reasons.
--
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]
