924060929 commented on code in PR #66914:
URL: https://github.com/apache/doris/pull/66914#discussion_r3838099867
##########
fe/fe-core/src/main/java/org/apache/doris/service/arrowflight/sessions/FlightSqlConnectPoolMgr.java:
##########
@@ -57,6 +59,12 @@ public int registerConnection(ConnectContext ctx) {
@Override
public void unregisterConnection(ConnectContext ctx) {
+ // Reject new publications first, then signal the active query before
waiting for an admitted
+ // GetFlightInfo publisher. Waiting before cancellation can deadlock
KILL CONNECTION behind the
+ // publisher whose query must be canceled in order to leave
publication.
+ ctx.sealFlightSqlDeferredExecutors();
+ ctx.cancelQuery(new Status(TStatusCode.CANCELLED, "arrow flight
connection closed"));
Review Comment:
Fixed in 445ec0d8262. Executor publication is now linearized with both the
Flight terminal seal and retained forward-cancel state; a late StmtExecutor
receives cancellation before execution admission. Added a real-executor
regression in StmtExecutorCancellationTest. Focused tests and the full FE build
pass.
##########
fe/fe-core/src/main/java/org/apache/doris/job/manager/JobManager.java:
##########
@@ -280,10 +280,10 @@ public void alterJobStatus(String jobName, JobStatus
jobStatus, FailureReason re
if (a.getJobName().equals(jobName)) {
try {
checkSameStatus(a, jobStatus);
- alterJobStatus(a.getJobId(), jobStatus);
if (a instanceof StreamingInsertJob) {
((StreamingInsertJob)
a).onManualStatusAltered(jobStatus, reason);
}
+ alterJobStatus(a.getJobId(), jobStatus);
Review Comment:
Fixed in 445ec0d8262. Manual reason, cancellation publication, status, and
epoch now share one job write-lock transition; blocking cancellation and reader
acknowledgement remain outside the lock. Invalid transitions cannot mutate
state, PAUSE/STOP cannot lose to a late callback, and STOP now cancels the
active task. Added invalid-transition, stale-tick, pause/resume, and STOP
late-success regressions. Focused tests and the full FE build pass.
##########
fe/fe-connector/fe-connector-iceberg/src/main/java/org/apache/doris/connector/iceberg/IcebergConnectorTransaction.java:
##########
@@ -1335,7 +1399,16 @@ public void rollback() {
@Override
public void close() {
- // No resources to release: the SDK transaction holds no connections
of its own.
+ IcebergStatementScope.TrackedTableLease lease;
+ synchronized (beginLock) {
+ closed = true;
+ lease = tableLease;
+ tableLease = null;
+ beginLock.notifyAll();
+ }
+ if (lease != null) {
+ lease.close();
Review Comment:
Fixed in 445ec0d8262. Connector rewrite groups now publish terminal
completion, replay cancellation to a late coordinator, and are canceled and
awaited before the shared transaction lease is rolled back or closed.
Submission failures also roll back the transient-task registration. Added
resource/partial-submission regressions; focused tests and the full FE build
pass.
##########
fe/fe-core/src/main/java/org/apache/doris/job/extensions/insert/streaming/StreamingInsertTask.java:
##########
@@ -173,24 +175,66 @@ protected void onFail(String errMsg) throws JobException {
@Override
public void cancel(boolean needWaitCancelComplete) {
super.cancel(needWaitCancelComplete);
- if (null != stmtExecutor) {
+ StmtExecutor executor = stmtExecutor;
+ if (null != executor) {
log.info("cancelling streaming insert task, job id is {}, task id
is {}",
getJobId(), getTaskId());
- stmtExecutor.cancel(new Status(TStatusCode.CANCELLED, "streaming
insert task cancelled"),
- needWaitCancelComplete);
+ executor.cancel(new Status(TStatusCode.CANCELLED, "streaming
insert task cancelled"), false);
+ }
+ if (needWaitCancelComplete) {
+ // Planning may still be blocked before stmtExecutor is published.
Do not let PAUSE wait
+ // forever; the scheduler owner remains responsible for exact-once
cleanup in execute().
+ awaitExecutionCompletion(CANCEL_WAIT_TIMEOUT_MS);
Review Comment:
Fixed in 445ec0d8262. The exact predecessor remains installed until its
execution owner reaches terminal handoff; scheduler admission rejects a
successor while that predecessor is retained, and callbacks use identity plus
status/epoch fences. Added predecessor and stale-callback regressions. Focused
tests and the full FE build pass.
--
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]
