[GitHub] [kafka] cmccabe commented on pull request #11969: KAFKA-13649: Implement early.start.listeners and fix StandardAuthorizer loading

[GitBox <gi...@apache.org>]

cmccabe commented on PR #11969:
URL: https://github.com/apache/kafka/pull/11969#issuecomment-1119953102

   I made two changes just now:
   
   1. Fixed it so TenantPartitionAssignor ignores fenced brokers, and added a unit test for this (as we discussed offline)
   
   2. Removed ClusterMetadataTest#testNodeReplicaCountsForExistingTenantTopicWithInvalidAssignment and replaced it with  ClusterMetadataTest#testNodeReplicaCountsForUnregisteredBrokers. Basically, the assumptions of the old test were invalid (it is actually possible to have partitions placed on brokers that are no longer in the cluster).
   
   Looking at the code, I see that it already took this into account when computing the nodeMetadata:
   
   ```
       String tenantPrefix = tenant + TenantContext.DELIMITER;
       for (Iterator<String> iterator = cluster.topicNames(); iterator.hasNext();) {
         String topic = iterator.next();
         boolean isTenantTopic = topic.startsWith(tenantPrefix);
         for (List<Integer> replicas : cluster.replicasForTopicName(topic)) {
           for (int i = 0; i < replicas.size(); i++) {
             Integer replica = replicas.get(i);
             NodeMetadata nodeMetadata = nodeMetadatas.computeIfAbsent(replica, node -> new NodeMetadata());
             nodeMetadata.incrementReplicas(i == 0, isTenantTopic);
           }
         }
       }
   ```
   
   So the code was already handling this case smoothly, which is what we want. But the very artificial unit test (injecting a null into the replicas array, which wouldn't happen in a real Cluster object) was confusing the issue. Hopefully this clears up the issue.


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