You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2020/12/01 18:49:38 UTC

[GitHub] [kafka] guozhangwang opened a new pull request #9667: MINOR: Do not print log4j for memberId required

guozhangwang opened a new pull request #9667:
URL: https://github.com/apache/kafka/pull/9667


   For MemberIdRequiredException, we would not print the exception at INFO with a full exception message since it may introduce more confusion that clearance.
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [kafka] guozhangwang commented on pull request #9667: MINOR: Do not print log4j for memberId required

Posted by GitBox <gi...@apache.org>.
guozhangwang commented on pull request #9667:
URL: https://github.com/apache/kafka/pull/9667#issuecomment-738931337


   > Maybe add an explanation here for why to skip the member.id required exception? (as it's transient)
   
   Ack, will do.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [kafka] guozhangwang merged pull request #9667: MINOR: Do not print log4j for memberId required

Posted by GitBox <gi...@apache.org>.
guozhangwang merged pull request #9667:
URL: https://github.com/apache/kafka/pull/9667


   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [kafka] guozhangwang commented on a change in pull request #9667: MINOR: Do not print log4j for memberId required

Posted by GitBox <gi...@apache.org>.
guozhangwang commented on a change in pull request #9667:
URL: https://github.com/apache/kafka/pull/9667#discussion_r536282550



##########
File path: clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractCoordinator.java
##########
@@ -465,7 +465,11 @@ boolean joinGroupIfNeeded(final Timer timer) {
                 }
             } else {
                 final RuntimeException exception = future.exception();
-                log.info("Rebalance failed.", exception);
+
+                if (!(exception instanceof MemberIdRequiredException)) {

Review comment:
       I think I agree with you: it is a bit redundant. I think originally it is because this future may contain the error that is either from join-response or from sync-response, but at the moment both errors are logged inside the Join/SyncGroupHandler anyways. Just to maintain compatibility in case some trouble shooting scenarios are dependent on this summary entry for now.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [kafka] bbejeck commented on pull request #9667: MINOR: Do not print log4j for memberId required

Posted by GitBox <gi...@apache.org>.
bbejeck commented on pull request #9667:
URL: https://github.com/apache/kafka/pull/9667#issuecomment-742799929


   cherry-picked to 2.7


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [kafka] chia7712 commented on a change in pull request #9667: MINOR: Do not print log4j for memberId required

Posted by GitBox <gi...@apache.org>.
chia7712 commented on a change in pull request #9667:
URL: https://github.com/apache/kafka/pull/9667#discussion_r535900161



##########
File path: clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractCoordinator.java
##########
@@ -465,7 +465,11 @@ boolean joinGroupIfNeeded(final Timer timer) {
                 }
             } else {
                 final RuntimeException exception = future.exception();
-                log.info("Rebalance failed.", exception);
+
+                if (!(exception instanceof MemberIdRequiredException)) {

Review comment:
       For another, does it need some comment to explain why ```MemberIdRequiredException``` is excluded.

##########
File path: clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractCoordinator.java
##########
@@ -465,7 +465,11 @@ boolean joinGroupIfNeeded(final Timer timer) {
                 }
             } else {
                 final RuntimeException exception = future.exception();
-                log.info("Rebalance failed.", exception);
+
+                if (!(exception instanceof MemberIdRequiredException)) {

Review comment:
       Is ```JoinGroupResponseHandler``` a better place to log error? For example, the error ```UNKNOWN_MEMBER_ID``` is log twice. 
   (https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/clients/consumer/internals/AbstractCoordinator.java#L605)
   




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [kafka] guozhangwang commented on pull request #9667: MINOR: Do not print log4j for memberId required

Posted by GitBox <gi...@apache.org>.
guozhangwang commented on pull request #9667:
URL: https://github.com/apache/kafka/pull/9667#issuecomment-736747672


   @hachikuji @abbccdda 


----------------------------------------------------------------
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:
users@infra.apache.org