philipnee commented on code in PR #12813: URL: https://github.com/apache/kafka/pull/12813#discussion_r1125051927
########## clients/src/main/java/org/apache/kafka/clients/NetworkClient.java: ########## @@ -753,7 +756,8 @@ public static AbstractResponse parseResponse(ByteBuffer responseBuffer, RequestH private void processDisconnection(List<ClientResponse> responses, Review Comment: I'm thinking, instead of passing boolean flags around, would it be better to have another function to explicitly calling out `processTimeoutDisconnection` ? ########## clients/src/main/java/org/apache/kafka/clients/NetworkClient.java: ########## @@ -324,11 +324,14 @@ public void disconnect(String nodeId) { log.info("Client requested disconnect from node {}", nodeId); selector.close(nodeId); long now = time.milliseconds(); - cancelInFlightRequests(nodeId, now, abortedSends); + cancelInFlightRequests(nodeId, now, abortedSends, false); connectionStates.disconnected(nodeId, now); } - private void cancelInFlightRequests(String nodeId, long now, Collection<ClientResponse> responses) { + private void cancelInFlightRequests(String nodeId, Review Comment: nit: final-lize all params ########## clients/src/main/java/org/apache/kafka/clients/NetworkClient.java: ########## @@ -1279,9 +1283,10 @@ public ClientResponse completed(AbstractResponse response, long timeMs) { false, null, null, response); } - public ClientResponse disconnected(long timeMs, AuthenticationException authenticationException) { + public ClientResponse disconnected(long timeMs, AuthenticationException authenticationException, boolean timedOut) { Review Comment: and maybe we could introduce `timeouted(long timeMs, AuthenticationException authenticationException)` ? if that makes everything more explicit about the timeout situation. -- 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: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org