KAFKA-16390 add `group.coordinator.rebalance.protocols=classic,consumer` to broker configs when system tests need the new coordinator by frankvicky · Pull Request #16715 · apache/kafka

frankvicky · 2024-07-29T12:25:41Z

Jira Link
Fix an issue that cause system test failing when using AsyncKafkaConsumer.
A configuration option, group.coordinator.rebalance.protocols, was introduced to specify the rebalance protocols used by the group coordinator. By default, the rebalance protocol is set to classic. When the new group coordinator is enabled, the rebalance protocols are set to classic,consumer.

Committer Checklist (excluded from commit message)

Verify design and implementation
Verify test coverage and CI build status
Verify documentation (including upgrade notes)

…d set correct value for it

chia7712 · 2024-07-29T13:00:50Z

@frankvicky could you please attach the test result? I will verify this patch on my local later

chia7712 · 2024-07-29T13:02:50Z

@@ -782,7 +782,8 @@ def prop_file(self, node):

        if self.use_new_coordinator:
            override_configs[config_property.NEW_GROUP_COORDINATOR_ENABLE] = 'true'


maybe we can remove this config? group.coordinator.rebalance.protocols=classic,consumer has covered that.

I would keep it for now. It is nice to have the ability to set it explicitly in tests.

I would keep it for now. It is nice to have the ability to set it explicitly in tests.

that is fine to me. BTW, will we remove group.coordinator.new.enable in the future?

my point was: why we need the scenario that server uses the new coordinator but "consumer protocol" is disallowed?

Yes. We will remove group.coordinator.new.enable in the near future. No, we don't have this case at the moment. I think that we have such cases, especially in integration environments where we want to test the new coordinator without the "consumer protocol".

frankvicky · 2024-07-29T14:31:54Z

Hi @chia7712
Following is the test result on my local machine:

================================================================================
SESSION REPORT (ALL TESTS)
ducktape version: 0.11.4
session_id:       2024-07-29--006
run time:         55 minutes 55.809 seconds
tests run:        28
passed:           28
flaky:            0
failed:           0
ignored:          0
================================================================================
test_id:    kafkatest.tests.core.consume_bench_test.ConsumeBenchTest.test_consume_bench.topics=.consume_bench_topic.0-5.0-4.metadata_quorum=ISOLATED_KRAFT.use_new_coordinator=False
status:     PASS
run time:   2 minutes 5.924 seconds
--------------------------------------------------------------------------------
test_id:    kafkatest.tests.core.consume_bench_test.ConsumeBenchTest.test_consume_bench.topics=.consume_bench_topic.0-5.0-4.metadata_quorum=ISOLATED_KRAFT.use_new_coordinator=True.group_protocol=classic
status:     PASS
run time:   2 minutes 7.900 seconds
--------------------------------------------------------------------------------
test_id:    kafkatest.tests.core.consume_bench_test.ConsumeBenchTest.test_consume_bench.topics=.consume_bench_topic.0-5.0-4.metadata_quorum=ISOLATED_KRAFT.use_new_coordinator=True.group_protocol=consumer
status:     PASS
run time:   2 minutes 6.928 seconds
--------------------------------------------------------------------------------
test_id:    kafkatest.tests.core.consume_bench_test.ConsumeBenchTest.test_consume_bench.topics=.consume_bench_topic.0-5.0-4.metadata_quorum=ZK.use_new_coordinator=False
status:     PASS
run time:   2 minutes 4.668 seconds
--------------------------------------------------------------------------------
test_id:    kafkatest.tests.core.consume_bench_test.ConsumeBenchTest.test_consume_bench.topics=.consume_bench_topic.0-5.metadata_quorum=ISOLATED_KRAFT.use_new_coordinator=False
status:     PASS
run time:   2 minutes 9.259 seconds
--------------------------------------------------------------------------------
test_id:    kafkatest.tests.core.consume_bench_test.ConsumeBenchTest.test_consume_bench.topics=.consume_bench_topic.0-5.metadata_quorum=ISOLATED_KRAFT.use_new_coordinator=True.group_protocol=classic
status:     PASS
run time:   2 minutes 7.732 seconds
--------------------------------------------------------------------------------
test_id:    kafkatest.tests.core.consume_bench_test.ConsumeBenchTest.test_consume_bench.topics=.consume_bench_topic.0-5.metadata_quorum=ISOLATED_KRAFT.use_new_coordinator=True.group_protocol=consumer
status:     PASS
run time:   2 minutes 8.855 seconds
--------------------------------------------------------------------------------
test_id:    kafkatest.tests.core.consume_bench_test.ConsumeBenchTest.test_consume_bench.topics=.consume_bench_topic.0-5.metadata_quorum=ZK.use_new_coordinator=False
status:     PASS
run time:   2 minutes 6.395 seconds
--------------------------------------------------------------------------------
test_id:    kafkatest.tests.core.consume_bench_test.ConsumeBenchTest.test_multiple_consumers_random_group_partitions.metadata_quorum=ISOLATED_KRAFT.use_new_coordinator=False
status:     PASS
run time:   2 minutes 17.896 seconds
--------------------------------------------------------------------------------
test_id:    kafkatest.tests.core.consume_bench_test.ConsumeBenchTest.test_multiple_consumers_random_group_partitions.metadata_quorum=ISOLATED_KRAFT.use_new_coordinator=True.group_protocol=classic
status:     PASS
run time:   2 minutes 18.293 seconds
--------------------------------------------------------------------------------
test_id:    kafkatest.tests.core.consume_bench_test.ConsumeBenchTest.test_multiple_consumers_random_group_partitions.metadata_quorum=ISOLATED_KRAFT.use_new_coordinator=True.group_protocol=consumer
status:     PASS
run time:   2 minutes 18.186 seconds
--------------------------------------------------------------------------------
test_id:    kafkatest.tests.core.consume_bench_test.ConsumeBenchTest.test_multiple_consumers_random_group_partitions.metadata_quorum=ZK.use_new_coordinator=False
status:     PASS
run time:   2 minutes 18.477 seconds
--------------------------------------------------------------------------------
test_id:    kafkatest.tests.core.consume_bench_test.ConsumeBenchTest.test_multiple_consumers_random_group_topics.metadata_quorum=ISOLATED_KRAFT.use_new_coordinator=False
status:     PASS
run time:   1 minute 59.997 seconds
--------------------------------------------------------------------------------
test_id:    kafkatest.tests.core.consume_bench_test.ConsumeBenchTest.test_multiple_consumers_random_group_topics.metadata_quorum=ISOLATED_KRAFT.use_new_coordinator=True.group_protocol=classic
status:     PASS
run time:   2 minutes 4.979 seconds
--------------------------------------------------------------------------------
test_id:    kafkatest.tests.core.consume_bench_test.ConsumeBenchTest.test_multiple_consumers_random_group_topics.metadata_quorum=ISOLATED_KRAFT.use_new_coordinator=True.group_protocol=consumer
status:     PASS
run time:   2 minutes 1.873 seconds
--------------------------------------------------------------------------------
test_id:    kafkatest.tests.core.consume_bench_test.ConsumeBenchTest.test_multiple_consumers_random_group_topics.metadata_quorum=ZK.use_new_coordinator=False
status:     PASS
run time:   1 minute 59.512 seconds
--------------------------------------------------------------------------------
test_id:    kafkatest.tests.core.consume_bench_test.ConsumeBenchTest.test_multiple_consumers_specified_group_partitions_should_raise.metadata_quorum=ISOLATED_KRAFT.use_new_coordinator=False
status:     PASS
run time:   1 minute 19.657 seconds
--------------------------------------------------------------------------------
test_id:    kafkatest.tests.core.consume_bench_test.ConsumeBenchTest.test_multiple_consumers_specified_group_partitions_should_raise.metadata_quorum=ISOLATED_KRAFT.use_new_coordinator=True.group_protocol=classic
status:     PASS
run time:   1 minute 18.882 seconds
--------------------------------------------------------------------------------
test_id:    kafkatest.tests.core.consume_bench_test.ConsumeBenchTest.test_multiple_consumers_specified_group_partitions_should_raise.metadata_quorum=ISOLATED_KRAFT.use_new_coordinator=True.group_protocol=consumer
status:     PASS
run time:   1 minute 15.674 seconds
--------------------------------------------------------------------------------
test_id:    kafkatest.tests.core.consume_bench_test.ConsumeBenchTest.test_multiple_consumers_specified_group_partitions_should_raise.metadata_quorum=ZK.use_new_coordinator=False
status:     PASS
run time:   1 minute 13.861 seconds
--------------------------------------------------------------------------------
test_id:    kafkatest.tests.core.consume_bench_test.ConsumeBenchTest.test_single_partition.metadata_quorum=ISOLATED_KRAFT.use_new_coordinator=False
status:     PASS
run time:   2 minutes 1.614 seconds
--------------------------------------------------------------------------------
test_id:    kafkatest.tests.core.consume_bench_test.ConsumeBenchTest.test_single_partition.metadata_quorum=ISOLATED_KRAFT.use_new_coordinator=True.group_protocol=classic
status:     PASS
run time:   2 minutes 1.605 seconds
--------------------------------------------------------------------------------
test_id:    kafkatest.tests.core.consume_bench_test.ConsumeBenchTest.test_single_partition.metadata_quorum=ISOLATED_KRAFT.use_new_coordinator=True.group_protocol=consumer
status:     PASS
run time:   1 minute 57.558 seconds
--------------------------------------------------------------------------------
test_id:    kafkatest.tests.core.consume_bench_test.ConsumeBenchTest.test_single_partition.metadata_quorum=ZK.use_new_coordinator=False
status:     PASS
run time:   1 minute 59.035 seconds
--------------------------------------------------------------------------------
test_id:    kafkatest.tests.core.consume_bench_test.ConsumeBenchTest.test_two_consumers_specified_group_topics.metadata_quorum=ISOLATED_KRAFT.use_new_coordinator=False
status:     PASS
run time:   2 minutes 9.197 seconds
--------------------------------------------------------------------------------
test_id:    kafkatest.tests.core.consume_bench_test.ConsumeBenchTest.test_two_consumers_specified_group_topics.metadata_quorum=ISOLATED_KRAFT.use_new_coordinator=True.group_protocol=classic
status:     PASS
run time:   2 minutes 7.757 seconds
--------------------------------------------------------------------------------
test_id:    kafkatest.tests.core.consume_bench_test.ConsumeBenchTest.test_two_consumers_specified_group_topics.metadata_quorum=ISOLATED_KRAFT.use_new_coordinator=True.group_protocol=consumer
status:     PASS
run time:   2 minutes 8.797 seconds
--------------------------------------------------------------------------------
test_id:    kafkatest.tests.core.consume_bench_test.ConsumeBenchTest.test_two_consumers_specified_group_topics.metadata_quorum=ZK.use_new_coordinator=False
status:     PASS
run time:   2 minutes 3.356 seconds
--------------------------------------------------------------------------------

lianetm · 2024-07-29T16:01:03Z

        if self.use_new_coordinator:
            override_configs[config_property.NEW_GROUP_COORDINATOR_ENABLE] = 'true'
-
+            override_configs[config_property.GROUP_COORDINATOR_REBALANCE_PROTOCOLS_CONFIG] = 'classic,consumer'


just to double check, here we're adding both as a default, but still respecting any specific value that a test may have right? (I guess the following logic on ln 788 ensures that?)

Yes, you're right, the logic respects specific test values while adding necessary defaults.

kirktrue

Thanks for the PR, @frankvicky!

This seems like a partial revert of the changes from #16482. If @dajac is happy with it, then I am as well 😄

Thanks!

chia7712 · 2024-07-30T16:42:49Z

This seems like a partial revert of the changes from #16482. If @dajac is happy with it, then I am as well 😄

yep, this PR reverts a part of #16482. For another, it seems adding the new coordinator configs to kraft_broker_configs is more straightforward. Maybe we can follow the style of #16482? @dajac @frankvicky WDYT?

frankvicky · 2024-07-31T06:02:45Z

This seems like a partial revert of the changes from #16482. If @dajac is happy with it, then I am as well 😄

yep, this PR reverts a part of #16482. For another, it seems adding the new coordinator configs to kraft_broker_configs is more straightforward. Maybe we can follow the style of #16482? @dajac @frankvicky WDYT?

I'm sold. 😺

chia7712 · 2024-08-01T16:09:55Z

@frankvicky any updates? I feel this PR need to be backport to 3.9 to make e2e stable. @cmccabe FYI

frankvicky · 2024-08-01T16:43:45Z

Hi @chia7712
I will update it ASAP.

jolshan · 2024-08-02T14:49:26Z

Can we update the title and/or description. There are many more tests that are failing (and hopefully should be fixed) by this change. More specifically any test with use_new_coordinator=True Thanks for the fix!

chia7712

LGTM. run consume_bench_test.py on my local. it works.

lianetm

Changes LGTM. Running on my local still but some already passed. Since it passes for @chia7712 too LGTM.

jolshan · 2024-08-02T23:17:59Z

I will also cherrypick to 3.9

…mer` to broker configs when system tests need the new coordinator (#16715) Fix an issue that cause system test failing when using AsyncKafkaConsumer. A configuration option, group.coordinator.rebalance.protocols, was introduced to specify the rebalance protocols used by the group coordinator. By default, the rebalance protocol is set to classic. When the new group coordinator is enabled, the rebalance protocols are set to classic,consumer. Reviewers: Chia-Ping Tsai <chia7712@gmail.com>, David Jacot <djacot@confluent.io>, Lianet Magrans <lianetmr@gmail.com>, Kirk True <kirk@kirktrue.pro>, Justine Olshan <jolshan@confluent.io>

chia7712 · 2024-08-03T01:57:20Z

Running on my local still but some already passed.

hi @lianetm, do you mean "consume_bench_test.py" still has some failed tests on your local? I run the test only once 😅 so maybe there are others flaky ...

KAFKA-16390: Add group.coordinator.rebalance.protocols into config an…

b4aeeef

…d set correct value for it

chia7712 reviewed Jul 29, 2024

View reviewed changes

lianetm reviewed Jul 29, 2024

View reviewed changes

kirktrue approved these changes Jul 29, 2024

View reviewed changes

kirktrue added ctr Consumer Threading Refactor (KIP-848) consumer tests Test fixes (including flaky tests) KIP-848 The Next Generation of the Consumer Rebalance Protocol labels Jul 29, 2024

KAFKA-16390: Configure props in kraft_broker_configs

4af1a91

chia7712 approved these changes Aug 2, 2024

View reviewed changes

chia7712 changed the title ~~KAFKA-16390: consume_bench_test.py failed using AsyncKafkaConsumer~~ KAFKA-16390 add group.coordinator.rebalance.protocols=classic,consumer to broker configs when E2E needs the new coordinator Aug 2, 2024

lianetm approved these changes Aug 2, 2024

View reviewed changes

jolshan approved these changes Aug 2, 2024

View reviewed changes

jolshan merged commit da14b5a into apache:trunk Aug 2, 2024

		@@ -782,7 +782,8 @@ def prop_file(self, node):

		if self.use_new_coordinator:
		override_configs[config_property.NEW_GROUP_COORDINATOR_ENABLE] = 'true'

Conversation

frankvicky commented Jul 29, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Committer Checklist (excluded from commit message)

Uh oh!

chia7712 commented Jul 29, 2024

Uh oh!

chia7712 Jul 29, 2024

Choose a reason for hiding this comment

Uh oh!

dajac Jul 29, 2024

Choose a reason for hiding this comment

Uh oh!

chia7712 Jul 29, 2024

Choose a reason for hiding this comment

Uh oh!

chia7712 Jul 29, 2024

Choose a reason for hiding this comment

Uh oh!

dajac Jul 29, 2024

Choose a reason for hiding this comment

Uh oh!

frankvicky commented Jul 29, 2024

Uh oh!

lianetm Jul 29, 2024

Choose a reason for hiding this comment

Uh oh!

frankvicky Jul 30, 2024

Choose a reason for hiding this comment

Uh oh!

kirktrue left a comment

Choose a reason for hiding this comment

Uh oh!

chia7712 commented Jul 30, 2024

Uh oh!

frankvicky commented Jul 31, 2024

Uh oh!

chia7712 commented Aug 1, 2024

Uh oh!

frankvicky commented Aug 1, 2024

Uh oh!

jolshan commented Aug 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chia7712 left a comment

Choose a reason for hiding this comment

Uh oh!

lianetm left a comment

Choose a reason for hiding this comment

Uh oh!

jolshan commented Aug 2, 2024

Uh oh!

chia7712 commented Aug 3, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

frankvicky commented Jul 29, 2024 •

edited

Loading

jolshan commented Aug 2, 2024 •

edited

Loading