mumrah commented on code in PR #15265:
URL: https://github.com/apache/kafka/pull/15265#discussion_r1497780170
##########
clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java:
##########
@@ -2129,63 +2167,183 @@ private Map<String, KafkaFuture<TopicDescription>>
handleDescribeTopicsByNames(f
}
}
final long now = time.milliseconds();
- Call call = new Call("describeTopics", calcDeadlineMs(now,
options.timeoutMs()),
- new LeastLoadedNodeProvider()) {
- private boolean supportsDisablingTopicCreation = true;
+ if (options.useDescribeTopicsApi()) {
+ RecurringCall call = new RecurringCall("DescribeTopics-Recurring",
calcDeadlineMs(now, options.timeoutMs()), runnable) {
Review Comment:
I think we can defer pipelining for now. If we do it the "dumb" way of just
loading the next page of results as we drain the iterator, it will be slower,
but it will work.
##########
clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java:
##########
@@ -2108,9 +2146,12 @@ void handleFailure(Throwable throwable) {
public DescribeTopicsResult describeTopics(final TopicCollection topics,
DescribeTopicsOptions options) {
if (topics instanceof TopicIdCollection)
return
DescribeTopicsResult.ofTopicIds(handleDescribeTopicsByIds(((TopicIdCollection)
topics).topicIds(), options));
- else if (topics instanceof TopicNameCollection)
+ else if (topics instanceof TopicNameCollection) {
+ if (options.useDescribeTopicsApi()) {
+ return DescribeTopicsResult.ofTopicNameIterator(new
DescribeTopicPartitionsIterator(((TopicNameCollection) topics).topicNames(),
options));
Review Comment:
DescribeTopicsResult is a Map based results class, which does not align the
streaming/paging approach we are trying to achieve with this new RPC. I don't
think we should try to reuse DescribeTopicsResult for the new API.
Something like
```
KafkaAdminClient#describeTopics(Consumer<TopicDescription>);
```
would be more natural for a streaming interface. With an interface like
this, we just need to be concerned with efficiently paging through the results.
The application can gather/filter/post-process the results as needed.
Since this is a new public API, and really the first time doing streaming in
KafkaAdminClient, I do think we should bring this up in the mailing list to
collect feedback. E.g., do we want to return an Iterator/Stream, or accept a
function like shown above.
--
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
