hachikuji commented on a change in pull request #9729:
URL: https://github.com/apache/kafka/pull/9729#discussion_r541218024
##########
File path:
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractCoordinator.java
##########
@@ -1355,7 +1362,7 @@ public void run() {
} else if (heartbeat.sessionTimeoutExpired(now)) {
// the session timeout has expired without seeing
a successful heartbeat, so we should
// probably make sure the coordinator is still
healthy.
- markCoordinatorUnknown();
+ markCoordinatorUnknown("session timed out without
heartbeat");
Review comment:
Seems ok. Maybe "without receiving a successful heartbeat response"?
##########
File path:
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractCoordinator.java
##########
@@ -890,13 +890,20 @@ private synchronized Node coordinator() {
return this.coordinator;
}
- protected synchronized void markCoordinatorUnknown() {
- markCoordinatorUnknown(false);
+
+ protected synchronized void markCoordinatorUnknown(Errors error) {
+ markCoordinatorUnknown(false, "error response {}" + error.message());
Review comment:
Wonder if we can use the name of the error. Some of the messages will
read awkwardly in the log message
##########
File path:
clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractCoordinator.java
##########
@@ -890,13 +890,20 @@ private synchronized Node coordinator() {
return this.coordinator;
}
- protected synchronized void markCoordinatorUnknown() {
- markCoordinatorUnknown(false);
+
+ protected synchronized void markCoordinatorUnknown(Errors error) {
+ markCoordinatorUnknown(false, "error response {}" + error.message());
+ }
+
+ protected synchronized void markCoordinatorUnknown(String cause) {
+ markCoordinatorUnknown(false, cause);
}
- protected synchronized void markCoordinatorUnknown(boolean isDisconnected)
{
+ protected synchronized void markCoordinatorUnknown(boolean isDisconnected,
String cause) {
if (this.coordinator != null) {
- log.info("Group coordinator {} is unavailable or invalid, will
attempt rediscovery", this.coordinator);
+ log.info("Group coordinator {} is unavailable or invalid due to
cause: {}."
+ + "isDisconnected: {}. Rediscovery will attempted.",
this.coordinator,
Review comment:
nit: "Rediscovery will **be** attempted"
----------------------------------------------------------------
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
