[refactor](session) Bind a ConnectContext to one ProtocolAdapter instead of a Flight subclass - #67835
Merged
Merged
Conversation
…ead of a Flight subclass ConnectContext carried the state of both wire protocols at once, and which of them was live was decided by FlightSqlConnectContext overriding six methods. Now a context is created with one ProtocolAdapter and keeps it for life: - MysqlProtocolAdapter: the MysqlChannel (a socket, a ProxyMysqlChannel on the master, or a DummyMysqlChannel for an internal context), the negotiated capabilities, the handshake packet, the SSL context, the COM_STMT_EXECUTE packet and cursor flag, and the xnio accept-query loop. It also owns the Connector/J cursor-terminator decision that StmtExecutor and FEOpExecutor used to compute from ConnectContext fields. - FlightProtocolAdapter: the peer identity, the FlightSqlChannel, the prepared queries, the endpoints, returnResultFromLocal and the deferred executors. It serializes the commands of a session under a per-session lock, which gRPC does not do and ConnectContext is not safe without: DorisFlightSqlProducer runs statements, prepared-statement actions, DoGet of frontend-side results and metadata requests through it. The protocol-specific getters of ConnectContext keep their signatures and delegate to the adapter, so the execution layer is untouched in this step. ConnectContext.forMysql / forMysqlProxy / forFlight replace the subclass and the (null, true, sessionId) constructor call. checkTimeout and the deferred executor reaper stay where they were. Verified against the golden baseline of apache#67789: neither MysqlPacketGoldenTest nor FlightResultGoldenTest changed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QFwVuLmK8e7sEdKVKB6QZJ
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
30 tasks
…otocol subpackage qe.protocol defines the interface; each front end implements it in its own protocol subpackage: mysql.protocol.MysqlProtocolAdapter and now service.arrowflight.protocol.FlightProtocolAdapter. The result senders of the next step land in the same two places. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QFwVuLmK8e7sEdKVKB6QZJ
Contributor
Author
|
run buildall |
1 similar comment
Contributor
Author
|
run buildall |
Contributor
Author
Local pipeline review — ✅ PASSschema: doris-repo-review/v1
status: PASS
pr: apache/doris#67835
commit: 22b9306171e097555946d8b9778d7cba13f7e036
base: b78724a75f704b059bf07cab4e7fa6601bdd8a79
reviewed_at: 2026-09-11T13:15+08:00
reviewer: morningman
model: claude-opus-5[1m]
effort: max
findings: {blocker: 0, major: 0, minor: 1, nit: 7}
rounds: 1
converged: trueNotes for maintainers
Reviewed locally with the |
924060929
approved these changes
Sep 11, 2026
Contributor
TPC-H: Total hot run time: 16798 ms |
Contributor
TPC-DS: Total hot run time: 83222 ms |
Contributor
ClickBench: Total hot run time: 14.9 s |
14 tasks
morningman
added a commit
to morningman/doris
that referenced
this pull request
Sep 11, 2026
…kage into org.apache.doris.arrowflight Arrow Flight SQL was added under org.apache.doris.service.arrowflight in 2023 (apache#24772) because it was wired up next to the thrift FrontendServiceImpl. It is now a full peer of the MySQL front end, whose code lives in the top-level org.apache.doris.mysql package, and apache#67835 already had to mirror a "protocol" sub-package on both sides. Move it up one level so the two front ends are symmetric before the next PR adds ResultSender implementations to that shape. Pure rename: git mv of fe-core main/test service/arrowflight/** (sub-packages kept), plus the manual Flight JDBC client FlightSqlJDBC that apache#27661 left in the service test package. Inside the files only package and import lines change, with the org.apache.doris import block re-sorted where "arrowflight" now sorts before "catalog"/"common"/"qe". Nothing references the old package name by string (no Class.forName, config key, pom, checkstyle suppression or log4j entry). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QFwVuLmK8e7sEdKVKB6QZJ
morningman
added a commit
that referenced
this pull request
Sep 11, 2026
…kage into org.apache.doris.arrowflight (#67866) ### What problem does this PR solve? Issue Number: #67577 -- the tracking issue for the protocol-agnostic session and execution layer. This is a preparatory, mechanical PR of its Stage 1 and does not close it. Arrow Flight SQL was added under `org.apache.doris.service.arrowflight` in 2023 (#24772) because it was wired up next to the thrift `FrontendServiceImpl`. It has since grown into a full peer of the MySQL front end (its own connect processor, sessions, auth, result channel, and, as of #67835, a `ProtocolAdapter`), while the MySQL side lives in the top-level `org.apache.doris.mysql` package. #67835 already had to mirror the `protocol` sub-package on both sides, and the next PR (`ResultSender`) would add one more implementation per side. Moving Flight out of `service/` now keeps the two front ends symmetric before more code lands on that shape: ``` org.apache.doris.mysql org.apache.doris.arrowflight (was service.arrowflight) .protocol.MysqlProtocolAdapter .protocol.FlightProtocolAdapter .protocol.MysqlResultSender (next PR) .protocol.FlightResultSender (next PR) ``` What moves (`git mv`, sub-packages kept as they were): - `fe-core/src/main/java/org/apache/doris/service/arrowflight/**` -> `.../doris/arrowflight/**` (19 files: 4 top-level, `auth2/` 5, `protocol/` 1, `results/` 3, `sessions/` 3, `tokens/` 3) - `fe-core/src/test/java/org/apache/doris/service/arrowflight/**` -> `.../doris/arrowflight/**` (7 tests) - `fe-core/src/test/java/org/apache/doris/service/FlightSqlJDBC.java` -> `.../doris/arrowflight/` (the manual Flight JDBC client left behind by #27661; it only sat in `service` because Flight did) What changes inside files: `package` declarations and `import` lines only, plus re-sorting the `org.apache.doris.*` import block where `arrowflight` now sorts before `catalog`/`common`/`qe` (checkstyle `CustomImportOrder`). 9 files outside the moved tree only update imports: `ConnectContext`, `ConnectScheduler`, `Coordinator`, `NereidsCoordinator`, `OssFeServerStarterProvider`, and the tests `ConnectionExceedTest`, `MysqlProtocolAdapterTest`, `AuditLogWorkloadGroupTest`, `FlightResultGoldenTest`. Nothing else references the old package name: no `Class.forName`, no configuration key, no `pom.xml` / checkstyle suppression / log4j entry, no regression-test or `.github` path. The four classes that are actually the thrift service (`ExecuteEnv`, `FeDiskInfo`, `FrontendOptions`, `FrontendServiceImpl`) stay in `org.apache.doris.service`.
8 tasks
morningman
added a commit
to morningman/doris
that referenced
this pull request
Sep 12, 2026
Second step of the protocol-independent session work (apache#67577). The result half of the two wire protocols is now behind qe.protocol.ResultSender, implemented by mysql.protocol.MysqlResultSender (the former sendMetaData / sendFields / sendTextResultRow / sendBinaryResultRow / sendStmtPrepareOK of StmtExecutor and the COM_FIELD_LIST body of ConnectProcessor, byte layout unchanged) and arrowflight.protocol.FlightResultSender (FlightSqlChannel). StmtExecutor no longer holds a MysqlSerializer or takes a MysqlChannel: executeAndSendResult, sendCachedValues and executeInternalQueryAndSend take a ResultSender, and the internal executor of an IVM dry run gets the caller's sender. handleExplainStmt / handleReplayStmt / handleExplainPlanProcessStmt go through the one sendResultSet, which gives EXPLAIN PLAN PROCESS a result on Arrow Flight SQL. ConnectProcessor loses its connectType field: the per-statement protocol work of a request is adapter.finishStatement (MySQL: SERVER_MORE_RESULTS_EXISTS and the intermediate response; Flight: the forwarded outcome and the single-result rule), finalizeCommand / getResultPacket move to MysqlProtocolAdapter.finishCommand / responsePacket, COM_FIELD_LIST to MysqlConnectProcessor, and the SQL-cache guard is supportsSqlCacheReplay(). Also folds in the leftovers of the apache#67835 review: the command lock waits up to getExecTimeoutS() and logs when it gives up, FlightRuntimeException passes through the producer's catch-alls, of() names a null adapter, and two tests bind their channels through the adapter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QFwVuLmK8e7sEdKVKB6QZJ
morningman
added a commit
that referenced
this pull request
Sep 12, 2026
### What problem does this PR solve? Issue Number: #67577 Related PR: #67835 (`ProtocolAdapter`), #67866 (package move), #67789 (protocol goldens) Problem Summary: Second step of the protocol-independent session work (#67577, stage 1). #67835 moved the *connection* half of the two wire protocols behind `ProtocolAdapter`; the *result* half was still spread over `StmtExecutor` and `ConnectProcessor` as `ConnectType` branches, a `MysqlSerializer` field, `MysqlChannel` parameters and three copies of "send a text result set". ``` +---------------------------+ +-----------------------------+ | MySQL client | | Arrow Flight SQL client | +-------------+-------------+ +--------------+--------------+ | | v v +---------------------------+ +-----------------------------+ | MysqlServer | | DorisFlightSqlProducer | | AcceptListener | | every call goes through | | ReadListener | | adapter.runCommand() | +-------------+-------------+ +--------------+--------------+ | | v v +---------------------------+ +-----------------------------+ | MysqlConnectProcessor | | FlightSqlConnectProcessor | | COM_FIELD_LIST, and | | | | finalizeCommand() = | | | | adapter.finishCommand() | | | +-------------+-------------+ +--------------+--------------+ | | +----------------------------+-----------------------------+ | v +-------------------------------------------------------------------------------------------+ | ConnectProcessor.executeQuery -- parse, one StmtExecutor per statement, audit | | for each statement: executor.execute() | | adapter.finishStatement(ctx, executor, i, n) <-- NEW | +---------------------------------------------+---------------------------------------------+ | v +-------------------------------------------------------------------------------------------+ | StmtExecutor -- plans and runs one statement, protocol-agnostic result path | | | | FE-side result (SHOW / EXPLAIN / REPLAY / dry run / FE-computable SELECT / forwarded): | | sendResultSet(rs) -> sender.sendResultSet(rs, fieldInfos, binaryRows) | | BE result stream: | | sender.sendFields(...) then sender.sendRow(row) per row | | internal executor streaming to the caller's client: | | executeInternalQueryAndSend(plan, callerCtx.getResultSender()) | | | | no MysqlSerializer field, no MysqlChannel parameter, no ConnectType branch on this path | +---------------------------------------------+---------------------------------------------+ | v +-------------------------------------------------------------------------------------------+ | ConnectContext -- the session, one per connection | | protocolAdapter : ProtocolAdapter getResultSender() = adapter.resultSender(this)| +---------------------------------------------+---------------------------------------------+ | v +-----------------------------------------------+ | <<interface>> qe.protocol.ProtocolAdapter | | type() remoteHostPortString(ctx) | | resultSinkType() connectPool(scheduler) | | afterStatement(ctx) closeConnection(ctx) | | resultSender(ctx) <-- NEW | | supportsSqlCacheReplay() <-- NEW | | finishStatement(ctx, executor, i, n) NEW | +-----------------------+-----------------------+ | +--------------------+----------------------------+ | | +-----------------------+----------------------+ +-----------------------+----------------------+ | mysql.protocol.MysqlProtocolAdapter | | arrowflight.protocol.FlightProtocolAdapter | | (unchanged state: channel, capability, | | (unchanged state: result cache, endpoints, | | handshake, SSL, execute packet, cursor) | | deferred executors, command lock) | | finishStatement: SERVER_MORE_RESULTS_EXISTS| | finishStatement: carry the outcome of a | | + flush the intermediate response when | | statement forwarded to the master; only | | CLIENT_MULTI_STATEMENTS | | the last statement may return a result | | finishCommand: OK/EOF/ERR or the master's | | | | packets; responsePacket(ctx) | | | +-----------------------+----------------------+ +-----------------------+----------------------+ | | v v +----------------------------------------------+ +----------------------------------------------+ | <<interface>> qe.protocol.ResultSender (NEW) | | | | sendResultSet(rs, fieldInfos, binaryRows) | | | | sendFields(names, fieldInfos, types) | | | | sendRow(wireRow) | | | | reset() | | | +----------------------------------------------+ +----------------------------------------------+ | mysql.protocol.MysqlResultSender | | arrowflight.protocol.FlightResultSender | | column count + column definitions + | | caches the ResultSet as Utf8 vectors under | | terminator (EOF / cursor OK) + text or | | the query id for the client's DoGet | | binary rows, through the channel's | | sendFields/sendRow: never called, the | | serializer; a raw row passes through | | client pulls BE results from the BE | | MySQL-only: sendStmtPrepareOK, | | reset: nothing pending | | sendFieldList | | | +----------------------------------------------+ +----------------------------------------------+ ``` **`ResultSender`** (`qe.protocol`, interface; implementations in `mysql.protocol` and `arrowflight.protocol`, mirroring the adapters): how a statement's result reaches the client. Four operations, all of them already used: `sendResultSet` for a result the frontend materialized, `sendFields` + `sendRow` for a backend result stream, `reset` for what `MysqlChannel.reset()` did at the start of a query. `MysqlResultSender` is the old `sendMetaData / sendFields / sendTextResultRow / sendBinaryResultRow / sendMetadataTerminatorIfNeeded / sendStmtPrepareOK` of `StmtExecutor` plus the `COM_FIELD_LIST` body of `ConnectProcessor`, moved without changes to the byte layout; it uses the channel's serializer, so the executor's `serializer` field is gone. `FlightResultSender` wraps `FlightSqlChannel.addResult` (every column still `Utf8`, typing them is stage 2). **`StmtExecutor`** no longer takes a `MysqlChannel`: `executeAndSendResult`, `sendCachedValues` and `executeInternalQueryAndSend` take a `ResultSender`. The three "text result" methods (`handleExplainStmt`, `handleReplayStmt`, `handleExplainPlanProcessStmt`) build a `ShowResultSet` and go through the one `sendResultSet`, which fixes `EXPLAIN PLAN PROCESS` returning nothing on an Arrow Flight SQL session (it had no Flight branch). The `MysqlChannel` overloads #67753 added for the IVM dry run become "hand the internal executor the caller's sender": `RefreshMTMVCommand` passes `ctx.getResultSender()`, and the internal executor's rows are encoded with the caller's negotiated capabilities instead of the internal context's defaults. **`ConnectProcessor`** loses its `connectType` field and every branch on it. The per-statement protocol work of a multi-statement request is one call, `adapter.finishStatement(ctx, executor, i, n)`: for MySQL it sets `SERVER_MORE_RESULTS_EXISTS` and flushes the intermediate response when the client negotiated `CLIENT_MULTI_STATEMENTS`; for Flight it carries a forwarded statement's outcome into the session (the former `carryForwardedOutcomeToFlightSession`) and enforces "only the last statement may return a result". `finalizeCommand` / `getResultPacket` move to `MysqlProtocolAdapter.finishCommand` / `responsePacket` and `COM_FIELD_LIST` to `MysqlConnectProcessor`, the only processor that dispatches it. The SQL-cache guard is `adapter.supportsSqlCacheReplay()` (true only for MySQL, whose wire rows the cache stores). Also folded in, from the local review of #67835: `FlightProtocolAdapter.acquireCommandLock` waits up to `getExecTimeoutS()` (a sync load may legitimately run past `query_timeout`) and logs when it gives up; `getFlightInfoStatement` / `streamMetadata` pass a `FlightRuntimeException` through instead of re-wrapping `UNAVAILABLE` as `INTERNAL`; `of(ctx)` names a null adapter instead of throwing NPE from the error path; `testFailedCommandReleasesTheSession` checks the lock from a second thread; `ConnectProcessorForwardProtocolTest` binds its recording channel through the adapter instead of overriding `getMysqlChannel()`. Not in this PR (next one, PR-1.3): the remaining `ConnectType` branches outside the result path (`returnResultFromLocal`, the Flight early return in `executeAndSendResult`, the retry condition, `supportHandleByFe`, the nine `getMysqlChannel().reset()` in insert / transaction commands, `FEOpExecutor`, the coordinators), which become capability bits on the adapter.
8 tasks
morningman
added a commit
that referenced
this pull request
Sep 14, 2026
…s with protocol capabilities (#67900) ### What problem does this PR solve? Issue Number: #67577 Related PR: #67883 (`ResultSender`), #67835 (`ProtocolAdapter`), #67789 (protocol goldens) Problem Summary: Third and last refactor step of stage 1 of #67577. After #67835 and #67883 the connection half and the result-encoding half of the two wire protocols live behind `ProtocolAdapter` / `ResultSender`, but the execution layer still asked `ctx.getConnectType()` in a dozen places: five times in `StmtExecutor`, in `ConnectContext.supportHandleByFe`, in nine insert / transaction `Command`s (each resetting the MySQL channel), in both coordinators, in the short-circuit rewrite rule and in `FEOpExecutor`; and the Flight-only `returnResultFromLocal` flag was flipped from four places outside the adapter. This PR replaces every one of them with a capability or a lifecycle hook on the adapter, one method per use: ``` +---------------------------+ +-----------------------------+ | MySQL client | | Arrow Flight SQL client | +-------------+-------------+ +--------------+--------------+ | | v v +---------------------------+ +-----------------------------+ | MysqlServer | | DorisFlightSqlProducer | | AcceptListener | | every call goes through | | ReadListener | | adapter.runCommand() | +-------------+-------------+ | executeQueryStatement: | | | adapter.beginRequest() NEW| v +--------------+--------------+ +---------------------------+ | | MysqlConnectProcessor | v | COM_FIELD_LIST | +-----------------------------+ | handleExecute: | | FlightSqlConnectProcessor | | adapter.beforeStatement NEW | | | finalizeCommand() = | | | | adapter.finishCommand() | | | +-------------+-------------+ +--------------+--------------+ | | +----------------------------+-----------------------------+ | v +-------------------------------------------------------------------------------------------+ | ConnectProcessor.executeQuery -- parse, one StmtExecutor per statement, audit | | for each statement: adapter.beforeStatement(ctx) <-- NEW | | executor.execute() | | adapter.finishStatement(ctx, executor, i, n) | | proxyExecute (master side): MysqlProtocolAdapter.restoreFromForwardRequest <-- NEW | +---------------------------------------------+---------------------------------------------+ | v +-------------------------------------------------------------------------------------------+ | StmtExecutor -- plans and runs one statement, protocol-agnostic | | | | forwarding a query to the master: adapter.canReplayForwardedQueryResult() <-- NEW | | retry after a failed attempt: adapter.canRetryQuery(ctx) <-- NEW | | execute(queryId), every attempt: adapter.beforeAttempt(ctx) <-- NEW | | executeAndSendResult: adapter.beforeQuery(ctx), then the coordinator; | | relay rows unless !ctx.isReturnResultFromLocal() | | FEOpExecutor.buildStmtForwardParams: adapter.fillForwardRequest(ctx, request) <-- NEW | | | | Commands (insert / txn): no channel reset any more, beforeStatement did it | | Coordinator / NereidsCoordinator / QueryProcessor: ctx.isReturnResultFromLocal() | | decides receivers vs. Flight endpoints, no ConnectType assertion | | LogicalResultSinkToShortCircuitPointQuery: adapter.supportsShortCircuitPointQuery() NEW| | no ConnectType branch and no getMysqlChannel() left in qe/** and nereids/** | +---------------------------------------------+---------------------------------------------+ | v +-------------------------------------------------------------------------------------------+ | ConnectContext -- the session, one per connection | | protocolAdapter : ProtocolAdapter getResultSender() = adapter.resultSender(this)| | supportHandleByFe() = adapter.supportsFeSideResult() && command != COM_STMT_EXECUTE | | isReturnResultFromLocal() = adapter.returnsResultFromLocal(this) (setter is gone) | +---------------------------------------------+---------------------------------------------+ | v +-----------------------------------------------+ | <<interface>> qe.protocol.ProtocolAdapter | | type() remoteHostPortString(ctx) | | resultSinkType() resultSender(ctx) | | connectPool(scheduler) closeConnection(ctx) | | | | capabilities: | | supportsSqlCacheReplay() | | canReplayForwardedQueryResult() <-- NEW | | supportsFeSideResult() <-- NEW | | supportsShortCircuitPointQuery() <-- NEW | | canRetryQuery(ctx) <-- NEW | | | | statement lifecycle: | | beforeStatement(ctx) <-- NEW | | beforeAttempt(ctx) <-- NEW | | beforeQuery(ctx) <-- NEW | | returnsResultFromLocal(ctx) <-- NEW | | finishStatement(ctx, executor, i, n) | | afterStatement(ctx) | | | | forwarding: | | fillForwardRequest(ctx, request) <-- NEW | +-----------------------+-----------------------+ | +--------------------+----------------------------+ | | +-----------------------+----------------------+ +-----------------------+----------------------+ | mysql.protocol.MysqlProtocolAdapter | | arrowflight.protocol.FlightProtocolAdapter | | capabilities: all true; | | capabilities: all false (the reasons are | | canRetryQuery = nothing flushed yet | | documented on each method) | | beforeStatement: channel.reset() -- drops | | beforeStatement: the statement's result is | | what the previous statement of the | | on this frontend until beforeQuery says | | request left unsent | | a backend produces it; the coordinator | | beforeAttempt: nothing | | beforeAttempt: result on this frontend, | | beforeQuery: nothing, rows are relayed | | endpoints of the failed attempt dropped | | | | then registers the endpoints | | returnsResultFromLocal: true | | returnsResultFromLocal: that flag | | fillForwardRequest: capability flags, | | fillForwardRequest: nothing | | deprecate-EOF, execute packet + cursor | | beginRequest(): deferred executors, result | | restoreFromForwardRequest (master side) | | cache, endpoints, flag (Flight-private) | | proxyResultPackets() (master side) | | | +-----------------------+----------------------+ +-----------------------+----------------------+ | | v v +----------------------------------------------+ +----------------------------------------------+ | <<interface>> qe.protocol.ResultSender | | | | sendResultSet(rs, fieldInfos, binaryRows) | | | | sendFields(names, fieldInfos, types) | | | | sendRow(wireRow) | | | | reset() -- now only for a retried query | | | +----------------------------------------------+ +----------------------------------------------+ | mysql.protocol.MysqlResultSender (unchanged) | | arrowflight.protocol.FlightResultSender | | | | sendResultSet no longer touches the | | | | result-location flag (EXPLAIN never marks | | | | the result as coming from a backend now) | +----------------------------------------------+ +----------------------------------------------+ ``` **Capabilities.** `canReplayForwardedQueryResult()` guards the refusal to forward a query to the master on a Flight session (#67569; the message is unchanged). `supportsFeSideResult()` is the protocol half of `ConnectContext.supportHandleByFe()`; `supportsShortCircuitPointQuery()` is the protocol half of `LogicalResultSinkToShortCircuitPointQuery.scanMatchShortCircuitCondition` (#67368); `canRetryQuery(ctx)` is the retry condition of `handleQueryWithRetry`: for MySQL "nothing was flushed to the socket yet", for Flight false (the client is not told which attempt its endpoints belong to). The reason each Flight answer is what it is moves onto the Flight implementation, out of the call sites. **Statement lifecycle.** `beforeStatement(ctx)` is called by `ConnectProcessor.executeQuery` before each statement (and by `MysqlConnectProcessor.handleExecute`): the MySQL adapter resets the channel there, which is what the query path and the nine `Command`s did each on their own; the Flight adapter puts the statement's result on this frontend. `beforeQuery(ctx)` is called at the top of `executeAndSendResult`, before a coordinator is built: the Flight adapter marks the result as staying on the backends, and `Coordinator` / `NereidsCoordinator` / `QueryProcessor` keep reading `ctx.isReturnResultFromLocal()` to register endpoints instead of receivers (the `checkState(ARROW_FLIGHT_SQL)` assertions go, the decision itself does not move). Because `beforeQuery` runs only where a coordinator follows, an `EXPLAIN` -- handled earlier in `handleQueryStmt` -- never marks its result as coming from a backend, and the `setReturnResultFromLocal(true)` that `FlightResultSender.sendResultSet` had to do in #67883 is gone with the setter. The four flips in `StmtExecutor` / `FlightSqlConnectProcessor` are gone; what a Flight request drops from its predecessor (deferred executors, result cache, endpoints, the flag) is `FlightProtocolAdapter.beginRequest()`. `beforeAttempt(ctx)` is called at the top of `StmtExecutor.execute(TUniqueId)`, i.e. for the first attempt and for each replan retry of `queryRetry` (a cloud `NEED_REPLAN` error); it replaces the `setReturnResultFromLocal(true)` that stood there before this PR, which the first version of this PR had folded into `beforeStatement` -- once per statement, outside the replan loop. A Flight attempt that had reached `beforeQuery` and then failed with a replan error left the next attempt in the "result on the backends" state; if that attempt failed before its own `beforeQuery`, `StatementContext.close` skipped the connector statement scope and the `finalizeQuery` in `handleQueryWithRetry` was skipped, with no deferred executor or DoGet ever releasing them. The Flight `beforeAttempt` puts the result back on this frontend and withdraws exactly the endpoints the failed attempt registered (the adapter records how many the request held when the statement started and trims back to that mark, so an earlier statement of the request keeps its own), which `queryRetry` -- unlike `handleQueryWithRetry`, which asks `canRetryQuery` -- did not withdraw before, so a replanned Flight query no longer hands the client the endpoints of a cancelled attempt next to its own. Covered by `FlightProtocolAdapterTest` (the attempt lifecycle) and `ProtocolCapabilityWiringTest.testEveryAttemptOfAReplannedFlightQueryStartsOnTheFrontend`, which runs a real Flight query whose first attempt fails on the mocked backend with an `E-230` status after registering its endpoint and whose second attempt fails while planned: the session ends with its result on the frontend and no endpoint (the test fails on the flag without the hook). **Forwarding.** `fillForwardRequest(ctx, request)` adds to a `TMasterOpRequest` what the master needs to know about the client: the MySQL adapter writes the negotiated capability flags, `CLIENT_DEPRECATE_EOF` and, for a `COM_STMT_EXECUTE`, the execute packet and the cursor flag (formerly two blocks in `FEOpExecutor.buildStmtForwardParams`, one of them behind `if (MYSQL)`); the Flight adapter writes nothing, its session consumes the master's status and rows rather than its packets (`carryForwardedOutcome`). The master's side, `ConnectProcessor.restoreForwardedMysqlContext`, becomes `MysqlProtocolAdapter.restoreFromForwardRequest`, and `proxyExecute` reads the proxy channel's packets through `MysqlProtocolAdapter.proxyResultPackets()` instead of `StmtExecutor.getProxyQueryResultBufList()` casting the channel. After this PR `grep -rn 'ConnectType\.\|getMysqlChannel()' fe-core/src/main/java/org/apache/doris/{qe,nereids}` outside `*/protocol/` finds only the `ConnectContext.getMysqlChannel()` delegate itself and two lines of `MysqlConnectProcessor` (reading the client's packet, the auth-switch handshake), which is MySQL protocol code by definition. **One behavior change, on the MySQL side, recorded in the golden.** The channel used to be reset at the start of a *query* and inside the insert / transaction commands, and nowhere else. A client that did not negotiate `CLIENT_MULTI_STATEMENTS` gets no intermediate response between the statements of a request, so whatever a query wrote stayed in the send buffer until the next query or insert reset it. When the next statement was neither -- `select 1; set @A = 1` -- the buffered result set of the `SELECT` went out together with the `OK` of the `SET`: a result set terminated by a `0x00` OK packet, which no MySQL client parses (it reads the OK as a row and waits for more). With the reset at the start of every statement, such a request delivers only its last statement's outcome, which is what `MysqlProtocolAdapter.finishStatement` has documented as the intent all along. The first commit makes `RecordingMysqlChannel` model the send buffer (a reset drops what was written after the last flush, the way `MysqlChannel.reset()` clears it) and records `select 1; set @A = 1` with and without the capability as it is today; the second commit's golden diff is exactly that: the three packets of `select 1` disappear from `multi-statement-without-capability-query-then-set`, the `OK` keeps its sequence id 4. The existing `multi-statement-without-capability` case (`select 1; select 2`) loses the three packets of `select 1` in the first commit only, because the recording channel now shows what reaches the client -- its sequence ids, 4 to 7, already were the ones on the wire. That the delivered response did not start at sequence id 1 was pre-existing -- `MysqlChannel.reset()` cleared the buffer but did not rewind the sequence id, so a client that checks sequence ids (pymysql, libmysqlclient; Connector/J does not by default) already failed `select 1; select 2` without the capability with "Packet sequence number wrong - got 5 expected 1" -- but the reset before every statement widened it: a request without the capability whose later statement failed (`select 1; select * from no_such_table`) used to deliver the buffered result set of `select 1` followed by the ERR, and now delivered a lone ERR with that hole, which pymysql rejects and go-sql-driver answers by closing the connection. So it is fixed here: `MysqlChannel` tracks the sequence id as of the last packet that reached the wire (the command packet received, the last flush) and `reset()` rewinds to it, `RecordingMysqlChannel` models the same, and the golden shows `select 1; select 2` and `select 1; set @A = 1` numbered from 1 and gains `select 1; select * from no_such_table` with and without the capability (ERR at 5 after the intermediate response, ERR at 1 without one). The same reset also fixes the retry of a forwarded query on the master: `ProxyMysqlChannel` never flushes, so `canRetryQuery` held, but its collected packets survived the reset and both attempts' packets travelled to the client; it now drops them on reset. Not in this PR: an internal adapter for the no-client context (it is still a MySQL context over a `DummyMysqlChannel`, now with nothing in the execution layer keyed on that), and the stage 1 performance baseline.
morningman
added a commit
that referenced
this pull request
Sep 15, 2026
…on PRs from master in merge order (#67753 #67802 #67814 #67837 #67853 #67876) (#68017) Cherry-picked from #67753, #67802, #67814, #67837, #67853, #67876 Batch pick of every merged PR carrying the `incremental-computation` label that `branch-incremental-computation` does not have yet (no `incremental-computation-picked` label), in the order they landed on master (`git log --first-parent`). One commit per PR, each created with `git cherry-pick -x` so the message ends with `(cherry picked from commit <master sha>)`. Follows the same convention as #67830. | # | Master commit | PR | Title | |---|---|---|---| | 1 | fe39f5b | #67753 | [fix](ivm) Answer FE-computable dry runs on the frontend instead of a placeholder backend | | 2 | f8ed33f | #67802 | [fix](ivm) Refresh the surviving partitions after an IVM baseline rebuild | | 3 | 7bd89a0 | #67814 | [fix](ivm) Stop the incremental delta from reading partitions the MV dropped | | 4 | 3050a9a | #67837 | [fix](ivm) Invalidate the baseline when a column used by the MV is dropped | | 5 | 22c95eb | #67853 | [fix](ivm) Carry the row-binlog hidden columns in the analyzed MTMV schema | | 6 | 3390a7a | #67876 | [test](ivm) Remove unnecessary cloud skips from IVM suites | Not included on purpose: - The 11 labelled PRs that already carry `incremental-computation-picked` (#62606 in the fork point, #67508 via #67712, the nine of #67830). - #67820 is still open on master; this branch already carries its content via #67861. ### Prerequisite check None of the six PRs declares a prerequisite, and none of them needs another master PR for its behavior. The only master commits that touch the same files and are not on this branch are unrelated to incremental computation (#66761 TIMESTAMP_NS, #67545 DLF, #67569 / #67520 / #67835 MySQL-protocol and session refactors, #67186 Hive partition batching, #67787 SQL cache user variables); they were left out, and two picks needed a mechanical adaptation because of that: - **#67753** conflicted in `StmtExecutor.sendMetaData`: master had already extracted the post-metadata EOF into `sendMetadataTerminatorIfNeeded(channel)` (#67520, a Connector/J cursor-fetch fix). The branch keeps its inline EOF block and now sends it on the given `channel` instead of `context.getMysqlChannel()`, which is exactly what the extracted helper does on master. Everything else in the pick is identical to the master commit. - **#67814** applied cleanly but did not compile: the new `MTMVPartitionUtil.generateRelatedBasePartitionIds()` returns an `Optional`, and on master `import java.util.Optional;` came with #67186. The import was added to the pick commit; that is the only difference from the master commit. The other four picks applied without conflicts and are byte-identical to their master commits (diffs compared with `index`/`@@` lines stripped). Both adaptations are recorded in the respective commit messages. ### Drift check against master After the six picks, every touched file is byte-identical to master at `3390a7a721f` except `MTMV.java`, `MTMVTask.java`, `MTMVPartitionUtil.java`, `MTMVPartitionUtilTest.java`, `MTMVTaskTest.java` (differences = #67186 + #67545 + #66761), `CreateTableInfo.java` (= #67787) and `StmtExecutor.java` (= #67520 + #67569 + #66761 + the later session refactors #67835 / #67883 + this branch's #67861). For the first six files, applying those unrelated master commits on top of the branch's versions reproduces master's files exactly; for `StmtExecutor.java`, the diff against master right after #67753 (`fe39f5b6a42`) consists only of #67520 / #67569 / #66761 / #67861 hunks. So nothing IVM-related is missing. The regression framework, plugins and the whole `mtmv_p0/ivm` suite/data directories are identical to master. ### Verification - FE: `run-fe-ut.sh --run` on this branch (regenerates thrift, compiles fe-core main + test) with the 17 test classes touched by the picks or extending the touched `IvmDeltaTestBase`: 17 classes, 404 tests, 0 failures, 0 errors, BUILD SUCCESS (5:19 min) — `MTMVPlanUtilTest` 24, `IvmAggDeltaHandlerTest` 33, `IvmDeltaRewriteHelperTest` 17, `IvmNormalizeMTMVJoinTest` 44, `IvmJoinDeltaHandlerTest` 23, `IvmDeltaRewriteStateTest` 10, `IvmPlanSignatureGeneratorTest` 22, `IvmBaselineRebuildTest` 28, `IvmLinearDeltaHandlerTest` 39, `IvmDeltaRewriterTest` 23, `IvmNormalizeMTMVUnionTest` 10, `MTMVTaskTest` 50, `MTMVPropertyUtilTest` 13, `MTMVPartitionUtilTest` 16, `SchemaChangeHandlerTest` 22, `StmtExecutorInternalQueryTest` 3, `StmtExecutorTest` 27. - FE checkstyle on fe-core: 0 violations. - No BE, cloud or thrift changes in this batch. - All 18 touched groovy files (framework `Suite.groovy`, `plugin_planner.groovy`, 16 suites) parse cleanly (groovy parser check). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: yujun <yujun@selectdb.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.
The shape after this PR:
In plain terms. A client session in the frontend is a
ConnectContext. Today that one classholds 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 membersitting 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, andbinds 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:
ConnectContextmixes 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,ConnectProcessorand the coordinators still call the sameConnectContextgetters, which now delegate.
What is changed?
qe/protocol/ProtocolAdapter-- the wire-protocol half of a connection:type(), the clientaddress 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.qe/protocolholds only the interface; each front end implements it in aprotocolsubpackage of its own package (mysql/protocol,service/arrowflight/protocol), which isalso where the result senders of the next step go.
mysql/protocol/MysqlProtocolAdapter-- owns theMysqlChannel(a socket, theProxyMysqlChannelof a forwarded statement on the master, or theDummyMysqlChannelof aninternal context), the server and negotiated capabilities, the handshake packet, the SSL context,
the
COM_STMT_EXECUTEpacket and its cursor flag, and the xnio accept-query loop thatAcceptListener/ReadListenerdrive. It also owns the decisionStmtExecutorandFEOpExecutorused to compute from
ConnectContextfields -- whether the Connector/J release on the other endconsumes the metadata terminator of a cursor result (#67520) -- as
clientConsumesCursorMetadataTerminator.service/arrowflight/protocol/FlightProtocolAdapter-- owns the peer identity (bearer token), theFlightSqlChannel, the prepared queries, the endpoints of the last query,returnResultFromLocaland the deferred executors of #62259 / #67503, together with their idle bound.
ConnectContextkeeps
checkTimeoutand 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
ConnectContextis not thread-safe(the existing
DorisFlightSqlProducerTestspells that out).runCommand/callCommandtake aper-session lock, make the session the thread's current
ConnectContextfor the duration, restorethe previous one afterwards, and give up with
UNAVAILABLEafter the session's query timeout ifanother command is still running.
DorisFlightSqlProducerruns statement execution, preparedstatement 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
VectorSchemaRootbeing streamed. Session teardown (token expiry,CloseSession,KILL) does nottake the lock; that path is reworked when the token becomes the session credential.
ConnectContext-- getsprotocolAdapterand three factories:forMysql(StreamConnection),forMysqlProxy(sessionId)(replaces thenew ConnectContext(null, true, sessionId)call inFrontendServiceImpl) andforFlight(peerIdentity)(replaces the subclass inFlightSessionsManager). The existing constructors stay as thin wrappers, so the ~115 test files thatcall
new ConnectContext()are untouched. Every protocol-specific getter keeps its signature anddelegates:
getMysqlChannel(),getCapability(),getFlightSqlChannel(),isReturnResultFromLocal()and so on. A getter that only makes sense on the other protocol throws
IllegalStateExceptionnaming the actual protocol (the subclass used to throw a
RuntimeExceptionforgetMysqlChannel();the base class used to return
null/ an empty list for the Flight ones, which no caller relied on).FlightSqlConnectContextis deleted: itsgetClientIP/getRemoteHostPortString/closeChannel/setQueryIdoverrides are the adapter'sremoteHostPortString/closeConnection/
connectPool, and itskilloverride only differed in log text.Removed as dead code while touching the class:
isSend/setIsSend(nothing read them; the realflag lives on
MysqlChannel),cloneContext()(no caller, and it would have to share a channelbetween two adapters), and the two lines of
resetConnection()that cleared Flight-only fields(
COM_RESET_CONNECTIONis only sent by MySQL clients).Not in this PR, deliberately: the execution layer still branches on
ConnectType, and an internalcontext is still a MySQL context over a
DummyMysqlChannel, exactly as before. Both go away in thefollow-up PRs that introduce the result sender and the capability bits.
Verification
MysqlPacketGoldenTest(27 cases, byte for byte) andFlightResultGoldenTestpass unchanged. Not a byte of the recorded traffic moved.FlightProtocolAdapterTest(commands of one session run one at a time, a waitingcommand fails with
UNAVAILABLEafter the query timeout, the thread's current context is set andrestored, a failing command releases the session,
KILLunregisters the session from the Flightpool, the trace id lands in the Flight pool) and
MysqlProtocolAdapterTest(internal and proxycontexts, the cursor-terminator decision and its per-statement reset, the accept-query loop and
close going through the channel).
FlightSqlConnectContext, pokedmysqlChannel/connectTypethrough reflection, or used a plainnew ConnectContext()as aFlight session (
ShortCircuitPointQueryTest,AuditLogWorkloadGroupTest,StmtExecutorTest,ConnectContextTest,MysqlProtoTest,ConnectionExceedTest). 26 test classes around thesession, the MySQL channel and the Flight producer: 166 tests, 0 failures.
arrow_flight_sql_p0(8 suites, includingthe forward-to-master, query-release, point-query, SQL cache and
DatabaseMetaData.getColumnspaths) and
prepared_stmt_p0(cursor fetch and server-side prepare over MySQL).checkstyle:checkon fe-core (main and test sources): 0 violations.Release note
None.
Check List (For Author)
Test
Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)
🤖 Generated with Claude Code
https://claude.ai/code/session_01QFwVuLmK8e7sEdKVKB6QZJ