github-actions[bot] commented on code in PR #67990:
URL: https://github.com/apache/doris/pull/67990#discussion_r4012576408
##########
fe/fe-core/src/main/java/org/apache/doris/mysql/ReadListener.java:
##########
@@ -46,23 +48,36 @@ public void handleEvent(ConduitStreamSourceChannel channel)
{
XnioIoThread.requireCurrentThread();
ctx.suspendAcceptQuery();
// start async query handle in task thread.
- channel.getWorker().execute(() -> {
- ctx.setThreadLocalInfo();
- try {
- connectProcessor.processOnce();
- if (!ctx.isKilled()) {
- ctx.resumeAcceptQuery();
- } else {
- ctx.stopAcceptQuery();
+ try {
+ channel.getWorker().execute(() -> {
+ ctx.setThreadLocalInfo();
+ try {
+ connectProcessor.processOnce();
+ if (!ctx.isKilled()) {
+ ctx.resumeAcceptQuery();
+ } else {
+ ctx.stopAcceptQuery();
+ ctx.cleanup();
+ }
+ } catch (Throwable e) {
+ LOG.warn("Exception happened in one session(" + ctx +
").", e);
+ ctx.setKilled();
ctx.cleanup();
+ } finally {
+ ConnectContext.remove();
}
- } catch (Throwable e) {
- LOG.warn("Exception happened in one session(" + ctx + ").", e);
+ });
+ } catch (RejectedExecutionException e) {
+ LOG.warn("Failed to submit query task for one session({}).", ctx,
e);
+ // Keep the same ConnectContext thread-local lifecycle as the
normal async path,
+ // so that cleanup()/close listener can access
ConnectContext.get() if needed.
+ ctx.setThreadLocalInfo();
+ try {
ctx.setKilled();
ctx.cleanup();
} finally {
Review Comment:
**[P1] Keep registered-session teardown off the XNIO I/O thread**
`handleEvent()` is still running on an XNIO I/O thread here, so this
`cleanup()` can turn worker-pool saturation into an event-loop outage. Closing
the connection invokes its close listener synchronously; for a registered
session that calls `unregisterConnection()` / `closeTxn()`, and observer
rollback can block in `MasterOpExecutor.execute()`. `cleanup()` then drops
every temporary table, forwarding one RPC per table on a follower. Those RPC
timeouts are derived from `1.2 * query_timeout` (900 seconds by default), while
the MySQL service has only four I/O threads by default. A handful of rejected
sessions with transactional or temporary-table state can therefore pin every
event loop for minutes and stall unrelated clients. `AcceptListener` is not an
equivalent precedent because its rejected context has not registered or
acquired session state. Please move the full registered-session teardown
(including close-listener rollback and temporary-table cleanup) to an executor
isolated f
rom the saturated query pool, or otherwise make this fallback bounded and
non-blocking.
--
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]
