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
