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 2022/09/07 08:44:58 UTC

[GitHub] [kafka] dajac opened a new pull request, #12598: KAFKA-14201; Consumer should not send group instance ID if committing with empty member ID

dajac opened a new pull request, #12598:
URL: https://github.com/apache/kafka/pull/12598

   The consumer group instance ID is used to support a notion of "static" consumer groups. The idea is to be able to identify the same group instance across restarts so that a rebalance is not needed. However, if the user sets `group.instance.id` in the consumer configuration, but uses "simple" assignment with `assign()`, then the instance ID nevertheless is sent in the OffsetCommit request to the coordinator. This may result in a surprising UNKNOWN_MEMBER_ID error.
   
   This PR attempts to fix this issue for existing consumers by relaxing the validation in this case. One way is to simply ignore the member id and the static id when the generation id is -1. -1 signals that the request comes from either the admin client or a consumer which does not use the group management. This does not apply to transactional offsets commit.
   
   ### 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.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [kafka] hachikuji merged pull request #12598: KAFKA-14201; Consumer should not send group instance ID if committing with empty member ID (server side)

Posted by GitBox <gi...@apache.org>.
hachikuji merged PR #12598:
URL: https://github.com/apache/kafka/pull/12598


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [kafka] dajac commented on a diff in pull request #12598: KAFKA-14201; Consumer should not send group instance ID if committing with empty member ID (server side)

Posted by GitBox <gi...@apache.org>.
dajac commented on code in PR #12598:
URL: https://github.com/apache/kafka/pull/12598#discussion_r966215071


##########
core/src/main/scala/kafka/coordinator/group/GroupCoordinator.scala:
##########
@@ -982,6 +982,12 @@ class GroupCoordinator(val brokerId: Int,
   ): Option[Errors] = {
     if (group.is(Dead)) {
       Some(Errors.COORDINATOR_NOT_AVAILABLE)
+    } else if (!isTransactional && generationId < 0 && group.is(Empty)) {

Review Comment:
   Removed it.



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [kafka] dajac commented on a diff in pull request #12598: KAFKA-14201; Consumer should not send group instance ID if committing with empty member ID (server side)

Posted by GitBox <gi...@apache.org>.
dajac commented on code in PR #12598:
URL: https://github.com/apache/kafka/pull/12598#discussion_r966208886


##########
core/src/main/scala/kafka/coordinator/group/GroupCoordinator.scala:
##########
@@ -982,6 +982,12 @@ class GroupCoordinator(val brokerId: Int,
   ): Option[Errors] = {
     if (group.is(Dead)) {
       Some(Errors.COORDINATOR_NOT_AVAILABLE)
+    } else if (!isTransactional && generationId < 0 && group.is(Empty)) {

Review Comment:
   I can remove that check to stay consistent.



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [kafka] dajac commented on pull request #12598: KAFKA-14201; Consumer should not send group instance ID if committing with empty member ID

Posted by GitBox <gi...@apache.org>.
dajac commented on PR #12598:
URL: https://github.com/apache/kafka/pull/12598#issuecomment-1239094026

   @hachikuji What do you think about doing something like this? I was debating wether ignoring the member id as well is a good thing or not in this case. That seems to be acceptable. 


-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [kafka] dajac commented on a diff in pull request #12598: KAFKA-14201; Consumer should not send group instance ID if committing with empty member ID (server side)

Posted by GitBox <gi...@apache.org>.
dajac commented on code in PR #12598:
URL: https://github.com/apache/kafka/pull/12598#discussion_r966198873


##########
core/src/main/scala/kafka/coordinator/group/GroupCoordinator.scala:
##########
@@ -982,6 +982,12 @@ class GroupCoordinator(val brokerId: Int,
   ): Option[Errors] = {
     if (group.is(Dead)) {
       Some(Errors.COORDINATOR_NOT_AVAILABLE)
+    } else if (!isTransactional && generationId < 0 && group.is(Empty)) {

Review Comment:
   I thought that transactional commits should never comes with a generation < 0. I suppose that this is wrong. I suppose that a producer could commit transactional offsets without using a consumer group. Is this right?



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [kafka] hachikuji commented on a diff in pull request #12598: KAFKA-14201; Consumer should not send group instance ID if committing with empty member ID (server side)

Posted by GitBox <gi...@apache.org>.
hachikuji commented on code in PR #12598:
URL: https://github.com/apache/kafka/pull/12598#discussion_r966196258


##########
core/src/main/scala/kafka/coordinator/group/GroupCoordinator.scala:
##########
@@ -982,6 +982,12 @@ class GroupCoordinator(val brokerId: Int,
   ): Option[Errors] = {
     if (group.is(Dead)) {
       Some(Errors.COORDINATOR_NOT_AVAILABLE)
+    } else if (!isTransactional && generationId < 0 && group.is(Empty)) {

Review Comment:
   I am not sure I follow the reason for the transactional check. Can you clarify?



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [kafka] hachikuji commented on a diff in pull request #12598: KAFKA-14201; Consumer should not send group instance ID if committing with empty member ID (server side)

Posted by GitBox <gi...@apache.org>.
hachikuji commented on code in PR #12598:
URL: https://github.com/apache/kafka/pull/12598#discussion_r966208033


##########
core/src/main/scala/kafka/coordinator/group/GroupCoordinator.scala:
##########
@@ -982,6 +982,12 @@ class GroupCoordinator(val brokerId: Int,
   ): Option[Errors] = {
     if (group.is(Dead)) {
       Some(Errors.COORDINATOR_NOT_AVAILABLE)
+    } else if (!isTransactional && generationId < 0 && group.is(Empty)) {

Review Comment:
   It seems to be allowed at the moment, though that is definitely not a common case.



-- 
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-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org