morningman commented on PR #67835:
URL: https://github.com/apache/doris/pull/67835#issuecomment-5629839542
<!-- doris-repo-review:v1:begin -->
### Local pipeline review — ✅ PASS
```yaml
schema: doris-repo-review/v1
status: PASS
pr: apache/doris#67835
commit: 22b9306171e097555946d8b9778d7cba13f7e036
base: b78724a75f704b059bf07cab4e7fa6601bdd8a79
reviewed_at: 2026-09-11T13:15+08:00
reviewer: morningman
model: claude-opus-5[1m]
effort: max
findings: {blocker: 0, major: 0, minor: 1, nit: 7}
rounds: 1
converged: true
```
**Notes for maintainers**
-
`fe/fe-core/src/main/java/org/apache/doris/service/arrowflight/protocol/FlightProtocolAdapter.java:270`
— the only Minor: a command waiting for the session lock gives up after
`query_timeout`, while the command holding it may be a sync-load statement
allowed `getExecTimeoutS()` = max(insert_timeout, query_timeout); a concurrent
metadata/prepare call during a long INSERT gets UNAVAILABLE too early. One-line
fix: bound the wait with `ctx.getExecTimeoutS()`.
-
`fe/fe-core/src/main/java/org/apache/doris/service/arrowflight/DorisFlightSqlProducer.java:320`
— the busy-session UNAVAILABLE reaches clients as UNAVAILABLE, INTERNAL, or
INTERNAL with an empty description depending on the entry point
(getFlightInfoStatement and the three metadata streams re-wrap it); consider
passing a FlightRuntimeException through unchanged.
-
`fe/fe-core/src/main/java/org/apache/doris/service/arrowflight/protocol/FlightProtocolAdapter.java:120`
— teardown (closeSession, token expiry, KILL) still runs
`unregisterConnection` concurrently with a command holding the lock;
pre-existing and stated in the javadoc, but the next PR of #67577 should close
it.
- The deleted Flight `kill` override also changed the cancel reason a killed
Flight query reports ("arrow flight query killed by user" -> "cancel query by
user from <addr>"), not only the log text; nothing asserts either string.
- Not verified locally: no build, unit test, regression or cluster run.
CheckStyle is green; TeamCity FE UT build 1043145 on this exact commit was
still running with 7118 passed / 0 failed when this comment was posted; the
golden baselines under `fe/fe-core/src/test/resources/protocol-golden/` are
untouched by the diff.
<sub>Reviewed locally with the `doris-repo-review` pipeline. Repository
policy may accept this receipt for the matching commit; it is not a human
Apache approval.</sub>
<!-- doris-repo-review:v1:end -->
--
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]
