dajac commented on code in PR #14524:
URL: https://github.com/apache/kafka/pull/14524#discussion_r1354878772


##########
tests/kafkatest/services/kafka/kafka.py:
##########
@@ -764,6 +769,9 @@ def prop_file(self, node):
             else:
                 override_configs[config_property.ZOOKEEPER_SSL_CLIENT_ENABLE] 
= 'false'
 
+        if self.use_new_coordinator:
+            override_configs[config_property.NEW_GROUP_COORDINATOR_ENABLE] = 
'true'

Review Comment:
   I don't see where we update the kafka config based on this flag. Is it 
missing?



##########
tests/kafkatest/tests/core/consume_bench_test.py:
##########
@@ -114,7 +126,11 @@ def test_single_partition(self, metadata_quorum=quorum.zk):
         self.logger.info("TASKS: %s\n" % json.dumps(tasks, sort_keys=True, 
indent=2))
 
     @cluster(num_nodes=10)
-    @matrix(metadata_quorum=quorum.all_non_upgrade)
+    @matrix(
+        metadata_quorum=quorum.all_non_upgrade,
+        se_new_coordinator=[True, False]
+    )
+    @skip_if_new_coordinator_and_zk
     def test_multiple_consumers_random_group_topics(self, 
metadata_quorum=quorum.zk):

Review Comment:
   Is having `use_new_coordinator` as a param optional?



##########
tests/kafkatest/tests/core/consume_bench_test.py:
##########
@@ -68,12 +67,21 @@ def produce_messages(self, topics, max_messages=10000):
         self.logger.debug("Produce workload finished")
 
     @cluster(num_nodes=10)
-    @matrix(topics=[["consume_bench_topic[0-5]"]], 
metadata_quorum=quorum.all_non_upgrade) # topic subscription
-    @matrix(topics=[["consume_bench_topic[0-5]:[0-4]"]], 
metadata_quorum=quorum.all_non_upgrade)  # manual topic assignment
-    def test_consume_bench(self, topics, metadata_quorum=quorum.zk):
+    @matrix(
+        topics=[
+            ["consume_bench_topic[0-5]"],
+            ["consume_bench_topic[0-5]:[0-4]"]
+        ],

Review Comment:
   Is this equivalent to the previous one? Let's keep the comments.



##########
tests/kafkatest/tests/core/consume_bench_test.py:
##########
@@ -68,12 +67,21 @@ def produce_messages(self, topics, max_messages=10000):
         self.logger.debug("Produce workload finished")
 
     @cluster(num_nodes=10)
-    @matrix(topics=[["consume_bench_topic[0-5]"]], 
metadata_quorum=quorum.all_non_upgrade) # topic subscription
-    @matrix(topics=[["consume_bench_topic[0-5]:[0-4]"]], 
metadata_quorum=quorum.all_non_upgrade)  # manual topic assignment
-    def test_consume_bench(self, topics, metadata_quorum=quorum.zk):
+    @matrix(
+        topics=[
+            ["consume_bench_topic[0-5]"],
+            ["consume_bench_topic[0-5]:[0-4]"]
+        ],
+        metadata_quorum=quorum.all_non_upgrade,
+        use_new_coordinator=[True, False]
+    ) # topic subscription
+    @skip_if_new_coordinator_and_zk
+    def test_consume_bench(self, topics, metadata_quorum=quorum.zk, 
use_new_coordinator=False):
         """
         Runs a ConsumeBench workload to consume messages
         """
+        self.kafka.use_new_coordinator = use_new_coordinator

Review Comment:
   I see that you only do this here, why that?



##########
tests/kafkatest/tests/core/consume_bench_test.py:
##########
@@ -114,7 +126,11 @@ def test_single_partition(self, metadata_quorum=quorum.zk):
         self.logger.info("TASKS: %s\n" % json.dumps(tasks, sort_keys=True, 
indent=2))
 
     @cluster(num_nodes=10)
-    @matrix(metadata_quorum=quorum.all_non_upgrade)
+    @matrix(
+        metadata_quorum=quorum.all_non_upgrade,
+        se_new_coordinator=[True, False]

Review Comment:
   typo. same for the others.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to