github-actions[bot] commented on code in PR #69996:
URL: https://github.com/apache/doris/pull/69996#discussion_r4469160220
##########
be/src/service/arrow_flight/arrow_flight_batch_reader.cpp:
##########
@@ -103,6 +105,46 @@ void ArrowFlightBatchReaderBase::close(const Status&
reason) {
_statement->result_addr.port == config::brpc_port) {
env->result_mgr()->cancel_arrow_flight_query(_statement->query_id,
reason);
}
+ // A result endpoint can outlive its local fragment. Find the owning FE by
buffer ID,
+ // then use its query-wide cancellation route, including for older result
BEs.
+ for (const auto& [address, info] : env->get_running_frontends()) {
+ Status status;
+ // One retry: the FE keeps the route when an attempt fails (for
example a BE missed the
+ // FE's one-second deadline), and a cached connection to the FE may
have gone stale.
+ // Neither applies to an FE that cannot be connected to, so that one
gets no retry.
+ for (int attempt = 0; attempt < 2; ++attempt) {
+ // An FE that sorts before the one that planned the query delays
every early close on
+ // every BE. So the connection comes from a cache that holds only
transports opened
+ // with this 2 s timeout, and acquiring it does not back off
(max_retries = 0).
+ FrontendServiceConnection
client(env->flight_cancel_frontend_client_cache(), address,
Review Comment:
[P2] Bound the total FE probe during stream teardown. `close()` asks every
running FE in address order, and `process_uuid == 0` entries are intentionally
treated as alive; with several stale FEs before the planner, each can consume
its connection/RPC retry budget before the loop reaches the route owner. The
dedicated cache fixes per-FE reuse/backoff but leaves cumulative delay
proportional to the number of stale FEs, blocking the DoGet/destructor thread
and postponing remote cancellation. Use one overall deadline,
remember/prioritize the planner FE, or probe the FEs concurrently.
##########
be/src/service/arrow_flight/arrow_flight_batch_reader.cpp:
##########
@@ -103,6 +105,46 @@ void ArrowFlightBatchReaderBase::close(const Status&
reason) {
_statement->result_addr.port == config::brpc_port) {
env->result_mgr()->cancel_arrow_flight_query(_statement->query_id,
reason);
}
+ // A result endpoint can outlive its local fragment. Find the owning FE by
buffer ID,
+ // then use its query-wide cancellation route, including for older result
BEs.
+ for (const auto& [address, info] : env->get_running_frontends()) {
Review Comment:
[P1] Avoid the unused structured-binding element here. `info` is never
referenced, while BE Clang builds enable `-Wunused` under `-Werror`, so this
new loop can fail compilation with an unused-variable diagnostic. Iterate the
map keys or use the repository's intentionally ignored binding convention.
##########
fe/fe-core/src/main/java/org/apache/doris/qe/StmtExecutor.java:
##########
@@ -1920,6 +1921,12 @@ public void executeAndSendResult(boolean isOutfileQuery,
boolean isSendFields,
if (!context.isReturnResultFromLocal()) {
profile.getSummaryProfile().setTempStartTime();
+ if (coordBase == coord) {
+ // A BE whose DoGet stream the client aborts reports it
through cancelFlightQuery;
+ // keep the query's backends so that call can cancel the
query on every one of
+ // them after the coordinator below is closed.
+ FlightSqlQueryCancellation.INSTANCE.register(context,
coord);
Review Comment:
[P2] Remove the cancellation route when GetFlightInfo fails. `register()`
runs at this line before `DorisFlightSqlProducer.fetchArrowFlightSchema(5000)`
and before the returned FlightInfo/ticket exists. If that schema RPC times out
or errors (or a later prepared-schema check rejects the result), the producer
catch only cancels/finalizes the coordinator; it never removes the route. The
result IDs and BE snapshots then occupy the global registry until timeout+5s,
so repeated failed requests can evict routes for successful queries and make
their later DoGet cancellation return NOT_FOUND. Register after schema
admission or explicitly unregister this failed query path.
--
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]
