FrankChen021 commented on code in PR #20313:
URL: https://github.com/apache/druid/pull/20313#discussion_r4015542397
##########
multi-stage-query/src/main/java/org/apache/druid/msq/dart/worker/DartWorkerClientImpl.java:
##########
@@ -184,6 +223,39 @@ private Pair<ServiceClient, Closeable>
getClientAndLocator(final String workerId
}
}
+ private static class RequestTrackingClient implements ServiceClient
+ {
+ private final ServiceClient delegate;
+ private final Set<ListenableFuture<?>> activeRequests;
+
+ private RequestTrackingClient(
+ final ServiceClient delegate,
+ final Set<ListenableFuture<?>> activeRequests
+ )
+ {
+ this.delegate = delegate;
+ this.activeRequests = activeRequests;
+ }
+
+ @Override
+ public <IntermediateType, FinalType> ListenableFuture<FinalType>
asyncRequest(
+ final RequestBuilder requestBuilder,
+ final HttpResponseHandler<IntermediateType, FinalType> handler
+ )
+ {
+ final ListenableFuture<FinalType> future =
delegate.asyncRequest(requestBuilder, handler);
+ activeRequests.add(future);
Review Comment:
[P2] Register delegated futures atomically with close
**Finding:** RequestTrackingClient.asyncRequest() calls the delegate before
adding the returned future to activeRequests. For non-postWorkOrder calls such
as fetches, postFinish, and stopWorker, close() can acquire clientMap after
dispatch but before this add, snapshot and clear the set, close the locators,
and leave the newly added future untracked. That request can therefore continue
retrying or remain in flight after query teardown, and a stop request can be
missed by close.
**Suggestion:** Serialize dispatch and registration with the client
lifecycle lock, or recheck the closed state after registration and cancel a
future that raced with close; add a test that blocks asyncRequest across close.
--
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]
