Doris-Breakwater commented on issue #70451: URL: https://github.com/apache/doris/issues/70451#issuecomment-6775993853
Breakwater-GitHub-Analysis-Slot: slot_fcef88c1775e The reported error-reporting gap is supported by source inspection. This merits FE correctness triage because an unsuccessful SELECT can be exposed as a successful, unrelated result. Please open the proposed PR. The exception causing the intermittent production occurrences remains unconfirmed. **Verified code path** I inspected master `9fb3e3a2a13f972a62cb999d5599fc8e8f17f306` and the local upstream `4.1.4` tag (commit `ad35a140c7fd0b842f18c23300bac581f7d04326`). The relevant gap exists in both; the Flight classes in 4.1.4 are under `org.apache.doris.service.arrowflight`, whereas master uses `org.apache.doris.arrowflight`. - In master, statement setup at lines 325–363 runs outside the inner `catch (Throwable)`. In particular, `resolveWorkloadGroupName()` runs after the state reset and before that catch. An exception there unwinds through the statement-context `finally` to `handleQuery`, whose `UserException` branch only logs and whose other `Exception` branch ignores it. Neither branch sets ERR or invokes auditing. The inner catch actually covers execution and subsequent result handling/auditing, rather than all statement setup. [ConnectProcessor](https://github.com/apache/doris/blob/9fb3e3a2a13f972a62cb999d5599fc8e8f17f306/fe/fe-core/src/main/java/org/apache/doris/qe/ConnectProcessor.java#L224-L397). - Flight initializes the request with an empty result channel and local-result routing; its per-statement hook also selects local routing. If setup throws before query execution changes that routing, the producer sees a non-ERR state and an empty local channel, then calls `addOKResult`. That method explicitly constructs one Utf8 column, `StatusResult`, containing `"0"`. This establishes the proposed failure mechanism independently of forwarded-query handling. [Request and statement state](https://github.com/apache/doris/blob/9fb3e3a2a13f972a62cb999d5599fc8e8f17f306/fe/fe-core/src/main/java/org/apache/doris/arrowflight/protocol/FlightProtocolAdapter.java), [producer ERR check and fallback](https://github.com/apache/doris/blob/9fb3e3a2a13f972a62cb999d5599fc8e8f17f306/fe/fe-core/src/main/java/org/apache/doris/arrowflight/DorisFlightSqlProducer.java#L240-L269), [synthetic row](https://github.com/apache/doris/blob/9fb3e3a2a13f972a62cb999d5599fc8e8f17f306/fe/fe-core/src/main/java/org/ap ache/doris/arrowflight/results/FlightSqlChannel.java#L87-L106). - `handleQueryException(exception, originStmt, null, null)` has existing paths for setting the appropriate error code/message and calling `auditAfterExec`; null statement/statistics are already used for parsing failures. Once ERR is set, the producer's existing check takes its exception path before synthesizing a result. Audit persistence still depends on the existing audit subsystem; invoking the handler is not a guarantee that an audit sink cannot fail. [Exception handler](https://github.com/apache/doris/blob/9fb3e3a2a13f972a62cb999d5599fc8e8f17f306/fe/fe-core/src/main/java/org/apache/doris/qe/ConnectProcessor.java#L515-L546). **PR review and regression coverage** The proposed outer fallback addresses the demonstrated pre-execution gap. Preserve `ConnectionException` propagation and the original error code/message for failures already handled during execution; ensure pre-execution `UserException` also reaches error handling, and log newly handled exceptions with their stack traces. Please include tests for: 1. The real base `handleQuery` path with an injected setup `RuntimeException`: ERR, nonempty error message, one audit invocation observing ERR, and a WARN with the exception. Include a pre-execution `UserException` case. 2. An execution failure already handled by the inner catch: its original error survives and auditing is not repeated. Confirm `ConnectionException` still propagates. 3. The real Flight producer path: setup failure fails GetFlightInfo and never publishes a `StatusResult` ticket; successful SELECT and SET/USE still behave correctly. **Injection detail:** `FlightSqlConnectProcessor.handleQuery` first calls `resolveWorkloadGroupName()` before `super.handleQuery`, then the base loop calls it again. An override that always throws only exercises the first, already-propagating path. Allow the first call and fail the per-statement call. [Flight processor](https://github.com/apache/doris/blob/9fb3e3a2a13f972a62cb999d5599fc8e8f17f306/fe/fe-core/src/main/java/org/apache/doris/arrowflight/FlightSqlConnectProcessor.java#L74-L99). 4. Reused-session and multi-statement cases, checking that the failed statement's audit attribution is correct and later statements do not execute. One boundary deserves explicit review: ERR is not an audit-completion flag. The outer catch can also receive an exception from `StatementContext.close()` after the normal audit call. That close invokes `releasePlannerResources()`, which can rethrow a resource-close failure. With a previously successful state, an unconditional non-ERR fallback could then invoke auditing again. Define or test that boundary before claiming exactly-once auditing for every exception reaching the outer catch. [Cleanup implementation](https://github.com/apache/doris/blob/9fb3e3a2a13f972a62cb999d5599fc8e8f17f306/fe/fe-core/src/main/java/org/apache/doris/nereids/StatementContext.java#L1583-L1601). **Information still needed for the production trigger** The supplied fault injection is sufficient to demonstrate the reporting gap, but it does not identify which setup operation fails in production. Please provide the PR/test, exact deployed FE build commit, a sanitized single-command SQL reproducer, Flight/ADBC client versions and API used, returned Arrow schema/row, and whether the session was reused or the request contained multiple statements. Capture the first newly visible exception stack trace with FE timestamp and request/session correlation, plus nearby FE logs and available audit records. A query ID or profile may not exist for a failure this early. The metrics, dialect-conversion, workload-resolution, and executor-setup blocks are candidate escape locations, not established production causes. Some exceptions within them are already caught locally (for example, `ComputeGroupException` in the metrics block and converter-plugin failures). There is currently no evidence establishing load, session concurrency, or a specific compute-group condition as the trigger. Validation here was read-only source inspection; I did not execute the reporter's regression test or a live Flight reproduction, and made no source changes. -- 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]
