You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2021/07/02 02:16:36 UTC

[GitHub] [kafka] jeffkbkim opened a new pull request #10955: KafkaAdminClient getListOffsetsCalls benchmark

jeffkbkim opened a new pull request #10955:
URL: https://github.com/apache/kafka/pull/10955


   new benchmark to test KafkaAdminClient.getListOffsetsCalls
   originated from https://github.com/apache/kafka/pull/10940
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   


-- 
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.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] jeffkbkim commented on pull request #10955: KafkaAdminClient getListOffsetsCalls benchmark

Posted by GitBox <gi...@apache.org>.
jeffkbkim commented on pull request #10955:
URL: https://github.com/apache/kafka/pull/10955#issuecomment-872667997


   ran benchmark with warm up = 5, iteration = 5
   
   With changes:
   ```
   Benchmark                                             (partitionCount)  (topicCount)  Mode  Cnt       Score       Error  Units
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls               100             1  avgt    5      40.040 ±     3.506  us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls               100            10  avgt    5     474.825 ±    95.547  us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls              1000             1  avgt    5     429.767 ±    60.605  us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls              1000            10  avgt    5    5069.417 ±   662.226  us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls             10000             1  avgt    5    6000.379 ±   235.654  us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls             10000            10  avgt    5  121240.173 ± 49894.963  us/op
   ```
   
   Without changes:
   ```
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls partition=100,       topic=1: Iteration   5: 4860.989 us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls partition=100,     topic=10: Iteration   1: 424064.225 us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls partition=1000,     topic=1: Iteration   5: 452289.400 us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls partition=1000,   topic=10: Iteration   2: 49306777.392 us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls partition=10000,   topic=1: Iteration   5: 53940393.545 us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls partition=10000, topic=10: Warm Up  1: 14664107350.721 us/op
   ```
   Running the benchmark pre-https://github.com/apache/kafka/pull/10940 took too long to finish. included the median result for each topic-partition pair. case for partition=10000, topic=10 took too long during warm up (each take +4 hours).
   


-- 
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.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] jeffkbkim edited a comment on pull request #10955: KafkaAdminClient getListOffsetsCalls benchmark

Posted by GitBox <gi...@apache.org>.
jeffkbkim edited a comment on pull request #10955:
URL: https://github.com/apache/kafka/pull/10955#issuecomment-872667997






-- 
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.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] dajac merged pull request #10955: MINOR: Add `KafkaAdminClient.getListOffsetsCalls` benchmark

Posted by GitBox <gi...@apache.org>.
dajac merged pull request #10955:
URL: https://github.com/apache/kafka/pull/10955


   


-- 
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.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] jeffkbkim edited a comment on pull request #10955: KafkaAdminClient getListOffsetsCalls benchmark

Posted by GitBox <gi...@apache.org>.
jeffkbkim edited a comment on pull request #10955:
URL: https://github.com/apache/kafka/pull/10955#issuecomment-872667997


   ran benchmark with warm up = 5, iteration = 5
   
   With changes:
   ```
   Benchmark                                             (partitionCount)  (topicCount)  Mode  Cnt       Score       Error  Units
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls               100             1  avgt    5      40.040 ±     3.506  us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls               100            10  avgt    5     474.825 ±    95.547  us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls              1000             1  avgt    5     429.767 ±    60.605  us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls              1000            10  avgt    5    5069.417 ±   662.226  us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls             10000             1  avgt    5    6000.379 ±   235.654  us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls             10000            10  avgt    5  121240.173 ± 49894.963  us/op
   ```
   
   Without changes:
   ```
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls partition=100,     topic=1: Iteration   5: 4860.989 us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls partition=100,    topic=10: Iteration   1: 424064.225 us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls partition=1000,    topic=1: Iteration   5: 452289.400 us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls partition=1000,  topic=10: Iteration   2: 49306777.392 us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls partition=10000,  topic=1: Iteration   5: 53940393.545 us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls partition=10000, topic=10: Warm Up  1: 14664107350.721 us/op
   ```
   Running the benchmark pre-https://github.com/apache/kafka/pull/10940 took too long to finish. included the median result for each topic-partition pair. case for partition=10000, topic=10 took too long during warm up (each take +4 hours).
   


-- 
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.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] dajac commented on a change in pull request #10955: KafkaAdminClient getListOffsetsCalls benchmark

Posted by GitBox <gi...@apache.org>.
dajac commented on a change in pull request #10955:
URL: https://github.com/apache/kafka/pull/10955#discussion_r664771393



##########
File path: clients/src/test/java/org/apache/kafka/clients/admin/AdminClientTestUtils.java
##########
@@ -102,4 +104,15 @@ public static DescribeTopicsResult describeTopicsResult(Map<String, TopicDescrip
     public static ListConsumerGroupOffsetsResult listConsumerGroupOffsetsResult(Map<TopicPartition, OffsetAndMetadata> offsets) {
         return new ListConsumerGroupOffsetsResult(KafkaFuture.completedFuture(offsets));
     }
+
+    /**
+     * Used for benchmark. KafkaAdminClient.getListOffsetsCalls is only accessible
+     * from within the admin package.
+     */
+    public static List<KafkaAdminClient.Call> getListOffsetsCalls(KafkaAdminClient adminClient, 

Review comment:
       Could we align the arguments?




-- 
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.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] jeffkbkim commented on pull request #10955: KafkaAdminClient getListOffsetsCalls benchmark

Posted by GitBox <gi...@apache.org>.
jeffkbkim commented on pull request #10955:
URL: https://github.com/apache/kafka/pull/10955#issuecomment-874952400


   @dajac thank you for the review and suggestion! i have addressed your comments.


-- 
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.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] dajac commented on pull request #10955: KafkaAdminClient getListOffsetsCalls benchmark

Posted by GitBox <gi...@apache.org>.
dajac commented on pull request #10955:
URL: https://github.com/apache/kafka/pull/10955#issuecomment-873889722


   @jeffkbkim We could add a `getListOffsetsCalls` method to `AdminClientTestUtils`, which is in the same package as the `KafkaAdminClient`, to access the package private method from the benchmark.


-- 
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.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] jeffkbkim edited a comment on pull request #10955: KafkaAdminClient getListOffsetsCalls benchmark

Posted by GitBox <gi...@apache.org>.
jeffkbkim edited a comment on pull request #10955:
URL: https://github.com/apache/kafka/pull/10955#issuecomment-872667997


   ran benchmark with warm up = 5, iteration = 5
   
   With changes:
   ```
   Benchmark                                             (partitionCount)  (topicCount)  Mode  Cnt       Score       Error  Units
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls               100             1  avgt    5      40.040 ±     3.506  us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls               100            10  avgt    5     474.825 ±    95.547  us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls              1000             1  avgt    5     429.767 ±    60.605  us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls              1000            10  avgt    5    5069.417 ±   662.226  us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls             10000             1  avgt    5    6000.379 ±   235.654  us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls             10000            10  avgt    5  121240.173 ± 49894.963  us/op
   ```
   
   Without changes:
   ```
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls partition=100,    topic=1: Iteration   5: 4860.989 us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls partition=100,   topic=10: Iteration   1: 424064.225 us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls partition=1000,   topic=1: Iteration   5: 452289.400 us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls partition=1000,  topic=10: Iteration   2: 49306777.392 us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls partition=10000,  topic=1: Iteration   5: 53940393.545 us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls partition=10000, topic=10: Warm Up Iter 1: 14664107350.721 us/op
   ```
   Running the benchmark pre-https://github.com/apache/kafka/pull/10940 took too long to finish. included the median result for each topic-partition pair. case for partition=10000, topic=10 took too long during warm up (each take +4 hours).
   


-- 
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.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] dajac commented on a change in pull request #10955: KafkaAdminClient getListOffsetsCalls benchmark

Posted by GitBox <gi...@apache.org>.
dajac commented on a change in pull request #10955:
URL: https://github.com/apache/kafka/pull/10955#discussion_r663716647



##########
File path: clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java
##########
@@ -3677,7 +3677,8 @@ public ListOffsetsResult listOffsets(Map<TopicPartition, OffsetSpec> topicPartit
         return new ListOffsetsResult(new HashMap<>(futures));
     }
 
-    private List<Call> getListOffsetsCalls(MetadataOperationContext<ListOffsetsResultInfo, ListOffsetsOptions> context,
+    // visible for benchmark
+    List<Call> getListOffsetsCalls(MetadataOperationContext<ListOffsetsResultInfo, ListOffsetsOptions> context,

Review comment:
       nit: Could we realign the arguments?




-- 
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.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] dajac commented on a change in pull request #10955: KafkaAdminClient getListOffsetsCalls benchmark

Posted by GitBox <gi...@apache.org>.
dajac commented on a change in pull request #10955:
URL: https://github.com/apache/kafka/pull/10955#discussion_r663716647



##########
File path: clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java
##########
@@ -3677,7 +3677,8 @@ public ListOffsetsResult listOffsets(Map<TopicPartition, OffsetSpec> topicPartit
         return new ListOffsetsResult(new HashMap<>(futures));
     }
 
-    private List<Call> getListOffsetsCalls(MetadataOperationContext<ListOffsetsResultInfo, ListOffsetsOptions> context,
+    // visible for benchmark
+    List<Call> getListOffsetsCalls(MetadataOperationContext<ListOffsetsResultInfo, ListOffsetsOptions> context,

Review comment:
       nit: Could we realign the arguments?




-- 
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.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] dajac commented on a change in pull request #10955: KafkaAdminClient getListOffsetsCalls benchmark

Posted by GitBox <gi...@apache.org>.
dajac commented on a change in pull request #10955:
URL: https://github.com/apache/kafka/pull/10955#discussion_r662815972



##########
File path: clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java
##########
@@ -3677,7 +3677,8 @@ public ListOffsetsResult listOffsets(Map<TopicPartition, OffsetSpec> topicPartit
         return new ListOffsetsResult(new HashMap<>(futures));
     }
 
-    private List<Call> getListOffsetsCalls(MetadataOperationContext<ListOffsetsResultInfo, ListOffsetsOptions> context,
+    // visible for benchmark
+    public List<Call> getListOffsetsCalls(MetadataOperationContext<ListOffsetsResultInfo, ListOffsetsOptions> context,

Review comment:
       I am a bit concerned by making the method public here. `KafkaAdminClient` is directly used by our users so there is a risk that they start calling this method. I wonder if we could at least keep it package private.




-- 
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.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] jeffkbkim edited a comment on pull request #10955: KafkaAdminClient getListOffsetsCalls benchmark

Posted by GitBox <gi...@apache.org>.
jeffkbkim edited a comment on pull request #10955:
URL: https://github.com/apache/kafka/pull/10955#issuecomment-872667997


   ran benchmark with warm up = 5, iteration = 5
   
   With changes:
   ```
   Benchmark                                             (partitionCount)  (topicCount)  Mode  Cnt       Score       Error  Units
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls               100             1  avgt    5      40.040 ±     3.506  us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls               100            10  avgt    5     474.825 ±    95.547  us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls              1000             1  avgt    5     429.767 ±    60.605  us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls              1000            10  avgt    5    5069.417 ±   662.226  us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls             10000             1  avgt    5    6000.379 ±   235.654  us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls             10000            10  avgt    5  121240.173 ± 49894.963  us/op
   ```
   
   Without changes:
   ```
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls partition=100,    topic=1: Iteration   5: 4860.989 us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls partition=100,   topic=10: Iteration   1: 424064.225 us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls partition=1000,   topic=1: Iteration   5: 452289.400 us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls partition=1000,  topic=10: Iteration   2: 49306777.392 us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls partition=10000,  topic=1: Iteration   5: 53940393.545 us/op
   GetListOffsetsCallsBenchmark.testGetListOffsetsCalls partition=10000, topic=10: Warm Up  1: 14664107350.721 us/op
   ```
   Running the benchmark pre-https://github.com/apache/kafka/pull/10940 took too long to finish. included the median result for each topic-partition pair. case for partition=10000, topic=10 took too long during warm up (each take +4 hours).
   


-- 
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.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [kafka] dajac commented on pull request #10955: KafkaAdminClient getListOffsetsCalls benchmark

Posted by GitBox <gi...@apache.org>.
dajac commented on pull request #10955:
URL: https://github.com/apache/kafka/pull/10955#issuecomment-873889722


   @jeffkbkim We could add a `getListOffsetsCalls` method to `AdminClientTestUtils`, which is in the same package as the `KafkaAdminClient`, to access the package private method from the benchmark.


-- 
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.

To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org