Re: Review Request 30809: Patch for KAFKA-1888
On March 31, 2015, 9:20 p.m., Joel Koshy wrote: core/src/main/scala/kafka/tools/ContinuousValidationTest.java, line 1 https://reviews.apache.org/r/30809/diff/4/?file=903374#file903374line1 This should definitely not be in tools - this should probably live somewhere under clients/test. I don't think those are currently exported though, so we will need to modify build.gradle. However, per other comments below I'm not sure this should be part of system tests since it is (by definition long running). Will do. On March 31, 2015, 9:20 p.m., Joel Koshy wrote: core/src/main/scala/kafka/tools/ContinuousValidationTest.java, line 49 https://reviews.apache.org/r/30809/diff/4/?file=903374#file903374line49 It would help a lot if you could add comments describing what validation is done. For e.g., I'm unclear on why we need the complicated file-based signaling mechanism. So a high-level description would help a lot. More importantly, I really think we should separate continuous validation from broker upgrade which is the focus of KAFKA-1888 In order to do a broker upgrade test, we don't need any additional code. We just instantiate the producer performance and consumer via system test utils. Keep those on the old jar. The cluster will start with the old jar as well and during the test we bounce in the latest jar (the system test utils will need to be updated to support this). We then do the standard system test validation - that all messages sent were received. I wanted to have two (topic, partition) tuples with leader on each broker. I have decided to use a single topic with multiple partitions rather than using two topics which could have also worked. The reason for picking the first approach was that essentially if I wanted to leverage continuous validation test outside of system test framework with in a test cluster with other topics. In order to illustrate why the second approach won't work in that scenario is that if we have 3 brokers with one partition if I create 3 topics (T1P1, T2P1, T3P1) then the following would be a valid assignment based on existing broker assignment algorithm. B1B2 B3 T1P1 TXP1 TXP2 T2P1 TYP1 TYP2 T3P1 where TX and TY are other production topics running in that cluster. In this case all the leaders have landed on the same broker. However the first approach precludes this possibility. The file signalling was to workaround the fact that the most commonly used client does not have capability to consume from a particular partition. The way I have set it up the file signalling acts as a barrier. We make sure all the producer/consumer pairs have been instantiated with the hope being that they have talked to zookeeper and reserved their parition. Once both the consumers have been instantiated we expect themselves to have bound themselves to a particular partition we can now let the producers run in both the instances and this way we are assured that the consumer should never receive data from same producer. On March 31, 2015, 9:20 p.m., Joel Koshy wrote: core/src/main/scala/kafka/tools/ContinuousValidationTest.java, line 52 https://reviews.apache.org/r/30809/diff/4/?file=903374#file903374line52 This appears to be for rate-limiting the producer but can be more general than that. It would help to add a comment describing its purpose. Also, should probably be private This is a poor man's rate limiter as compared to guava rate limiter. I will make it private. On March 31, 2015, 9:20 p.m., Joel Koshy wrote: system_test/broker_upgrade/bin/test-broker-upgrade.sh, line 1 https://reviews.apache.org/r/30809/diff/4/?file=903376#file903376line1 This appears to be a one-off script to set up the test. This needs to be done within the system test framework which already has a number of utilities that do similar things. One other comment is that the patch is for an upgrade test, but I think it is a bit confusing to mix this with CVT. The continuous validation test will be useful outside of the system test framework. This was an attempt to leverage CVT in the system test setting. I think since strong objections have been raised against adopting this approach I will leave a comment on this patch accordingly. - Abhishek --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30809/#review78270 --- 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:
Re: Review Request 30809: Patch for KAFKA-1888
On April 2, 2015, 1:38 a.m., Jun Rao wrote: core/src/main/scala/kafka/tools/ContinuousValidationTest.java, lines 431-437 https://reviews.apache.org/r/30809/diff/4/?file=903374#file903374line431 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)? The data which is generated is very simple - increasing sequence of longs with timestamp. The producer keeps track of the newest sequence number, timestamp which it has sent. The consumer keeps track of the last sequence number and timestamp which it has received. The system test will interrupt the CVT and compare the sequence numbers between the producer and the sender. If they do not line up then it is an error. (If either the producer or consumer threads terminate un-expectedly before they have been interrupted it will be flagged as an error) If the test fails then the data logs from the producer and consumer are not removed and can be inspected. The idea behind putting the consumer and producer in the same JVM was orthogonal to system test and was in case it is used in a test cluster hosting other topics it makes easy to get hands on some things like delta etc. However, I think there is very strong objection to adopting this for system tests which are short-lived in nature. Unless there is support for the approach I have taken so far I plan to revert to the existing approach of spawning multiple JVMs for producer and consumer. I will change the bash script to be in python similar to what other system tests do. On April 2, 2015, 1:38 a.m., Jun Rao wrote: core/src/main/scala/kafka/tools/ContinuousValidationTest.java, lines 440-454 https://reviews.apache.org/r/30809/diff/4/?file=903374#file903374line440 Could we add a description of each command line option? I need to add more documentation. I will add this in. - Abhishek --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30809/#review78630 --- 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
Re: Review Request 30809: Patch for KAFKA-1888
--- 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
Re: Review Request 30809: Patch for KAFKA-1888
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30809/#review78270 --- bin/kafka-run-class.sh https://reviews.apache.org/r/30809/#comment126788 Can you remove this (and the echo that Ashish pointed out)? core/src/main/scala/kafka/tools/ContinuousValidationTest.java https://reviews.apache.org/r/30809/#comment126790 This should definitely not be in tools - this should probably live somewhere under clients/test. I don't think those are currently exported though, so we will need to modify build.gradle. However, per other comments below I'm not sure this should be part of system tests since it is (by definition long running). core/src/main/scala/kafka/tools/ContinuousValidationTest.java https://reviews.apache.org/r/30809/#comment126791 Can remove this core/src/main/scala/kafka/tools/ContinuousValidationTest.java https://reviews.apache.org/r/30809/#comment126822 It would help a lot if you could add comments describing what validation is done. For e.g., I'm unclear on why we need the complicated file-based signaling mechanism. So a high-level description would help a lot. More importantly, I really think we should separate continuous validation from broker upgrade which is the focus of KAFKA-1888 In order to do a broker upgrade test, we don't need any additional code. We just instantiate the producer performance and consumer via system test utils. Keep those on the old jar. The cluster will start with the old jar as well and during the test we bounce in the latest jar (the system test utils will need to be updated to support this). We then do the standard system test validation - that all messages sent were received. core/src/main/scala/kafka/tools/ContinuousValidationTest.java https://reviews.apache.org/r/30809/#comment126794 This appears to be for rate-limiting the producer but can be more general than that. It would help to add a comment describing its purpose. Also, should probably be private core/src/main/scala/kafka/tools/ContinuousValidationTest.java https://reviews.apache.org/r/30809/#comment126793 1 - one core/src/main/scala/kafka/tools/ContinuousValidationTest.java https://reviews.apache.org/r/30809/#comment126792 Stray println core/src/main/scala/kafka/tools/ContinuousValidationTest.java https://reviews.apache.org/r/30809/#comment126795 Stray println core/src/main/scala/kafka/tools/ContinuousValidationTest.java https://reviews.apache.org/r/30809/#comment126829 Can you use the logger formatter we use elsewhere? i.e., curly braces instead of an explicit String.format system_test/broker_upgrade/bin/test-broker-upgrade.sh https://reviews.apache.org/r/30809/#comment127013 This appears to be a one-off script to set up the test. This needs to be done within the system test framework which already has a number of utilities that do similar things. One other comment is that the patch is for an upgrade test, but I think it is a bit confusing to mix this with CVT. system_test/broker_upgrade/bin/test-broker-upgrade.sh https://reviews.apache.org/r/30809/#comment127010 may be better to just use mktemp for temporary files system_test/broker_upgrade/bin/test-broker-upgrade.sh https://reviews.apache.org/r/30809/#comment127011 should these (and below) be in basedir? That said I don't see this created anywhere. - Joel Koshy 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
Re: Review Request 30809: Patch for KAFKA-1888
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30809/#review77732 --- bin/kafka-run-class.sh https://reviews.apache.org/r/30809/#comment125922 Probably a leftover debug print. core/src/main/scala/kafka/tools/ContinuousValidationTest.java https://reviews.apache.org/r/30809/#comment125923 Not sure if this belongs to tools package. Any specific reason why we cannot use existing producer consumer tests' variants? Can they be extended to avoid creating yet another producer consumer test? system_test/broker_upgrade/bin/test-broker-upgrade.sh https://reviews.apache.org/r/30809/#comment125924 Relying on greps for validation is probably not the best way. system_test/broker_upgrade/bin/test-broker-upgrade.sh https://reviews.apache.org/r/30809/#comment125925 Probably, extract out in a upgrade method for reusability and readability. - Ashish Singh 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
Re: Review Request 30809: Patch for KAFKA-1888
--- 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) --- Updated the RB with Gwen's comments, Beckett's comments and a subset of Guozhang's comments Diffs (updated) - 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
Re: Review Request 30809: Patch for KAFKA-1888
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30809/#review76835 --- Also, I think we can move ValidatingProducer/Consumer and BoostrapConsumer to kafka.perf if we could do the refactoring based on Producer/ConsumerPerformance. - Guozhang Wang On March 9, 2015, 11:55 p.m., Abhishek Nigam wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30809/ --- (Updated March 9, 2015, 11:55 p.m.) Review request for kafka. Bugs: KAFKA-1888 https://issues.apache.org/jira/browse/KAFKA-1888 Repository: kafka Description --- Fixing the tests based on Mayuresh comments, code cleanup after proper IDE setup Diffs - build.gradle 0f0fe60a74542efa91a0e727146e896edcaa38af core/src/main/scala/kafka/tools/ContinuousValidationTest.java PRE-CREATION system_test/broker_upgrade/bin/kafka-run-class.sh PRE-CREATION system_test/broker_upgrade/bin/test.sh PRE-CREATION system_test/broker_upgrade/configs/server1.properties PRE-CREATION system_test/broker_upgrade/configs/server2.properties PRE-CREATION system_test/broker_upgrade/configs/zookeeper_source.properties 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
Re: Review Request 30809: Patch for KAFKA-1888
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30809/#review76157 --- This looks like a very good start. I think the framework is flexible enough to allow us to add a variety of upgrade tests. I'm looking forward to it. I have few comments, but mostly I'm still confused on how this will be used. Perhaps more comments or even a readme is in order You wrote that we invoke test.sh dir1 dir2, what should each directory contain? just the kafka jar of different versions? or an entire installation (including bin/ and conf/)? Which one of the directories should be the newer and which is older? does it matter? Which version of clients will be used. Perhaps a more descriptive name for test.sh can help too. I'm guessing we'll have a whole collection of those test scripts soon. Gwen build.gradle https://reviews.apache.org/r/30809/#comment123636 This should probably be a test dependency (if needed at all) Packaging Guava will be a pain, since so many systems use different versions of Guava and they are all incompatible. core/src/main/scala/kafka/tools/ContinuousValidationTest.java https://reviews.apache.org/r/30809/#comment123635 Do we really want to do this? We are using joptsimple for a bunch of other tools. It is easier to read, maintain, nice error messages, help screen, etc. system_test/broker_upgrade/bin/kafka-run-class.sh https://reviews.apache.org/r/30809/#comment123638 Why did we decide to duplicate this entire file? - Gwen Shapira On March 9, 2015, 11:55 p.m., Abhishek Nigam wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30809/ --- (Updated March 9, 2015, 11:55 p.m.) Review request for kafka. Bugs: KAFKA-1888 https://issues.apache.org/jira/browse/KAFKA-1888 Repository: kafka Description --- Fixing the tests based on Mayuresh comments, code cleanup after proper IDE setup Diffs - build.gradle 0f0fe60a74542efa91a0e727146e896edcaa38af core/src/main/scala/kafka/tools/ContinuousValidationTest.java PRE-CREATION system_test/broker_upgrade/bin/kafka-run-class.sh PRE-CREATION system_test/broker_upgrade/bin/test.sh PRE-CREATION system_test/broker_upgrade/configs/server1.properties PRE-CREATION system_test/broker_upgrade/configs/server2.properties PRE-CREATION system_test/broker_upgrade/configs/zookeeper_source.properties 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
Re: Review Request 30809: Patch for KAFKA-1888
On March 12, 2015, 12:13 a.m., Jiangjie Qin wrote: core/src/main/scala/kafka/tools/ContinuousValidationTest.java, line 183 https://reviews.apache.org/r/30809/diff/3/?file=889855#file889855line183 This is essentially a sync approach, can we use callback to do this? Abhishek Nigam wrote: This is intentional. We want to make sure the event has successfully reached the brokers. This enables us to form a reasonable expectation of what the consumer should expect. The callback should be able to make sure everything goes well otherwise you can chose stop sending or do whatever you want. One big issue about this approach is that you will only send a single message out for each batch, and that would be very slow especially if you don's set linger time to be some thing very small even zero. Ideally the test case should work with differnt producer settings, I would say at least ack=1 and ack=-1, also with different batch size and linger time. Sending a single message out for each batch does not look a very useful test case. On March 12, 2015, 12:13 a.m., Jiangjie Qin wrote: core/src/main/scala/kafka/tools/ContinuousValidationTest.java, line 184 https://reviews.apache.org/r/30809/diff/3/?file=889855#file889855line184 When a send fails, should we at least log the sequence number? Abhishek Nigam wrote: I log the exception and the logger gives me the timestamp in the logs. Maybe I am missing something. Can you explain the rationale of why we would want to log the sequence number on the producer side when send fails. Suppose someone is reading the log because something went wrong, wouldn't it be much faster to see how many and which messages are lost if you have sequence number logged? For example, with sequence number, you can see producer saying that I messge 1,2,3 sent successfully while message 4 failed. And consumer would say, I expect to see 1,2,3 but I just got 1,3. 2 is lost. In your current log, what you can see is just a message wasn't sent successfully, and one message was not consumed as expected. It's more complicated to debug, right? - Jiangjie --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30809/#review76173 --- On March 9, 2015, 11:55 p.m., Abhishek Nigam wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30809/ --- (Updated March 9, 2015, 11:55 p.m.) Review request for kafka. Bugs: KAFKA-1888 https://issues.apache.org/jira/browse/KAFKA-1888 Repository: kafka Description --- Fixing the tests based on Mayuresh comments, code cleanup after proper IDE setup Diffs - build.gradle 0f0fe60a74542efa91a0e727146e896edcaa38af core/src/main/scala/kafka/tools/ContinuousValidationTest.java PRE-CREATION system_test/broker_upgrade/bin/kafka-run-class.sh PRE-CREATION system_test/broker_upgrade/bin/test.sh PRE-CREATION system_test/broker_upgrade/configs/server1.properties PRE-CREATION system_test/broker_upgrade/configs/server2.properties PRE-CREATION system_test/broker_upgrade/configs/zookeeper_source.properties 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
Re: Review Request 30809: Patch for KAFKA-1888
On March 12, 2015, 12:13 a.m., Jiangjie Qin wrote: core/src/main/scala/kafka/tools/ContinuousValidationTest.java, line 183 https://reviews.apache.org/r/30809/diff/3/?file=889855#file889855line183 This is essentially a sync approach, can we use callback to do this? This is intentional. We want to make sure the event has successfully reached the brokers. This enables us to form a reasonable expectation of what the consumer should expect. On March 12, 2015, 12:13 a.m., Jiangjie Qin wrote: core/src/main/scala/kafka/tools/ContinuousValidationTest.java, line 184 https://reviews.apache.org/r/30809/diff/3/?file=889855#file889855line184 When a send fails, should we at least log the sequence number? I log the exception and the logger gives me the timestamp in the logs. Maybe I am missing something. Can you explain the rationale of why we would want to log the sequence number on the producer side when send fails. On March 12, 2015, 12:13 a.m., Jiangjie Qin wrote: core/src/main/scala/kafka/tools/ContinuousValidationTest.java, line 321 https://reviews.apache.org/r/30809/diff/3/?file=889855#file889855line321 Similar to producer, can we log the expected sequence number and the seq we actually saw? Sure in the cases where this a mismatch I could do that. On March 12, 2015, 12:13 a.m., Jiangjie Qin wrote: core/src/main/scala/kafka/tools/ContinuousValidationTest.java, line 386 https://reviews.apache.org/r/30809/diff/3/?file=889855#file889855line386 Can we use KafkaThread here? I will take a look at that. - Abhishek --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30809/#review76173 --- On March 9, 2015, 11:55 p.m., Abhishek Nigam wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30809/ --- (Updated March 9, 2015, 11:55 p.m.) Review request for kafka. Bugs: KAFKA-1888 https://issues.apache.org/jira/browse/KAFKA-1888 Repository: kafka Description --- Fixing the tests based on Mayuresh comments, code cleanup after proper IDE setup Diffs - build.gradle 0f0fe60a74542efa91a0e727146e896edcaa38af core/src/main/scala/kafka/tools/ContinuousValidationTest.java PRE-CREATION system_test/broker_upgrade/bin/kafka-run-class.sh PRE-CREATION system_test/broker_upgrade/bin/test.sh PRE-CREATION system_test/broker_upgrade/configs/server1.properties PRE-CREATION system_test/broker_upgrade/configs/server2.properties PRE-CREATION system_test/broker_upgrade/configs/zookeeper_source.properties 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
Re: Review Request 30809: Patch for KAFKA-1888
On March 11, 2015, 11:12 p.m., Gwen Shapira wrote: This looks like a very good start. I think the framework is flexible enough to allow us to add a variety of upgrade tests. I'm looking forward to it. I have few comments, but mostly I'm still confused on how this will be used. Perhaps more comments or even a readme is in order You wrote that we invoke test.sh dir1 dir2, what should each directory contain? just the kafka jar of different versions? or an entire installation (including bin/ and conf/)? Which one of the directories should be the newer and which is older? does it matter? Which version of clients will be used. Perhaps a more descriptive name for test.sh can help too. I'm guessing we'll have a whole collection of those test scripts soon. Gwen The directory containing the kafka jars. kafka_2.10-0.8.3-SNAPSHOT.jar kafka-clients-0.8.3-SNAPSHOT.jar The other jars are shared between both the kafka brokers. On March 11, 2015, 11:12 p.m., Gwen Shapira wrote: build.gradle, line 209 https://reviews.apache.org/r/30809/diff/3/?file=889854#file889854line209 This should probably be a test dependency (if needed at all) Packaging Guava will be a pain, since so many systems use different versions of Guava and they are all incompatible. Guava provides an excellent rate limiter which I am using in the test and have used in the past. When you talk about packaging we are already pulling in other external libraries like zookeeper with a specific version which the applications might be using extensively and might similarly run into conflicts. If you have a suggestion for a library which provides rate limiting(less popular) than guava then I can use that instead otherwise I will move this dependency to the test for now. On March 11, 2015, 11:12 p.m., Gwen Shapira wrote: core/src/main/scala/kafka/tools/ContinuousValidationTest.java, lines 409-440 https://reviews.apache.org/r/30809/diff/3/?file=889855#file889855line409 Do we really want to do this? We are using joptsimple for a bunch of other tools. It is easier to read, maintain, nice error messages, help screen, etc. Thanks, I will switch to jobOpts. On March 11, 2015, 11:12 p.m., Gwen Shapira wrote: system_test/broker_upgrade/bin/kafka-run-class.sh, lines 152-156 https://reviews.apache.org/r/30809/diff/3/?file=889856#file889856line152 Why did we decide to duplicate this entire file? The only difference is that it takes an additional argument which contains the directory from which the kafka jars should be pulled. Would you recommend adding it to the original script as an optional argument? - Abhishek --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30809/#review76157 --- On March 9, 2015, 11:55 p.m., Abhishek Nigam wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30809/ --- (Updated March 9, 2015, 11:55 p.m.) Review request for kafka. Bugs: KAFKA-1888 https://issues.apache.org/jira/browse/KAFKA-1888 Repository: kafka Description --- Fixing the tests based on Mayuresh comments, code cleanup after proper IDE setup Diffs - build.gradle 0f0fe60a74542efa91a0e727146e896edcaa38af core/src/main/scala/kafka/tools/ContinuousValidationTest.java PRE-CREATION system_test/broker_upgrade/bin/kafka-run-class.sh PRE-CREATION system_test/broker_upgrade/bin/test.sh PRE-CREATION system_test/broker_upgrade/configs/server1.properties PRE-CREATION system_test/broker_upgrade/configs/server2.properties PRE-CREATION system_test/broker_upgrade/configs/zookeeper_source.properties 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
Re: Review Request 30809: Patch for KAFKA-1888
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30809/#review76173 --- core/src/main/scala/kafka/tools/ContinuousValidationTest.java https://reviews.apache.org/r/30809/#comment123652 This is essentially a sync approach, can we use callback to do this? core/src/main/scala/kafka/tools/ContinuousValidationTest.java https://reviews.apache.org/r/30809/#comment123653 When a send fails, should we at least log the sequence number? core/src/main/scala/kafka/tools/ContinuousValidationTest.java https://reviews.apache.org/r/30809/#comment123659 Similar to producer, can we log the expected sequence number and the seq we actually saw? core/src/main/scala/kafka/tools/ContinuousValidationTest.java https://reviews.apache.org/r/30809/#comment123655 Can we use KafkaThread here? - Jiangjie Qin On March 9, 2015, 11:55 p.m., Abhishek Nigam wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30809/ --- (Updated March 9, 2015, 11:55 p.m.) Review request for kafka. Bugs: KAFKA-1888 https://issues.apache.org/jira/browse/KAFKA-1888 Repository: kafka Description --- Fixing the tests based on Mayuresh comments, code cleanup after proper IDE setup Diffs - build.gradle 0f0fe60a74542efa91a0e727146e896edcaa38af core/src/main/scala/kafka/tools/ContinuousValidationTest.java PRE-CREATION system_test/broker_upgrade/bin/kafka-run-class.sh PRE-CREATION system_test/broker_upgrade/bin/test.sh PRE-CREATION system_test/broker_upgrade/configs/server1.properties PRE-CREATION system_test/broker_upgrade/configs/server2.properties PRE-CREATION system_test/broker_upgrade/configs/zookeeper_source.properties 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
Re: Review Request 30809: Patch for KAFKA-1888
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30809/#review73382 --- build.gradle https://reviews.apache.org/r/30809/#comment119686 Additional dependency: core/src/main/scala/kafka/tools/ContinuousValidationTest.java https://reviews.apache.org/r/30809/#comment119687 Better have specific imports than wildcards. core/src/main/scala/kafka/tools/ContinuousValidationTest.java https://reviews.apache.org/r/30809/#comment119689 ..Test- + time? - Guozhang Wang On March 9, 2015, 11:55 p.m., Abhishek Nigam wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30809/ --- (Updated March 9, 2015, 11:55 p.m.) Review request for kafka. Bugs: KAFKA-1888 https://issues.apache.org/jira/browse/KAFKA-1888 Repository: kafka Description --- Fixing the tests based on Mayuresh comments, code cleanup after proper IDE setup Diffs - build.gradle 0f0fe60a74542efa91a0e727146e896edcaa38af core/src/main/scala/kafka/tools/ContinuousValidationTest.java PRE-CREATION system_test/broker_upgrade/bin/kafka-run-class.sh PRE-CREATION system_test/broker_upgrade/bin/test.sh PRE-CREATION system_test/broker_upgrade/configs/server1.properties PRE-CREATION system_test/broker_upgrade/configs/server2.properties PRE-CREATION system_test/broker_upgrade/configs/zookeeper_source.properties 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
Re: Review Request 30809: Patch for KAFKA-1888
On March 10, 2015, 11:44 p.m., Guozhang Wang wrote: This is from some old review comments, I will upload rest of them soon. - Guozhang --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30809/#review73382 --- On March 9, 2015, 11:55 p.m., Abhishek Nigam wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30809/ --- (Updated March 9, 2015, 11:55 p.m.) Review request for kafka. Bugs: KAFKA-1888 https://issues.apache.org/jira/browse/KAFKA-1888 Repository: kafka Description --- Fixing the tests based on Mayuresh comments, code cleanup after proper IDE setup Diffs - build.gradle 0f0fe60a74542efa91a0e727146e896edcaa38af core/src/main/scala/kafka/tools/ContinuousValidationTest.java PRE-CREATION system_test/broker_upgrade/bin/kafka-run-class.sh PRE-CREATION system_test/broker_upgrade/bin/test.sh PRE-CREATION system_test/broker_upgrade/configs/server1.properties PRE-CREATION system_test/broker_upgrade/configs/server2.properties PRE-CREATION system_test/broker_upgrade/configs/zookeeper_source.properties 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
Re: Review Request 30809: Patch for KAFKA-1888
On Feb. 18, 2015, 12:06 a.m., Mayuresh Gharat wrote: core/src/main/scala/kafka/tools/ContinuousValidationTest.java, line 400 https://reviews.apache.org/r/30809/diff/1/?file=859055#file859055line400 The common format of commenting is : // this is a comment Personally I don't mind, but thats kind of a standard that I understood from the reviews that I got. IDE setup was a little messed up. I looked up kafka coding guidelines and there does not seem to be anything about comments so made the indenting consistent using the IDE. - Abhishek --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30809/#review72786 --- On March 9, 2015, 11:55 p.m., Abhishek Nigam wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30809/ --- (Updated March 9, 2015, 11:55 p.m.) Review request for kafka. Bugs: KAFKA-1888 https://issues.apache.org/jira/browse/KAFKA-1888 Repository: kafka Description --- Fixing the tests based on Mayuresh comments, code cleanup after proper IDE setup Diffs - build.gradle 0f0fe60a74542efa91a0e727146e896edcaa38af core/src/main/scala/kafka/tools/ContinuousValidationTest.java PRE-CREATION system_test/broker_upgrade/bin/kafka-run-class.sh PRE-CREATION system_test/broker_upgrade/bin/test.sh PRE-CREATION system_test/broker_upgrade/configs/server1.properties PRE-CREATION system_test/broker_upgrade/configs/server2.properties PRE-CREATION system_test/broker_upgrade/configs/zookeeper_source.properties 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
Re: Review Request 30809: Patch for KAFKA-1888
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30809/ --- (Updated March 9, 2015, 11:55 p.m.) Review request for kafka. Bugs: KAFKA-1888 https://issues.apache.org/jira/browse/KAFKA-1888 Repository: kafka Description (updated) --- Fixing the tests based on Mayuresh comments, code cleanup after proper IDE setup Diffs (updated) - build.gradle 0f0fe60a74542efa91a0e727146e896edcaa38af core/src/main/scala/kafka/tools/ContinuousValidationTest.java PRE-CREATION system_test/broker_upgrade/bin/kafka-run-class.sh PRE-CREATION system_test/broker_upgrade/bin/test.sh PRE-CREATION system_test/broker_upgrade/configs/server1.properties PRE-CREATION system_test/broker_upgrade/configs/server2.properties PRE-CREATION system_test/broker_upgrade/configs/zookeeper_source.properties 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
Re: Review Request 30809: Patch for KAFKA-1888
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30809/#review73061 --- core/src/main/scala/kafka/tools/ContinuousValidationTest.java https://reviews.apache.org/r/30809/#comment119244 Flip is needed to reset the pointer to beginning of byte buffer. - Abhishek Nigam On March 9, 2015, 11:55 p.m., Abhishek Nigam wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30809/ --- (Updated March 9, 2015, 11:55 p.m.) Review request for kafka. Bugs: KAFKA-1888 https://issues.apache.org/jira/browse/KAFKA-1888 Repository: kafka Description --- Fixing the tests based on Mayuresh comments, code cleanup after proper IDE setup Diffs - build.gradle 0f0fe60a74542efa91a0e727146e896edcaa38af core/src/main/scala/kafka/tools/ContinuousValidationTest.java PRE-CREATION system_test/broker_upgrade/bin/kafka-run-class.sh PRE-CREATION system_test/broker_upgrade/bin/test.sh PRE-CREATION system_test/broker_upgrade/configs/server1.properties PRE-CREATION system_test/broker_upgrade/configs/server2.properties PRE-CREATION system_test/broker_upgrade/configs/zookeeper_source.properties 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
Re: Review Request 30809: Patch for KAFKA-1888
On Feb. 18, 2015, 12:06 a.m., Mayuresh Gharat wrote: core/src/main/scala/kafka/tools/ContinuousValidationTest.java, line 168 https://reviews.apache.org/r/30809/diff/1/?file=859055#file859055line168 same here can we use isInterrupted()? http://docs.oracle.com/javase/tutorial/essential/concurrency/interrupt.html I want to check if the current thread is interrupted. The link you sent out is useful if I wanted to query whether another thread was interrupted. On Feb. 18, 2015, 12:06 a.m., Mayuresh Gharat wrote: core/src/main/scala/kafka/tools/ContinuousValidationTest.java, line 280 https://reviews.apache.org/r/30809/diff/1/?file=859055#file859055line280 Can we put this in a separate method like init(). Constructor can be used mainly for assignment. what do you think? Moved the launching of the threads to an init method. On Feb. 18, 2015, 12:06 a.m., Mayuresh Gharat wrote: core/src/main/scala/kafka/tools/ContinuousValidationTest.java, line 298 https://reviews.apache.org/r/30809/diff/1/?file=859055#file859055line298 When is the blockingCallInterrupted set to true? Got rid of this. On Feb. 18, 2015, 12:06 a.m., Mayuresh Gharat wrote: core/src/main/scala/kafka/tools/ContinuousValidationTest.java, line 324 https://reviews.apache.org/r/30809/diff/1/?file=859055#file859055line324 formatting spaces. Will there be a case where : (evt.sequenceId lastEventSeenSequenceId.get() evt.eventProducedTimestamp lastEventSeenTimeProduced.get() This will happen when the sequence numbers wraparound. On Feb. 18, 2015, 12:06 a.m., Mayuresh Gharat wrote: core/src/main/scala/kafka/tools/ContinuousValidationTest.java, line 61 https://reviews.apache.org/r/30809/diff/1/?file=859055#file859055line61 Any reason for not making this final? static variables should come before Instance variables. Its a common standard to specify instance variables with _ like : _groupId. Fixed this. - Abhishek --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30809/#review72786 --- On March 9, 2015, 11:55 p.m., Abhishek Nigam wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30809/ --- (Updated March 9, 2015, 11:55 p.m.) Review request for kafka. Bugs: KAFKA-1888 https://issues.apache.org/jira/browse/KAFKA-1888 Repository: kafka Description --- Fixing the tests based on Mayuresh comments, code cleanup after proper IDE setup Diffs - build.gradle 0f0fe60a74542efa91a0e727146e896edcaa38af core/src/main/scala/kafka/tools/ContinuousValidationTest.java PRE-CREATION system_test/broker_upgrade/bin/kafka-run-class.sh PRE-CREATION system_test/broker_upgrade/bin/test.sh PRE-CREATION system_test/broker_upgrade/configs/server1.properties PRE-CREATION system_test/broker_upgrade/configs/server2.properties PRE-CREATION system_test/broker_upgrade/configs/zookeeper_source.properties 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
Re: Review Request 30809: Patch for KAFKA-1888
On Feb. 18, 2015, 12:06 a.m., Mayuresh Gharat wrote: core/src/main/scala/kafka/tools/ContinuousValidationTest.java, line 207 https://reviews.apache.org/r/30809/diff/1/?file=859055#file859055line207 This might end up in infinite loop if something goes wrong with cluster, right? Should we have a maximum numnber of retries? What do you think? This will not be an issue since for timed runs we will interrupt the thread anyway after a fixed time. This is the mode which is being used in the upgrade test. On Feb. 18, 2015, 12:06 a.m., Mayuresh Gharat wrote: core/src/main/scala/kafka/tools/ContinuousValidationTest.java, line 424 https://reviews.apache.org/r/30809/diff/1/?file=859055#file859055line424 Are you assuming that first argument will be some key? If you take a look at the script I am expecting alternate parameters like -timedRun -timeToSpawn Key is essentially the parameter name. On Feb. 18, 2015, 12:06 a.m., Mayuresh Gharat wrote: core/src/main/scala/kafka/tools/ContinuousValidationTest.java, line 474 https://reviews.apache.org/r/30809/diff/1/?file=859055#file859055line474 what do you mean by rebuild state later? What I meant was that between two runs for the rolling upgrade test we will not re-use any state from zookeeper or the brokers so I do not need to worry about clean shutdown. On Feb. 18, 2015, 12:06 a.m., Mayuresh Gharat wrote: core/src/main/scala/kafka/tools/ContinuousValidationTest.java, line 77 https://reviews.apache.org/r/30809/diff/1/?file=859055#file859055line77 Why we need a flip? The flip is needed to reset the get pointer in byte buffer to beginning of the byte buffer else we will get underflow exception. - Abhishek --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30809/#review72786 --- On Feb. 18, 2015, 1:59 a.m., Abhishek Nigam wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30809/ --- (Updated Feb. 18, 2015, 1:59 a.m.) Review request for kafka. Bugs: KAFKA-1888 https://issues.apache.org/jira/browse/KAFKA-1888 Repository: kafka Description --- patch for KAFKA-1888 Diffs - build.gradle 0f0fe60a74542efa91a0e727146e896edcaa38af core/src/main/scala/kafka/tools/ContinuousValidationTest.java PRE-CREATION system_test/broker_upgrade/bin/kafka-run-class.sh PRE-CREATION system_test/broker_upgrade/bin/test.sh PRE-CREATION system_test/broker_upgrade/configs/server1.properties PRE-CREATION system_test/broker_upgrade/configs/server2.properties PRE-CREATION system_test/broker_upgrade/configs/zookeeper_source.properties 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
Re: Review Request 30809: Patch for KAFKA-1888
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30809/#review72786 --- core/src/main/scala/kafka/tools/ContinuousValidationTest.java https://reviews.apache.org/r/30809/#comment118863 Can you remove this core/src/main/scala/kafka/tools/ContinuousValidationTest.java https://reviews.apache.org/r/30809/#comment118889 Any reason for not making this final? static variables should come before Instance variables. Its a common standard to specify instance variables with _ like : _groupId. core/src/main/scala/kafka/tools/ContinuousValidationTest.java https://reviews.apache.org/r/30809/#comment118890 same here : any reason for not making it final? core/src/main/scala/kafka/tools/ContinuousValidationTest.java https://reviews.apache.org/r/30809/#comment118913 Why we need a flip? core/src/main/scala/kafka/tools/ContinuousValidationTest.java https://reviews.apache.org/r/30809/#comment118940 same here can we use isInterrupted()? http://docs.oracle.com/javase/tutorial/essential/concurrency/interrupt.html core/src/main/scala/kafka/tools/ContinuousValidationTest.java https://reviews.apache.org/r/30809/#comment118933 This might end up in infinite loop if something goes wrong with cluster, right? Should we have a maximum numnber of retries? What do you think? core/src/main/scala/kafka/tools/ContinuousValidationTest.java https://reviews.apache.org/r/30809/#comment118941 You might want to do : Thread.currentThread().interrupt() . Then you might not require the blockingCallInterrupted flag : http://www.ibm.com/developerworks/library/j-jtp05236/ http://www.javamex.com/tutorials/threads/thread_interruption_2.shtml What do you think? core/src/main/scala/kafka/tools/ContinuousValidationTest.java https://reviews.apache.org/r/30809/#comment118911 Can we put this in a separate method like init(). Constructor can be used mainly for assignment. what do you think? core/src/main/scala/kafka/tools/ContinuousValidationTest.java https://reviews.apache.org/r/30809/#comment118915 When is the blockingCallInterrupted set to true? core/src/main/scala/kafka/tools/ContinuousValidationTest.java https://reviews.apache.org/r/30809/#comment118939 Thread.interrupted resets the interrupt flag. Can we use isInterrupted()? http://docs.oracle.com/javase/tutorial/essential/concurrency/interrupt.html core/src/main/scala/kafka/tools/ContinuousValidationTest.java https://reviews.apache.org/r/30809/#comment118925 formatting spaces. Will there be a case where : (evt.sequenceId lastEventSeenSequenceId.get() evt.eventProducedTimestamp lastEventSeenTimeProduced.get() core/src/main/scala/kafka/tools/ContinuousValidationTest.java https://reviews.apache.org/r/30809/#comment118885 The common format of commenting is : // this is a comment Personally I don't mind, but thats kind of a standard that I understood from the reviews that I got. core/src/main/scala/kafka/tools/ContinuousValidationTest.java https://reviews.apache.org/r/30809/#comment118893 Are you assuming that first argument will be some key? core/src/main/scala/kafka/tools/ContinuousValidationTest.java https://reviews.apache.org/r/30809/#comment118907 what do you mean by rebuild state later? - Mayuresh Gharat On Feb. 9, 2015, 11:53 p.m., Abhishek Nigam wrote: --- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30809/ --- (Updated Feb. 9, 2015, 11:53 p.m.) Review request for kafka. Bugs: KAFKA-1888 https://issues.apache.org/jira/browse/KAFKA-1888 Repository: kafka Description --- Essentially this test does the following: a) Start a java process with 3 threads Producer - producing continuously Consumer - consuming from latest Bootstrap consumer - started after a pause to bootstrap from beginning. It uses sequentially increasing numbers and timestamps to make sure we are not receiving out of order messages and do real-time validation. b) Script which wraps this and takes two directories which contain the kafka version specific jars: kafka_2.10-0.8.3-SNAPSHOT-test.jar kafka_2.10-0.8.3-SNAPSHOT.jar The first argument is the directory containing the older version of the jars. The second argument is the directory containing the newer version of the jars. The reason for choosing directories was because there are two jars in these directories: Diffs - build.gradle c3e6bb839ad65c512c9db4695d2bb49b82c80da5 core/src/main/scala/kafka/tools/ContinuousValidationTest.java PRE-CREATION
Re: Review Request 30809: Patch for KAFKA-1888
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30809/ --- (Updated Feb. 18, 2015, 1:59 a.m.) Review request for kafka. Bugs: KAFKA-1888 https://issues.apache.org/jira/browse/KAFKA-1888 Repository: kafka Description (updated) --- patch for KAFKA-1888 Diffs (updated) - build.gradle 0f0fe60a74542efa91a0e727146e896edcaa38af core/src/main/scala/kafka/tools/ContinuousValidationTest.java PRE-CREATION system_test/broker_upgrade/bin/kafka-run-class.sh PRE-CREATION system_test/broker_upgrade/bin/test.sh PRE-CREATION system_test/broker_upgrade/configs/server1.properties PRE-CREATION system_test/broker_upgrade/configs/server2.properties PRE-CREATION system_test/broker_upgrade/configs/zookeeper_source.properties 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
Re: Review Request 30809: Patch for KAFKA-1888
--- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/30809/ --- (Updated Feb. 9, 2015, 11:53 p.m.) Review request for kafka. Bugs: KAFKA-1888 https://issues.apache.org/jira/browse/KAFKA-1888 Repository: kafka Description (updated) --- Essentially this test does the following: a) Start a java process with 3 threads Producer - producing continuously Consumer - consuming from latest Bootstrap consumer - started after a pause to bootstrap from beginning. It uses sequentially increasing numbers and timestamps to make sure we are not receiving out of order messages and do real-time validation. b) Script which wraps this and takes two directories which contain the kafka version specific jars: kafka_2.10-0.8.3-SNAPSHOT-test.jar kafka_2.10-0.8.3-SNAPSHOT.jar The first argument is the directory containing the older version of the jars. The second argument is the directory containing the newer version of the jars. The reason for choosing directories was because there are two jars in these directories: Diffs - build.gradle c3e6bb839ad65c512c9db4695d2bb49b82c80da5 core/src/main/scala/kafka/tools/ContinuousValidationTest.java PRE-CREATION system_test/broker_upgrade/bin/kafka-run-class.sh PRE-CREATION system_test/broker_upgrade/bin/test.sh PRE-CREATION system_test/broker_upgrade/configs/server1.properties PRE-CREATION system_test/broker_upgrade/configs/server2.properties PRE-CREATION system_test/broker_upgrade/configs/zookeeper_source.properties PRE-CREATION Diff: https://reviews.apache.org/r/30809/diff/ Testing (updated) --- Scripted it to run 20 times without any failures. Command-line: broker-upgrade/bin/test.sh dir1 dir2 Thanks, Abhishek Nigam
