FrankChen021 commented on code in PR #19927:
URL: https://github.com/apache/druid/pull/19927#discussion_r3740841839
##########
multi-stage-query/src/main/java/org/apache/druid/msq/dart/worker/DartWorkerClientImpl.java:
##########
@@ -173,6 +168,22 @@ protected Pair<ServiceClient, Closeable>
makeNewClient(final WorkerId workerId)
return Pair.of(client, locator);
}
+ private Pair<ServiceClient, Closeable> getClientAndLocator(final String
workerIdString)
+ {
+ final WorkerId workerId = WorkerId.fromString(workerIdString);
+ if (!queryId.equals(workerId.getQueryId())) {
+ throw DruidException.defensive("Unexpected queryId[%s]. Expected
queryId[%s]", workerId.getQueryId(), queryId);
+ }
+
+ synchronized (clientMap) {
+ if (closed) {
+ throw DruidException.defensive("%s is closed", getClass().getName());
+ }
+
+ return clientMap.computeIfAbsent(workerId.getHostAndPort(), ignored ->
makeNewClient(workerId));
Review Comment:
[P2] Retains clients for unrelated node removals
DartMessageRelays invokes serverRemoved for every historical node, before
controller.hasWorker(...) is checked. This computeIfAbsent therefore creates
and retains a closed client/locator for nodes never used by the query; repeated
node churn can grow each active query's cache until completion. Avoid retaining
entries for unrelated workers while preserving the pre-first-use removal race.
--
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]
