github-actions[bot] commented on code in PR #67004:
URL: https://github.com/apache/doris/pull/67004#discussion_r3822017953
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/CollectRelation.java:
##########
@@ -268,6 +268,15 @@ private void collectMTMVCandidates(TableIf table,
CascadesContext cascadesContex
LOG.debug("table {} related mv set is {}", new
BaseTableInfo(table), mtmvSet);
}
for (MTMV mtmv : mtmvSet) {
+ // Skip MTMVs that currently have an INSERT OVERWRITE in
flight.
+ if (Env.getCurrentEnv().getInsertOverwriteManager()
Review Comment:
[P1] Coordinate the state check with planner locking
This check releases `runningLock` well before `StatementContext.lock()`
takes the candidate's table read lock. For example, planner Q can observe `mv1`
as idle and continue through collection/external preload; refresh R can then
finish its own planning, register `mv1`, and enter temp-partition creation
while holding `mv1`'s table write lock (the cloud path even keeps it across
MetaService RPC/retry); Q then reaches `tryReadLock(1, MINUTES)` and reproduces
the timeout this change is meant to prevent. A second unlocked status check
only narrows the window. Please coordinate the idle decision with acquisition
of the planner read lock (and remove any earlier retained candidate on a state
change) so registration cannot slip between the decision and locking.
##########
fe/fe-core/src/main/java/org/apache/doris/insertoverwrite/InsertOverwriteManager.java:
##########
@@ -360,6 +360,19 @@ public void dropRunningRecord(DatabaseIf db, TableIf
targetTable) throws Excepti
dropRunningRecord(db.getId(), targetTable.getId());
}
+ /**
+ * Whether the given MTMV table currently has an INSERT OVERWRITE running
on this FE master.
+ */
+ public boolean hasRunningTask(long dbId, long tableId) {
+ runningLock.readLock().lock();
+ try {
+ Set<Long> tables = runningTables.get(dbId);
Review Comment:
[P1] Make the marker follow the remote overwrite lifecycle
This transient, one-time marker cannot be authoritative for remote
overwrites in either failure direction. If master A applies ADD but its
response is lost, the tokenless retry is rejected and the caller exits before
cleanup, leaving A stale-true indefinitely. Conversely, if A fails after ADD
but before task registration, `masterCallWithRetry` redirects the later
task/add/replace RPCs to promoted master B; B has no marker or task to recover,
yet now takes B's MTMV write locks, so planners on B see false and can hit the
same one-minute timeout. Please use an idempotent operation identity/lease that
is reconciled or re-registered on the current master, and test both ambiguous
ADD and failover between marker and task registration.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/rules/analysis/CollectRelation.java:
##########
@@ -268,6 +268,15 @@ private void collectMTMVCandidates(TableIf table,
CascadesContext cascadesContex
LOG.debug("table {} related mv set is {}", new
BaseTableInfo(table), mtmvSet);
}
for (MTMV mtmv : mtmvSet) {
+ // Skip MTMVs that currently have an INSERT OVERWRITE in
flight.
+ if (Env.getCurrentEnv().getInsertOverwriteManager()
+ .hasRunningTask(mtmv.getDatabase().getId(),
mtmv.getId())) {
Review Comment:
[P1] Exclude candidates whose MTMV dependencies are running
This only checks the candidate object, but hierarchical MVs
deterministically bypass it. With `mv1 := SELECT ... FROM t4` and `mv2 :=
SELECT ... FROM mv1`, a query over `t4` discovers both candidates. Running
`mv1` is skipped, but non-running `mv2` is accepted;
`mv2.getRelation().getBaseTables()` contains `mv1`, so the loop below adds
`mv1` back to `mtmvRelatedTables`, and `StatementContext.lock()` still waits up
to one minute on it. Reject the whole candidate when any MTMV in the dependency
closure used for planner locking is running, and cover this nested-MV shape
with a deterministic test.
--
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]
