rajinisivaram commented on a change in pull request #10649:
URL: https://github.com/apache/kafka/pull/10649#discussion_r662986120
##########
File path:
clients/src/main/java/org/apache/kafka/clients/ClusterConnectionStates.java
##########
@@ -105,11 +105,13 @@ public boolean isBlackedOut(String id, long now) {
public long connectionDelay(String id, long now) {
NodeConnectionState state = nodeState.get(id);
if (state == null) return 0;
Review comment:
The javadoc still seems confusing to me. It seems fine for
`disconnected` state because we are returning remaining backoff. For
`connecting` state, we are now returning setup timeout, isn't that to cover
stalled state? And for `connected` state, we are relying on wakeup based on
other events, so this method doesn't actually `handle slow/stalled connections`?
##########
File path:
clients/src/main/java/org/apache/kafka/clients/ClusterConnectionStates.java
##########
@@ -273,6 +276,7 @@ public void authenticationFailed(String id, long now,
AuthenticationException ex
nodeState.state = ConnectionState.AUTHENTICATION_FAILED;
nodeState.lastConnectAttemptMs = now;
updateReconnectBackoff(nodeState);
+ connectingNodes.remove(id);
Review comment:
Can we get here with id in `connectingNodes`?
--
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
