[GitHub] [kafka] tang7526 commented on pull request #10588: KAFKA-12662: add unit test for ProducerPerformance
tang7526 commented on pull request #10588: URL: https://github.com/apache/kafka/pull/10588#issuecomment-855235007 > @tang7526 thanks for your patch. Could you run `benchmark_test.py` to make sure this refactor does not make performance regression? @chia7712 Thank you for your opinion. I have run `benchmark_test.py`. The patch's result is almost the same as trunk. **Before** average run time is 147.171 seconds. average records per sec is 26.8866. average mb_per_sec is 2.564. **After** average run time is 146.313 seconds. average records per sec is 27.1467. average mb_per_sec is 2.589.  -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [kafka] tang7526 commented on pull request #10588: KAFKA-12662: add unit test for ProducerPerformance
tang7526 commented on pull request #10588: URL: https://github.com/apache/kafka/pull/10588#issuecomment-850770445 > > BTW, could you add tests for the code used to generate data? (https://github.com/apache/kafka/blob/trunk/tools/src/main/java/org/apache/kafka/tools/ProducerPerformance.java#L127) > > Ah, right! Thanks for reminder, @chia7712 ! > @tang7526 , this is the PR(#10469) to fix the random data generation. I agree we should add some tests to protect this change. FYI Thank you. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [kafka] tang7526 commented on pull request #10588: KAFKA-12662: add unit test for ProducerPerformance
tang7526 commented on pull request #10588: URL: https://github.com/apache/kafka/pull/10588#issuecomment-830735655 > @tang7526 , thanks for the patch! Could we add some tests for the `argParser`? ex: try pass unexpected arguments and verify the error thrown. Thanks. @showuon Thank you for your opinion. I have added a test for `argParser`. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
[GitHub] [kafka] tang7526 commented on pull request #10588: KAFKA-12662: add unit test for ProducerPerformance
tang7526 commented on pull request #10588: URL: https://github.com/apache/kafka/pull/10588#issuecomment-825051567 @chia7712 Could you help to review this PR? Thanks. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: us...@infra.apache.org
