morningman opened a new pull request, #68266:
URL: https://github.com/apache/doris/pull/68266
### What problem does this PR solve?
Issue Number: #67577
Related PR: #68101 (the one connection pool this builds on), #67966 (the
Flight session teardown contract this keeps, D-13)
Problem Summary:
Stage 3, item 2 of #67577 (connection governance): the bearer token of an
Arrow Flight SQL session is the credential of exactly one session and lives
exactly as long as it.
A bearer token used to be an object of its own next to the session. It was
issued at the handshake into a Guava cache — LRU, sized to the Flight
sub-quota, expiring 24 hours after write
(`arrow_flight_token_alive_time_second`) — plus a per-user LRU of
`max_user_connections / 2` tokens, while the session (`ConnectContext`) was
created on the token's first request and registered in the connection pool. The
two coupled only one way: a token leaving the cache unregistered its session, a
session leaving the pool left its token behind. What that produced:
- A full Flight sub-quota showed as the eviction of the least recently used
token — someone else's live session with it, its transaction rolled back, its
deferred query finalized — rather than as the pool's refusal, which was in
practice unreachable for the sub-quota (the cache is sized to it, and Guava
evicts per segment before the total is reached). A second, undocumented
per-user limit of `max_user_connections / 2` tokens evicted the same way.
- An active session was cut off 24 hours after its handshake, however busy
it was: the expiry counted from the write, not from use.
- After `KILL CONNECTION` or `wait_timeout` the token stayed in the cache,
so the session's next request first tripped the created-session guard
(`INTERNAL: UserSession expire after access`) and only the one after that was
`UNAUTHENTICATED`.
- Session creation needed a frontend-wide lock and a re-validation of the
token after registration to close the race between the two owners (#68101 left
a note that the token's lifecycle would move under one owner in this PR).
Now the token is nothing but the peer identity a session is registered in
the pool under, and that index is the only place a token exists.
- `FlightSessionsManager` has three operations: `openSession` — at the
handshake that authenticated the password (`FlightCredentialValidator`): mint
the token, build the `ConnectContext`, register it in the pool; the pool's
refusal is the handshake's `RESOURCE_EXHAUSTED`, in the words a MySQL client is
refused in, and nothing is left of a refused session, no token either —
`getConnectContext` — the pool's lookup by peer identity; none →
`UNAUTHENTICATED`, used by the header authenticator for every call and by the
producer to resolve a call's session — and `closeConnectContext` —
`CloseSession`: what `COM_QUIT` does for a MySQL connection
(`ConnectContext.cleanup`: the channel's cached results, the deferred
executors, the transaction, the temporary tables, the pool's slots), plus
cancelling a statement still running on another thread, since a Flight
session's commands need not run on the thread that closes it.
- `FlightSessionsInConnectPool` is the one implementation, serving both the
header authenticator and the producer. The `tokens` package
(`FlightTokenManager`, `FlightTokenManagerImpl`, `FlightTokenDetails`), both
caches, the cleanup thread, the created-session flag and the creation lock with
its re-validation are deleted. `FlightTokenDetails.expiresAt`, computed with
minutes for seconds so that the in-code expiry check never fired, goes with the
class.
- A session ends with `CloseSession`, `KILL CONNECTION` or `wait_timeout`
(or the frontend restarting), and from that moment the very next call under its
token is `UNAUTHENTICATED`, whatever kind of call it is. Nothing else ends a
session: no session that fits is evicted for one that does not (the pool's
quotas are the only admission control, as decided for #67577 D-3), and there is
no idle timeout of Flight's own — `wait_timeout`, which a Flight client can set
through its session options since #67966, governs both protocols.
- `arrow_flight_token_cache_size` and `arrow_flight_token_alive_time_second`
are deprecated no-ops (`@Deprecated`, the way `enable_query_hive_views` is),
kept for one release so a fe.conf that sets them still parses; a value other
than the default is reported at startup by name.
- `arrow_flight_max_connections`' description is rewritten around how a
session now ends. `ConnectPoolMgr`, `FlightProtocolAdapter` and the producer's
comments no longer name token expiry or eviction as teardown paths.
Two things that did not change, stated so nobody looks for them:
`getStreamStatement` still resolves the executing session from the ticket's
peer identity rather than the call's token (a client that opens a second
connection for an endpoint may authenticate again on it), and the
deferred-query contract of #67966 is untouched — teardown still does not wait
for a running command.
One known edge: a client that authenticates again for each connection it
opens to fetch a result opens a session each time, which stays until
`wait_timeout` (before this PR it left a token in the cache instead). The
Flight SQL JDBC driver did that before 15.0.0; 15.0.0 and later reuse the
bearer token for the endpoint connections they open, and the ADBC drivers pass
the token along too. The config description says so.
### Release note
The bearer token of an Arrow Flight SQL session is now valid exactly as long
as the session it was issued with: the session opens, and the token is issued,
at the handshake that authenticates the user (`authenticateBasicToken`), and
the session ends with `CloseSession`, a `KILL CONNECTION` from another
connection, or `wait_timeout`. From that moment every call made with the token
is refused as `UNAUTHENTICATED`, with a message naming the token by its masked
id and the ways the session may have ended. A session is no longer ended by
anything else: not by a 24-hour token expiry (an active session is never cut
off any more), and not by the eviction of a token from a cache when another
session is opened — a Flight session that does not fit `qe_max_connection`,
`arrow_flight_max_connections` or the user's `max_user_connections` is refused
at its handshake, with the same `Reach limit of connections ...` message as a
MySQL connection, and no session already open is evicted for it. The
per-user cap of `max_user_connections / 2` bearer tokens is gone with the
cache: a user may hold up to `max_user_connections` connections over both
protocols, as documented.
`arrow_flight_token_cache_size` and `arrow_flight_token_alive_time_second`
are deprecated and no longer read; a fe.conf that sets them still starts, with
a warning naming the setting, and they will be removed in a later release.
Sessions whose clients have gone without `CloseSession` (the ADBC drivers send
it; the Flight SQL JDBC driver only for a connection opened with a `catalog`)
stay in the pool until `wait_timeout` (8 hours by default), which is the one
knob for reclaiming them sooner: lower it globally, per user, or for the
session (Flight clients can set it through their session options).
`CloseSession` now also drops the session's temporary tables and cancels a
statement of the session that is still running, as a MySQL client's disconnect
does.
### Check List (For Author)
- Test
- [x] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
**Unit tests.** `FlightSessionsInConnectPoolTest` (new): opening a session
registers it under the token it issues (one token per session, each naming only
its own); a token no session is registered under is `UNAUTHENTICATED`, named by
its masked id and never by the token itself; `CloseSession`, `KILL CONNECTION`
and `wait_timeout` (through the pool's timeout checker) each end the session
and with it the token, `CloseSession` cancelling a statement still running and
being harmless on a session already gone; a session that does not fit is
refused with the pool's own words and leaves nothing, the session already open
untouched; tokens are 130 random bits in base 32. `ConnectionExceedTest`
reworked around `openSession` (the refusal is the handshake's
`RESOURCE_EXHAUSTED` with the MySQL wording; the lazy-creation race test is
gone with lazy creation). `DorisFlightSqlServiceTest`: the startup report names
only the deprecated settings that differ from their defaults, which are the conf
ig fields' own. `DorisFlightSqlProducerTest`, `FlightProtocolAdapterTest`,
`FlightSessionClientIpTest`, `FlightSqlDeferredQueryIdleTimeoutTest`,
`FlightSessionOptionsTest`, `ConnectPoolMgrTest`, `TokenMaskerTest` green.
Protocol goldens `MysqlPacketGoldenTest` (33 cases) / `FlightResultGoldenTest`
(9 statements) unchanged.
**Regression.** `arrow_flight_sql_p0/test_connection_quota`:
`max_user_connections` back to 1 (it was 4 to dodge the per-user token cap):
the first Flight session is the user's one connection from its handshake on and
answers a statement; the second is refused at its handshake in MySQL's words,
leaving the first untouched; a MySQL connection is refused in the same words;
`CloseSession` frees the slot (the closed session's token is `UNAUTHENTICATED`
at once), a MySQL connection takes it and a Flight session is refused for it,
and once that connection is closed a Flight session opens again.
`arrow_flight_sql_p0/test_bearer_token_lifecycle` (new): `KILL CONNECTION` from
the control connection, `wait_timeout` (the session sets its own to 3 seconds)
and `CloseSession` each make the very next call under the token
`UNAUTHENTICATED` — a statement, a metadata request and a session action alike,
with the new message — and a later session of the same user does not revive an
ended token
. Whole `arrow_flight_sql_p0` green locally: 12 suites, 0 failed, against an
FE built from this branch. With the Python ADBC driver against that FE: one
connection is one session from the handshake on across FE-side (`SHOW`, `SET`)
and BE-side (`SELECT`) statements, `KILL CONNECTION` makes the driver's next
statement fail `UNAUTHENTICATED`, and `Connection.close()` ends the session.
- Behavior changed:
- [x] Yes. See the release note: refusal at the handshake instead of at
the first request; no token expiry and no eviction of a session for another; no
per-user cap of `max_user_connections / 2` tokens; `UNAUTHENTICATED`
immediately after `KILL CONNECTION` / `wait_timeout` (it was `INTERNAL` once,
then `UNAUTHENTICATED`); the two settings deprecated; `CloseSession` drops
temporary tables and cancels a running statement.
- Does this need documentation?
- [x] Yes. Docs PR to apache/doris-website to follow (together with
#68101's, not opened yet): the Arrow Flight SQL page and the FE config page —
how a session ends and what the token's validity is, the refusal at the
handshake, the two deprecated settings, `wait_timeout` as the knob for
abandoned sessions.
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should
merge into -->
--
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]
