Uh oh!
There was an error while loading. Please reload this page.
[fix](arrow-flight) Release a finished Flight query's coordinator instead of holding it until wait_timeout - #67504
Conversation
hello-stephen
commented
Sep 3, 2026
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
…instead of killing the session Follow-up to the first revision of apache#67504 after review: - Narrow the apache#64799 deferral gate: only a coordinator that still hands out splits to the BE (an external-table scan in batch mode, see the new ScanNode/Coordinator.hasBatchSplitSource) outlives GetFlightInfo. Every other Arrow Flight query closes its coordinator at the end of GetFlightInfo again, releasing the workload-group queue slot and the active_queries entry right away. Finalizing the FE side does not cancel BE execution, so DoGet is unaffected. - Replace arrow_flight_session_idle_timeout_second by arrow_flight_deferred_query_idle_timeout_second: the connection timeout checker now finalizes the deferred executors of a sleeping Flight session and leaves the session alive; wait_timeout still governs the session. A killed session would have made the client's next call fail with "UserSession expire after access". - Freeze the execution timeout when the executor is deferred, so a SET_VAR query_timeout hint (reverted at the end of execute()) still floors the bound. - Tests: FlightSqlDeferredQueryIdleTimeoutTest drives checkTimeout, ArrowFlightDeferralGateTest covers the predicates, StmtExecutorTest covers the frozen timeout; regression cases for the internal-table release (arrow_flight_sql_p0) and the idle reaper on a batch-mode Iceberg scan. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ApxvNkMK8TiNycv44P8Ezk
morningman
commented
Sep 5, 2026
Thanks for the precise write-up, the problem is real and #67503 nails the mechanism. I reviewed the branch and pushed a revision on top of your commit (the PR allows maintainer edits); the description is updated to match. What changed and why:
Pick labels: #64799 is in 4.0.8 / 4.1.4, so this needs |
morningman
commented
Sep 5, 2026
run buildall |
…rom wait_timeout A Flight SQL session idles under the same wait_timeout as a MySQL connection (28800s by default). Since the coordinator of a BE-served Flight query is kept alive across GetFlightInfo -> DoGet until the session's next query or its close, an abandoned session - a client that opens a session per query and never sends CloseSession - keeps that query's workload-group queue slot for the whole wait_timeout. Eight such sessions fill a max_concurrency=8 group and every later query in it fails with "query queue timeout". Add a mutable FE config, arrow_flight_session_idle_timeout_second (default 3600), applied by the existing connection timeout checker to ARROW_FLIGHT_SQL contexts only as min(wait_timeout, max(config, exec timeout)). The exec-timeout floor matters: a Flight session is COM_SLEEP while the client drains the result via DoGet and its idle clock runs from the query's start, so a bound below query_timeout would kill a long result stream before the query's own timeout could. 0 disables the bound. MySQL-protocol connections are unchanged. The kill log line now reports the effective idle timeout. Signed-off-by: Raghvendra Singh <raghav@cashify.in> Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…instead of killing the session Follow-up to the first revision of apache#67504 after review: - Narrow the apache#64799 deferral gate: only a coordinator that still hands out splits to the BE (an external-table scan in batch mode, see the new ScanNode/Coordinator.hasBatchSplitSource) outlives GetFlightInfo. Every other Arrow Flight query closes its coordinator at the end of GetFlightInfo again, releasing the workload-group queue slot and the active_queries entry right away. Finalizing the FE side does not cancel BE execution, so DoGet is unaffected. - Replace arrow_flight_session_idle_timeout_second by arrow_flight_deferred_query_idle_timeout_second: the connection timeout checker now finalizes the deferred executors of a sleeping Flight session and leaves the session alive; wait_timeout still governs the session. A killed session would have made the client's next call fail with "UserSession expire after access". - Freeze the execution timeout when the executor is deferred, so a SET_VAR query_timeout hint (reverted at the end of execute()) still floors the bound. - Tests: FlightSqlDeferredQueryIdleTimeoutTest drives checkTimeout, ArrowFlightDeferralGateTest covers the predicates, StmtExecutorTest covers the frozen timeout; regression cases for the internal-table release (arrow_flight_sql_p0) and the idle reaper on a batch-mode Iceberg scan. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ApxvNkMK8TiNycv44P8Ezk
212a026 to
c662d85Comparemorningman
commented
Sep 5, 2026
Rebased onto current master ( |
morningman
commented
Sep 5, 2026
run buildall |
hello-stephen
commented
Sep 5, 2026
TPC-H: Total hot run time: 16940 ms |
hello-stephen
commented
Sep 5, 2026
TPC-DS: Total hot run time: 83156 ms |
hello-stephen
commented
Sep 5, 2026
ClickBench: Total hot run time: 14.81 s |
hello-stephen
commented
Sep 5, 2026
FE UT Coverage ReportIncrement line coverage |
…elease path Follow-up to apache#67504 after review: the deferral gate this PR narrows moves exactly one case - an external-table scan that is NOT in batch mode - from "coordinator deferred past GetFlightInfo" to "coordinator closed at the end of GetFlightInfo". That case had no Arrow Flight coverage anywhere: the only external-catalog Flight suite forces batch mode on its session and asserts it, so all four of its data queries exercise the deferred side, and the new arrow_flight_sql_p0 case covers the eager side only for an internal table, where neither a split source nor a connector read session exists. Add the missing cell to test_iceberg_arrow_flight_split_source, reusing the catalog and Flight connection it already sets up: - A negative control mirroring the existing batch assertion: "(approximate)" is emitted only when isBatchMode(), so its absence proves the scan really is on the synchronous split path. Without it the block could silently run in batch mode and pass. enable_external_table_batch_mode=false is a reliable off switch here - IcebergScanPlanProvider.streamingSplitEstimate returns -1 when it is unset, and Iceberg does not override supportsBatchScan (SPI default false), so the partition-count flavor cannot route around it. - A full scan that must return every row: the FE has closed the coordinator by then, and the BE buffers the result independently of it. - A check that the release really was eager - the query is gone from active_queries right after the client has the rows. No polling is needed because finalizeQuery() runs inside GetFlightInfo. A distinct limit keeps the query text apart from the other scans and the LIKE pattern is built with CONCAT so the probe cannot match itself. Batch mode is restored afterwards, since the idle-reaper assertions below need a deferred coordinator to release. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WGqBt7EdtV7rc7tD4nmEqx
morningman
commented
Sep 5, 2026
run buildall |
hello-stephen
commented
Sep 5, 2026
FE Regression Coverage ReportIncrement line coverage |
hello-stephen
commented
Sep 5, 2026
TPC-H: Total hot run time: 16970 ms |
hello-stephen
commented
Sep 5, 2026
TPC-DS: Total hot run time: 82259 ms |
hello-stephen
commented
Sep 5, 2026
ClickBench: Total hot run time: 14.75 s |
What problem does this PR solve?
Issue Number: close#67503
Related PR: #64799 (kept the coordinator alive across GetFlightInfo → DoGet so the BE can fetch external-table splits; this PR narrows that to the queries that actually need it and bounds how long an abandoned session can hold the rest)
Problem Summary:
Since #64799 every Arrow Flight SQL query whose results are produced on the BE kept its coordinator alive after
GetFlightInfo, until the session ran its next query or was closed. The coordinator holds the query's workload-group queue slot and itsactive_queriesregistration. Most Flight clients open a session per query and never sendCloseSession(the ADBC/JDBC drivers don't do it on connection close, as the comment inDorisFlightSqlProducer.closeSessionnotes), so each abandoned session pinned one slot for up towait_timeout(8h by default).Observed on a 2-FE cluster with a
max_concurrency=8workload group: eight finished Flight queries sat ininformation_schema.active_queriesas RUNNING for over an hour, and every later query in the group failed withquery queue timeout, timeout: 60000 ms. Full write-up in #67503.This PR fixes it in two steps:
Only defer the coordinator when the BE still needs it. The deferral added by [fix](arrow-flight) Keep coordinator alive across GetFlightInfo/DoGet for external table scan #64799 is required only for an external-table scan in batch mode, where the BE lazily fetches splits during
DoGetfrom the split source the coordinator holds. Every other query (internal tables, external tables in non-batch mode) now closes its coordinator at the end ofGetFlightInfoagain, releasing the queue slot and theactive_queriesentry right away. That is safe: the BE buffers its results independently of the coordinator, and finalizing the FE side does not cancel BE execution (QeProcessorImpl.reportExecStatusaccepts reports for an unregistered query, andCoordinator.close()only releases the queue token and the split sources). New predicatesScanNode.hasBatchSplitSource()/Coordinator.hasBatchSplitSource()drive the gate inStmtExecutor.executeAndSendResult.Bound the remaining deferred queries without killing the session. New mutable FE config
arrow_flight_deferred_query_idle_timeout_second(default 3600). Once a Flight session has been sleeping for longer than this since its last query started, the connection timeout checker finalizes the session's deferred executors (releasing the slot and the registration) and leaves the session alive;wait_timeoutstill governs the session itself. The bound is floored at the execution timeout the deferred query actually ran with, captured when the executor is deferred (SET_VARhint values are reverted at the end ofexecute(), so the session value cannot be read later).0disables the bound.Why not kill the session (the first revision of this PR): a killed Flight session's bearer token stays in the token cache marked as already used, so the client's next call on it fails with
UserSession expire after accessand has to re-handshake. With a 1h bound, every pooled or BI-tool Flight connection that idles for an hour would fail once on its next use. Reaping only the deferred query releases the leaked resources with no client-visible change.Release note
Arrow Flight SQL: a query no longer holds its workload-group queue slot and
active_queriesentry afterGetFlightInfounless it is an external-table scan in batch mode, the only case where the BE still fetches splits from the FE duringDoGet. For that case a new FE configarrow_flight_deferred_query_idle_timeout_second(default 1h) releases the coordinator of an idle, never-closed session without killing the session.Check List (For Author)
Test
arrow_flight_sql_p0/test_arrow_flight_query_release: a finished Flight query on an internal table no longer occupies amax_concurrency=1group and is gone fromactive_queries;external_table_p0/iceberg/test_iceberg_arrow_flight_split_source: the batch-mode scan stays registered afterDoGetand is released by the idle reaper while the session survives)FlightSqlDeferredQueryIdleTimeoutTest: the reaper throughcheckTimeout, the exec-timeout floor,0disables, nothing deferred, MySQL untouched;ArrowFlightDeferralGateTest: the batch-split-source predicates;StmtExecutorTest.testDeferForArrowFlightFreezesExecTimeoutInEffect)Behavior changed:
active_queriesentry at the end ofGetFlightInfo, as it did before [fix](arrow-flight) Keep coordinator alive across GetFlightInfo/DoGet for external table scan #64799. (2) The deferred coordinator of a batch-mode scan on an idle session is released after 1h by default instead of atwait_timeout; setarrow_flight_deferred_query_idle_timeout_second=0for the previous behavior. Sessions are never killed by this change.Does this need documentation?
arrow_flight_deferred_query_idle_timeout_second— doris-website PR to follow once this is reviewed.Check List (For Reviewer who merge this PR)