yujun777 commented on code in PR #62606:
URL: https://github.com/apache/doris/pull/62606#discussion_r3915019007
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/InsertIntoTableCommand.java:
##########
@@ -221,7 +231,28 @@ public void
setInsertExecutorListener(InsertExecutorListener insertExecutorListe
@Override
public void run(ConnectContext ctx, StmtExecutor executor) throws
Exception {
- runInternal(ctx, executor);
+ isRunning.set(true);
+ try {
+ runInternal(ctx, executor);
+ } finally {
+ isRunning.set(false);
+ }
+ }
+
+ @Override
+ public void cancel() {
+ isCancelled.set(true);
+ }
+
+ @Override
+ public void waitNotRunning() {
+ long waitMaxTimeSecond = 10L;
+ try {
+ Awaitility.await().atMost(waitMaxTimeSecond,
TimeUnit.SECONDS).untilFalse(isRunning);
+ } catch (Exception e) {
+ LOG.warn("waiting time exceeds {} second, stop wait, labelName:
{}",
+ waitMaxTimeSecond, labelName.orElse(""), e);
+ }
}
public void runWithUpdateInfo(ConnectContext ctx, StmtExecutor executor,
Review Comment:
Fixed in 10295e68806. `run()` now delegates to `runWithUpdateInfo`, so both
direct statements and the scheduled InsertTask entrypoint share the single
`isRunning` try/finally lifecycle; `waitNotRunning()` therefore waits for the
running write (coordinating/committing included) on the task path too. Covered
by `testRunWithUpdateInfoMaintainsRunningFlag`.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/insert/InsertIntoTableCommand.java:
##########
@@ -336,6 +378,12 @@ public void beforeComplete(AbstractInsertExecutor
insertExecutor, StmtExecutor e
continue;
}
if (insertExecutor.requiresTransaction()) {
+ if (isCancelled.get()) {
+ LOG.info("insert is cancelled before beginTransaction,
queryId: {}",
+ ctx.getQueryIdentifier());
+ newestTargetTableIf.readUnlock();
Review Comment:
Fixed in 10295e68806. The target-table read lock is now released by a single
finally around the check/beginTransaction block; the retry-loop continue
branches and the cancellation throw no longer unlock manually, so the catch
path can no longer double-unlock (IllegalMonitorStateException) and
`insertExecutor.onFail` (txn abort) runs for the real error.
--
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]
