----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15201/#review28781 -----------------------------------------------------------
core/src/main/scala/kafka/tools/ReplicaVerificationTool.scala <https://reviews.apache.org/r/15201/#comment55792> fetchsize -> fetch-size for consistency with other options? core/src/main/scala/kafka/tools/ReplicaVerificationTool.scala <https://reviews.apache.org/r/15201/#comment55793> We can probably change this to ConsumerConfig.FetchSize. Anytime we change the max message size on the broker, we will probably change default fetch size on consumer, so that can serve as the source of truth for this tool as well. core/src/main/scala/kafka/tools/ReplicaVerificationTool.scala <https://reviews.apache.org/r/15201/#comment55794> Should we just do .replaceAll("""["']""", "") instead? core/src/main/scala/kafka/tools/ReplicaVerificationTool.scala <https://reviews.apache.org/r/15201/#comment55799> val fetcherThreads = for ((i, element) <- topicAndPartitionsPerBroker.view.zipWithIndex) yield new ReplicaFetcher(...., doVerification = if (i == 0) true else false) to avoid variable = true and then variable = false? core/src/main/scala/kafka/tools/ReplicaVerificationTool.scala <https://reviews.apache.org/r/15201/#comment55807> Minor comment: Can we rename this to initialOffsetMap (we use the offsetMap name in ReplicaFetchThread), I got confused on the first glace.. core/src/main/scala/kafka/tools/ReplicaVerificationTool.scala <https://reviews.apache.org/r/15201/#comment55805> I thought this loop is supposed to go through all the messages that can be returned by the messageIterator, but looks like it will check for only the first message obtained via each messageIterator. core/src/main/scala/kafka/tools/ReplicaVerificationTool.scala <https://reviews.apache.org/r/15201/#comment55806> Wondering why we don't exit when checksums don't match? core/src/main/scala/kafka/tools/ReplicaVerificationTool.scala <https://reviews.apache.org/r/15201/#comment55808> typo core/src/main/scala/kafka/tools/ReplicaVerificationTool.scala <https://reviews.apache.org/r/15201/#comment55809> typo Another caveat seems to be that the tool cannot handle changes in 1. partition leadership change 2. topic configuration change (number of partitions). - Swapnil Ghike On Nov. 12, 2013, 4:34 p.m., Jun Rao wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/15201/ > ----------------------------------------------------------- > > (Updated Nov. 12, 2013, 4:34 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1117 > https://issues.apache.org/jira/browse/KAFKA-1117 > > > Repository: kafka > > > Description > ------- > > kafka-1117; fix 3 > > > kafka-1117; fix 2 > > > kafka-1117; fix 1 > > > kafka-1117 > > > Diffs > ----- > > config/tools-log4j.properties 79240490149835656e2a013a9702c5aa41c104f1 > core/src/main/scala/kafka/api/OffsetResponse.scala > 08dc3cd3d166efba6b2b43f6e148f636b175affe > core/src/main/scala/kafka/tools/ReplicaVerificationTool.scala PRE-CREATION > > Diff: https://reviews.apache.org/r/15201/diff/ > > > Testing > ------- > > > Thanks, > > Jun Rao > >
