You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "frankvicky (via GitHub)" <gi...@apache.org> on 2024/06/01 06:30:04 UTC
[PR] KAFKA-16861: Don't convert to group to classic if the size is larger than group max size. [kafka]
frankvicky opened a new pull request, #16163:
URL: https://github.com/apache/kafka/pull/16163
Fix the bug where the group downgrade to a classic one when a member leaves, even though the consumer group size is still larger than `classicGroupMaxSize`.
### 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
Re: [PR] KAFKA-16861: Don't convert to group to classic if the size is larger than group max size. [kafka]
Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on PR #16163:
URL: https://github.com/apache/kafka/pull/16163#issuecomment-2143745346
@frankvicky Thanks for this! As I said, I am fine with merging the fix. We should definitely keep it. I just wanted to signal that there is an ongoing effort from @dongnuo123 to add integration and system tests for the upgrade/downgrade paths.
--
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
Re: [PR] KAFKA-16861: Don't convert to group to classic if the size is larger than group max size. [kafka]
Posted by "frankvicky (via GitHub)" <gi...@apache.org>.
frankvicky commented on code in PR #16163:
URL: https://github.com/apache/kafka/pull/16163#discussion_r1624235852
##########
group-coordinator/src/test/java/org/apache/kafka/coordinator/group/GroupMetadataManagerTest.java:
##########
@@ -13166,6 +13166,50 @@ public void testClassicGroupLeaveToConsumerGroupWithoutValidLeaveGroupMember() {
assertEquals(Collections.emptyList(), leaveResult.records());
}
+ @Test
+ public void testNoConversionWhenSizeExceedsClassicMaxGroupSize() throws Exception {
+ MockPartitionAssignor assignor = new MockPartitionAssignor("range");
Review Comment:
Oops, my bad, I will remove 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
Re: [PR] KAFKA-16861: Don't convert to group to classic if the size is larger than group max size. [kafka]
Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on PR #16163:
URL: https://github.com/apache/kafka/pull/16163#issuecomment-2143718116
@dongnuo123 is working in adding integration tests for all this work stream. We should wait.
@chia7712 We do already have unit tests but we missed one for this particular 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
Re: [PR] KAFKA-16861: Don't convert to group to classic if the size is larger than group max size. [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #16163:
URL: https://github.com/apache/kafka/pull/16163#discussion_r1624060061
##########
group-coordinator/src/test/java/org/apache/kafka/coordinator/group/GroupMetadataManagerTest.java:
##########
@@ -13166,6 +13166,50 @@ public void testClassicGroupLeaveToConsumerGroupWithoutValidLeaveGroupMember() {
assertEquals(Collections.emptyList(), leaveResult.records());
}
+ @Test
+ public void testNoConversionWhenSizeExceedsClassicMaxGroupSize() throws Exception {
+ MockPartitionAssignor assignor = new MockPartitionAssignor("range");
Review Comment:
It seems this variable is unused.
--
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
Re: [PR] KAFKA-16861: Don't convert to group to classic if the size is larger than group max size. [kafka]
Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac merged PR #16163:
URL: https://github.com/apache/kafka/pull/16163
--
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
Re: [PR] KAFKA-16861: Don't convert to group to classic if the size is larger than group max size. [kafka]
Posted by "frankvicky (via GitHub)" <gi...@apache.org>.
frankvicky commented on PR #16163:
URL: https://github.com/apache/kafka/pull/16163#issuecomment-2143744193
Hi @dajac,
As @chia7712 mentioned, I am cooperating with @chia7712 on this issue. It seems that @dongnuo123 is working on the integration test for this. If that is the case, please feel free to incorporate this fix into your PR.
Thank you! 😺
--
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
Re: [PR] KAFKA-16861: Don't convert to group to classic if the size is larger than group max size. [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #16163:
URL: https://github.com/apache/kafka/pull/16163#issuecomment-2143746525
@frankvicky @dajac thanks for all your responses. I will take a look at this PR later. @dajac it would be great if this PR can have reviews from you :)
--
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
Re: [PR] KAFKA-16861: Don't convert to group to classic if the size is larger than group max size. [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #16163:
URL: https://github.com/apache/kafka/pull/16163#issuecomment-2143737885
> if you want to help, we can sync with @dongnuo123.
@dongnuo123 Please feel free to make this fix be a part of your PR if you are adding integration tests for all this work stream.
--
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
Re: [PR] KAFKA-16861: Don't convert to group to classic if the size is larger than group max size. [kafka]
Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on PR #16163:
URL: https://github.com/apache/kafka/pull/16163#issuecomment-2144482983
@chia7712 Do you want to review it too?
--
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
Re: [PR] KAFKA-16861: Don't convert to group to classic if the size is larger than group max size. [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #16163:
URL: https://github.com/apache/kafka/pull/16163#issuecomment-2143558213
I noticed that we don't have IT for downgrading to classic group. Maybe we can add IT in this PR?
--
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
Re: [PR] KAFKA-16861: Don't convert to group to classic if the size is larger than group max size. [kafka]
Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on PR #16163:
URL: https://github.com/apache/kafka/pull/16163#issuecomment-2143726817
@frankvicky Out of curiosity, how did you find the bug? Regarding the integration tests, if you want to help, we can sync with @dongnuo123.
--
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
Re: [PR] KAFKA-16861: Don't convert to group to classic if the size is larger than group max size. [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #16163:
URL: https://github.com/apache/kafka/pull/16163#issuecomment-2143737187
> Out of curiosity, how did you find the bug?
@dajac the ticket is created by me (see https://issues.apache.org/jira/browse/KAFKA-16861), and I noticed that issue when reading the new coordinator.
--
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
Re: [PR] KAFKA-16861: Don't convert to group to classic if the size is larger than group max size. [kafka]
Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on PR #16163:
URL: https://github.com/apache/kafka/pull/16163#issuecomment-2145877889
Merged to trunk and to 3.8.
--
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
Re: [PR] KAFKA-16861: Don't convert to group to classic if the size is larger than group max size. [kafka]
Posted by "frankvicky (via GitHub)" <gi...@apache.org>.
frankvicky commented on PR #16163:
URL: https://github.com/apache/kafka/pull/16163#issuecomment-2143675270
Sounds good, I will write an integration test for downgrading to classic 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.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16861: Don't convert to group to classic if the size is larger than group max size. [kafka]
Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on PR #16163:
URL: https://github.com/apache/kafka/pull/16163#issuecomment-2143718243
@dongnuo123 Could you please review this PR?
--
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
Re: [PR] KAFKA-16861: Don't convert to group to classic if the size is larger than group max size. [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #16163:
URL: https://github.com/apache/kafka/pull/16163#issuecomment-2144640823
> Do you want to review it too?
I will take a look later!
--
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
Re: [PR] KAFKA-16861: Don't convert to group to classic if the size is larger than group max size. [kafka]
Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on PR #16163:
URL: https://github.com/apache/kafka/pull/16163#issuecomment-2143741695
@chia7712 We can merge this one with the unit test. That’s fine. I just wanted to ensure that we don’t do duplicate work on the integration tests part of 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
Re: [PR] KAFKA-16861: Don't convert to group to classic if the size is larger than group max size. [kafka]
Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on PR #16163:
URL: https://github.com/apache/kafka/pull/16163#issuecomment-2143747462
Sure. I can review it tomorrow.
--
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