morningman opened a new pull request, #67835:
URL: https://github.com/apache/doris/pull/67835
### What problem does this PR solve?
Issue Number: #67577 -- the tracking issue for the protocol-agnostic session
and execution
layer. This is the second PR of its Stage 1 (after the golden baseline
#67789) and does not close it.
**In plain terms.** A client session in the frontend is a `ConnectContext`.
Today that one class
holds the state of both wire protocols at the same time: the MySQL socket,
the capabilities
negotiated with the MySQL client, the handshake and SSL state, the
prepared-statement packet being
executed -- and, next to them, the Arrow Flight SQL result cache, the
backend endpoints of the last
Flight query, the prepared queries and the deferred coordinators. Which half
is real is decided by a
subclass, `FlightSqlConnectContext`, that overrides six methods and leaves
every other Flight member
sitting on the base class, where a MySQL connection carries it as dead
weight and a Flight session
throws from the MySQL ones. This PR gives each protocol its own object, a
`ProtocolAdapter`, and
binds a session to exactly one of them when it is created. Nothing a client
sees changes: the golden
byte-for-byte baseline recorded in #67789 is identical before and after.
Problem Summary:
`ConnectContext` mixes three things: the session (user, catalog and
database, session variables,
transaction, prepared statements, the running statement), the MySQL protocol
state, and the Arrow
Flight SQL protocol state. The next steps of #67577 move the result path of
both protocols onto one
shared implementation, which needs a place for "what only this protocol
knows" that is not the
session itself. This PR creates that place and moves the state, without
touching the execution layer
yet: `StmtExecutor`, `ConnectProcessor` and the coordinators still call the
same `ConnectContext`
getters, which now delegate.
### What is changed?
**`qe/protocol/ProtocolAdapter`** -- the wire-protocol half of a connection:
`type()`, the client
address for processlist and the audit log, the result sink type the backend
must use, the pool the
connection is registered in (there is still one per protocol), a
per-statement cleanup hook and
`closeConnection`.
**`mysql/protocol/MysqlProtocolAdapter`** -- owns the `MysqlChannel` (a
socket, the
`ProxyMysqlChannel` of a forwarded statement on the master, or the
`DummyMysqlChannel` of an
internal context), the server and negotiated capabilities, the handshake
packet, the SSL context,
the `COM_STMT_EXECUTE` packet and its cursor flag, and the xnio accept-query
loop that
`AcceptListener` / `ReadListener` drive. It also owns the decision
`StmtExecutor` and `FEOpExecutor`
used to compute from `ConnectContext` fields -- whether the Connector/J
release on the other end
consumes the metadata terminator of a cursor result (#67520) -- as
`clientConsumesCursorMetadataTerminator`.
**`service/arrowflight/FlightProtocolAdapter`** -- owns the peer identity
(bearer token), the
`FlightSqlChannel`, the prepared queries, the endpoints of the last query,
`returnResultFromLocal`
and the deferred executors of #62259 / #67503, together with their idle
bound. `ConnectContext`
keeps `checkTimeout` and the idle reaper unchanged; only the list moved.
It also serializes the commands of a session. gRPC runs each call of a
session on whatever thread it
likes and nothing in the Flight transport orders them, while
`ConnectContext` is not thread-safe
(the existing `DorisFlightSqlProducerTest` spells that out). `runCommand` /
`callCommand` take a
per-session lock, make the session the thread's current `ConnectContext` for
the duration, restore
the previous one afterwards, and give up with `UNAVAILABLE` after the
session's query timeout if
another command is still running. `DorisFlightSqlProducer` runs statement
execution, prepared
statement creation and close, DoGet of a frontend-side result and the
catalog / schema / table
metadata requests through it. DoGet of a frontend-side result streams under
the lock on purpose: the
next statement of the session resets the channel, whose removal listener
closes the
`VectorSchemaRoot` being streamed. Session teardown (token expiry,
`CloseSession`, `KILL`) does not
take the lock; that path is reworked when the token becomes the session
credential.
**`ConnectContext`** -- gets `protocolAdapter` and three factories:
`forMysql(StreamConnection)`,
`forMysqlProxy(sessionId)` (replaces the `new ConnectContext(null, true,
sessionId)` call in
`FrontendServiceImpl`) and `forFlight(peerIdentity)` (replaces the subclass
in
`FlightSessionsManager`). The existing constructors stay as thin wrappers,
so the ~115 test files that
call `new ConnectContext()` are untouched. Every protocol-specific getter
keeps its signature and
delegates: `getMysqlChannel()`, `getCapability()`, `getFlightSqlChannel()`,
`isReturnResultFromLocal()`
and so on. A getter that only makes sense on the other protocol throws
`IllegalStateException`
naming the actual protocol (the subclass used to throw a `RuntimeException`
for `getMysqlChannel()`;
the base class used to return `null` / an empty list for the Flight ones,
which no caller relied on).
`FlightSqlConnectContext` is deleted: its `getClientIP` /
`getRemoteHostPortString` /
`closeChannel` / `setQueryId` overrides are the adapter's
`remoteHostPortString` / `closeConnection`
/ `connectPool`, and its `kill` override only differed in log text.
Removed as dead code while touching the class: `isSend` / `setIsSend`
(nothing read them; the real
flag lives on `MysqlChannel`), `cloneContext()` (no caller, and it would
have to share a channel
between two adapters), and the two lines of `resetConnection()` that cleared
Flight-only fields
(`COM_RESET_CONNECTION` is only sent by MySQL clients).
Not in this PR, deliberately: the execution layer still branches on
`ConnectType`, and an internal
context is still a MySQL context over a `DummyMysqlChannel`, exactly as
before. Both go away in the
follow-up PRs that introduce the result sender and the capability bits.
### Verification
- **Golden baseline of #67789**: `MysqlPacketGoldenTest` (27 cases, byte for
byte) and
`FlightResultGoldenTest` pass unchanged. Not a byte of the recorded
traffic moved.
- New unit tests: `FlightProtocolAdapterTest` (commands of one session run
one at a time, a waiting
command fails with `UNAVAILABLE` after the query timeout, the thread's
current context is set and
restored, a failing command releases the session, `KILL` unregisters the
session from the Flight
pool, the trace id lands in the Flight pool) and
`MysqlProtocolAdapterTest` (internal and proxy
contexts, the cursor-terminator decision and its per-statement reset, the
accept-query loop and
close going through the channel).
- Existing tests adjusted to the factories: the ones that built a
`FlightSqlConnectContext`, poked
`mysqlChannel` / `connectType` through reflection, or used a plain `new
ConnectContext()` as a
Flight session (`ShortCircuitPointQueryTest`, `AuditLogWorkloadGroupTest`,
`StmtExecutorTest`,
`ConnectContextTest`, `MysqlProtoTest`, `ConnectionExceedTest`). 26 test
classes around the
session, the MySQL channel and the Flight producer: 166 tests, 0 failures.
- Regression on a local cluster built from this branch:
`arrow_flight_sql_p0` (8 suites, including
the forward-to-master, query-release, point-query, SQL cache and
`DatabaseMetaData.getColumns`
paths) and `prepared_stmt_p0` (cursor fetch and server-side prepare over
MySQL).
- `checkstyle:check` on fe-core (main and test sources): 0 violations.
### Release note
None.
### Check List (For Author)
- Test
- [x] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- Behavior changed:
- [x] No.
- [ ] Yes.
- Does this need documentation?
- [x] No.
- [ ] Yes.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01QFwVuLmK8e7sEdKVKB6QZJ
--
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]
