morningman commented on PR #67576:
URL: https://github.com/apache/doris/pull/67576#issuecomment-5614876331
<!-- doris-repo-review:v1:begin -->
### Local pipeline review — ✅ PASS
```yaml
schema: doris-repo-review/v1
status: PASS
pr: apache/doris#67576
commit: 512e686c2e6948bc28d1fe8e6310bfa9684b9c46
base: 18967f28a3976e73c236b4db02fa53ffa9ed8e92
reviewed_at: 2026-09-10T15:34+08:00
reviewer: morningman
model: claude-opus-5[1m]
effort: max
findings: {blocker: 0, major: 0, minor: 2, nit: 6}
rounds: 1
converged: true
```
**Notes for maintainers**
-
`regression-test/suites/arrow_flight_sql_p0/test_processlist_client_ip.groovy:44`
— the `information_schema.processlist` lookup keys on `Id` alone, but
connection ids come from a per-FE counter
(`fe/fe-core/src/main/java/org/apache/doris/qe/ConnectScheduler.java:59`) while
the table aggregates every alive frontend
(`fe/fe-core/src/main/java/org/apache/doris/planner/SchemaScanNode.java:114`).
Single-FE pipelines are safe; on a multi-FE cluster a foreign session with the
same id breaks `assertEquals(1, hosts.size())`. Adding `AND FE =
'${ownRow[13]}'` scopes it.
-
`fe/fe-core/src/main/java/org/apache/doris/service/arrowflight/sessions/FlightSqlConnectContext.java:97`
— the null/empty branch is unreachable: `buildConnectContext` always calls
`setRemoteIP` before publication, the Immutables builder enforces non-null
`remoteIp`, and the tracer never returns empty. It is also the only reason
`UNKNOWN_REMOTE_IP` had to become `public`. Consequence:
`testFallsBackWhenTheAddressWasNotResolved` covers a state no live session can
be in, while the real unresolved case (`remoteIP == "0.0.0.0"`) takes the other
branch and is untested.
-
`regression-test/suites/arrow_flight_sql_p0/test_processlist_client_ip.groovy:36`
— the comment calls information_schema "a different code path", but both
surfaces produce `Host` from the same `ThreadInfo.toRow()`; the block proves
the thrift + BE-scanner hop, not the address itself.
`assertFalse(host.isEmpty())` at line 32 cannot fail; line 33 is the whole gate.
- Verified clear, for the record: nothing outside fe-core parses the value
(BE scanner casts only Id/Time/LoginTime, `Host` stays a raw `StringRef`); no
`.out` or assertion pins the old `0.0.0.0:0`; and reading `remoteIP`
cross-thread is safe because publication goes through `ConnectPoolMgr`'s
`ConcurrentHashMap`.
- An IPv6 Flight peer is genuinely reachable (the `0.0.0.0` bind is
dual-stack) and its address is reported unbracketed, unlike MySQL rows in the
same column. No consumer notices, and routing through
`NetUtils.getHostPortInAccessibleFormat(ip, 0)` would be wrong — it re-adds
`:0`. One sentence in the comment at lines 94-96 would settle it.
<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]
