You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Guozhang Wang <gu...@linkedin.com> on 2014/08/01 23:27:18 UTC
Re: Review Request 24006: Patch for KAFKA-1420
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24006/#review49382
-----------------------------------------------------------
core/src/test/scala/unit/kafka/admin/AdminTest.scala
<https://reviews.apache.org/r/24006/#comment86410>
RunningAsBroker is not used any more.
core/src/test/scala/unit/kafka/admin/AdminTest.scala
<https://reviews.apache.org/r/24006/#comment86411>
What I was actually thinking is that probably we can define these two variables at the top of testTopicCreationInZK and crate the expectedReplicaAssignment and leaderForPartitionMap programmatically instead of hand-written them.
core/src/test/scala/unit/kafka/admin/AdminTest.scala
<https://reviews.apache.org/r/24006/#comment86413>
After reading this test more clearly, I think it was originally trying to "testResumePartitionReassignmentThatWasIncomplet", i.e. when the brokers are started up they need to finish whatever reassignment tasks is left on ZK. In this case, we cannot create the brokers before hand.
What we can actually do, is to 1) create the brokers and create the topics, 2) shutdown brokers, 3) write the reassignment in ZK, 4) restart-brokers.
core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala
<https://reviews.apache.org/r/24006/#comment86420>
Could we rename these to "replicaIdsForPartition" and "serversHostingPartition"?
core/src/test/scala/unit/kafka/integration/UncleanLeaderElectionTest.scala
<https://reviews.apache.org/r/24006/#comment86421>
We do not need to declare these variables if they are not re-usable other than the createTopic() call. Same blew.
- Guozhang Wang
On July 30, 2014, 6:24 p.m., Jonathan Natkins wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24006/
> -----------------------------------------------------------
>
> (Updated July 30, 2014, 6:24 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1420
> https://issues.apache.org/jira/browse/KAFKA-1420
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1420 Replace AdminUtils.createOrUpdateTopicPartitionAssignmentPathInZK with TestUtils.createTopic in unit tests
>
>
> Diffs
> -----
>
> core/src/test/scala/unit/kafka/admin/AdminTest.scala e28979827110dfbbb92fe5b152e7f1cc973de400
> core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala 29cc01bcef9cacd8dec1f5d662644fc6fe4994bc
> core/src/test/scala/unit/kafka/integration/UncleanLeaderElectionTest.scala f44568cb25edf25db857415119018fd4c9922f61
> core/src/test/scala/unit/kafka/utils/TestUtils.scala c4e13c5240c8303853d08cc3b40088f8c7dae460
>
> Diff: https://reviews.apache.org/r/24006/diff/
>
>
> Testing
> -------
>
> Automated
>
>
> Thanks,
>
> Jonathan Natkins
>
>
Re: Review Request 24006: Patch for KAFKA-1420
Posted by Jonathan Natkins <na...@cloudera.com>.
> On Aug. 1, 2014, 9:27 p.m., Guozhang Wang wrote:
> > core/src/test/scala/unit/kafka/admin/AdminTest.scala, line 134
> > <https://reviews.apache.org/r/24006/diff/3/?file=646111#file646111line134>
> >
> > What I was actually thinking is that probably we can define these two variables at the top of testTopicCreationInZK and crate the expectedReplicaAssignment and leaderForPartitionMap programmatically instead of hand-written them.
This would admittedly be easier in later versions of Scala, which have some combinatorial functionality, but I've written a little code to do this. Take a look at it...I'm not totally sure it's what you had in mind, so let me know if you just meant for me to use AdminTools.assignReplicasToBrokers or something like that.
- Jonathan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24006/#review49382
-----------------------------------------------------------
On Aug. 2, 2014, 6:04 p.m., Jonathan Natkins wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24006/
> -----------------------------------------------------------
>
> (Updated Aug. 2, 2014, 6:04 p.m.)
>
>
> Review request for kafka.
>
>
> Bugs: KAFKA-1420
> https://issues.apache.org/jira/browse/KAFKA-1420
>
>
> Repository: kafka
>
>
> Description
> -------
>
> KAFKA-1420 Replace AdminUtils.createOrUpdateTopicPartitionAssignmentPathInZK with TestUtils.createTopic in unit tests
>
>
> Diffs
> -----
>
> core/src/test/scala/unit/kafka/admin/AdminTest.scala e28979827110dfbbb92fe5b152e7f1cc973de400
> core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala 29cc01bcef9cacd8dec1f5d662644fc6fe4994bc
> core/src/test/scala/unit/kafka/integration/UncleanLeaderElectionTest.scala f44568cb25edf25db857415119018fd4c9922f61
> core/src/test/scala/unit/kafka/utils/TestUtils.scala c4e13c5240c8303853d08cc3b40088f8c7dae460
>
> Diff: https://reviews.apache.org/r/24006/diff/
>
>
> Testing
> -------
>
> Automated
>
>
> Thanks,
>
> Jonathan Natkins
>
>