morningman commented on PR #68101:
URL: https://github.com/apache/doris/pull/68101#issuecomment-5710385407
<!-- doris-repo-review:v1:begin -->
### Local pipeline review — ✅ PASS
```yaml
schema: doris-repo-review/v1
status: PASS
pr: apache/doris#68101
commit: 237b81a43e618cb532f48b61b41b32b0c30068b9
base: dea1b99e528f57b96d9b3bd9fa68f4dc3e726d32
reviewed_at: 2026-09-17T15:01+08:00
reviewer: morningman
model: claude-opus-5
effort: max
findings: {blocker: 0, major: 0, minor: 0, nit: 4}
rounds: 1
converged: true
```
**Notes for maintainers**
- Third review (supersedes the `0437c74` receipt). Increment since
`0437c74`: the previous review's Minor and Nits
closed (session-end declaration incl. token expiry and the per-user
`max_user_connections / 2` bound; diagnostics;
real test gates for the two-argument index removal, the
MySQL-unregister/Flight-counter invariant, distinct
`(current: N)` fixtures and a failing channel close; suite cleanup), two
declared production changes (the refusal
text names the Flight share and usage to both protocols whenever Flight
sessions are in the pool; a refused MySQL
login is logged at WARN), and the maintainer's default decision: `-1` =
half of `qe_max_connection` (512 with the
default pool; token cache follows to 512), explicit values still capped at
the pool with a WARN.
- Verified read-only: every production reader of the default goes through
`effectiveFlightMaxConnections`; the unit
tests pin 10->5 / 1024->512 / 1->0 and enforce the half; the changed
refusal text has exactly the consumers that
were updated (two unit-test literals, the suite's prefix-anchored regex);
the retained `ArrowBuf` of the new
channel-close-fails test trips no allocator check under fe-core's surefire
configuration (arrow-memory-core 19.0.0).
No build, unit test or cluster run by the reviewer; the author's UT and
`arrow_flight_sql_p0` results are from the handoff.
- Four Nits remain: the PR body's problem summary still says the default
"follows" `qe_max_connection`; the release
note's "the bound one user meets first" holds only below 1024 connections
per user; the capped-value WARN does not
say that an old `fe.conf` pinning 4096 hands Flight the whole pool on a
mixed FE; the doc PR to doris-website is
still to be opened.
<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]
