apoorvmittal10 commented on code in PR #19602:
URL: https://github.com/apache/kafka/pull/19602#discussion_r2074223876
##########
tools/src/main/java/org/apache/kafka/tools/consumer/group/ShareGroupCommand.java:
##########
@@ -320,18 +321,29 @@ TreeMap<String, Entry<ShareGroupDescription,
Collection<SharePartitionOffsetInfo
groupSpecs.put(groupId, offsetsSpec);
try {
- Map<TopicPartition, Long> earliestResult =
adminClient.listShareGroupOffsets(groupSpecs).all().get().get(groupId);
+ Map<TopicPartition, OffsetAndMetadata> startOffsets =
adminClient.listShareGroupOffsets(groupSpecs).all().get().get(groupId);
Set<SharePartitionOffsetInformation> partitionOffsets =
new HashSet<>();
- for (Entry<TopicPartition, Long> tp :
earliestResult.entrySet()) {
- SharePartitionOffsetInformation partitionOffsetInfo =
new SharePartitionOffsetInformation(
- groupId,
- tp.getKey().topic(),
- tp.getKey().partition(),
-
Optional.ofNullable(earliestResult.get(tp.getKey()))
- );
- partitionOffsets.add(partitionOffsetInfo);
+ for (Entry<TopicPartition, OffsetAndMetadata> tp :
startOffsets.entrySet()) {
+ OffsetAndMetadata offset =
startOffsets.get(tp.getKey());
Review Comment:
ni: Please feel free to ignore.
Do you think `offsetAndMetadata` would be a better variable name? Also why
not to use `tp.getValue()`, instead of fetching back from map? Also would be
better to have `forEach` over map to get tp and offsetAndMetadata directly for
usage.
##########
clients/src/main/resources/common/message/ReadShareGroupStateSummaryResponse.json:
##########
@@ -41,6 +41,8 @@
"about": "The error message, or null if there was no error." },
{ "name": "StateEpoch", "type": "int32", "versions": "0+",
"about": "The state epoch of the share-partition." },
+ { "name": "LeaderEpoch", "type": "int32", "versions": "0+",
+ "about": "The leader epoch of the share-partition." },
Review Comment:
Maybe it was not used in AK 4.0 and shouldn't impact 4.0 clients, is that
correct understading?
##########
clients/src/main/resources/common/message/ReadShareGroupStateSummaryResponse.json:
##########
@@ -41,6 +41,8 @@
"about": "The error message, or null if there was no error." },
{ "name": "StateEpoch", "type": "int32", "versions": "0+",
"about": "The state epoch of the share-partition." },
+ { "name": "LeaderEpoch", "type": "int32", "versions": "0+",
+ "about": "The leader epoch of the share-partition." },
Review Comment:
As the json was part of AK 4.0 as well so do we need to bump the version?
--
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]
