hachikuji commented on code in PR #15323:
URL: https://github.com/apache/kafka/pull/15323#discussion_r1480620848
##########
clients/src/main/java/org/apache/kafka/clients/producer/internals/RecordAccumulator.java:
##########
@@ -647,27 +647,27 @@ private long batchReady(boolean exhausted, TopicPartition
part, Node leader,
}
/**
- * Iterate over partitions to see which one have batches ready and collect
leaders of those partitions
- * into the set of ready nodes. If partition has no leader, add the topic
to the set of topics with
- * no leader. This function also calculates stats for adaptive
partitioning.
+ * Iterate over partitions to see which one have batches ready and collect
leaders of those
+ * partitions into the set of ready nodes. If partition has no leader,
add the topic to the set
+ * of topics with no leader. This function also calculates stats for
adaptive partitioning.
*
- * @param metadata The cluster metadata
- * @param nowMs The current time
- * @param topic The topic
- * @param topicInfo The topic info
+ * @param cluster The cluster metadata
+ * @param nowMs The current time
+ * @param topic The topic
+ * @param topicInfo The topic info
* @param nextReadyCheckDelayMs The delay for next check
- * @param readyNodes The set of ready nodes (to be filled in)
- * @param unknownLeaderTopics The set of topics with no leader (to be
filled in)
+ * @param readyNodes The set of ready nodes (to be filled in)
+ * @param unknownLeaderTopics The set of topics with no leader (to be
filled in)
* @return The delay for next check
*/
- private long partitionReady(Metadata metadata, long nowMs, String topic,
+ private long partitionReady(Cluster cluster, long nowMs, String topic,
Review Comment:
In some ways, this is a step backwards. We have been trying to reduce the
reliance on `Cluster` internally because it is public. With a lot of internal
usage, we end up making changes to the API which are only needed for the
internal implementation (as we are doing in this PR). Have you considered
alternatives? Perhaps we could expose something like `Cluster`, but with a
reduced scope?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
