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/02/08 14:12:10 UTC

[GitHub] [kafka] chia7712 opened a new pull request #10082: MINOR: use 'mapKey' to avoid unnecessary grouped data

chia7712 opened a new pull request #10082:
URL: https://github.com/apache/kafka/pull/10082


   as title. Thanks to great KRPC :)
   
   ### 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.

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



[GitHub] [kafka] chia7712 commented on a change in pull request #10082: MINOR: use 'mapKey' to avoid unnecessary grouped data

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



##########
File path: clients/src/main/resources/common/message/ElectLeadersRequest.json
##########
@@ -28,7 +28,7 @@
     { "name": "TopicPartitions", "type": "[]TopicPartitions", "versions": "0+", "nullableVersions": "0+",
       "about": "The topic partitions to elect leaders.",
       "fields": [
-        { "name": "Topic", "type": "string", "versions": "0+", "entityType": "topicName",
+        { "name": "Topic", "type": "string", "versions": "0+", "entityType": "topicName", "mapKey": true,
           "about": "The name of a topic." },
         { "name": "PartitionId", "type": "[]int32", "versions": "0+",

Review comment:
       make sense




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [kafka] dajac commented on a change in pull request #10082: MINOR: use 'mapKey' to avoid unnecessary grouped data

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



##########
File path: clients/src/main/resources/common/message/ElectLeadersRequest.json
##########
@@ -28,7 +28,7 @@
     { "name": "TopicPartitions", "type": "[]TopicPartitions", "versions": "0+", "nullableVersions": "0+",
       "about": "The topic partitions to elect leaders.",
       "fields": [
-        { "name": "Topic", "type": "string", "versions": "0+", "entityType": "topicName",
+        { "name": "Topic", "type": "string", "versions": "0+", "entityType": "topicName", "mapKey": true,
           "about": "The name of a topic." },
         { "name": "PartitionId", "type": "[]int32", "versions": "0+",

Review comment:
       nit: While we are here, should we rename this field to `Partitions`? I was confused by this while reading the code. When I read `PartitionId`, I don't really expect an array.




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [kafka] chia7712 commented on pull request #10082: MINOR: use 'mapKey' to avoid unnecessary grouped data

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


   the error is traced by #10024


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [kafka] ijuma commented on pull request #10082: MINOR: use 'mapKey' to avoid unnecessary grouped data

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


   Looks like ConsumerProtocolAssignment was also changed to have mapKey, so worth updating the PR to include that information.


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [kafka] ijuma commented on a change in pull request #10082: MINOR: use 'mapKey' to avoid unnecessary grouped data

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



##########
File path: clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java
##########
@@ -2581,10 +2581,10 @@ public DescribeReplicaLogDirsResult describeReplicaLogDirs(Collection<TopicParti
                 List<Integer> partitionIndex = new ArrayList<>();

Review comment:
       Change this variable name too?




----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [kafka] chia7712 commented on pull request #10082: MINOR: use 'mapKey' to avoid unnecessary grouped data

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


   > Looks like ConsumerProtocolAssignment was also changed to have mapKey, so worth updating the PR to include that information.
   
   done


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [kafka] chia7712 merged pull request #10082: MINOR: use 'mapKey' to avoid unnecessary grouped data

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


   


----------------------------------------------------------------
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:
users@infra.apache.org



[GitHub] [kafka] dajac commented on a change in pull request #10082: MINOR: use 'mapKey' to avoid unnecessary grouped data

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



##########
File path: clients/src/main/java/org/apache/kafka/clients/admin/KafkaAdminClient.java
##########
@@ -2581,10 +2581,10 @@ public DescribeReplicaLogDirsResult describeReplicaLogDirs(Collection<TopicParti
                 List<Integer> partitionIndex = new ArrayList<>();

Review comment:
       I do agree.




----------------------------------------------------------------
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:
users@infra.apache.org