morningman commented on code in PR #69996:
URL: https://github.com/apache/doris/pull/69996#discussion_r4444136789
##########
fe/fe-core/src/main/java/org/apache/doris/arrowflight/results/FlightSqlChannel.java:
##########
@@ -124,8 +128,16 @@ public long getAllocatedMemory() {
return allocator.getAllocatedMemory();
}
- public void reset() {
+ public synchronized void registerRemoteQuery(TUniqueId queryId,
List<TUniqueId> resultIds,
+ List<TNetworkAddress> backends, int timeoutSeconds) {
+ FlightSqlQueryCancellation.INSTANCE.register(queryId, resultIds,
backends, timeoutSeconds);
+ remoteResultIds.addAll(resultIds);
+ }
+
+ public synchronized void reset() {
resultCache.invalidateAll();
+ FlightSqlQueryCancellation.INSTANCE.unregister(remoteResultIds);
Review Comment:
Fixed in `ba0c1d9926d`. Agreed, and the local run shows the case is wider
than overlapping streams: the BE reports an abort from its `DoGet` thread, so a
client that closes a stream and goes straight to its next request can start
that request before the report reaches the FE. In `test_flight_cancel_cleanup`,
before this change, the report of the first abort came after the suite's
health-check request and found no route.
A route now belongs to no session. `FlightSqlChannel` is back to its master
form, and `StmtExecutor` registers through
`FlightSqlQueryCancellation.register(ConnectContext, Coordinator)`, which keeps
nothing of the session. A route goes when its cancellation succeeds, when its
query's execution timeout plus 5 s has passed, or, oldest first and as a whole,
when the registry would hold more than 100,000 result ids. Without the session
as a bound, that cap keeps the registry from growing with the query rate times
the timeout. The registry is an insertion-ordered map instead of the Caffeine
cache, whose size eviction can reject a new entry instead of evicting the
oldest one.
Tests: `routesOutliveTheSessionsNextRequestAndTeardown` registers from a
Flight session, runs `beginRequest()` and `releaseProtocolSession()`, and still
cancels; `boundDropsWholeRoutesOldestFirst` covers the cap;
`test_flight_cancel_cleanup` gains a mode that runs the session's next request
before the abort. On a local cluster, the FE cancels all three aborts of that
suite through the route; before the change it cancelled one of them.
--
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]
