You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Sriharsha Chintalapani <ha...@hortonworks.com> on 2014/05/08 23:58:41 UTC

Review Request 21239: Patch for KAFKA-1431

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

Review request for kafka.


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


Repository: kafka


Description
-------

KAFKA-1431. ConsoleConsumer - Option to clean zk consumer path.


Diffs
-----

  core/src/main/scala/kafka/consumer/ConsoleConsumer.scala 0f62819be0562f62c0f778bd20ead053f01a6f2f 

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


Testing
-------


Thanks,

Sriharsha Chintalapani


Re: Review Request 21239: Patch for KAFKA-1431

Posted by Sriharsha Chintalapani <ha...@hortonworks.com>.

> On May 15, 2014, 3:01 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/consumer/ConsoleConsumer.scala, lines 165-166
> > <https://reviews.apache.org/r/21239/diff/3/?file=581744#file581744line165>
> >
> >     Since this is a command line tool, we should print the message to stdout, instead of logging, so that users can see the message on the console.
> 
> Neha Narkhede wrote:
>     stdout->stderr

There is tools-log4j.properties file thats being used by kafka-run-class.sh. It has ConsoleAppender so by using error it does print to the console.
But if we would like to have it has stdout print than I'll change that , there are few other places error being used to print to console.


- Sriharsha


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


On May 14, 2014, 7:08 p.m., Sriharsha Chintalapani wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21239/
> -----------------------------------------------------------
> 
> (Updated May 14, 2014, 7:08 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1431
>     https://issues.apache.org/jira/browse/KAFKA-1431
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1431. ConsoleConsumer - Option to clean zk consumer path changed options --delete-consumer-offsets. It checks if existing offsets for givent topic & groupId exists and throws error if user didn't specifiy --delete-consumer-offsets
> 
> 
> KAFKA-1431.ConsoleConsumer - Option to clean zk consumer path.  just checking consumers/groupId/offsets is good enough, removed for checking topicId under offsets.
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/consumer/ConsoleConsumer.scala 0f62819be0562f62c0f778bd20ead053f01a6f2f 
> 
> Diff: https://reviews.apache.org/r/21239/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sriharsha Chintalapani
> 
>


Re: Review Request 21239: Patch for KAFKA-1431

Posted by Neha Narkhede <ne...@gmail.com>.

> On May 15, 2014, 3:01 p.m., Jun Rao wrote:
> > core/src/main/scala/kafka/consumer/ConsoleConsumer.scala, lines 165-166
> > <https://reviews.apache.org/r/21239/diff/3/?file=581744#file581744line165>
> >
> >     Since this is a command line tool, we should print the message to stdout, instead of logging, so that users can see the message on the console.

stdout->stderr


- Neha


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


On May 14, 2014, 7:08 p.m., Sriharsha Chintalapani wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21239/
> -----------------------------------------------------------
> 
> (Updated May 14, 2014, 7:08 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1431
>     https://issues.apache.org/jira/browse/KAFKA-1431
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1431. ConsoleConsumer - Option to clean zk consumer path changed options --delete-consumer-offsets. It checks if existing offsets for givent topic & groupId exists and throws error if user didn't specifiy --delete-consumer-offsets
> 
> 
> KAFKA-1431.ConsoleConsumer - Option to clean zk consumer path.  just checking consumers/groupId/offsets is good enough, removed for checking topicId under offsets.
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/consumer/ConsoleConsumer.scala 0f62819be0562f62c0f778bd20ead053f01a6f2f 
> 
> Diff: https://reviews.apache.org/r/21239/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sriharsha Chintalapani
> 
>


Re: Review Request 21239: Patch for KAFKA-1431

Posted by Jun Rao <ju...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21239/#review43113
-----------------------------------------------------------



core/src/main/scala/kafka/consumer/ConsoleConsumer.scala
<https://reviews.apache.org/r/21239/#comment77127>

    Since this is a command line tool, we should print the message to stdout, instead of logging, so that users can see the message on the console.


- Jun Rao


On May 14, 2014, 7:08 p.m., Sriharsha Chintalapani wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21239/
> -----------------------------------------------------------
> 
> (Updated May 14, 2014, 7:08 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1431
>     https://issues.apache.org/jira/browse/KAFKA-1431
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1431. ConsoleConsumer - Option to clean zk consumer path changed options --delete-consumer-offsets. It checks if existing offsets for givent topic & groupId exists and throws error if user didn't specifiy --delete-consumer-offsets
> 
> 
> KAFKA-1431.ConsoleConsumer - Option to clean zk consumer path.  just checking consumers/groupId/offsets is good enough, removed for checking topicId under offsets.
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/consumer/ConsoleConsumer.scala 0f62819be0562f62c0f778bd20ead053f01a6f2f 
> 
> Diff: https://reviews.apache.org/r/21239/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sriharsha Chintalapani
> 
>


Re: Review Request 21239: Patch for KAFKA-1431

Posted by Sriharsha Chintalapani <ha...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21239/
-----------------------------------------------------------

(Updated May 14, 2014, 7:08 p.m.)


Review request for kafka.


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


Repository: kafka


Description (updated)
-------

KAFKA-1431. ConsoleConsumer - Option to clean zk consumer path changed options --delete-consumer-offsets. It checks if existing offsets for givent topic & groupId exists and throws error if user didn't specifiy --delete-consumer-offsets


KAFKA-1431.ConsoleConsumer - Option to clean zk consumer path.  just checking consumers/groupId/offsets is good enough, removed for checking topicId under offsets.


Diffs (updated)
-----

  core/src/main/scala/kafka/consumer/ConsoleConsumer.scala 0f62819be0562f62c0f778bd20ead053f01a6f2f 

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


Testing
-------


Thanks,

Sriharsha Chintalapani


Re: Review Request 21239: Patch for KAFKA-1431

Posted by Sriharsha Chintalapani <ha...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21239/
-----------------------------------------------------------

(Updated May 14, 2014, 6:33 p.m.)


Review request for kafka.


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


Repository: kafka


Description (updated)
-------

KAFKA-1431. ConsoleConsumer - Option to clean zk consumer path changed options --delete-consumer-offsets. It checks if existing offsets for givent topic & groupId exists and throws error if user didn't specifiy --delete-consumer-offsets


Diffs (updated)
-----

  core/src/main/scala/kafka/consumer/ConsoleConsumer.scala 0f62819be0562f62c0f778bd20ead053f01a6f2f 

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


Testing
-------


Thanks,

Sriharsha Chintalapani


Re: Review Request 21239: Patch for KAFKA-1431

Posted by Jun Rao <ju...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21239/#review42664
-----------------------------------------------------------



core/src/main/scala/kafka/consumer/ConsoleConsumer.scala
<https://reviews.apache.org/r/21239/#comment76487>

    Could we change the description to sth like "if specified, the consumer path in zookeeper is deleted when starting up".


- Jun Rao


On May 8, 2014, 9:58 p.m., Sriharsha Chintalapani wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21239/
> -----------------------------------------------------------
> 
> (Updated May 8, 2014, 9:58 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1431
>     https://issues.apache.org/jira/browse/KAFKA-1431
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1431. ConsoleConsumer - Option to clean zk consumer path.
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/consumer/ConsoleConsumer.scala 0f62819be0562f62c0f778bd20ead053f01a6f2f 
> 
> Diff: https://reviews.apache.org/r/21239/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sriharsha Chintalapani
> 
>


Re: Review Request 21239: Patch for KAFKA-1431

Posted by Sriharsha Chintalapani <ha...@hortonworks.com>.

> On May 8, 2014, 10:48 p.m., Neha Narkhede wrote:
> >

Thanks for the feedback.  I added checkZkPathExists method to check if the offsets for given topic & groupid exists and removed tryCleanZookeeper method its not being used anywhere else in the code.


- Sriharsha


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


On May 14, 2014, 6:33 p.m., Sriharsha Chintalapani wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21239/
> -----------------------------------------------------------
> 
> (Updated May 14, 2014, 6:33 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1431
>     https://issues.apache.org/jira/browse/KAFKA-1431
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1431. ConsoleConsumer - Option to clean zk consumer path changed options --delete-consumer-offsets. It checks if existing offsets for givent topic & groupId exists and throws error if user didn't specifiy --delete-consumer-offsets
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/consumer/ConsoleConsumer.scala 0f62819be0562f62c0f778bd20ead053f01a6f2f 
> 
> Diff: https://reviews.apache.org/r/21239/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sriharsha Chintalapani
> 
>


Re: Review Request 21239: Patch for KAFKA-1431

Posted by Neha Narkhede <ne...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/21239/#review42550
-----------------------------------------------------------



core/src/main/scala/kafka/consumer/ConsoleConsumer.scala
<https://reviews.apache.org/r/21239/#comment76355>

    This is great. In addition to this, if the user picks the --from-beginning option and we find existing offset information, we should probably throw an error saying "Found previous offset information for this group <group-name>. Please use the --delete-consumer-path option to delete the previous offset metadata".
    
    Also, what if the user runs the console consumer with both --from-beginning and --delete-consumer-path? This should be allowed, I guess.
    
    Another suggestion - can we rename the new option to --delete-consumer-offsets?


- Neha Narkhede


On May 8, 2014, 9:58 p.m., Sriharsha Chintalapani wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/21239/
> -----------------------------------------------------------
> 
> (Updated May 8, 2014, 9:58 p.m.)
> 
> 
> Review request for kafka.
> 
> 
> Bugs: KAFKA-1431
>     https://issues.apache.org/jira/browse/KAFKA-1431
> 
> 
> Repository: kafka
> 
> 
> Description
> -------
> 
> KAFKA-1431. ConsoleConsumer - Option to clean zk consumer path.
> 
> 
> Diffs
> -----
> 
>   core/src/main/scala/kafka/consumer/ConsoleConsumer.scala 0f62819be0562f62c0f778bd20ead053f01a6f2f 
> 
> Diff: https://reviews.apache.org/r/21239/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sriharsha Chintalapani
> 
>