smjn commented on code in PR #17656:
URL: https://github.com/apache/kafka/pull/17656#discussion_r1825854011
##########
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:
If you plan to use it then it should be fine. Otherwise, serves no purpose.
It can be argued that doing this causes additional problems, both with
FIND_COORD RPC as well as well as persistence RPCs.
If we forgo the topicName from both the `SharePartition` and the
ShareCoordinator side, we could directly invoke SPK.hashCode() method in the
`partitionFor` method thus completely abstracting out the reliance on
`groupId:topicId:partitionId`.
And as pointed out above we do not have access to the topic name in the
above RPCs.
--
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]
