This is an automated email from the ASF dual-hosted git repository.
yujun777 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/master by this push:
new 9e98d362389 [improvement](mtmv) Journal MV alter ops under the MV lock
and read IVM info lock-free (#69256)
9e98d362389 is described below
commit 9e98d3623892bf133dc44149af2fcdc14226ecc1
Author: yujun <[email protected]>
AuthorDate: Tue Oct 20 15:55:50 2026 +0800
[improvement](mtmv) Journal MV alter ops under the MV lock and read IVM
info lock-free (#69256)
Two lock/journaling gaps in the MTMV metadata path.
`ALTER_REFRESH_INFO` and `ALTER_STATUS` changed the MV metadata under
the MV write lock but enqueued the matching journal record only after
that lock was released (`Alter.processAlterMTMV`). A task result
journaled in between could be replayed on the other side of the alter,
and a live `ALTER_STATUS` could lose its record if a later step threw
after the metadata had already moved.
`MTMV.getIvmInfo()` took the MV write lock on the query path (`isIvm()`
reads it). `enableIvm` is decided at CREATE and no ALTER path changes
it, and the only other mutable field, the sequence prefix, is read when
a refresh plans and advanced when the same refresh's transaction commits
-- both under the MV's `MTMVJob` write lock, which serializes refreshes
of one MV.
Changes:
- Apply `ALTER_REFRESH_INFO` / `ALTER_STATUS` and submit their journal
record under one MV write lock; `processAlterMTMV` awaits the entry
after the lock is released, like the other journaled MTMV changes.
- Make `getIvmInfo()` a lock-free read and mark the payload reference
`volatile`, so a replay that replaces it publishes it safely. `isIvm()`
and `getNextSequencePrefix()` no longer queue for the MV write lock.
- Correct the `MTMVTask` comment that claimed base table snapshots were
captured under a read lock: they are captured before the read, without
one.
- The remaining item of the same list, the `addTaskResult` race on
`getPartitionNames()`, was fixed separately by #69166.
---
.../main/java/org/apache/doris/alter/Alter.java | 14 ++--
.../main/java/org/apache/doris/catalog/MTMV.java | 80 +++++++++++++++++-----
.../apache/doris/job/extensions/mtmv/MTMVTask.java | 9 ++-
3 files changed, 78 insertions(+), 25 deletions(-)
diff --git a/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java
b/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java
index d64e5a94a07..52d28703321 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/alter/Alter.java
@@ -95,6 +95,7 @@ import
org.apache.doris.nereids.trees.plans.commands.info.ReplaceTableOp;
import org.apache.doris.persist.AlterMTMV;
import org.apache.doris.persist.AlterViewInfo;
import org.apache.doris.persist.BatchModifyPartitionsInfo;
+import org.apache.doris.persist.EditLog.EditLogItem;
import org.apache.doris.persist.ModifyCommentOperationLog;
import org.apache.doris.persist.ModifyPartitionInfo;
import org.apache.doris.persist.ModifyTableEngineOperationLog;
@@ -1357,15 +1358,20 @@ public class Alter {
public void processAlterMTMV(AlterMTMV alterMTMV, boolean isReplay) {
TableNameInfo tbl = alterMTMV.getMvName();
MTMV mtmv = null;
+ // ALTER_REFRESH_INFO and ALTER_STATUS journal themselves from inside
the MV write lock that
+ // applies them: mutation and enqueue have to be the same step, or a
record submitted in
+ // between can be replayed on either side of this one. Their journal
write is awaited after the
+ // lock is released, like the other journaled MTMV changes.
+ EditLogItem editLogItem = null;
try {
Database db =
Env.getCurrentInternalCatalog().getDbOrDdlException(tbl.getDb());
mtmv = (MTMV) db.getTableOrMetaException(tbl.getTbl(),
TableType.MATERIALIZED_VIEW);
switch (alterMTMV.getOpType()) {
case ALTER_REFRESH_INFO:
- mtmv.alterRefreshInfo(alterMTMV.getRefreshInfo());
+ editLogItem = mtmv.alterRefreshInfo(alterMTMV, isReplay);
break;
case ALTER_STATUS:
- mtmv.alterStatus(alterMTMV.getStatus());
+ editLogItem = mtmv.alterStatus(alterMTMV, isReplay);
break;
case ALTER_PROPERTY:
// Live ALTER PROPERTY statements call
processAlterMTMVProperty directly
@@ -1401,8 +1407,8 @@ public class Alter {
if (alterMTMV.isNeedRebuildJob()) {
Env.getCurrentEnv().getMtmvService().alterJob(mtmv, isReplay);
}
- if (!isReplay) {
- Env.getCurrentEnv().getEditLog().logAlterMTMV(alterMTMV);
+ if (editLogItem != null) {
+ editLogItem.await();
}
} catch (UserException e) {
// if MTMV has been dropped, ignore this exception
diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/MTMV.java
b/fe/fe-core/src/main/java/org/apache/doris/catalog/MTMV.java
index 4b651ebd36e..9805be4e157 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/catalog/MTMV.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/MTMV.java
@@ -111,7 +111,7 @@ public class MTMV extends OlapTable {
@SerializedName("rs")
private MTMVRefreshSnapshot refreshSnapshot = new MTMVRefreshSnapshot();
@SerializedName("ii")
- private IvmInfo ivmInfo = new IvmInfo();
+ private volatile IvmInfo ivmInfo = new IvmInfo();
/**
* The refresh epoch of every MV partition, keyed by MV partition name.
*
@@ -223,13 +223,25 @@ public class MTMV extends OlapTable {
}
}
- public MTMVRefreshInfo alterRefreshInfo(MTMVRefreshInfo newRefreshInfo) {
+ /**
+ * Applies an ALTER_REFRESH_INFO op and journals it under one MV write
lock, so that the record
+ * order is the order the change took effect in (see {@link
#submitAlterLog}). The journal write
+ * itself is awaited by the caller after the lock is released.
+ *
+ * @return the pending journal entry, or null on replay (which has nothing
to record)
+ */
+ public EditLogItem alterRefreshInfo(AlterMTMV alterMTMV, boolean isReplay)
{
+ EditLogItem editLogItem = null;
writeMvLock();
try {
- return refreshInfo.updateNotNull(newRefreshInfo);
+ refreshInfo.updateNotNull(alterMTMV.getRefreshInfo());
+ if (!isReplay) {
+ editLogItem = submitAlterLog(alterMTMV);
+ }
} finally {
writeMvUnlock();
}
+ return editLogItem;
}
/**
@@ -238,8 +250,8 @@ public class MTMV extends OlapTable {
* the transparent rewrite serving rows from it.
*
* <p>This is the step that <em>applies</em> a change, not the one that
records it -- it takes the MV
- * lock and moves all three, but nothing here is journaled. It is what
{@code Alter#processAlterMTMV}
- * calls for an {@code ALTER_STATUS} op, both live and on replay; a live
caller reaches it through
+ * lock and moves all three, but nothing here is journaled. {@code
Alter#processAlterMTMV} uses the
+ * overload that journals under the same lock; a live invalidation reaches
it through
* {@link #invalidateWholeMv}, which goes the journaled way round. A new
invalidation belongs there:
* calling this directly would leave the MV in a state a restart forgets.
The one caller that goes
* direct is {@link #processBaseViewChange}, and it is not new: the alter
or the drop of the view is
@@ -252,13 +264,40 @@ public class MTMV extends OlapTable {
public MTMVStatus alterStatus(MTMVStatus newStatus) {
writeMvLock();
try {
- // only can update state, refresh state will be change by add task
- this.schemaChangeVersion++;
- this.refreshSnapshot = new MTMVRefreshSnapshot();
- return this.status.updateStateAndDetail(newStatus);
+ return applyStatus(newStatus);
+ } finally {
+ writeMvUnlock();
+ }
+ }
+
+ /**
+ * Applies an ALTER_STATUS op and journals it under one MV write lock. An
ALTER_STATUS is applied
+ * together with what it stands for -- the version bump and the snapshot
drop -- and the record has
+ * to describe that same step, not a later one: a result journaled in
between would be replayed on
+ * either side of it.
+ *
+ * @return the pending journal entry, or null on replay (which has nothing
to record)
+ */
+ public EditLogItem alterStatus(AlterMTMV alterMTMV, boolean isReplay) {
+ EditLogItem editLogItem = null;
+ writeMvLock();
+ try {
+ applyStatus(alterMTMV.getStatus());
+ if (!isReplay) {
+ editLogItem = submitAlterLog(alterMTMV);
+ }
} finally {
writeMvUnlock();
}
+ return editLogItem;
+ }
+
+ // Assumes the MV write lock is held.
+ private MTMVStatus applyStatus(MTMVStatus newStatus) {
+ // only can update state, refresh state will be change by add task
+ this.schemaChangeVersion++;
+ this.refreshSnapshot = new MTMVRefreshSnapshot();
+ return this.status.updateStateAndDetail(newStatus);
}
/**
@@ -757,16 +796,21 @@ public class MTMV extends OlapTable {
}
}
+ /**
+ * The MV's IVM metadata, readable without the MV lock.
+ *
+ * <p>{@code isIvm()} is on the query path, and what it reads is fixed for
the MV's lifetime:
+ * {@code enableIvm} is decided at CREATE and no ALTER path changes it.
The rest of the payload is
+ * touched only by the refresh of this MV -- the sequence prefix is read
when a refresh plans and
+ * advanced when its transaction commits, and the plan signature is
published by the same task's
+ * result -- and one MV's tasks run one at a time, under its job's write
lock (see
+ * {@code MTMVTask#runTask}). So no reader here races a writer.
+ *
+ * <p>Never null: the field is created with the MV, and {@link
#gsonPostProcess} covers an image
+ * that carries it as null.
+ */
public IvmInfo getIvmInfo() {
- writeMvLock();
- try {
- if (ivmInfo == null) {
- ivmInfo = new IvmInfo();
- }
- return ivmInfo;
- } finally {
- writeMvUnlock();
- }
+ return ivmInfo;
}
// ALTER_IVM_INFO replay applies a detached snapshot here. Live IVM
changes submit their journal
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/job/extensions/mtmv/MTMVTask.java
b/fe/fe-core/src/main/java/org/apache/doris/job/extensions/mtmv/MTMVTask.java
index 50ffaef0a4a..1ac1cd7477d 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/job/extensions/mtmv/MTMVTask.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/job/extensions/mtmv/MTMVTask.java
@@ -893,9 +893,12 @@ public class MTMVTask extends AbstractTask {
return IvmIncrRefreshResult.success();
}
IvmIncrRefreshManager ivmIncrRefreshManager = new
IvmIncrRefreshManager();
- // Capture base table snapshots under read lock before execution, same
as
- // partition-based refresh. This ensures snapshot versions are
consistent
- // with the data the INSERT will read.
+ // Capture the base table snapshots before execution, same as the
partition-based path. No
+ // base table read lock is held: what the record must guarantee is
that it is no newer than
+ // the data the INSERT reads, and capturing first gives exactly that
-- a version committed
+ // in between is simply missing from the record, which leaves the MV
looking less fresh than
+ // it is. Partitions added or dropped in the same window are
reconciled by the refresh's own
+ // partition sync, not by this record.
Map<String, MTMVRefreshPartitionSnapshot> capturedSnapshots;
try {
capturedSnapshots = MTMVPartitionUtil.generatePartitionSnapshots(
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
