chickenchickenlove commented on code in PR #23742:
URL: https://github.com/apache/kafka/pull/23742#discussion_r4259114952
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/modern/Assignment.java:
##########
@@ -42,9 +42,14 @@ public class Assignment implements MemberAssignment {
public Assignment(
Map<Uuid, Set<Integer>> partitions
) {
+ Objects.requireNonNull(partitions);
// Assignments are used as input to assignors, which expect to receive
immutable
// assignment maps, otherwise they will be modified in place.
- this.partitions =
Collections.unmodifiableMap(Objects.requireNonNull(partitions));
+ Map<Uuid, Set<Integer>> copy = new HashMap<>(partitions.size());
+ partitions.forEach(
+ (topicId, partitionsIds) -> copy.put(topicId,
Set.copyOf(partitionsIds))
+ );
Review Comment:
Ah, that makes sense. You're right. 🙇♂️
Even if the assignor returns a mutable assignment, replay reconstructs an
immutable assignment from the target assignment records. Subsequent assignor
calls use this replayed state, and the coordinator only reads the assignor's
output, so we don't need a defensive copy in `TargetAssignmentBuilder`.
I've addressed it.
--
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]
