github-actions[bot] commented on code in PR #68353:
URL: https://github.com/apache/doris/pull/68353#discussion_r4070145626
##########
fe/fe-core/src/main/java/org/apache/doris/job/extensions/insert/streaming/StreamingInsertTask.java:
##########
@@ -239,6 +239,15 @@ public void closeOrReleaseResources() {
taskCommand = null;
}
if (null != ctx) {
+ // This attempt's statement is over: end its StatementContext the
way TaskProcessor does
+ // for the tasks it runs (this task is run by the streaming
scheduler instead). The plan
+ // before() built only to rewrite the TVF never gets a
coordinator, so what its scan nodes
+ // opened for one - the Flight SQL session of a remote Doris scan
joined with the TVF - is
+ // released here rather than left to the remote frontend's
wait_timeout.
+ StatementContext statementContext = ctx.getStatementContext();
+ if (statementContext != null) {
+ statementContext.close();
Review Comment:
[P1] Close canceled attempts only after planning has quiesced
This cleanup is not guaranteed to run at a terminal task boundary. `STOPPED`
only calls `cancelAllTasks(false)`, which marks the task `CANCELED`;
`StreamingInsertJob.updateJobStatus` force-cleans only `PAUSED`, while
`AbstractStreamingTask.execute()` skips its own cleanup for every canceled
task, so this line is never reached. There is also a PAUSED race before
`stmtExecutor` is assigned: the control thread can close and clear the
statement while `baseCommand.initPlan(...)` is still running, after which a
`RemoteDorisScanNode` can register into the already-drained `StatementContext`
and the canceled task again skips a final close. In both cases the pre-rewrite
plan has no coordinator to close that Flight SQL session, leaving it until
remote `wait_timeout`. Please make the task thread perform one final close
after `before()`/`run()` can no longer register nodes for every terminal
status, and cover STOPPED plus cancel-during-planning.
##########
regression-test/framework/src/main/groovy/org/apache/doris/regression/RegressionTest.groovy:
##########
@@ -176,6 +179,15 @@ class RegressionTest {
// anything else has to bound that wait itself, as SuiteCluster does
for its doris-compose
// subprocesses.
Awaitility.pollInSameThread()
+ // And no Awaitility condition answers for threads it does not poll:
by default every await()
+ // installs itself as the JVM's default uncaught-exception handler and
rethrows, from the awaiting
+ // thread, whatever any thread of the JVM threw uncaught meanwhile.
With suites running in parallel
+ // that is the wrong suite by construction (a poller
test_active_queries left running failed
+ // test_partial_update_insert_schema_change, which happened to be
awaiting a schema change), and
+ // the handler in place is whichever suite entered an await() last, so
the same exception may just
+ // as well reach nobody. A thread's failure reaches its suite through
Suite.thread() and the future
+ // it returns; a thread a suite started itself is the suite's to join
and check.
+ Awaitility.doNotCatchUncaughtExceptionsByDefault()
Review Comment:
[P1] Preserve a suite verdict for raw worker failures
This avoids cross-suite misattribution by making uncaught raw-worker
failures unable to fail any suite. The tree still has many `Thread.start`/`new
Thread` workers that run SQL or assertions and whose parents only call
`join()`; `join()` never rethrows, and the framework installs no per-suite
uncaught-error collector. `Suite.thread()` can propagate through its future,
but those existing raw-thread callers do not use it. The manual result in this
PR also explicitly shows the new behavior: a failing daemon only prints to
stderr and fails no suite. Please add owner-local propagation (for example,
migrate these workers to futures plus `get()`, or add a per-suite collector
checked after joins) before disabling the only process-level signal, so product
regressions cannot turn green.
--
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]
