Re: [PR] KAFKA-16463 System test for reverting migration to ZK [kafka]

2024-05-31 Thread via GitHub 


mimaison commented on PR #15754:
URL: https://github.com/apache/kafka/pull/15754#issuecomment-2142163784

   @mumrah @soarez Can we try to get this into 3.8? Otherwise we may just drop 
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

Re: [PR] KAFKA-16463 System test for reverting migration to ZK [kafka]

2024-04-24 Thread via GitHub 


mumrah commented on code in PR #15754:
URL: https://github.com/apache/kafka/pull/15754#discussion_r1577998958


##
tests/kafkatest/tests/core/zookeeper_migration_test.py:
##
@@ -86,10 +87,35 @@ def do_migration(self, roll_controller = False, 
downgrade_to_zk = False):
 controller.stop_node(node)
 controller.start_node(node)
 
+if downgrade_to_zk:
+self.logger.info("Shutdown brokers to avoid waiting on unclean 
shutdown")
+for node in self.kafka.nodes:
+self.kafka.stop_node(node)
+metadata_log_dir = KafkaService.METADATA_LOG_DIR + 
"/__cluster_metadata-0"
+for node in self.kafka.nodes:
+assert path_exists(node, metadata_log_dir), "Should still 
have a metadata log on the brokers."
+
+self.logger.info("Shutdown KRaft quorum")
+for node in controller.nodes:
+controller.stop_node(node)
+
+self.logger.info("Deleting controller ZNode")
+self.zk.delete(path="/controller", recursive=True)
+
+self.logger.info("Rolling brokers back to ZK mode")
+self.kafka.downgrade_kraft_broker_to_zk(controller)
+for node in self.kafka.nodes:
+self.kafka.start_node(node)
+
+# This blocks until all brokers have a full ISR
+self.wait_until_rejoin()

Review Comment:
   No, it doesn't require a full shutdown. Let me see if I can fix this here. 
Good catch



-- 
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-16463 System test for reverting migration to ZK [kafka]

2024-04-24 Thread via GitHub 


mumrah commented on code in PR #15754:
URL: https://github.com/apache/kafka/pull/15754#discussion_r1577997204


##
tests/kafkatest/services/kafka/kafka.py:
##
@@ -463,6 +463,18 @@ def reconfigure_zk_for_migration(self, kraft_quorum):
 # This is not added to "advertised.listeners" because of 
configured_for_zk_migration=True
 self.port_mappings[kraft_quorum.controller_listener_names] = 
kraft_quorum.port_mappings.get(kraft_quorum.controller_listener_names)
 
+def downgrade_kraft_broker_to_zk(self, kraft_quorum):
+self.configured_for_zk_migration = True
+self.quorum_info = quorum.ServiceQuorumInfo(quorum.zk, self)
+self.controller_quorum = kraft_quorum
+
+# Set the migration properties
+self.server_prop_overrides.extend([
+["zookeeper.metadata.migration.enable", "true"],
+["controller.quorum.voters", 
kraft_quorum.controller_quorum_voters],
+["controller.listener.names", 
kraft_quorum.controller_listener_names]
+])

Review Comment:
   Well, as long as `process.roles` is not set, a broker is in ZK mode. When 
reverting back to ZK mode, we can still have the migration configs set. Since 
the behavior added in KAFKA-16463 is only enabled when 
`zookeeper.metadata.migration.enable` is set, we include it in this test



-- 
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-16463 System test for reverting migration to ZK [kafka]

2024-04-22 Thread via GitHub 


soarez commented on code in PR #15754:
URL: https://github.com/apache/kafka/pull/15754#discussion_r1574994375


##
tests/kafkatest/tests/core/zookeeper_migration_test.py:
##
@@ -86,10 +87,35 @@ def do_migration(self, roll_controller = False, 
downgrade_to_zk = False):
 controller.stop_node(node)
 controller.start_node(node)
 
+if downgrade_to_zk:
+self.logger.info("Shutdown brokers to avoid waiting on unclean 
shutdown")
+for node in self.kafka.nodes:
+self.kafka.stop_node(node)
+metadata_log_dir = KafkaService.METADATA_LOG_DIR + 
"/__cluster_metadata-0"
+for node in self.kafka.nodes:
+assert path_exists(node, metadata_log_dir), "Should still 
have a metadata log on the brokers."
+
+self.logger.info("Shutdown KRaft quorum")
+for node in controller.nodes:
+controller.stop_node(node)
+
+self.logger.info("Deleting controller ZNode")
+self.zk.delete(path="/controller", recursive=True)
+
+self.logger.info("Rolling brokers back to ZK mode")
+self.kafka.downgrade_kraft_broker_to_zk(controller)
+for node in self.kafka.nodes:
+self.kafka.start_node(node)
+
+# This blocks until all brokers have a full ISR
+self.wait_until_rejoin()

Review Comment:
   The test name is `test_online_migration` but we're shutting every broker 
down to apply the downgrade, it seems like a contradiction? Does the downgrade 
require a full shutdown of the cluster?



##
tests/kafkatest/services/kafka/kafka.py:
##
@@ -463,6 +463,18 @@ def reconfigure_zk_for_migration(self, kraft_quorum):
 # This is not added to "advertised.listeners" because of 
configured_for_zk_migration=True
 self.port_mappings[kraft_quorum.controller_listener_names] = 
kraft_quorum.port_mappings.get(kraft_quorum.controller_listener_names)
 
+def downgrade_kraft_broker_to_zk(self, kraft_quorum):
+self.configured_for_zk_migration = True
+self.quorum_info = quorum.ServiceQuorumInfo(quorum.zk, self)
+self.controller_quorum = kraft_quorum
+
+# Set the migration properties
+self.server_prop_overrides.extend([
+["zookeeper.metadata.migration.enable", "true"],
+["controller.quorum.voters", 
kraft_quorum.controller_quorum_voters],
+["controller.listener.names", 
kraft_quorum.controller_listener_names]
+])

Review Comment:
   In https://kafka.apache.org/documentation/#kraft_zk_migration under 
"Reverting to ZooKeeper mode During the Migration" we say:
   
   > On each broker, remove the zookeeper.metadata.migration.enable, 
controller.listener.names, and controller.quorum.voters configurations.
   
   And before this method is called we log `"Rolling brokers back to ZK mode"`.
   
   I'm guessing I'm probably missing something, shouldn't these three 
properties be removed?



-- 
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-16463 System test for reverting migration to ZK [kafka]

2024-04-19 Thread via GitHub 


mumrah commented on PR #15754:
URL: https://github.com/apache/kafka/pull/15754#issuecomment-2066454711

   ```
   

   SESSION REPORT (ALL TESTS)
   ducktape version: 0.11.4
   session_id:   2024-04-18--017
   run time: 26 minutes 43.661 seconds
   tests run:8
   passed:   8
   flaky:0
   failed:   0
   ignored:  0
   

   test_id:
kafkatest.tests.core.zookeeper_migration_test.TestMigration.test_online_migration.roll_controller=False.downgrade_to_zk=False
   status: PASS
   run time:   2 minutes 54.546 seconds
   

   test_id:
kafkatest.tests.core.zookeeper_migration_test.TestMigration.test_online_migration.roll_controller=False.downgrade_to_zk=True
   status: PASS
   run time:   3 minutes 32.044 seconds
   

   test_id:
kafkatest.tests.core.zookeeper_migration_test.TestMigration.test_online_migration.roll_controller=True.downgrade_to_zk=False
   status: PASS
   run time:   2 minutes 56.290 seconds
   

   test_id:
kafkatest.tests.core.zookeeper_migration_test.TestMigration.test_online_migration.roll_controller=True.downgrade_to_zk=True
   status: PASS
   run time:   3 minutes 38.101 seconds
   

   test_id:
kafkatest.tests.core.zookeeper_migration_test.TestMigration.test_online_migration.roll_controller=False.downgrade_to_zk=False
   status: PASS
   run time:   2 minutes 52.691 seconds
   

   test_id:
kafkatest.tests.core.zookeeper_migration_test.TestMigration.test_online_migration.roll_controller=False.downgrade_to_zk=True
   status: PASS
   run time:   3 minutes 56.394 seconds
   

   test_id:
kafkatest.tests.core.zookeeper_migration_test.TestMigration.test_online_migration.roll_controller=True.downgrade_to_zk=False
   status: PASS
   run time:   3 minutes 8.253 seconds
   

   test_id:
kafkatest.tests.core.zookeeper_migration_test.TestMigration.test_online_migration.roll_controller=True.downgrade_to_zk=True
   status: PASS
   run time:   3 minutes 44.203 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