----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30809/#review78630 -----------------------------------------------------------
Thanks for the patch. A few high level comments. 1. I think we are standardizing on python for test system tests. Should we write test-broker-upgrade.sh in python? 2. The existing system tests also generate loads and do verification. However, that part of logic is a bit hacky. For example, we hacked ProducerPerfomance a bit to generate data with sequential ids. Do we plan to replace those with ContinuousValidationTest in the future? Ideally, it would be great if we can write the common drive code (e.g., loading data, reading data, verifying) once and reuse it in all tests. 3. When doing upgrades, we typically upgrade the brokers first, followed by the clients. To simulate that, the clients need to run on an old version. Have you thought about how to do that? core/src/main/scala/kafka/tools/ContinuousValidationTest.java <https://reviews.apache.org/r/30809/#comment127544> Could we add a description of the test (what kind of data is generated, how does consumer to the verification, what kind of output is generated, etc)? core/src/main/scala/kafka/tools/ContinuousValidationTest.java <https://reviews.apache.org/r/30809/#comment127543> Could we add a description of each command line option? - Jun Rao On March 23, 2015, 6:54 p.m., Abhishek Nigam wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/30809/ > ----------------------------------------------------------- > > (Updated March 23, 2015, 6:54 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1888 > https://issues.apache.org/jira/browse/KAFKA-1888 > > > Repository: kafka > > > Description > ------- > > Updated the RB with Gwen's comments, Beckett's comments and a subset of > Guozhang's comments > > > Diffs > ----- > > bin/kafka-run-class.sh 881f578a8f5c796fe23415b978c1ad35869af76e > core/src/main/scala/kafka/tools/ContinuousValidationTest.java PRE-CREATION > core/src/main/scala/kafka/utils/ShutdownableThread.scala > fc226c863095b7761290292cd8755cd7ad0f155c > system_test/broker_upgrade/bin/test-broker-upgrade.sh PRE-CREATION > > Diff: https://reviews.apache.org/r/30809/diff/ > > > Testing > ------- > > Scripted it to run 20 times without any failures. > Command-line: broker-upgrade/bin/test.sh <dir1> <dir2> > > > Thanks, > > Abhishek Nigam > >
