[ 
https://issues.apache.org/jira/browse/KAFKA-776?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13589091#comment-13589091
 ] 

Neha Narkhede commented on KAFKA-776:
-------------------------------------

Thanks for the patch. Overall looks good, few review comments -
1. ConsumerOffsetChecker
Fix error() statement in getConsumer() to include the throwable e as the 2nd 
argument to error(). That will ensure that the stack trace is pretty printed

2. ExportZkOffsets
Maybe the following 2 lines should be in the "case: Some(m)" block or protected 
by if(offsetVal != null)
 fileWriter.write(offsetPath + ":" + offsetVal + "\n")
            debug(offsetPath + " => " + offsetVal)

3. ShutdownBroker
Would you mind changing the error() line to the following -
error("Operation failed due to controller failure", t)
                
> Changing ZK format breaks some tools
> ------------------------------------
>
>                 Key: KAFKA-776
>                 URL: https://issues.apache.org/jira/browse/KAFKA-776
>             Project: Kafka
>          Issue Type: Bug
>    Affects Versions: 0.8
>            Reporter: Sriram Subramanian
>            Assignee: Swapnil Ghike
>            Priority: Blocker
>              Labels: kafka-0.8, p1
>             Fix For: 0.8
>
>         Attachments: kafka-776-v1.patch
>
>
> There are some tools that parse the zk output and they might break.1 has been 
> verified to break.
> Few that read the zk output are 
> 1. Shutdown Broker
> 2. PreferredReplicaLeader
> 3. ConsumerOffsetChecker
> There could be few others but I have not checked everything

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to