KAFKA-19624: Improving consistency of command-line arguments for consumer performance tests #20385

aheev · 2025-08-20T14:40:54Z

resolves https://issues.apache.org/jira/browse/KAFKA-19624

Reviewers: @brandboat, @AndrewJSchofield, @m1a2st

…umer performance tests

aheev · 2025-08-20T14:41:28Z

@AndrewJSchofield can you please review this?

brandboat · 2025-08-25T02:11:38Z

ping @aheev, could you please fix the conflicts?

# Conflicts: # tools/src/main/java/org/apache/kafka/tools/ConsumerPerformance.java # tools/src/test/java/org/apache/kafka/tools/ConsumerPerformanceTest.java

aheev · 2025-08-25T03:07:42Z

ping @aheev, could you please fix the conflicts?

done

brandboat

Thanks for the patch, left some comments below.

tools/src/main/java/org/apache/kafka/tools/ConsumerPerformance.java

tools/src/main/java/org/apache/kafka/tools/ShareConsumerPerformance.java

aheev · 2025-08-25T07:16:09Z

FAILED ❌ RestoreIntegrationTest > "shouldInvokeUserDefinedGlobalStateRestoreListener(boolean).useNewProtocol=false"

This test seems to be flaky. It succeeds on my machine and fails sometimes. Also the changes are not even related to the test

Just tested. Same issue on trunk too

tools/src/main/java/org/apache/kafka/tools/ConsumerPerformance.java

m1a2st

Thanks for this patch, some comments left

tools/src/main/java/org/apache/kafka/tools/ConsumerPerformance.java

AndrewJSchofield

Thanks for the PR. Please add some tests for the variation combinations of new/old options to make sure the rules in the KIP are correctly implemented.

tests/kafkatest/services/performance/consumer_performance.py

tests/kafkatest/services/performance/share_consumer_performance.py

tools/src/main/java/org/apache/kafka/tools/ConsumerPerformance.java

tools/src/main/java/org/apache/kafka/tools/ShareConsumerPerformance.java

…nd `messages` traces from the tools

brandboat

Thanks for the update, some minor comments.

tools/src/main/java/org/apache/kafka/tools/ShareConsumerPerformance.java

brandboat · 2025-08-27T09:03:50Z

tools/src/test/java/org/apache/kafka/tools/ConsumerPerformanceTest.java

@@ -136,7 +174,9 @@ public void testConfigWithoutTopicAndInclude() {

    @Test
    public void testClientIdOverride() throws IOException {
-        File tempFile = Files.createFile(tempDir.resolve("test_consumer_config.conf")).toFile();
+        Path configPath = tempDir.resolve("test_consumer_config.conf");
+        Files.deleteIfExists(configPath);


This is not required, tempDir will be cleanup automatically after junit test finished.
Same thing in other places.

While I was testing, a test ended abruptly and caused FileAlreadyExists exceptions on next runs. Hence, I have added this code

I see, we have multiple test files with the same name. We could give them distinct names—for example, by adding a random suffix or simply choosing different names. This is just my personal nit though.

Changed the file names to test_name_test_class.conf

tools/src/test/java/org/apache/kafka/tools/ConsumerPerformanceTest.java

tools/src/test/java/org/apache/kafka/tools/ShareConsumerPerformanceTest.java

brandboat

LGTM, thanks!

tools/src/test/java/org/apache/kafka/tools/ConsumerPerformanceTest.java

m1a2st

Overall, LGTM. Could you run the e2e test and share the results?

KAFKA-19487: Improving consistency of command-line arguments for cons…

931dfaf

…umer performance tests

github-actions bot added triage PRs from the community tools labels Aug 20, 2025

aheev changed the title ~~KAFKA-19487: Improving consistency of command-line arguments for consumer performance tests~~ KAFKA-19624: Improving consistency of command-line arguments for consumer performance tests Aug 20, 2025

AndrewJSchofield added ci-approved and removed triage PRs from the community labels Aug 22, 2025

Merge branch 'trunk' of https://github.com/apache/kafka into KAFKA-19487

17460c7

# Conflicts: # tools/src/main/java/org/apache/kafka/tools/ConsumerPerformance.java # tools/src/test/java/org/apache/kafka/tools/ConsumerPerformanceTest.java

brandboat reviewed Aug 25, 2025

View reviewed changes

tools/src/main/java/org/apache/kafka/tools/ConsumerPerformance.java Show resolved Hide resolved

tools/src/main/java/org/apache/kafka/tools/ShareConsumerPerformance.java Show resolved Hide resolved

Add deprecation cycle for num-records

34651a9

aheev requested a review from brandboat August 25, 2025 06:27

brandboat reviewed Aug 25, 2025

View reviewed changes

tools/src/main/java/org/apache/kafka/tools/ConsumerPerformance.java Outdated Show resolved Hide resolved

tools/src/main/java/org/apache/kafka/tools/ConsumerPerformance.java Outdated Show resolved Hide resolved

m1a2st reviewed Aug 25, 2025

View reviewed changes

tools/src/main/java/org/apache/kafka/tools/ConsumerPerformance.java Show resolved Hide resolved

tools/src/main/java/org/apache/kafka/tools/ConsumerPerformance.java Outdated Show resolved Hide resolved

AndrewJSchofield self-requested a review August 25, 2025 14:42

AndrewJSchofield requested changes Aug 25, 2025

View reviewed changes

aheev added 4 commits August 26, 2025 16:32

Merge branch 'trunk' of https://github.com/apache/kafka into KAFKA-19487

529eb36

Add deprecation signatures to old options; Remove consumer config a…

d82ce4f

…nd `messages` traces from the tools

Add checks for deprecated options

c4cd51e

Merge branch 'trunk' of https://github.com/apache/kafka into KAFKA-19487

b8b2dc4

aheev requested review from brandboat, m1a2st and AndrewJSchofield August 26, 2025 20:01

brandboat reviewed Aug 27, 2025

View reviewed changes

aheev added 2 commits August 27, 2025 14:55

Rename tests to old config names

5c8fd2b

Merge branch 'trunk' of https://github.com/apache/kafka into KAFKA-19487

d252457

brandboat approved these changes Aug 27, 2025

View reviewed changes

tools/src/test/java/org/apache/kafka/tools/ConsumerPerformanceTest.java Outdated Show resolved Hide resolved

Rename consumer config files to unique prefix-suffixes

9d9a8c6

m1a2st approved these changes Aug 27, 2025

View reviewed changes

KAFKA-19624: Improving consistency of command-line arguments for consumer performance tests #20385

Are you sure you want to change the base?

KAFKA-19624: Improving consistency of command-line arguments for consumer performance tests #20385

Conversation

aheev commented Aug 20, 2025 • edited by github-actions bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aheev commented Aug 20, 2025

Uh oh!

brandboat commented Aug 25, 2025

Uh oh!

aheev commented Aug 25, 2025

Uh oh!

brandboat left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

aheev commented Aug 25, 2025

Uh oh!

Uh oh!

Uh oh!

m1a2st left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

AndrewJSchofield left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

brandboat left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

brandboat Aug 27, 2025

Choose a reason for hiding this comment

Uh oh!

aheev Aug 27, 2025

Choose a reason for hiding this comment

Uh oh!

brandboat Aug 27, 2025

Choose a reason for hiding this comment

Uh oh!

aheev Aug 27, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

brandboat left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

m1a2st left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

aheev commented Aug 20, 2025 •

edited by github-actions bot

Loading