smjn commented on code in PR #17656:
URL: https://github.com/apache/kafka/pull/17656#discussion_r1825566446
##########
share-coordinator/src/main/java/org/apache/kafka/coordinator/share/ShareCoordinatorService.java:
##########
@@ -539,7 +539,7 @@ public void onNewMetadataImage(MetadataImage newImage,
MetadataDelta delta) {
}
private TopicPartition topicPartitionFor(SharePartitionKey key) {
- return new TopicPartition(Topic.SHARE_GROUP_STATE_TOPIC_NAME,
partitionFor(key.toString()));
+ return new TopicPartition(Topic.SHARE_GROUP_STATE_TOPIC_NAME,
partitionFor(key.asCoordinatorKey()));
Review Comment:
@mumrah Changing `partitionFor` would be counter productive, IMO. This is
because it will be invoked by the KafkaApis FIND_COORD API handler.
The find coordinator request comprises of the coordinator keys as Strings
(as its a JSON defined RPC spec). The find coordinator handler will simply
invoke `partitionFor` with the key specified in the request argument.
If we change `partitionFor` signature to accept the `SharePartitionKey`, we
would not have enough information in FIND_COORD API handler to create a
`SharePartitionKey` object and call `partitionFor` unless we resort to
splitting and formatting string in the request and creating the required object
and pass to `partitionFor`.
cc: @AndrewJSchofield
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
