morningman commented on PR #67900:
URL: https://github.com/apache/doris/pull/67900#issuecomment-5659146286
<!-- doris-repo-review:v1:begin -->
### Local pipeline review — ✅ PASS
```yaml
schema: doris-repo-review/v1
status: PASS
pr: apache/doris#67900
commit: 55c4ba1b4081945b4abc65fdea88a7e25987ddbc
base: efe7dec93d048e27b84edb0a3e2506e4cd872b53
reviewed_at: 2026-09-14T12:45+08:00
reviewer: morningman
model: claude-opus-5
effort: max
findings: {blocker: 0, major: 0, minor: 4, nit: 4}
rounds: 1
converged: true
```
**Notes for maintainers**
- The three Majors of the previous review are fixed and verified: the
`wireSequenceId` rewind holds on every send/receive/handshake/SSL/16
MB/direct-send path of `MysqlChannel` (goldens re-derived from the real
channel's rules); the Flight endpoint watermark restores base behaviour for
earlier statements of a request and withdraws only the failed attempt's
endpoints (nested `EXECUTE`, retries and prepared statements included);
`ProxyMysqlChannel.reset()` drops the failed attempt's packets and nothing
resets between the last write and `proxyExecute`.
- `fe/fe-core/src/main/java/org/apache/doris/mysql/MysqlChannel.java:598` —
F-01 (Minor): the rewind is exercised by no test on the real class (the goldens
re-implement the rule in `RecordingMysqlChannel`); a `MysqlChannelTest` case
with the mocked sink capturing header bytes across flush/reset would pin it.
-
`regression-test/suites/arrow_flight_sql_p0/test_arrow_flight_session_lifecycle.groovy:120`
— F-02 (Minor): the restored two-backend-query Flight cell is pinned only at
the adapter level; add `SELECT ...; SELECT ...` to point 6 (both statements'
rows) or a wiring-test case asserting two endpoints.
- `fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java:1207` —
F-03 (Minor, pre-existing): a Flight attempt that fails after `beforeQuery`
with a replan error is never finalized (registration / per-user instance count
leak); `finalizeQuery()` is idempotent, call it on the exception path.
- No CI has run on this head (`code-review | pending`, last reports for
`d60a614`) although `905da206f8e` changes the packet numbering of every MySQL
connection — please `run buildall` before merging; the 2-FE docker suite still
never executes in any pipeline; the PR body still says "8/8", "Release note:
None" (four declared wire changes) and "One behavior change" (F-04/F-07).
<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]
