Doris-Breakwater commented on issue #67369: URL: https://github.com/apache/doris/issues/67369#issuecomment-5490630310
Breakwater-GitHub-Analysis-Slot: slot_d813a37c5b64 ## Preliminary maintainer analysis **Judgment:** This is a valid, high-severity FE / Arrow Flight SQL correctness defect. The exact 4x multiplication is not yet proven from the supplied evidence, but the reported `INTERNAL` error-after-side-effect is directly explained by the 4.1.3-rc02 source. Until fixed, PL/SQL stored-procedure `CALL` should be treated as unsafe over Flight SQL. The issue currently has no labels; please add the repository's bug and Arrow Flight SQL / FE area labels. ### Verified from commit `31263df4dc1d4d3a27517d264802cd4d6b92c874` 1. Flight SQL executes the SQL during `GetFlightInfo`: `DorisFlightSqlProducer.executeQueryStatement()` calls `FlightSqlConnectProcessor.handleQuery()` and turns an error in `ConnectContext` into an `INTERNAL` response ([source](https://github.com/apache/doris/blob/31263df4dc1d4d3a27517d264802cd4d6b92c874/fe/fe-core/src/main/java/org/apache/doris/service/arrowflight/DorisFlightSqlProducer.java#L187-L203)). 2. A stored-procedure call reaches `CallProcedure.run()`, which invokes `PlSqlOperation.execute()` ([source](https://github.com/apache/doris/blob/31263df4dc1d4d3a27517d264802cd4d6b92c874/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/plans/commands/call/CallProcedure.java#L42-L50)). There is no Arrow Flight protocol gate in `CallCommand` before that invocation. 3. `PlSqlOperation.execute()` evaluates the procedure via `exec.parseAndEval(args)` first, then unconditionally executes `ctx.getMysqlChannel().reset()` in `finally` ([source](https://github.com/apache/doris/blob/31263df4dc1d4d3a27517d264802cd4d6b92c874/fe/fe-core/src/main/java/org/apache/doris/plsql/executor/PlSqlOperation.java#L46-L75)). On a Flight session, `FlightSqlConnectContext.getMysqlChannel()` always throws exactly `getMysqlChannel not in mysql connection` ([source](https://github.com/apache/doris/blob/31263df4dc1d4d3a27517d264802cd4d6b92c874/fe/fe-core/src/main/java/org/apache/doris/service/arrowflight/sessions/FlightSqlConnectContext.java#L53-L56)). The exception is caught and converted to an error state only after the procedure body may already have committed its DML. This confirms the partial-success ambiguity and the observed client error. 4. This is broader than the final `reset()`: `PlsqlResult` also sends rows, metadata, and EOF through `MysqlChannel` ([source](https://github.com/apache/doris/blob/31263df4dc1d4d3a27517d264802cd4d6b92c874/fe/fe-core/src/main/java/org/apache/doris/plsql/executor/PlsqlResult.java#L89-L170)). Removing or guarding only the final reset would not establish Flight SQL support for PL/SQL results. 5. Doris defaults `max_query_retry_time` to 3, which makes "initial attempt + 3 retries = 4" a plausible clue ([source](https://github.com/apache/doris/blob/31263df4dc1d4d3a27517d264802cd4d6b92c874/fe/fe-common/src/main/java/org/apache/doris/common/Config.java#L1078-L1084)). It is not sufficient proof of the four executions: this particular MySQL-channel exception is caught inside `PlSqlOperation` and returned as query state, while `StmtExecutor.queryRetry()` retries only specific thrown failures. Static inspection therefore does not establish whether the repeated execution is caused by an outer Flight/ADBC `GetFlightInfo` replay, an independent retry while running the inner `INSERT`, or another layer. ### Information needed to prove the 4x path - Exact versions of `adbc_driver_flightsql`, `adbc_driver_manager`, and `pyarrow` (for example, the output of `python -m pip freeze | grep -E 'adbc|pyarrow'`). The DB-API path prepares statements automatically in current ADBC implementations, so the driver version matters. - FE topology and mode: cloud vs. non-cloud, number of FEs, whether `127.0.0.1:41070` is the master FE, and the effective `max_query_retry_time` value. - FE audit records and the surrounding `fe.log` for one fresh unique key, including the outer `CALL`, inner `INSERT`, query IDs, timestamps, and the full server stack trace. On an isolated test cluster, enabling `enable_print_request_before_execution` would make attempt counting clearer. - A client/RPC trace that counts `CreatePreparedStatement`, `GetFlightInfoPreparedStatement`, and/or `GetFlightInfoStatement` requests for the single `cur.execute()`. Please also compare the DB-API prepared path with a low-level ADBC statement executed without calling `prepare()`. - On an isolated test cluster, repeat once with `max_query_retry_time = 0`. A change from four rows to one would implicate a Doris retry path; no change would shift attention to Flight/ADBC replay. Do not use this as a production mitigation because one write can still commit before the error. The audit pattern will localize the replay boundary: - Four outer `CALL` audit entries plus four inner `INSERT` entries: the Flight request is being replayed/resubmitted. - One outer `CALL` entry plus four inner `INSERT` entries: retry occurs inside procedure/DML execution. - One outer and one inner entry despite four rows: investigate the storage/result-counting path and capture transaction/load labels. ### Recommended next steps 1. **Immediate containment:** reject stored-procedure `CALL` on `ConnectType.ARROW_FLIGHT_SQL` before `PlSqlOperation.execute()` is entered. This is the only currently evidenced way to guarantee no side effect before an unsupported-protocol error. Keep the documented MySQL/JDBC workaround and the warning not to retry ambiguous Flight errors. 2. **If Flight SQL support is intended:** make PL/SQL result delivery and finalization protocol-aware end to end; do not fix only `getMysqlChannel().reset()`. Define how repeated `GetFlightInfo` requests are deduplicated or otherwise prevented from re-running side-effecting statements. 3. Add regression coverage for both prepared and direct Flight SQL paths. Each must assert either (a) a pre-execution unsupported error with zero inserted rows, or (b) success with exactly one row. Also add a focused FE test proving rejection happens before the PL/SQL executor is invoked. 4. Correlate the outer and inner query IDs before selecting the final retry fix. The current evidence is enough for containment, but not enough to claim the root cause of the exact four attempts. -- 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]
