dajac commented on code in PR #13240:
URL: https://github.com/apache/kafka/pull/13240#discussion_r1131114261
##########
clients/src/main/java/org/apache/kafka/common/requests/OffsetCommitResponse.java:
##########
@@ -119,28 +131,52 @@ public boolean shouldClientThrottle(short version) {
return version >= 4;
}
+ public short version() {
+ return version;
+ }
+
public static class Builder {
OffsetCommitResponseData data = new OffsetCommitResponseData();
HashMap<String, OffsetCommitResponseTopic> byTopicName = new
HashMap<>();
+ private final TopicResolver topicResolver;
+ private final short version;
+
+ public Builder(TopicResolver topicResolver, short version) {
Review Comment:
How about doing the following?
We change the signature of `GroupCoordinator.handleCommitOffsets` to the
following:
```
def handleCommitOffsets(groupId: String,
memberId: String,
groupInstanceId: Option[String],
generationId: Int,
offsetMetadata: immutable.Map[TopicIdPartition,
OffsetAndMetadata],
responseCallback: immutable.Map[TopicIdPartition,
Errors] => Unit,
requestLocal: RequestLocal =
RequestLocal.NoCaching): Unit = {
```
Note the change from `TopicPartition` to `TopicIdPartition` for
`offsetMetadata` and `responseCallback`.
Then, we have to adapt the implementation of `handleCommitOffsets` to get
the `TopicPartition` from the `TopicIdPartition` where required. We can keep
`pendingOffsetCommits` and `offsets` keyed by `TopicPartition` for now in
`GroupMetadataManager`.
This allows the preservation of the topic ids provided to the
GroupCoordinator but it does not provide any stronger guarantee for the offsets
yet (as you pointed out). With this approach, we don't depend on the resolver
at all.
--
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-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
