morningman commented on PR #68101:
URL: https://github.com/apache/doris/pull/68101#issuecomment-5731709416
<!-- doris-repo-review:v1:begin -->
### Local pipeline review — ✅ PASS
```yaml
schema: doris-repo-review/v1
status: PASS
pr: apache/doris#68101
commit: a435eae450b15b045d909b82cb1e2a7e308ab995
base: dea1b99e528f57b96d9b3bd9fa68f4dc3e726d32
reviewed_at: 2026-09-18T22:47+08:00
reviewer: morningman
model: claude-opus-5
effort: max
findings: {blocker: 0, major: 0, minor: 2, nit: 4}
rounds: 1
converged: true
```
**Notes for maintainers**
- Sixth review of this PR (reviewed locally before the push at the
maintainer's request; the head equals the reviewed
commit). Since the `9ff3965` head: the review bot's four items - admission
of the pool / Flight / user quotas as one
critical section (verified outcome-equivalent to the earlier code in every
cell; leaf lock, limit read before it);
one `ConnectContext` per bearer token under a creation lock with an index
re-check, plus a re-validation of the token
after registration that unregisters an orphan; the token cache floored
only on the sub-quota term (`min(max(1,
subQuota), arrow_flight_token_cache_size)`, so a legal sub-quota of 0
refuses with RESOURCE_EXHAUSTED while an
illegal cache size keeps its loud failure); the retained-buffer test
removed. The suite treats only RESOURCE_EXHAUSTED
as a refusal, logs and tolerates INTERNAL on the probe query after
admission, and count-gates every step.
- Two Minors remain (test gates): the deterministic admission test gates the
old increment-before-user-check flaw, not
the lock's check-then-increment atomicity (no test races registrations at
the boundary; the test comment's "fails
without the lock" claim is wrong); the post-registration re-validation has
no test. Four Nits: test hygiene, the
floor missing from two descriptions, the ungated `catch (Throwable)`
around the channel close (declined seam), the
doc PR still to be opened.
- Verified read-only from the pinned artifacts: Mockito 4.11.0 thread-local
static mocking (the six-thread latch test
is sound), guava 33.2.1 `maximumSize` argument check, arrow-memory-core
19.0.0 allocator semantics. No build, unit
test or cluster run by the reviewer; the author's FE UT and two-FE
`arrow_flight_sql_p0` results are from the handoff.
The pipeline's INTERNAL on the probe query after admission is not
attributable to this PR from the code (no change on
an admitted session's first-statement path) and is now logged by the suite
for diagnosis.
<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]
