akhileshchg commented on code in PR #12998:
URL: https://github.com/apache/kafka/pull/12998#discussion_r1050297925


##########
metadata/src/main/java/org/apache/kafka/metadata/migration/KRaftMigrationDriver.java:
##########
@@ -88,14 +83,23 @@ private void initializeMigrationState() {
     }
 
     private boolean isControllerQuorumReadyForMigration() {
-        // TODO
-        return false;
+        return true;
     }
 
     private boolean areZkBrokersReadyForMigration() {
-        // TODO: Check available broker registrations and known topic 
assignments to confirm of
-        //  all Zk brokers are registered before beginning migration.
-        return false;
+        Set<Integer> kraftRegisteredZkBrokers = 
image.cluster().brokers().values()
+            .stream()
+            .filter(BrokerRegistration::isMigratingZkBroker)
+            .map(BrokerRegistration::id)
+            .collect(Collectors.toSet());
+        Set<Integer> zkRegisteredZkBrokers = zkMigrationClient.readBrokerIds();
+        zkRegisteredZkBrokers.removeAll(kraftRegisteredZkBrokers);
+        if (zkRegisteredZkBrokers.isEmpty()) {
+            return true;
+        } else {

Review Comment:
   I forgot about the approach we decided to take. Remind me again, are we not 
looking at topic assignments at this point? I thought that is more decisive 
regarding the zkBrokers which need to register with the kraft controller.



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

Reply via email to