Re: [PR] KAFKA-16579: Revert Consumer Rolling Upgrade [kafka]

2024-04-19 Thread via GitHub


lianetm commented on code in PR #15753:
URL: https://github.com/apache/kafka/pull/15753#discussion_r1572569170


##
tests/kafkatest/tests/client/consumer_rolling_upgrade_test.py:
##
@@ -56,12 +56,7 @@ def _verify_roundrobin_assignment(self, consumer):
 metadata_quorum=[quorum.isolated_kraft],
 use_new_coordinator=[True, False]
 )
-@matrix(
-metadata_quorum=quorum.all_kraft,
-use_new_coordinator=[True],
-group_protocol=consumer_group.all_group_protocols

Review Comment:
   You're right, I was only concerned about the new coordinator + classic 
combination, missed that it's coming from the default and other combination, 
all good then. Thanks! 



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

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



Re: [PR] KAFKA-16579: Revert Consumer Rolling Upgrade [kafka]

2024-04-19 Thread via GitHub


lianetm commented on code in PR #15753:
URL: https://github.com/apache/kafka/pull/15753#discussion_r1572569170


##
tests/kafkatest/tests/client/consumer_rolling_upgrade_test.py:
##
@@ -56,12 +56,7 @@ def _verify_roundrobin_assignment(self, consumer):
 metadata_quorum=[quorum.isolated_kraft],
 use_new_coordinator=[True, False]
 )
-@matrix(
-metadata_quorum=quorum.all_kraft,
-use_new_coordinator=[True],
-group_protocol=consumer_group.all_group_protocols

Review Comment:
   You're right, I was only concerned about the new coordinator + classic 
combination, missed that it's coming from the default, all good then. Thanks! 



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

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



Re: [PR] KAFKA-16579: Revert Consumer Rolling Upgrade [kafka]

2024-04-19 Thread via GitHub


lucasbru commented on code in PR #15753:
URL: https://github.com/apache/kafka/pull/15753#discussion_r1572534438


##
tests/kafkatest/tests/client/consumer_rolling_upgrade_test.py:
##
@@ -56,12 +56,7 @@ def _verify_roundrobin_assignment(self, consumer):
 metadata_quorum=[quorum.isolated_kraft],
 use_new_coordinator=[True, False]
 )
-@matrix(
-metadata_quorum=quorum.all_kraft,
-use_new_coordinator=[True],
-group_protocol=consumer_group.all_group_protocols

Review Comment:
   Hey @lianetm. My understanding was that this does test with the new 
coordinator also after the update, and the default is group_protocol=classic, 
so this should be fine right? Unless you want to add `combined_kraft`, but not 
sure if that's an interesting 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-unsubscr...@kafka.apache.org

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



Re: [PR] KAFKA-16579: Revert Consumer Rolling Upgrade [kafka]

2024-04-19 Thread via GitHub


lianetm commented on code in PR #15753:
URL: https://github.com/apache/kafka/pull/15753#discussion_r1572508072


##
tests/kafkatest/tests/client/consumer_rolling_upgrade_test.py:
##
@@ -56,12 +56,7 @@ def _verify_roundrobin_assignment(self, consumer):
 metadata_quorum=[quorum.isolated_kraft],
 use_new_coordinator=[True, False]
 )
-@matrix(
-metadata_quorum=quorum.all_kraft,
-use_new_coordinator=[True],
-group_protocol=consumer_group.all_group_protocols

Review Comment:
   Hey, sorry I'm late to this party, but important concern. Don't we want to 
keep testing this with the new coordinator and the classic protocol? I would 
say yes. I would expect we only want to exclude the consumer protocol, so we 
would want to keep the test with the params it had, but changing 
`group_protocol=consumer_group.classic`



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

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



Re: [PR] KAFKA-16579: Revert Consumer Rolling Upgrade [kafka]

2024-04-19 Thread via GitHub


lucasbru merged PR #15753:
URL: https://github.com/apache/kafka/pull/15753


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

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



Re: [PR] KAFKA-16579: Revert Consumer Rolling Upgrade [kafka]

2024-04-18 Thread via GitHub


philipnee commented on PR #15753:
URL: https://github.com/apache/kafka/pull/15753#issuecomment-2064308513

   @lucasbru - This is just to remove the consumer protocol from testing as it 
is not suited for this test. Much appreciated if you get a chance to look at 
this.


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

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



[PR] KAFKA-16579: Revert Consumer Rolling Upgrade [kafka]

2024-04-18 Thread via GitHub


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

   Consumer Rolling Upgrade is meant to test the protocol upgrade for the old 
protocol.  Therefore, I am removing old changes.
   
   ```
   

   SESSION REPORT (ALL TESTS)
   ducktape version: 0.11.4
   session_id:   2024-04-18--004
   run time: 1 minute 38.006 seconds
   tests run:3
   passed:   3
   flaky:0
   failed:   0
   ignored:  0
   

   test_id:
kafkatest.tests.client.consumer_rolling_upgrade_test.ConsumerRollingUpgradeTest.rolling_update_test.metadata_quorum=ISOLATED_KRAFT.use_new_coordinator=False
   status: PASS
   run time:   33.562 seconds
   

   test_id:
kafkatest.tests.client.consumer_rolling_upgrade_test.ConsumerRollingUpgradeTest.rolling_update_test.metadata_quorum=ISOLATED_KRAFT.use_new_coordinator=True
   status: PASS
   run time:   34.902 seconds
   

   test_id:
kafkatest.tests.client.consumer_rolling_upgrade_test.ConsumerRollingUpgradeTest.rolling_update_test.metadata_quorum=ZK.use_new_coordinator=False
   status: PASS
   run time:   29.447 seconds
   

   ```


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

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