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/06/05 17:01:47 UTC

[GitHub] [kafka] abbccdda opened a new pull request #8816: MINOR: Print all members during join complete

abbccdda opened a new pull request #8816:
URL: https://github.com/apache/kafka/pull/8816


   For better visibility on the group rebalance, we are trying to print out the included members inside the group coordinator during rebalance complete.
   
   ### 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 #8816: MINOR: Print all removed dynamic members during join complete

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






----------------------------------------------------------------
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] abbccdda commented on a change in pull request #8816: MINOR: Print all members during join complete

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



##########
File path: core/src/main/scala/kafka/coordinator/group/GroupCoordinator.scala
##########
@@ -1150,7 +1150,6 @@ class GroupCoordinator(val brokerId: Int,
       group.notYetRejoinedMembers.filterNot(_.isStaticMember) foreach { failedMember =>
         removeHeartbeatForLeavingMember(group, failedMember)
         group.remove(failedMember.memberId)
-        group.removeStaticMember(failedMember.groupInstanceId)

Review comment:
       This is a cleanup. In the current logic we should never hit a static member in this block




----------------------------------------------------------------
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 #8816: MINOR: Print all removed dynamic members during join complete

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


   


----------------------------------------------------------------
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 #8816: MINOR: Print all removed dynamic members during join complete

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


   okay to test


----------------------------------------------------------------
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] hachikuji commented on a change in pull request #8816: MINOR: Print all members during join complete

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



##########
File path: core/src/main/scala/kafka/coordinator/group/GroupCoordinator.scala
##########
@@ -1186,6 +1185,9 @@ class GroupCoordinator(val brokerId: Int,
           for (member <- group.allMemberMetadata) {
             val joinResult = JoinGroupResult(
               members = if (group.isLeader(member.memberId)) {
+                info(s"Group ${group.groupId} replies the leader with " +

Review comment:
       Do we need to print this for every member? That is O(n^2). Keep in mind that groups in the thousands are not unheard of. I think it would be better if we just ensured that we log the id of every member entering and leaving the group.




----------------------------------------------------------------
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] abbccdda commented on a change in pull request #8816: MINOR: Print all members during join complete

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



##########
File path: core/src/main/scala/kafka/coordinator/group/GroupCoordinator.scala
##########
@@ -1186,6 +1185,8 @@ class GroupCoordinator(val brokerId: Int,
           for (member <- group.allMemberMetadata) {
             val joinResult = JoinGroupResult(
               members = if (group.isLeader(member.memberId)) {
+                info(s"Reply the group leader with the following dynamic members' metadata: ${group.allDynamicMembers}" +
+                  s"and static members' metadata: ${group.allStaticMembers}")

Review comment:
       Good idea




----------------------------------------------------------------
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] vvcephei commented on pull request #8816: MINOR: Print all removed dynamic members during join complete

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






----------------------------------------------------------------
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] dajac commented on a change in pull request #8816: MINOR: Print all members during join complete

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



##########
File path: core/src/main/scala/kafka/coordinator/group/GroupCoordinator.scala
##########
@@ -1186,6 +1185,8 @@ class GroupCoordinator(val brokerId: Int,
           for (member <- group.allMemberMetadata) {
             val joinResult = JoinGroupResult(
               members = if (group.isLeader(member.memberId)) {
+                info(s"Reply the group leader with the following dynamic members' metadata: ${group.allDynamicMembers}" +
+                  s"and static members' metadata: ${group.allStaticMembers}")

Review comment:
       Shouldn't we include the group id in the log as well?




----------------------------------------------------------------
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 #8816: MINOR: Print all removed dynamic members during join complete

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



##########
File path: core/src/main/scala/kafka/coordinator/group/GroupCoordinator.scala
##########
@@ -1146,12 +1146,16 @@ class GroupCoordinator(val brokerId: Int,
 
   def onCompleteJoin(group: GroupMetadata): Unit = {
     group.inLock {
-      // remove dynamic members who haven't joined the group yet
-      group.notYetRejoinedMembers.filterNot(_.isStaticMember) foreach { failedMember =>
-        removeHeartbeatForLeavingMember(group, failedMember)
-        group.remove(failedMember.memberId)
-        group.removeStaticMember(failedMember.groupInstanceId)

Review comment:
       Nice catch.




----------------------------------------------------------------
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