> 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.
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? - Jason ----------------------------------------------------------- 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 > >
