Doris-Breakwater commented on issue #67503: URL: https://github.com/apache/doris/issues/67503#issuecomment-5528055664
Breakwater-GitHub-Analysis-Slot: slot_ca94d2a74982 ### Initial triage **Assessment: confirmed code-level lifecycle bug on `master` at `219c6193f24`, with high operational impact for Arrow Flight SQL users whose workload groups have finite `max_concurrency`.** The reported behavior is a direct consequence of the coordinator deferral added by #64799, rather than an unexplained runtime anomaly. No labels are currently set; please add the repository's Bug and Arrow Flight SQL/FE component labels if available. ### Verified facts - Every BE-served Flight query using the normal coordinator is registered in `QeProcessorImpl`, then marked deferred. Its eager `coordBase.close()` is skipped. The code explicitly notes that this broad gate covers internal as well as external remote-result queries and retains the queue slot/query registration until later cleanup ([`StmtExecutor`](https://github.com/apache/doris/blob/219c6193f24/fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java#L1513-L1522), [deferral](https://github.com/apache/doris/blob/219c6193f24/fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java#L1540-L1558), [skipped close](https://github.com/apache/doris/blob/219c6193f24/fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java#L1667-L1673)). - The processor changes the connection to `COM_SLEEP`, deliberately skips finalization for deferred executors, and then clears `ConnectContext.executor` ([`FlightSqlConnectProcessor.close`](https://github.com/apache/doris/blob/219c6193f24/fe/fe-core/src/main/java/org/apache/doris/service/arrowflight/FlightSqlConnectProcessor.java#L196-L214)). Cleanup therefore occurs only on the next query, a failed `GetFlightInfo`, or connection teardown ([`DorisFlightSqlProducer`](https://github.com/apache/doris/blob/219c6193f24/fe/fe-core/src/main/java/org/apache/doris/service/arrowflight/DorisFlightSqlProducer.java#L187-L198), [error path](https://github.com/apache/doris/blob/219c6193f24/fe/fe-core/src/main/java/org/apache/doris/service/arrowflight/DorisFlightSqlProducer.java#L286-L301), [pool teardown](https://github.com/apache/doris/blob/219c6193f24/fe/fe-core/src/main/java/org/apache/doris/service/arrowflight/sessions/FlightSqlConnectPoolMgr.java#L59-L85)). - A sleeping connection is checked only against session `wait_timeout`; the default is 28,800 seconds. The same `startTime` was set when the query began, and BE-served `DoGet` is directed to a BE endpoint, so it does not refresh FE activity ([timeout check](https://github.com/apache/doris/blob/219c6193f24/fe/fe-core/src/main/java/org/apache/doris/qe/ConnectContext.java#L1295-L1326), [BE endpoint construction and existing callback TODO](https://github.com/apache/doris/blob/219c6193f24/fe/fe-core/src/main/java/org/apache/doris/service/arrowflight/DorisFlightSqlProducer.java#L235-L283)). - `Coordinator.close()` is the operation that releases the workload-group queue token, while `finalizeArrowFlightQuery()` closes the coordinator and unregisters the query. Retaining the deferred executor therefore explains both the occupied slot and the stale `active_queries` row ([queue release](https://github.com/apache/doris/blob/219c6193f24/fe/fe-core/src/main/java/org/apache/doris/qe/Coordinator.java#L793-L818), [finalization](https://github.com/apache/doris/blob/219c6193f24/fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java#L1068-L1090)). It also explains why `KILL CONNECTION` releases both immediately. - The `SELECT 1` observation is consistent with the implementation: queries without a non-schema scan node do not enter this workload-group queue, and short-circuit/point queries are not deferred by the `coordBase == coord` gate. - The direct BE `DoGet` path currently only reads/forwards batches; there is no notification back to the FE when the stream reaches EOS ([BE `DoGetStatement`](https://github.com/apache/doris/blob/219c6193f24/be/src/service/arrow_flight/flight_sql_service.cpp#L79-L96), [batch EOS](https://github.com/apache/doris/blob/219c6193f24/be/src/service/arrow_flight/arrow_flight_batch_reader.cpp#L93-L108)). Thus "release when fetched" requires a new BE-to-FE completion mechanism; it cannot be implemented solely in the existing FE stream handler. I verified this path by source inspection at the reported commit. I did not run the end-to-end reproduction in this environment. ### Version scope correction The deferral path is present at the current heads of `master`, `branch-4.0`, and `branch-4.1`. It is **not** present at the current heads of `branch-3.0` or `branch-3.1`; those branches still eagerly close/finalize the coordinator at the end of `GetFlightInfo`. Unless a different 3.x branch/tag contains a separate backport, the issue's “3.x/4.x lineage” statement should be narrowed to master/4.x. ### Missing information No additional logs or profile are required to accept the lifecycle bug: the supplied reproduction and the current code are sufficient. For an automated integration test, it would still help to record the exact ADBC/JDBC driver and version used, plus whether all `DoGet` batches were consumed before the process exited. A Doris query profile is not needed for this resource-lifetime issue. ### Recommended next steps 1. Treat a Flight-specific cleanup bound as the practical short-term fix, reusing the existing connection timeout checker and the existing `unregisterConnection` cleanup path. The timeout must be measured from the query's recorded start and must be at least that deferred query's immutable execution timeout. 2. Do **not** calculate that floor from `ConnectContext.getExecTimeoutS()` after `GetFlightInfo`: `FlightSqlConnectProcessor.close()` clears `ConnectContext.executor`, and per-query `SET_VAR(query_timeout=...)` values are reverted after statement execution. Capture the coordinator's `TQueryOptions.execution_timeout` (or an absolute deadline) when the executor is deferred, so the cleanup checker cannot terminate a long query based on a restored/default session value. 3. Longer term, implement the callback already noted in `DorisFlightSqlProducer` so BE execution/EOS can finalize the FE coordinator promptly. Keep the timeout as a fallback for abandoned/stalled consumers and lost callbacks. 4. Add coverage for: (a) two abandoned Flight sessions no longer blocking a third scanning query past the configured bound; (b) a long `DoGet` not being cleaned before its captured query deadline, including a `SET_VAR(query_timeout=...)` case; (c) immediate cleanup on `CloseSession`/`KILL CONNECTION`; (d) concurrent timeout/next-query teardown remaining single-finalization; and (e) master plus affected 4.x branches. -- 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]
