----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/23655/#review48165 -----------------------------------------------------------
core/src/main/scala/kafka/consumer/ConsumerConfig.scala <https://reviews.apache.org/r/23655/#comment84474> In the new consumer config, we call this partition.assignment.strategy. It would be good if we make them consistent. core/src/main/scala/kafka/consumer/PartitionAllocator.scala <https://reviews.apache.org/r/23655/#comment84475> License header. core/src/main/scala/kafka/consumer/PartitionAllocator.scala <https://reviews.apache.org/r/23655/#comment84946> Could we make this api a bit more general so that we can reuse it in the new consumer? I was thinking of sth like def allocate(partitions: List[TopicPartition], consumers: List[ConsumerItem]) case class ConsumerItem(consumerId: String, subscribedTopics: List[String]) core/src/main/scala/kafka/consumer/PartitionAllocator.scala <https://reviews.apache.org/r/23655/#comment84948> Not sure if we need both Symmetric and round-robin, if we order the list of consumers by sth like their hashcode. It would also be useful to add an example in the comment that illustrates how the assignment works. core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala <https://reviews.apache.org/r/23655/#comment84949> Perhaps we can keep the name and the corresponding allocation instance in sth like a PartitionAllocatorFactory? core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala <https://reviews.apache.org/r/23655/#comment84950> Do we need this metric? Would the ConsumerOffsetChecker be suffice? core/src/test/scala/unit/kafka/consumer/PartitionAllocatorTest.scala <https://reviews.apache.org/r/23655/#comment84953> License header. - Jun Rao On July 18, 2014, 10:57 p.m., Joel Koshy wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/23655/ > ----------------------------------------------------------- > > (Updated July 18, 2014, 10:57 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-687 > https://issues.apache.org/jira/browse/KAFKA-687 > > > Repository: kafka > > > Description > ------- > > The updated diff contains the mbeans for ownership counts. > The comments in the code and the summary are pretty self-explanatory. > > Things to think about: > * Naming - do symmetric/range/roundrobin make sense? > * The comments briefly summarize why we needed a separate symmetric mode but > let me know if that is unclear. > * Rebalance time will be slightly higher - I have not measured (will do that) > > > Diffs > ----- > > core/src/main/scala/kafka/consumer/ConsumerConfig.scala > 1cf2f62ba02e4aa66bfa7575865e5d57baf82212 > core/src/main/scala/kafka/consumer/PartitionAllocator.scala PRE-CREATION > core/src/main/scala/kafka/consumer/TopicCount.scala > c79311097c5bd6718cb6a7fc403f804a1a939353 > core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala > 65f518d47c7555c42c4bff39c211814831f4b8b6 > core/src/main/scala/kafka/metrics/KafkaMetricsGroup.scala > a20ab90165cc7ebb1cf44078efe23a53938c8df6 > core/src/main/scala/kafka/utils/ZkUtils.scala > dcdc1ce2b02c996294e19cf480736106aaf29511 > core/src/test/scala/unit/kafka/consumer/PartitionAllocatorTest.scala > PRE-CREATION > > Diff: https://reviews.apache.org/r/23655/diff/ > > > Testing > ------- > > * I did the unit tests (including the new one) as well as mirror maker system > test suite with roundrobin. While this is being reviewed I will run the > system tests with symmetric > > > Thanks, > > Joel Koshy > >
