xiangfu0 commented on code in PR #18649:
URL: https://github.com/apache/pinot/pull/18649#discussion_r3367292945
##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseSingleStageBrokerRequestHandler.java:
##########
@@ -338,6 +340,76 @@ protected boolean handleCancel(long queryId, int
timeoutMs, Executor executor, H
return true;
}
+ @Override
+ @Nullable
+ public QueryProgressStats getQueryProgressStats(long queryId, int timeoutMs,
Executor executor,
+ HttpClientConnectionManager connMgr)
+ throws Exception {
+ Preconditions.checkState(isQueryCancellationEnabled(), "Query cancellation
is not enabled on broker");
+ QueryServers queryServers = _serversById.get(queryId);
Review Comment:
This new SSE progress path assumes `_serversById` and `_clientQueryIds` stay
populated for the lifetime of the outer query, but `IN_SUBQUERY` still executes
`doHandleRequest(requestId, ...)` with the same request id. The inner call's
`finally { onQueryFinish(requestId); }` clears those maps as soon as the
subquery returns, so `/query/{clientQueryId}/progress` can flip to 404 while
the outer scatter-gather is still running. Please give subqueries a distinct
internal request id or ref-count the tracking so only the outermost request
removes the state.
--
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]
