> On Nov. 13, 2013, 7:43 a.m., Swapnil Ghike wrote: > > core/src/main/scala/kafka/tools/ReplicaVerificationTool.scala, line 70 > > <https://reviews.apache.org/r/15201/diff/3/?file=382691#file382691line70> > > > > fetchsize -> fetch-size for consistency with other options?
changed. > On Nov. 13, 2013, 7:43 a.m., Swapnil Ghike wrote: > > core/src/main/scala/kafka/tools/ReplicaVerificationTool.scala, line 74 > > <https://reviews.apache.org/r/15201/diff/3/?file=382691#file382691line74> > > > > 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. changed. > On Nov. 13, 2013, 7:43 a.m., Swapnil Ghike wrote: > > core/src/main/scala/kafka/tools/ReplicaVerificationTool.scala, lines 110-111 > > <https://reviews.apache.org/r/15201/diff/3/?file=382691#file382691line110> > > > > Should we just do .replaceAll("""["']""", "") instead? Not long used. > On Nov. 13, 2013, 7:43 a.m., Swapnil Ghike wrote: > > core/src/main/scala/kafka/tools/ReplicaVerificationTool.scala, lines 165-181 > > <https://reviews.apache.org/r/15201/diff/3/?file=382691#file382691line165> > > > > 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? Changed, in a different way. > On Nov. 13, 2013, 7:43 a.m., Swapnil Ghike wrote: > > core/src/main/scala/kafka/tools/ReplicaVerificationTool.scala, lines 242-244 > > <https://reviews.apache.org/r/15201/diff/3/?file=382691#file382691line242> > > > > Minor comment: Can we rename this to initialOffsetMap (we use the > > offsetMap name in ReplicaFetchThread), I got confused on the first glace.. done. > On Nov. 13, 2013, 7:43 a.m., Swapnil Ghike wrote: > > core/src/main/scala/kafka/tools/ReplicaVerificationTool.scala, line 272 > > <https://reviews.apache.org/r/15201/diff/3/?file=382691#file382691line272> > > > > 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. This loop is just iterating one message from every replica. The outer loop iterates through all messages. > On Nov. 13, 2013, 7:43 a.m., Swapnil Ghike wrote: > > core/src/main/scala/kafka/tools/ReplicaVerificationTool.scala, lines 291-295 > > <https://reviews.apache.org/r/15201/diff/3/?file=382691#file382691line291> > > > > Wondering why we don't exit when checksums don't match? It's probably better to report all occurrences of mismatched checksums. > On Nov. 13, 2013, 7:43 a.m., Swapnil Ghike wrote: > > core/src/main/scala/kafka/tools/ReplicaVerificationTool.scala, line 376 > > <https://reviews.apache.org/r/15201/diff/3/?file=382691#file382691line376> > > > > typo changed. > On Nov. 13, 2013, 7:43 a.m., Swapnil Ghike wrote: > > core/src/main/scala/kafka/tools/ReplicaVerificationTool.scala, line 385 > > <https://reviews.apache.org/r/15201/diff/3/?file=382691#file382691line385> > > > > typo changed. On Nov. 13, 2013, 7:43 a.m., Jun Rao wrote: > > Another caveat seems to be that the tool cannot handle changes in 1. > > partition leadership change 2. topic configuration change (number of > > partitions). The tool doesn't care about leader changes since it has to read from every replica. It won't pick up newly created partitions or partitions moved by the reassignment tool. For those cases, we can just restart the tool. - Jun ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/15201/#review28781 ----------------------------------------------------------- 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 > >
