> 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. > > Jiangjie Qin wrote: > 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.
I think what you bring about ack=1 and ack=-1 is a very good point but orthogonal to this discussion. This can be a config paramter and we could have different configurations as you pointed out. This is however not intended to be a performance test since there are other system tests which do that much better. LeaderNotAvailableException and UnkownHostOrTopicException are actual exceptions that I see in my logs every time I ran this test and it does not make sense to stop the producer thread in this case. To clarify the purpose of this test: a) Data integrity - old consumer can pull from the new broker without any issues. There is no data format change which will cause issues for the old consumers consuming from new broker. (This is tested through bootstrap) b) Data is received in order and this guarantee is not violated despite the rolling bounce. > 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. > > Jiangjie Qin wrote: > 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? Since the sender re-tries the message till it succeeds I am not sure how we would arrive at the issue you are mentioning but maybe I am missing something. - 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 > >
