> On July 19, 2015, 1:11 a.m., Jason Gustafson wrote:
> >
>
> Ashish Singh wrote:
> Jason, thanks for your review! I looked into ConsumerNetworkClient/
> NetwrokClient, Metadata and Cluster classes. On receiving metadataUpdate,
> cluster instance in metadata is updated. However, when a topic is added by
> consumer, it is added to metadata.topics. After considering various options,
> I have updated the patch with what I think is the least obtrusive changes.
> So, we still keep metadata.topics as the list of topics we are interested in
> maintaining the state for, however we can choose to get metadata for all
> topics by setting metadata.needMetadataForAllTopics.
>
> One thing to notice is that in the current implementation there is no
> caching for allTopics metadata, which might be OK depending on how we are
> planning to use it. We can discuss further once you take a look at the latest
> patch.
>
> Jason Gustafson wrote:
> Hey Ashish, that makes sense and I agree that it seems less obtrusive.
> One concern I have is that we're using the same Cluster object in Metadata
> for representing both the set of all metadata and for just a subset (those
> topics that have been added through subscriptions). It seems like there might
> be potential for conflict there. Additionally I'm not sure how we'll be able
> to extend this to handle regex subscriptions. Basically we need to be able to
> "listen" for metadata changes and update our subscriptions based on any topic
> changes. We could block to get all metdata, but it's probably best if we can
> do this asynchronously. Do you have any thoughts on this?
{quote}
One concern I have is that we're using the same Cluster object in Metadata for
representing both the set of all metadata and for just a subset (those topics
that have been added through subscriptions). It seems like there might be
potential for conflict there.
{quote}
Maybe I should move the flag, indicating cluster has metadata for all topics or
subset of topics, to Cluster. Makes sense?
{quote}
Additionally I'm not sure how we'll be able to extend this to handle regex
subscriptions. Basically we need to be able to "listen" for metadata changes
and update our subscriptions based on any topic changes. We could block to get
all metdata, but it's probably best if we can do this asynchronously. Do you
have any thoughts on this?
{quote}
I do not think there is a way to directly subscribe to metadata changes as of
now. Correct me if my understanding is wrong. One would have to periodically
poll to get metadata updates. Now, the question becomes where should this
polling be done? With the current modification, the regex subscriber will have
to manage the polling logic. We can definitely push the polling logic down to
say Network client, but then the question will be is it required? Let me know
your thoughts.
- Ashish
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/36590/#review92194
-----------------------------------------------------------
On July 20, 2015, 5:44 p.m., Ashish Singh wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/36590/
> -----------------------------------------------------------
>
> (Updated July 20, 2015, 5:44 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-2275
> https://issues.apache.org/jira/browse/KAFKA-2275
>
>
> Repository: kafka
>
>
> Description
> -------
>
> Add logic to get all topics when needMetadataForAllTopics is set on metadata
>
>
> Return metadata for all topics if empty list is passed to partitionsFor
>
>
> KAFKA-2275: Add a "Map<String, List<PartitionInfo>> partitionsFor(String...
> topics)" API to the new consumer
>
>
> Diffs
> -----
>
> clients/src/main/java/org/apache/kafka/clients/Metadata.java
> 0387f2602c93a62cd333f1b3c569ca6b66b5b779
> clients/src/main/java/org/apache/kafka/clients/NetworkClient.java
> 48fe7961e2215372d8033ece4af739ea06c6457b
> clients/src/main/java/org/apache/kafka/clients/consumer/Consumer.java
> 252b759c0801f392e3526b0f31503b4b8fbf1c8a
> clients/src/main/java/org/apache/kafka/clients/consumer/KafkaConsumer.java
> bea3d737c51be77d5b5293cdd944d33b905422ba
> clients/src/main/java/org/apache/kafka/clients/consumer/MockConsumer.java
> c14eed1e95f2e682a235159a366046f00d1d90d6
>
> clients/src/main/java/org/apache/kafka/clients/consumer/internals/ConsumerNetworkClient.java
> 9517d9d0cd480d5ba1d12f1fde7963e60528d2f8
> clients/src/main/java/org/apache/kafka/common/Cluster.java
> 60594a7dce90130911a626ea80cf80d815aeb46e
> core/src/test/scala/integration/kafka/api/ConsumerTest.scala
> 3eb5f95731a3f06f662b334ab2b3d0ad7fa9e1ca
>
> Diff: https://reviews.apache.org/r/36590/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ashish Singh
>
>
