kafka icon indicating copy to clipboard operation
kafka copied to clipboard

MINOR: Fix `leader` param description in TopicPartitionInfo constructor

Open mingyen066 opened this issue 1 month ago • 1 comments

The description is inconsistent with method leader(). After checking the code, we always use pass null to TopicPartitionInfo instead of noNode The current description is inconsistent with the leader() method. After reviewing the code, we actually pass null to TopicPartitionInfo instead of using noNode() in KafkaAdminClient

        for (PartitionInfo partitionInfo : partitionInfos) {
            TopicPartitionInfo topicPartitionInfo = new
TopicPartitionInfo(
                partitionInfo.partition(), leader(partitionInfo),
Arrays.asList(partitionInfo.replicas()),
                Arrays.asList(partitionInfo.inSyncReplicas()));
            partitions.add(topicPartitionInfo);
        }

    private Node leader(PartitionInfo partitionInfo) {
        if (partitionInfo.leader() == null ||
partitionInfo.leader().id() == Node.noNode().id())
            return null;
        return partitionInfo.leader();
    }

https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java#L2440-L2454

mingyen066 avatar Dec 04 '25 15:12 mingyen066

The new param description doesn't look fully accurate either?

According to the current logic, it passes null instead of noNode. We should revisit the usage in the test code


    private Node leader(PartitionInfo partitionInfo) {
        if (partitionInfo.leader() == null || partitionInfo.leader().id() == Node.noNode().id())
            return null;
        return partitionInfo.leader();
    }

        return new TopicPartitionInfo(
            partition.partitionIndex(),
            nodes.get(partition.leaderId()),
            partition.replicaNodes().stream().map(id -> nodes.getOrDefault(id, new Node(id, "", -1))).collect(Collectors.toList()),
            partition.isrNodes().stream().map(id -> nodes.getOrDefault(id, new Node(id, "", -1))).collect(Collectors.toList()),
            partition.eligibleLeaderReplicas().stream().map(id -> nodes.getOrDefault(id, new Node(id, "", -1))).collect(Collectors.toList()),
            partition.lastKnownElr().stream().map(id -> nodes.getOrDefault(id, new Node(id, "", -1))).collect(Collectors.toList()));

chia7712 avatar Dec 08 '25 07:12 chia7712