----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25420/#review52980 -----------------------------------------------------------
Thanks for the patch. Overall it is a great clean-up. Some comments below: core/src/main/scala/kafka/controller/PartitionStateMachine.scala <https://reviews.apache.org/r/25420/#comment92277> The controller will only be shutdown upon kafka server shutting down, we should probably just call controller.onControllerResignation() core/src/main/scala/kafka/utils/ZkUtils.scala <https://reviews.apache.org/r/25420/#comment92282> Do we still need this function any more? Shall we just change the usages of getReplicasForPartition to getPartitionAssignmentForTopic? core/src/main/scala/kafka/utils/ZkUtils.scala <https://reviews.apache.org/r/25420/#comment92285> Why do we want it to be mutable? core/src/main/scala/kafka/utils/ZkUtils.scala <https://reviews.apache.org/r/25420/#comment92286> Shall we also use failAsValue on catching ZkNoNodeException in getChildrenParentMayNotExist, and make its return type an Option? core/src/main/scala/kafka/utils/ZkUtils.scala <https://reviews.apache.org/r/25420/#comment92288> Can we rename to "consumerOffsetForPartitionDir" for naming consistency? - Guozhang Wang On Sept. 7, 2014, 7:22 p.m., Viktor Tarananeko wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25420/ > ----------------------------------------------------------- > > (Updated Sept. 7, 2014, 7:22 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-686 > https://issues.apache.org/jira/browse/KAFKA-686 > > > Repository: kafka > > > Description > ------- > > Merge branch 'trunk' into fix-null-pointer-in-zk-utils > > > unify topic partition path constructing; refactoring and better code reuse > > > reuse method to fetch broker data > > > unify fetching topics > > > raise InvalidTopicException if unable to properly parse json data from > Zookeeper > > > TopicRegistrationInfo class > > > base controller failure test on invalid topic data > > > Diffs > ----- > > core/src/main/scala/kafka/common/TopicRegistrationInfo.scala PRE-CREATION > core/src/main/scala/kafka/consumer/TopicCount.scala > 0954b3c3ff8b3b7a7a4095436bc9e6c494a38c37 > core/src/main/scala/kafka/consumer/ZookeeperConsumerConnector.scala > fbc680fde21b02f11285a4f4b442987356abd17b > core/src/main/scala/kafka/controller/PartitionStateMachine.scala > e20b63a6ec1c1a848bc3823701b0f8ceaeb6100d > core/src/main/scala/kafka/server/KafkaHealthcheck.scala > 4acdd70fe9c1ee78d6510741006c2ece65450671 > core/src/main/scala/kafka/tools/ConsumerOffsetChecker.scala > d1e7c434e77859d746b8dc68dd5d5a3740425e79 > core/src/main/scala/kafka/tools/ExportZkOffsets.scala > 4d051bc2db12f0e15aa6a3289abeb9dd25d373d2 > core/src/main/scala/kafka/tools/UpdateOffsetsInZK.scala > 111c9a8b94ce45d95551482e9fd3f8c1cccbf548 > core/src/main/scala/kafka/utils/ZkUtils.scala > a7b1fdcb50d5cf930352d37e39cb4fc9a080cb12 > core/src/test/scala/unit/kafka/integration/AutoOffsetResetTest.scala > 95303e098d40cd790fb370e9b5a47d20860a6da3 > core/src/test/scala/unit/kafka/server/ControllerFailureTest.scala > PRE-CREATION > core/src/test/scala/unit/kafka/utils/TestUtils.scala > c4e13c5240c8303853d08cc3b40088f8c7dae460 > > Diff: https://reviews.apache.org/r/25420/diff/ > > > Testing > ------- > > > Thanks, > > Viktor Tarananeko > >
