cmccabe commented on a change in pull request #10281:
URL: https://github.com/apache/kafka/pull/10281#discussion_r595633668



##########
File path: 
clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java
##########
@@ -1089,29 +1106,61 @@ private long sendEligibleCalls(long now) {
                     continue;
                 }
                 Node node = entry.getKey();
+                if (!callsInFlight.getOrDefault(node.idString(), 
Collections.emptyList()).isEmpty()) {
+                    log.trace("Still waiting for other calls to finish on node 
{}.", node);
+                    nodeReadyDeadlines.remove(node);
+                    continue;

Review comment:
       Notice that we set `maxInFlightRequestsPerConnection` to 1 when 
constructing the `NetworkClient`.  We don't support sending multiple requests 
to a single node on a single connection in `AdminClient`.  I think we could add 
this support, but we'd have to check how the server handled it since we've 
never done it before.  Maybe there should be a JIRA.
   
   Also, if we do choose to add this support for multiple outstanding requests 
per node per socket we'd need some way to distinguish between "waiting for a 
chance to use this connection" from "waiting for this connection to be opened" 
in NetworkClient.  Currently ready just returns a boolean, which isn't enough 
information to distinguish these two cases.  We could probably add a new method 
that returned an enum or something.




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to