You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "mingyen066 (via GitHub)" <gi...@apache.org> on 2025/05/23 09:15:58 UTC
[PR] HOTFIX: Fix GroupMetadataManager to use InitMapValue in addInitializingTopicsRecords [kafka]
mingyen066 opened a new pull request, #19792:
URL: https://github.com/apache/kafka/pull/19792
Delete this text and replace it with a detailed description of your change. The
PR title and body will become the squashed commit message.
If you would like to tag individuals, add some commentary, upload images, or
include other supplemental information that should not be part of the eventual
commit message, please use a separate comment.
If applicable, please include a summary of the testing strategy (including
rationale) for the proposed change. Unit and/or integration tests are expected
for any behavior change and system tests should be considered for larger
changes.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HOTFIX: Fix GroupMetadataManager#completeAlterShareGroupOffsets to use InitMapValue in addInitializingTopicsRecords [kafka]
Posted by "smjn (via GitHub)" <gi...@apache.org>.
smjn commented on code in PR #19792:
URL: https://github.com/apache/kafka/pull/19792#discussion_r2104313411
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupMetadataManager.java:
##########
@@ -8136,10 +8137,14 @@ public Map.Entry<AlterShareGroupOffsetsResponseData, InitializeShareGroupStatePa
}
});
- initializingTopics.put(topicId, topic.partitions().stream()
- .map(AlterShareGroupOffsetsRequestData.AlterShareGroupOffsetsRequestPartition::partitionIndex)
- .filter(existingPartitions::contains)
- .collect(Collectors.toSet()));
+ initializingTopics.put(topicId, new InitMapValue(
Review Comment:
Not required:
can call attachInitValues on the map.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HOTFIX: Fix GroupMetadataManager to use InitMapValue in addInitializingTopicsRecords [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #19792:
URL: https://github.com/apache/kafka/pull/19792#discussion_r2104202970
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupMetadataManager.java:
##########
@@ -8136,10 +8136,14 @@ public Map.Entry<AlterShareGroupOffsetsResponseData, InitializeShareGroupStatePa
}
});
- initializingTopics.put(topicId, topic.partitions().stream()
- .map(AlterShareGroupOffsetsRequestData.AlterShareGroupOffsetsRequestPartition::partitionIndex)
- .filter(existingPartitions::contains)
- .collect(Collectors.toSet()));
+ initializingTopics.put(topicId, new InitMapValue(
+ topic.topicName(),
+ topic.partitions().stream()
+ .map(AlterShareGroupOffsetsRequestData.AlterShareGroupOffsetsRequestPartition::partitionIndex)
+ .filter(existingPartitions::contains)
+ .collect(Collectors.toSet()),
+ time.milliseconds()
Review Comment:
Those initializing topics should have same timestamp, right?
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HOTFIX: Fix GroupMetadataManager#completeAlterShareGroupOffsets to use InitMapValue in addInitializingTopicsRecords [kafka]
Posted by "apoorvmittal10 (via GitHub)" <gi...@apache.org>.
apoorvmittal10 merged PR #19792:
URL: https://github.com/apache/kafka/pull/19792
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HOTFIX: Fix GroupMetadataManager#completeAlterShareGroupOffsets to use InitMapValue in addInitializingTopicsRecords [kafka]
Posted by "Yunyung (via GitHub)" <gi...@apache.org>.
Yunyung commented on code in PR #19792:
URL: https://github.com/apache/kafka/pull/19792#discussion_r2104219802
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupMetadataManager.java:
##########
@@ -8136,10 +8136,15 @@ public Map.Entry<AlterShareGroupOffsetsResponseData, InitializeShareGroupStatePa
}
});
- initializingTopics.put(topicId, topic.partitions().stream()
- .map(AlterShareGroupOffsetsRequestData.AlterShareGroupOffsetsRequestPartition::partitionIndex)
- .filter(existingPartitions::contains)
- .collect(Collectors.toSet()));
+ long curTimestamp = time.milliseconds();
Review Comment:
Move time.milliseconds() out of the loop so that all iterations have the same timestamp.
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HOTFIX: Fix GroupMetadataManager#completeAlterShareGroupOffsets to use InitMapValue in addInitializingTopicsRecords [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #19792:
URL: https://github.com/apache/kafka/pull/19792#discussion_r2104333197
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/GroupMetadataManager.java:
##########
@@ -8136,10 +8137,14 @@ public Map.Entry<AlterShareGroupOffsetsResponseData, InitializeShareGroupStatePa
}
});
- initializingTopics.put(topicId, topic.partitions().stream()
- .map(AlterShareGroupOffsetsRequestData.AlterShareGroupOffsetsRequestPartition::partitionIndex)
- .filter(existingPartitions::contains)
- .collect(Collectors.toSet()));
+ initializingTopics.put(topicId, new InitMapValue(
Review Comment:
The current version is better because `initializingTopics` already contains complete information (ID and name), and `attachInitValue` would create an additional map. WDYT?
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HOTFIX: Fix GroupMetadataManager#completeAlterShareGroupOffsets to use InitMapValue in addInitializingTopicsRecords [kafka]
Posted by "apoorvmittal10 (via GitHub)" <gi...@apache.org>.
apoorvmittal10 commented on PR #19792:
URL: https://github.com/apache/kafka/pull/19792#issuecomment-2904214519
@smjn I have merged the PR for fixing the build. We can discuss the right approach in your PR as there is this [open](https://github.com/apache/kafka/pull/19792#discussion_r2104333197) question on the comment
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HOTFIX: Fix GroupMetadataManager#completeAlterShareGroupOffsets to use InitMapValue in addInitializingTopicsRecords [kafka]
Posted by "smjn (via GitHub)" <gi...@apache.org>.
smjn commented on PR #19792:
URL: https://github.com/apache/kafka/pull/19792#issuecomment-2904025640
https://github.com/apache/kafka/pull/19794
--
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-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org