You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "aschmidt402 (via GitHub)" <gi...@apache.org> on 2026/09/02 17:28:09 UTC

[PR] KAFKA-20960 - Block ZK-to-KRaft migration finalization until brokers re-register [kafka]

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

   
   Added a controller-side check to confirm that brokers have re-registered with KRaft before finalizing ZK migration. If brokers have not re-registered, then the migration is blocked and the offending brokers are listed.
   
   Also added `testMigrationNotFinalizedWhileBrokersStillRegisteredAsZk()`, which tests and confirms that migration is blocked when brokers have failed to re-register with KRaft
   


-- 
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-20960 - Block ZK-to-KRaft migration finalization until brokers re-register [kafka]

Posted by "aschmidt402 (via GitHub)" <gi...@apache.org>.
aschmidt402 commented on PR #23338:
URL: https://github.com/apache/kafka/pull/23338#issuecomment-5618078388

   <img width="2258" height="1532" alt="image" src="https://github.com/user-attachments/assets/a040989b-87d7-4be9-86c9-6ed9ce81677b" />
   


-- 
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-20960 - Block ZK-to-KRaft migration finalization until brokers re-register [kafka]

Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on code in PR #23338:
URL: https://github.com/apache/kafka/pull/23338#discussion_r3980332938


##########
metadata/src/main/java/org/apache/kafka/controller/QuorumController.java:
##########
@@ -1240,11 +1243,16 @@ class CompleteActivationEvent implements ControllerWriteOperation<Void> {
         @Override
         public ControllerResult<Void> generateRecordsAndResult() {
             try {
+                Set<Integer> stillZkRegisteredBrokerIds = clusterControl.brokerRegistrations().values().stream()
+                    .filter(BrokerRegistration::isMigratingZkBroker)
+                    .map(BrokerRegistration::id)
+                    .collect(Collectors.toCollection(TreeSet::new));

Review Comment:
   I am guessing you are putting these into a TreeSet so they will appear in order in the log message?
   
   If that's the case, maybe name this variable `sortedStillZkRegisteredBrokerIds` or similar. 



##########
metadata/src/main/java/org/apache/kafka/controller/ActivationRecordsGenerator.java:
##########
@@ -182,10 +184,17 @@ static ControllerResult<Void> recordsForNonEmptyLog(
                         // This can happen if controller leadership transfers to a controller with migrations enabled
                         // after another controller had finalized the migration. For example, during a rolling restart
                         // of the controller quorum during which the migration config is being set to false.
-                        logMessageBuilder
-                            .append("Completing the ZK migration since this controller was configured with ")
-                            .append("'zookeeper.metadata.migration.enable' set to 'false'. ");
-                        records.add(ZkMigrationState.POST_MIGRATION.toRecord());
+                        if (!stillZkRegisteredBrokerIds.isEmpty()) {
+                            logMessageBuilder
+                                .append("Cannot complete ZK migration because the following broker(s) are still registered as ZK brokers: ")

Review Comment:
   For the actual error message, we should indicate what the problem is (like you've done) but also how to fix it.
   
   ```
   Staying in ZK migration mode even though 'zookeeper.metadata.migration.enable' set to 'false' because the following broker(s) are still registered as ZK brokers: [0, 1]. These brokers must be migrated to KRaft before the controller can finalize the migration.
   ```
   



-- 
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-20960 - Block ZK-to-KRaft migration finalization until brokers re-register [kafka]

Posted by "aschmidt402 (via GitHub)" <gi...@apache.org>.
aschmidt402 commented on PR #23338:
URL: https://github.com/apache/kafka/pull/23338#issuecomment-6196357543

   > Thanks for the patch @aschmidt402! I agree that this is a useful guard. One thing to note is that there is no guarantee that we will make another 3.9.x release. This improvement may never actually get released.
   > 
   > For the actual change, WDYT about logging vs throwing in this scenario? One concern with just logging during activation is that a user may not realize anything has gone wrong and just continue on forever in the MIGRATION state with ZK brokers.
   
   I chose this approach because I was worried that there might be downstream issues with throwing, or cases where having the migration fail would be more cumbersome than letting it go through and then needing to restart brokers.


-- 
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-20960 - Block ZK-to-KRaft migration finalization until brokers re-register [kafka]

Posted by "kevin-wu24 (via GitHub)" <gi...@apache.org>.
kevin-wu24 commented on code in PR #23338:
URL: https://github.com/apache/kafka/pull/23338#discussion_r3917806513


##########
metadata/src/main/java/org/apache/kafka/controller/ActivationRecordsGenerator.java:
##########
@@ -182,10 +185,17 @@ static ControllerResult<Void> recordsForNonEmptyLog(
                         // This can happen if controller leadership transfers to a controller with migrations enabled
                         // after another controller had finalized the migration. For example, during a rolling restart
                         // of the controller quorum during which the migration config is being set to false.
-                        logMessageBuilder
-                            .append("Completing the ZK migration since this controller was configured with ")
-                            .append("'zookeeper.metadata.migration.enable' set to 'false'. ");
-                        records.add(ZkMigrationState.POST_MIGRATION.toRecord());
+                        if (!stillZkRegisteredBrokerIds.isEmpty()) {
+                            logMessageBuilder
+                                .append("Cannot complete ZK migration because the following broker(s) are still registered as ZK brokers: ")
+                                .append(new TreeSet<>(stillZkRegisteredBrokerIds))

Review Comment:
   What about the following?
   ```suggestion
                                   .append(stillZkRegisteredBrokerIds)
   ```
   



##########
metadata/src/main/java/org/apache/kafka/controller/ActivationRecordsGenerator.java:
##########
@@ -182,10 +185,17 @@ static ControllerResult<Void> recordsForNonEmptyLog(
                         // This can happen if controller leadership transfers to a controller with migrations enabled
                         // after another controller had finalized the migration. For example, during a rolling restart
                         // of the controller quorum during which the migration config is being set to false.
-                        logMessageBuilder
-                            .append("Completing the ZK migration since this controller was configured with ")
-                            .append("'zookeeper.metadata.migration.enable' set to 'false'. ");
-                        records.add(ZkMigrationState.POST_MIGRATION.toRecord());
+                        if (!stillZkRegisteredBrokerIds.isEmpty()) {
+                            logMessageBuilder
+                                .append("Cannot complete ZK migration because the following broker(s) are still registered as ZK brokers: ")
+                                .append(new TreeSet<>(stillZkRegisteredBrokerIds))
+                                .append(". Restart these brokers in KRaft mode before finalizing the migration. ");

Review Comment:
   ```suggestion
                                   .append(". Restart these brokers in KRaft mode before restarting the controllers to finalize the migration. ");
   ```



-- 
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-20960 - Block ZK-to-KRaft migration finalization until brokers re-register [kafka]

Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah merged PR #23338:
URL: https://github.com/apache/kafka/pull/23338


-- 
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