Doris-Breakwater commented on issue #70145: URL: https://github.com/apache/doris/issues/70145#issuecomment-6614962322
The audit-timing gap is supported by the current source. The delayed FE cleanup is also supported for **deferred coordinators**, but its scope is narrower than “any SELECT” on current branches. The proposed direction is useful, with an important qualification: **pipeline completion is not equivalent to completion of result fetching**. I inspected upstream `master` at `7866f5a36c02e15dc014aaef30e537a395f4059d` and `branch-4.1` at `dd72933ec78a8a4daf3e1d204a6ea96d6443ada7`. This is static source verification; I have not independently reproduced the timings or tested the proposed patch. The issue currently has no labels; suggested routing is Flight SQL / FE query lifecycle / audit. Verified mechanisms: - `ConnectProcessor` calls `auditAfterExec` after statement execution/finishing, while the BE-served Flight path returns after dispatch without entering the FE result-fetch loop. `AuditLogHelper` captures `queryTime` as elapsed time at that point. More precisely, this **constructs/submits** the audit event then; actual emission can happen later. `WorkloadRuntimeStatusMgr` replaces resource counters but does not recompute `queryTime`. Thus delayed audit emission alone cannot repair the duration. See [dispatch and early return](https://github.com/apache/doris/blob/7866f5a36c02e15dc014aaef30e537a395f4059d/fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java#L1931-L1959), [audit duration capture](https://github.com/apache/doris/blob/7866f5a36c02e15dc014aaef30e537a395f4059d/fe/fe-core/src/main/java/org/apache/doris/qe/AuditLogHelper.java#L227-L267), and [audit enrichment](https://github.com/apache/doris/blob/7866f5a36c02e15dc014aaef30e537a395f4059d/fe/fe-core/src/main/java/or g/apache/doris/resource/workloadschedpolicy/WorkloadRuntimeStatusMgr.java#L81-L108). - Current master defers only when `coord.mustOutliveDispatch()` is true: external batch split sources or another scan resource requiring continued FE ownership, including remote Doris Flight sessions. Current 4.1 uses `coord.hasBatchSplitSource()`. Other BE-served queries close/unregister promptly, although their audit duration still omits subsequent execution/fetch. A fix gated solely on `isDeferredForArrowFlight()` would therefore leave part of the audit problem unresolved. See [master eligibility and queue release](https://github.com/apache/doris/blob/7866f5a36c02e15dc014aaef30e537a395f4059d/fe/fe-core/src/main/java/org/apache/doris/qe/Coordinator.java#L774-L813) and [4.1 eligibility](https://github.com/apache/doris/blob/dd72933ec78a8a4daf3e1d204a6ea96d6443ada7/fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java#L1465-L1487). - Deferred executors retain their coordinator until a later request, teardown, or the reaper. Finalization closes the coordinator, releases any acquired workload-group queue token, and unregisters the query. This supports the reported admission-control impact **when those queries acquired queue tokens**; a lingering query entry alone does not establish queue occupancy. The [Flight lifecycle implementation](https://github.com/apache/doris/blob/7866f5a36c02e15dc014aaef30e537a395f4059d/fe/fe-core/src/main/java/org/apache/doris/arrowflight/protocol/FlightProtocolAdapter.java#L70-L87) explicitly documents the missing BE-completion signal. Points to address in the PR: 1. **Define the duration boundary.** The result buffer applies backpressure, but `close()` can mark producers finished with queued batches still present; `get_batch()` serves those batches before EOF. Consequently, a small buffered result can finish producing before DoGet starts, and a slow consumer can keep fetching after pipeline completion. This is visible in [buffer close, backpressure, and fetch](https://github.com/apache/doris/blob/7866f5a36c02e15dc014aaef30e537a395f4059d/be/src/runtime/result_block_buffer.cpp#L60-L182). Last-task duration can improve execution accounting, but does not guarantee the requested fetch-inclusive semantics. Cover delayed DoGet and slow consumption explicitly; consider a result-stream terminal signal if fetch-inclusive timing is the contract. Also define non-overlapping FE/BE timing intervals: dispatch can overlap BE execution, so adding dispatch-inclusive FE time to the longest BE duration is an approximation. 2. **Use the expected participant set, not only BEs already observed reporting.** A delayed/missing BE must not make an incomplete set appear finished. Current master already has a bounded expected-backend completion barrier in [`WorkloadRuntimeStatusMgr`](https://github.com/apache/doris/blob/7866f5a36c02e15dc014aaef30e537a395f4059d/fe/fe-core/src/main/java/org/apache/doris/resource/workloadschedpolicy/WorkloadRuntimeStatusMgr.java#L168-L234); current 4.1 has the older time-based audit queue. Build on the appropriate branch implementation. 3. **Keep audit fallback separate from safe resource release.** An audit wait cap or queue overflow is not proof that active BEs no longer need external split sources. Cleanup must remain effective even if audit enqueueing fails, and must finalize the matching query exactly once when racing with next-request, session close, cancellation, and the reaper. Preserve per-query identity/state rather than reading a reused session. Account for failed/abandoned streams and old BEs lacking any new optional statistics field. Please attach the exact FE/BE build SHAs for both reported versions, a minimal SQL/schema and ADBC script with driver version, table/catalog type, and whether all endpoints were drained to EOF while the connection remained open. For one query ID, include the client start/EOF timestamps, audit row, timestamped `active_queries` observations, workload-group configuration and running/queued counts, and relevant FE/BE completion/error logs. Observe from a separate connection so the probe does not itself finalize the Flight query. These details are especially needed to reconcile “any SELECT” with the narrower current deferral gate. Recommended next step: submit the proposed PR with tests for ordinary internal-table Flight audit timing, a genuinely deferred external scan, small buffered results with delayed DoGet, slow/multi-endpoint fetch, multi-BE delayed/missing reports, errors/cancellation, and cleanup races. Verify that a subsequent queued query is admitted after safe completion without issuing another statement on the original connection, and that MySQL timing remains unchanged. The reported before/after measurements are encouraging evidence for that workload, but do not yet establish fetch-inclusive timing across these boundaries. Breakwater-GitHub-Analysis-Slot: slot_33d388b50404 -- 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]
