You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@kafka.apache.org by Sriram Subramanian <sr...@gmail.com> on 2013/10/09 08:48:02 UTC

Review Request 14554: Move AddPartitions into TopicCommand in trunk

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14554/
-----------------------------------------------------------

Review request for kafka.


Bugs: KAFKA-1052
    https://issues.apache.org/jira/browse/KAFKA-1052


Repository: kafka


Description
-------

Move AddPartitions into TopicCommand


Diffs
-----

  core/src/main/scala/kafka/admin/AddPartitionsCommand.scala c74d9c224565d475d4104467ba36c0a287b68239 
  core/src/main/scala/kafka/admin/AdminUtils.scala 6560fc668e8b8feedc1e3f63c11aac2ff370c8d0 
  core/src/main/scala/kafka/admin/TopicCommand.scala 06bbd37a5af1ea5bd5f6c42cb9063cc7d6fd381b 
  core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala 09254ccac2a4d5885a372f1f21b0a5813a930ded 

Diff: https://reviews.apache.org/r/14554/diff/


Testing
-------


Thanks,

Sriram Subramanian

Re: Review Request 14554: Move AddPartitions into TopicCommand in trunk

Posted by Sriram Subramanian <sr...@gmail.com>.


> On Oct. 9, 2013, 5:23 p.m., Jun Rao wrote:
> > Got the following error when running the tool.
> > 
> > bin/kafka-topics.sh --zookeeper localhost:2181 --alter --partitions 3 --topic test
> > partitions can only be added when topic has no key
> > Exception in thread "main" java.lang.NullPointerException
> > 	at kafka.admin.AdminUtils$.getManualReplicaAssignment(AdminUtils.scala:122)
> > 	at kafka.admin.AdminUtils$.addPartitions(AdminUtils.scala:106)
> > 	at kafka.admin.TopicCommand$.alterTopic(TopicCommand.scala:91)
> > 	at kafka.admin.TopicCommand$.main(TopicCommand.scala:50)
> > 	at kafka.admin.TopicCommand.main(TopicCommand.scala)
> >

my bad. Had the change but had not commited it and the tool did not pick it up.


- Sriram


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14554/#review26821
-----------------------------------------------------------


On Oct. 9, 2013, 6:48 a.m., Sriram Subramanian wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14554/
> -----------------------------------------------------------
> 
> (Updated Oct. 9, 2013, 6:48 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1052
>     https://issues.apache.org/jira/browse/KAFKA-1052
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> Move AddPartitions into TopicCommand
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/admin/AddPartitionsCommand.scala c74d9c224565d475d4104467ba36c0a287b68239 
>   core/src/main/scala/kafka/admin/AdminUtils.scala 6560fc668e8b8feedc1e3f63c11aac2ff370c8d0 
>   core/src/main/scala/kafka/admin/TopicCommand.scala 06bbd37a5af1ea5bd5f6c42cb9063cc7d6fd381b 
>   core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala 09254ccac2a4d5885a372f1f21b0a5813a930ded 
> 
> Diff: https://reviews.apache.org/r/14554/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sriram Subramanian
> 
>

Re: Review Request 14554: Move AddPartitions into TopicCommand in trunk

Posted by Jun Rao <ju...@gmail.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14554/#review26821
-----------------------------------------------------------


Got the following error when running the tool.

bin/kafka-topics.sh --zookeeper localhost:2181 --alter --partitions 3 --topic test
partitions can only be added when topic has no key
Exception in thread "main" java.lang.NullPointerException
	at kafka.admin.AdminUtils$.getManualReplicaAssignment(AdminUtils.scala:122)
	at kafka.admin.AdminUtils$.addPartitions(AdminUtils.scala:106)
	at kafka.admin.TopicCommand$.alterTopic(TopicCommand.scala:91)
	at kafka.admin.TopicCommand$.main(TopicCommand.scala:50)
	at kafka.admin.TopicCommand.main(TopicCommand.scala)



core/src/main/scala/kafka/admin/TopicCommand.scala
<https://reviews.apache.org/r/14554/#comment52172>

    Technically, one can still increase # partitions even if messages have keys. The user just needs to be aware of the impact, i.e., messages with the same key may move to a different partition.



core/src/main/scala/kafka/admin/TopicCommand.scala
<https://reviews.apache.org/r/14554/#comment52173>

    Ditto for the comment above.


- Jun Rao


On Oct. 9, 2013, 6:48 a.m., Sriram Subramanian wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14554/
> -----------------------------------------------------------
> 
> (Updated Oct. 9, 2013, 6:48 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1052
>     https://issues.apache.org/jira/browse/KAFKA-1052
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> Move AddPartitions into TopicCommand
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/admin/AddPartitionsCommand.scala c74d9c224565d475d4104467ba36c0a287b68239 
>   core/src/main/scala/kafka/admin/AdminUtils.scala 6560fc668e8b8feedc1e3f63c11aac2ff370c8d0 
>   core/src/main/scala/kafka/admin/TopicCommand.scala 06bbd37a5af1ea5bd5f6c42cb9063cc7d6fd381b 
>   core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala 09254ccac2a4d5885a372f1f21b0a5813a930ded 
> 
> Diff: https://reviews.apache.org/r/14554/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sriram Subramanian
> 
>

Re: Review Request 14554: Move AddPartitions into TopicCommand in trunk

Posted by Sriram Subramanian <sr...@gmail.com>.


> On Oct. 9, 2013, 5:31 p.m., Guozhang Wang wrote:
> > core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala, line 122
> > <https://reviews.apache.org/r/14554/diff/1/?file=362781#file362781line122>
> >
> >     Why need to increment the added # partitions by 1 here?

The addPartitions input is the total number of partitions now.


- Sriram


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14554/#review26823
-----------------------------------------------------------


On Oct. 9, 2013, 6:48 a.m., Sriram Subramanian wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14554/
> -----------------------------------------------------------
> 
> (Updated Oct. 9, 2013, 6:48 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1052
>     https://issues.apache.org/jira/browse/KAFKA-1052
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> Move AddPartitions into TopicCommand
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/admin/AddPartitionsCommand.scala c74d9c224565d475d4104467ba36c0a287b68239 
>   core/src/main/scala/kafka/admin/AdminUtils.scala 6560fc668e8b8feedc1e3f63c11aac2ff370c8d0 
>   core/src/main/scala/kafka/admin/TopicCommand.scala 06bbd37a5af1ea5bd5f6c42cb9063cc7d6fd381b 
>   core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala 09254ccac2a4d5885a372f1f21b0a5813a930ded 
> 
> Diff: https://reviews.apache.org/r/14554/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sriram Subramanian
> 
>

Re: Review Request 14554: Move AddPartitions into TopicCommand in trunk

Posted by Guozhang Wang <gu...@linkedin.com>.

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14554/#review26823
-----------------------------------------------------------



core/src/main/scala/kafka/admin/TopicCommand.scala
<https://reviews.apache.org/r/14554/#comment52177>

    I think keyed-messages and none-key messages can be sent to a topic at the same time, and we cannot control that client behavior. SO I suggest changing this to "WARNING: sending keyed-messages to topics whose partitions are expanded may not reserve the order anymore"



core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala
<https://reviews.apache.org/r/14554/#comment52176>

    Why need to increment the added # partitions by 1 here?


- Guozhang Wang


On Oct. 9, 2013, 6:48 a.m., Sriram Subramanian wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14554/
> -----------------------------------------------------------
> 
> (Updated Oct. 9, 2013, 6:48 a.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1052
>     https://issues.apache.org/jira/browse/KAFKA-1052
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> Move AddPartitions into TopicCommand
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/admin/AddPartitionsCommand.scala c74d9c224565d475d4104467ba36c0a287b68239 
>   core/src/main/scala/kafka/admin/AdminUtils.scala 6560fc668e8b8feedc1e3f63c11aac2ff370c8d0 
>   core/src/main/scala/kafka/admin/TopicCommand.scala 06bbd37a5af1ea5bd5f6c42cb9063cc7d6fd381b 
>   core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala 09254ccac2a4d5885a372f1f21b0a5813a930ded 
> 
> Diff: https://reviews.apache.org/r/14554/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sriram Subramanian
> 
>