guozhangwang commented on a change in pull request #11449:
URL: https://github.com/apache/kafka/pull/11449#discussion_r740417424
##########
File path: clients/src/main/java/org/apache/kafka/clients/NetworkClient.java
##########
@@ -319,23 +319,29 @@ public void disconnect(String nodeId) {
if (connectionStates.isDisconnected(nodeId))
return;
+ log.info("Client requested disconnect from node {} (invoking callbacks
for in-flight requests)", nodeId);
Review comment:
It seems we only have two types of callers for this: 1) admin-client
proactively disconnecting, where the caller always log an info explaining why,
2) consumer-client async disconnect the coordinator.
If our target is primarily covering 2) here, what about: 1) make this
function `disconnect(nodeId, reason)` where `reason` is a formatted string; 2)
rename the `pendingDisconnects` (which is only used for coordinator
disconnects) as `pendingCoordinatorDisconnects` and pass in a reason like
"since the node was previously recognized as the group coordinator but now it
become unknown".
##########
File path: clients/src/main/java/org/apache/kafka/clients/NetworkClient.java
##########
@@ -923,7 +930,7 @@ private void handleApiVersionsResponse(List<ClientResponse>
responses,
private void handleDisconnections(List<ClientResponse> responses, long
now) {
for (Map.Entry<String, ChannelState> entry :
this.selector.disconnected().entrySet()) {
String node = entry.getKey();
- log.debug("Node {} disconnected.", node);
+ log.info("Node {} disconnected.", node);
Review comment:
It makes me thinking: why not consolidating `disconnect` to call
`processDisconnections` but have two call paths here?
##########
File path: clients/src/main/java/org/apache/kafka/clients/NetworkClient.java
##########
@@ -355,6 +361,7 @@ private void cancelInFlightRequests(String nodeId, long
now, Collection<ClientRe
*/
@Override
public void close(String nodeId) {
+ log.info("Client requested disconnect from node {} (not invoking
callbacks for in-flight requests)", nodeId);
Review comment:
nit: request closing the socket from node?
--
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
