----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15201/#review28186 -----------------------------------------------------------
core/src/main/scala/kafka/tools/ReplicaVerificationTool.scala <https://reviews.apache.org/r/15201/#comment54838> Default max.message.size is (1MB + header). Default fetch size should be a little larger than 1MB. How about 1.1 MB? core/src/main/scala/kafka/tools/ReplicaVerificationTool.scala <https://reviews.apache.org/r/15201/#comment54840> This can throw an exception if the leader doesn't exist Replicas that don't have a leader when this tool is run cannot set the initial offset. Should we skip such partitions? core/src/main/scala/kafka/tools/ReplicaVerificationTool.scala <https://reviews.apache.org/r/15201/#comment54847> It will be useful to know the fetch offset of the chunk that is undergoing verification. core/src/main/scala/kafka/tools/ReplicaVerificationTool.scala <https://reviews.apache.org/r/15201/#comment54853> map{ -> map { core/src/main/scala/kafka/tools/ReplicaVerificationTool.scala <https://reviews.apache.org/r/15201/#comment54856> It will be good to include a relevant message with every assert core/src/main/scala/kafka/tools/ReplicaVerificationTool.scala <https://reviews.apache.org/r/15201/#comment54862> Let's add a comment explaining what this loop does. core/src/main/scala/kafka/tools/ReplicaVerificationTool.scala <https://reviews.apache.org/r/15201/#comment54854> This is not a messageSet but a message. Can we rename this to messageInfoFromFirstReplica ? core/src/main/scala/kafka/tools/ReplicaVerificationTool.scala <https://reviews.apache.org/r/15201/#comment54861> What happens if one replica returns empty message set because it is not caught up and other replicas return some valid messages? Then ideally the fetch offset should not be updated since we need to repeat the verification for that fetch offset core/src/main/scala/kafka/tools/ReplicaVerificationTool.scala <https://reviews.apache.org/r/15201/#comment54858> Let's rename isDone to something meaningful. It is a little confusing that we update the fetch offset even when isDone is false. - Neha Narkhede On Nov. 4, 2013, 12:34 a.m., Jun Rao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/15201/ > ----------------------------------------------------------- > > (Updated Nov. 4, 2013, 12:34 a.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1117 > https://issues.apache.org/jira/browse/KAFKA-1117 > > > Repository: kafka > > > Description > ------- > > kafka-1117 > > > Diffs > ----- > > core/src/main/scala/kafka/tools/ReplicaVerificationTool.scala PRE-CREATION > > Diff: https://reviews.apache.org/r/15201/diff/ > > > Testing > ------- > > > Thanks, > > Jun Rao > >
