dajac commented on code in PR #23888:
URL: https://github.com/apache/kafka/pull/23888#discussion_r4334204382
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/modern/share/ShareGroup.java:
##########
Review Comment:
While we are here, should we ensure that the two Maps and the Set are
immutable too?
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/modern/share/ShareGroup.java:
##########
@@ -97,6 +99,33 @@ public record InitMapValue(
Set<Integer> partitions,
long timestamp
) {
+ public InitMapValue {
+ partitions = Set.copyOf(partitions);
+ }
+
+ public InitMapValue(String name, Collection<Integer> partitions, long
timestamp) {
+ this(name, Set.copyOf(partitions), timestamp);
+ }
+
+ /**
+ * @param added The partitions to add.
+ * @return A copy of this value with the partitions added.
+ */
+ public InitMapValue withAddedPartitions(Collection<Integer> added) {
+ Set<Integer> newPartitions = new HashSet<>(partitions);
+ newPartitions.addAll(added);
+ return new InitMapValue(name, newPartitions, timestamp);
Review Comment:
It is a tad wasteful to allocate an HashSet here to then copy it in
InitMapValue. Have you considered using unmodifiableSet?
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/modern/share/ShareGroup.java:
##########
@@ -97,6 +99,33 @@ public record InitMapValue(
Set<Integer> partitions,
long timestamp
) {
+ public InitMapValue {
+ partitions = Set.copyOf(partitions);
+ }
+
+ public InitMapValue(String name, Collection<Integer> partitions, long
timestamp) {
+ this(name, Set.copyOf(partitions), timestamp);
+ }
+
+ /**
+ * @param added The partitions to add.
+ * @return A copy of this value with the partitions added.
+ */
+ public InitMapValue withAddedPartitions(Collection<Integer> added) {
+ Set<Integer> newPartitions = new HashSet<>(partitions);
+ newPartitions.addAll(added);
+ return new InitMapValue(name, newPartitions, timestamp);
+ }
+
+ /**
+ * @param removed The partitions to remove.
+ * @return A copy of this value with the partitions removed.
+ */
+ public InitMapValue withRemovedPartitions(Collection<Integer> removed)
{
+ Set<Integer> newPartitions = new HashSet<>(partitions);
+ newPartitions.removeAll(removed);
+ return new InitMapValue(name, newPartitions, timestamp);
Review Comment:
ditto.
--
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]
