You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Viktor Tarananeko <vi...@gmail.com> on 2014/09/07 21:22:26 UTC
Review Request 25420: Patch for KAFKA-686
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/25420/
-----------------------------------------------------------
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
Re: Review Request 25420: Patch for KAFKA-686
Posted by Guozhang Wang <gu...@linkedin.com>.
-----------------------------------------------------------
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
>
>