AndrewJSchofield commented on code in PR #18864:
URL: https://github.com/apache/kafka/pull/18864#discussion_r1957425369
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/assignor/SimpleAssignor.java:
##########
@@ -72,36 +77,178 @@ private GroupAssignment assignHomogenous(
if (subscribeTopicIds.isEmpty())
return new GroupAssignment(Collections.emptyMap());
- Map<Uuid, Set<Integer>> targetPartitions = computeTargetPartitions(
+ // Subscribed topic partitions for the share group.
+ List<TargetPartition> targetPartitions = computeTargetPartitions(
subscribeTopicIds, subscribedTopicDescriber);
- return new
GroupAssignment(groupSpec.memberIds().stream().collect(Collectors.toMap(
- Function.identity(), memberId -> new
MemberAssignmentImpl(targetPartitions))));
+ // The current assignment from topic partition to members.
+ Map<TargetPartition, List<String>> currentAssignment =
currentAssignment(groupSpec);
+ return newAssignmentHomogeneous(groupSpec, subscribeTopicIds,
targetPartitions, currentAssignment);
}
private GroupAssignment assignHeterogeneous(
GroupSpec groupSpec,
SubscribedTopicDescriber subscribedTopicDescriber
) {
- Map<String, MemberAssignment> members = new HashMap<>();
+ Map<String, List<TargetPartition>> memberToPartitionsSubscription =
new HashMap<>();
for (String memberId : groupSpec.memberIds()) {
MemberSubscription spec = groupSpec.memberSubscription(memberId);
if (spec.subscribedTopicIds().isEmpty())
continue;
- Map<Uuid, Set<Integer>> targetPartitions = computeTargetPartitions(
+ // Subscribed topic partitions for the share group member.
+ List<TargetPartition> targetPartitions = computeTargetPartitions(
spec.subscribedTopicIds(), subscribedTopicDescriber);
+ memberToPartitionsSubscription.put(memberId, targetPartitions);
+ }
+
+ // The current assignment from topic partition to members.
+ Map<TargetPartition, List<String>> currentAssignment =
currentAssignment(groupSpec);
+ return newAssignmentHeterogeneous(groupSpec,
memberToPartitionsSubscription, currentAssignment);
+ }
- members.put(memberId, new MemberAssignmentImpl(targetPartitions));
+ // Get the current assignment for subscribed topic partitions to share
group members.
+ private Map<TargetPartition, List<String>> currentAssignment(GroupSpec
groupSpec) {
+ Map<TargetPartition, List<String>> assignment = new HashMap<>();
+
+ for (String member : groupSpec.memberIds()) {
+ Map<Uuid, Set<Integer>> assignedTopicPartitions =
groupSpec.memberAssignment(member).partitions();
+ assignedTopicPartitions.forEach((topicId, partitions) ->
partitions.forEach(
+ partition -> assignment.computeIfAbsent(new
TargetPartition(topicId, partition), k -> new ArrayList<>()).add(member)));
}
+ return assignment;
+ }
+
+ private GroupAssignment newAssignmentHomogeneous(
+ GroupSpec groupSpec,
+ Set<Uuid> subscribeTopicIds,
+ List<TargetPartition> targetPartitions,
+ Map<TargetPartition, List<String>> currentAssignment) {
+
+ Map<TargetPartition, List<String>> newAssignment = new HashMap<>();
+ // Step 1: Hash member IDs to partitions.
+ memberHashAssignment(targetPartitions, groupSpec.memberIds(),
newAssignment);
+
+ // Step 2: Round-robin assignment for unassigned partitions which do
not have members already assigned in the current assignment.
+ Set<TargetPartition> assignedPartitions = new
HashSet<>(newAssignment.keySet());
+ List<TargetPartition> unassignedPartitions = targetPartitions.stream()
+ .filter(targetPartition ->
!assignedPartitions.contains(targetPartition))
+ .filter(targetPartition ->
!currentAssignment.containsKey(targetPartition))
+ .collect(Collectors.toList());
+
+ roundRobinAssignment(groupSpec.memberIds(), unassignedPartitions,
newAssignment);
+
+ Map<String, Set<TargetPartition>> finalAssignment = new HashMap<>();
+
+ // When combining current assignment, we need to only consider the
topics in current assignment that are also being
+ // subscribed in the new assignment as well.
+ currentAssignment.forEach((targetPartition, members) -> {
+ if (subscribeTopicIds.contains(targetPartition.topicId))
+ members.forEach(member -> {
+ if (groupSpec.memberIds().contains(member))
+ finalAssignment.computeIfAbsent(member, k -> new
HashSet<>()).add(targetPartition);
+ });
+ });
+ newAssignment.forEach((targetPartition, members) ->
members.forEach(member ->
+ finalAssignment.computeIfAbsent(member, k -> new
HashSet<>()).add(targetPartition)));
+
+ return groupAssignment(finalAssignment, groupSpec.memberIds());
+ }
+
+ private GroupAssignment newAssignmentHeterogeneous(
+ GroupSpec groupSpec,
+ Map<String, List<TargetPartition>> memberToPartitionsSubscription,
+ Map<TargetPartition, List<String>> currentAssignment) {
+
+ // Exhaustive set of all subscribed topic partitions.
+ Set<TargetPartition> targetPartitions = new LinkedHashSet<>();
+
memberToPartitionsSubscription.values().forEach(targetPartitions::addAll);
+
+ // Create a map for topic to members subscription.
+ Map<Uuid, Set<String>> topicToMemberSubscription = new HashMap<>();
+ memberToPartitionsSubscription.forEach((member, partitions) ->
+ partitions.forEach(partition ->
topicToMemberSubscription.computeIfAbsent(partition.topicId(), k -> new
LinkedHashSet<>()).add(member)));
+
+ Map<TargetPartition, List<String>> newAssignment = new HashMap<>();
+ // Step 1: Hash member IDs to partitions.
+ memberToPartitionsSubscription.forEach((member, partitions) ->
+ memberHashAssignment(partitions,
Collections.singletonList(member), newAssignment));
+
+ // Step 2: Round-robin assignment for unassigned partitions which do
not have members already assigned in the current assignment.
+ Set<TargetPartition> assignedPartitions = new
LinkedHashSet<>(newAssignment.keySet());
+ Map<Uuid, List<TargetPartition>> unassignedPartitions = new
HashMap<>();
+ targetPartitions.forEach(targetPartition -> {
+ if (!assignedPartitions.contains(targetPartition) &&
!currentAssignment.containsKey(targetPartition))
+
unassignedPartitions.computeIfAbsent(targetPartition.topicId(), k -> new
ArrayList<>()).add(targetPartition);
+ });
+
+ unassignedPartitions.keySet().forEach(unassignedTopic ->
+
roundRobinAssignment(topicToMemberSubscription.get(unassignedTopic),
unassignedPartitions.get(unassignedTopic), newAssignment));
+
+ // Step 3: We combine current assignment and new assignment.
+ Map<String, Set<TargetPartition>> finalAssignment = new HashMap<>();
+
+ // When combining current assignment, we need to only consider the
member topic subscription in current assignment
+ // which is being subscribed in the new assignment as well.
+ currentAssignment.forEach((targetPartition, members) ->
members.forEach(member -> {
+ if
(topicToMemberSubscription.getOrDefault(targetPartition.topicId(),
Collections.emptySet()).contains(member))
+ finalAssignment.computeIfAbsent(member, k -> new
HashSet<>()).add(targetPartition);
+ }));
+ newAssignment.forEach((targetPartition, members) ->
members.forEach(member ->
+ finalAssignment.computeIfAbsent(member, k -> new
HashSet<>()).add(targetPartition)));
+
+ return groupAssignment(finalAssignment, groupSpec.memberIds());
+ }
+
+ private GroupAssignment groupAssignment(
+ Map<String, Set<TargetPartition>> assignmentByMember,
+ Collection<String> allGroupMembers) {
+ Map<String, MemberAssignment> members = new HashMap<>();
+ for (Map.Entry<String, Set<TargetPartition>> entry :
assignmentByMember.entrySet()) {
+ Map<Uuid, Set<Integer>> targetPartitions = new HashMap<>();
+ entry.getValue().forEach(targetPartition ->
targetPartitions.computeIfAbsent(targetPartition.topicId(), k -> new
HashSet<>()).add(targetPartition.partition()));
+ members.put(entry.getKey(), new
MemberAssignmentImpl(targetPartitions));
+ }
+ allGroupMembers.forEach(member -> {
+ if (!members.containsKey(member))
+ members.put(member, new MemberAssignmentImpl(new HashMap<>()));
+ });
+
return new GroupAssignment(members);
}
- private Map<Uuid, Set<Integer>> computeTargetPartitions(
+ // Visible for testing.
+ void memberHashAssignment(
+ List<TargetPartition> targetPartitions,
+ Collection<String> memberIds,
+ Map<TargetPartition, List<String>> assignment) {
Review Comment:
nit: As a general formatting point, indenting the arguments by 4 spaces, and
then leaving the closing `) {` on the same line as the final argument means
that the arguments and the following code statements are indented identically
with no gap between them. Please reformat so the statements and the arguments
are distinct. Personally, I would either place `) {` on a separate line, or
indent all of the arguments at the end of the method name.
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/assignor/SimpleAssignor.java:
##########
@@ -72,36 +77,178 @@ private GroupAssignment assignHomogenous(
if (subscribeTopicIds.isEmpty())
return new GroupAssignment(Collections.emptyMap());
Review Comment:
Should be able to use `Map.of()` rather than `Collections.emptyMap()` across
this class.
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/assignor/SimpleAssignor.java:
##########
@@ -26,12 +26,17 @@
import
org.apache.kafka.coordinator.group.api.assignor.SubscribedTopicDescriber;
import org.apache.kafka.coordinator.group.modern.MemberAssignmentImpl;
+import java.util.ArrayList;
+import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
+import java.util.Iterator;
+import java.util.LinkedHashSet;
+import java.util.List;
import java.util.Map;
+import java.util.Objects;
import java.util.Set;
-import java.util.function.Function;
import java.util.stream.Collectors;
import static
org.apache.kafka.coordinator.group.api.assignor.SubscriptionType.HOMOGENEOUS;
Review Comment:
The class javadoc comment for the simple assignor is now wrong.
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/assignor/SimpleAssignor.java:
##########
@@ -72,36 +77,178 @@ private GroupAssignment assignHomogenous(
if (subscribeTopicIds.isEmpty())
return new GroupAssignment(Collections.emptyMap());
- Map<Uuid, Set<Integer>> targetPartitions = computeTargetPartitions(
+ // Subscribed topic partitions for the share group.
+ List<TargetPartition> targetPartitions = computeTargetPartitions(
subscribeTopicIds, subscribedTopicDescriber);
- return new
GroupAssignment(groupSpec.memberIds().stream().collect(Collectors.toMap(
- Function.identity(), memberId -> new
MemberAssignmentImpl(targetPartitions))));
+ // The current assignment from topic partition to members.
+ Map<TargetPartition, List<String>> currentAssignment =
currentAssignment(groupSpec);
+ return newAssignmentHomogeneous(groupSpec, subscribeTopicIds,
targetPartitions, currentAssignment);
}
private GroupAssignment assignHeterogeneous(
GroupSpec groupSpec,
SubscribedTopicDescriber subscribedTopicDescriber
) {
- Map<String, MemberAssignment> members = new HashMap<>();
+ Map<String, List<TargetPartition>> memberToPartitionsSubscription =
new HashMap<>();
for (String memberId : groupSpec.memberIds()) {
MemberSubscription spec = groupSpec.memberSubscription(memberId);
if (spec.subscribedTopicIds().isEmpty())
continue;
- Map<Uuid, Set<Integer>> targetPartitions = computeTargetPartitions(
+ // Subscribed topic partitions for the share group member.
+ List<TargetPartition> targetPartitions = computeTargetPartitions(
spec.subscribedTopicIds(), subscribedTopicDescriber);
+ memberToPartitionsSubscription.put(memberId, targetPartitions);
+ }
+
+ // The current assignment from topic partition to members.
+ Map<TargetPartition, List<String>> currentAssignment =
currentAssignment(groupSpec);
+ return newAssignmentHeterogeneous(groupSpec,
memberToPartitionsSubscription, currentAssignment);
+ }
- members.put(memberId, new MemberAssignmentImpl(targetPartitions));
+ // Get the current assignment for subscribed topic partitions to share
group members.
+ private Map<TargetPartition, List<String>> currentAssignment(GroupSpec
groupSpec) {
+ Map<TargetPartition, List<String>> assignment = new HashMap<>();
+
+ for (String member : groupSpec.memberIds()) {
+ Map<Uuid, Set<Integer>> assignedTopicPartitions =
groupSpec.memberAssignment(member).partitions();
+ assignedTopicPartitions.forEach((topicId, partitions) ->
partitions.forEach(
+ partition -> assignment.computeIfAbsent(new
TargetPartition(topicId, partition), k -> new ArrayList<>()).add(member)));
}
+ return assignment;
+ }
+
+ private GroupAssignment newAssignmentHomogeneous(
+ GroupSpec groupSpec,
+ Set<Uuid> subscribeTopicIds,
+ List<TargetPartition> targetPartitions,
+ Map<TargetPartition, List<String>> currentAssignment) {
+
+ Map<TargetPartition, List<String>> newAssignment = new HashMap<>();
+ // Step 1: Hash member IDs to partitions.
+ memberHashAssignment(targetPartitions, groupSpec.memberIds(),
newAssignment);
+
+ // Step 2: Round-robin assignment for unassigned partitions which do
not have members already assigned in the current assignment.
+ Set<TargetPartition> assignedPartitions = new
HashSet<>(newAssignment.keySet());
+ List<TargetPartition> unassignedPartitions = targetPartitions.stream()
+ .filter(targetPartition ->
!assignedPartitions.contains(targetPartition))
+ .filter(targetPartition ->
!currentAssignment.containsKey(targetPartition))
+ .collect(Collectors.toList());
+
+ roundRobinAssignment(groupSpec.memberIds(), unassignedPartitions,
newAssignment);
+
+ Map<String, Set<TargetPartition>> finalAssignment = new HashMap<>();
+
+ // When combining current assignment, we need to only consider the
topics in current assignment that are also being
+ // subscribed in the new assignment as well.
+ currentAssignment.forEach((targetPartition, members) -> {
+ if (subscribeTopicIds.contains(targetPartition.topicId))
+ members.forEach(member -> {
+ if (groupSpec.memberIds().contains(member))
+ finalAssignment.computeIfAbsent(member, k -> new
HashSet<>()).add(targetPartition);
+ });
+ });
+ newAssignment.forEach((targetPartition, members) ->
members.forEach(member ->
+ finalAssignment.computeIfAbsent(member, k -> new
HashSet<>()).add(targetPartition)));
+
+ return groupAssignment(finalAssignment, groupSpec.memberIds());
+ }
+
+ private GroupAssignment newAssignmentHeterogeneous(
+ GroupSpec groupSpec,
+ Map<String, List<TargetPartition>> memberToPartitionsSubscription,
+ Map<TargetPartition, List<String>> currentAssignment) {
+
+ // Exhaustive set of all subscribed topic partitions.
+ Set<TargetPartition> targetPartitions = new LinkedHashSet<>();
+
memberToPartitionsSubscription.values().forEach(targetPartitions::addAll);
+
+ // Create a map for topic to members subscription.
+ Map<Uuid, Set<String>> topicToMemberSubscription = new HashMap<>();
+ memberToPartitionsSubscription.forEach((member, partitions) ->
+ partitions.forEach(partition ->
topicToMemberSubscription.computeIfAbsent(partition.topicId(), k -> new
LinkedHashSet<>()).add(member)));
+
+ Map<TargetPartition, List<String>> newAssignment = new HashMap<>();
+ // Step 1: Hash member IDs to partitions.
+ memberToPartitionsSubscription.forEach((member, partitions) ->
+ memberHashAssignment(partitions,
Collections.singletonList(member), newAssignment));
+
+ // Step 2: Round-robin assignment for unassigned partitions which do
not have members already assigned in the current assignment.
+ Set<TargetPartition> assignedPartitions = new
LinkedHashSet<>(newAssignment.keySet());
+ Map<Uuid, List<TargetPartition>> unassignedPartitions = new
HashMap<>();
+ targetPartitions.forEach(targetPartition -> {
+ if (!assignedPartitions.contains(targetPartition) &&
!currentAssignment.containsKey(targetPartition))
+
unassignedPartitions.computeIfAbsent(targetPartition.topicId(), k -> new
ArrayList<>()).add(targetPartition);
+ });
+
+ unassignedPartitions.keySet().forEach(unassignedTopic ->
+
roundRobinAssignment(topicToMemberSubscription.get(unassignedTopic),
unassignedPartitions.get(unassignedTopic), newAssignment));
+
+ // Step 3: We combine current assignment and new assignment.
+ Map<String, Set<TargetPartition>> finalAssignment = new HashMap<>();
+
+ // When combining current assignment, we need to only consider the
member topic subscription in current assignment
+ // which is being subscribed in the new assignment as well.
+ currentAssignment.forEach((targetPartition, members) ->
members.forEach(member -> {
+ if
(topicToMemberSubscription.getOrDefault(targetPartition.topicId(),
Collections.emptySet()).contains(member))
+ finalAssignment.computeIfAbsent(member, k -> new
HashSet<>()).add(targetPartition);
+ }));
+ newAssignment.forEach((targetPartition, members) ->
members.forEach(member ->
+ finalAssignment.computeIfAbsent(member, k -> new
HashSet<>()).add(targetPartition)));
+
+ return groupAssignment(finalAssignment, groupSpec.memberIds());
+ }
+
+ private GroupAssignment groupAssignment(
+ Map<String, Set<TargetPartition>> assignmentByMember,
+ Collection<String> allGroupMembers) {
+ Map<String, MemberAssignment> members = new HashMap<>();
+ for (Map.Entry<String, Set<TargetPartition>> entry :
assignmentByMember.entrySet()) {
+ Map<Uuid, Set<Integer>> targetPartitions = new HashMap<>();
+ entry.getValue().forEach(targetPartition ->
targetPartitions.computeIfAbsent(targetPartition.topicId(), k -> new
HashSet<>()).add(targetPartition.partition()));
+ members.put(entry.getKey(), new
MemberAssignmentImpl(targetPartitions));
+ }
+ allGroupMembers.forEach(member -> {
+ if (!members.containsKey(member))
+ members.put(member, new MemberAssignmentImpl(new HashMap<>()));
+ });
+
return new GroupAssignment(members);
}
- private Map<Uuid, Set<Integer>> computeTargetPartitions(
+ // Visible for testing.
+ void memberHashAssignment(
Review Comment:
This is one class for which comprehensive comments would be helpful for
future maintainers.
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/assignor/SimpleAssignor.java:
##########
@@ -111,12 +258,26 @@ private Map<Uuid, Set<Integer>> computeTargetPartitions(
);
}
- Set<Integer> partitions = new HashSet<>();
for (int i = 0; i < numPartitions; i++) {
- partitions.add(i);
+ targetPartitions.add(new TargetPartition(topicId, i));
}
- targetPartitions.put(topicId, partitions);
});
return targetPartitions;
}
+
+ record TargetPartition(Uuid topicId, int partition) {
+
+ @Override
+ public boolean equals(Object o) {
Review Comment:
I've surprised that you implemented `equals` and `hashCode`. I believe the
default implementation provided by the use of `record` would be sufficient.
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/assignor/SimpleAssignor.java:
##########
@@ -72,36 +77,178 @@ private GroupAssignment assignHomogenous(
if (subscribeTopicIds.isEmpty())
return new GroupAssignment(Collections.emptyMap());
- Map<Uuid, Set<Integer>> targetPartitions = computeTargetPartitions(
+ // Subscribed topic partitions for the share group.
+ List<TargetPartition> targetPartitions = computeTargetPartitions(
subscribeTopicIds, subscribedTopicDescriber);
- return new
GroupAssignment(groupSpec.memberIds().stream().collect(Collectors.toMap(
- Function.identity(), memberId -> new
MemberAssignmentImpl(targetPartitions))));
+ // The current assignment from topic partition to members.
+ Map<TargetPartition, List<String>> currentAssignment =
currentAssignment(groupSpec);
+ return newAssignmentHomogeneous(groupSpec, subscribeTopicIds,
targetPartitions, currentAssignment);
}
private GroupAssignment assignHeterogeneous(
GroupSpec groupSpec,
SubscribedTopicDescriber subscribedTopicDescriber
) {
- Map<String, MemberAssignment> members = new HashMap<>();
+ Map<String, List<TargetPartition>> memberToPartitionsSubscription =
new HashMap<>();
for (String memberId : groupSpec.memberIds()) {
MemberSubscription spec = groupSpec.memberSubscription(memberId);
if (spec.subscribedTopicIds().isEmpty())
continue;
- Map<Uuid, Set<Integer>> targetPartitions = computeTargetPartitions(
+ // Subscribed topic partitions for the share group member.
+ List<TargetPartition> targetPartitions = computeTargetPartitions(
spec.subscribedTopicIds(), subscribedTopicDescriber);
+ memberToPartitionsSubscription.put(memberId, targetPartitions);
+ }
+
+ // The current assignment from topic partition to members.
+ Map<TargetPartition, List<String>> currentAssignment =
currentAssignment(groupSpec);
+ return newAssignmentHeterogeneous(groupSpec,
memberToPartitionsSubscription, currentAssignment);
+ }
- members.put(memberId, new MemberAssignmentImpl(targetPartitions));
+ // Get the current assignment for subscribed topic partitions to share
group members.
+ private Map<TargetPartition, List<String>> currentAssignment(GroupSpec
groupSpec) {
+ Map<TargetPartition, List<String>> assignment = new HashMap<>();
+
+ for (String member : groupSpec.memberIds()) {
+ Map<Uuid, Set<Integer>> assignedTopicPartitions =
groupSpec.memberAssignment(member).partitions();
+ assignedTopicPartitions.forEach((topicId, partitions) ->
partitions.forEach(
+ partition -> assignment.computeIfAbsent(new
TargetPartition(topicId, partition), k -> new ArrayList<>()).add(member)));
}
+ return assignment;
+ }
+
+ private GroupAssignment newAssignmentHomogeneous(
+ GroupSpec groupSpec,
+ Set<Uuid> subscribeTopicIds,
+ List<TargetPartition> targetPartitions,
+ Map<TargetPartition, List<String>> currentAssignment) {
+
+ Map<TargetPartition, List<String>> newAssignment = new HashMap<>();
+ // Step 1: Hash member IDs to partitions.
+ memberHashAssignment(targetPartitions, groupSpec.memberIds(),
newAssignment);
+
+ // Step 2: Round-robin assignment for unassigned partitions which do
not have members already assigned in the current assignment.
+ Set<TargetPartition> assignedPartitions = new
HashSet<>(newAssignment.keySet());
+ List<TargetPartition> unassignedPartitions = targetPartitions.stream()
+ .filter(targetPartition ->
!assignedPartitions.contains(targetPartition))
+ .filter(targetPartition ->
!currentAssignment.containsKey(targetPartition))
+ .collect(Collectors.toList());
+
+ roundRobinAssignment(groupSpec.memberIds(), unassignedPartitions,
newAssignment);
+
+ Map<String, Set<TargetPartition>> finalAssignment = new HashMap<>();
+
+ // When combining current assignment, we need to only consider the
topics in current assignment that are also being
+ // subscribed in the new assignment as well.
+ currentAssignment.forEach((targetPartition, members) -> {
+ if (subscribeTopicIds.contains(targetPartition.topicId))
+ members.forEach(member -> {
+ if (groupSpec.memberIds().contains(member))
+ finalAssignment.computeIfAbsent(member, k -> new
HashSet<>()).add(targetPartition);
+ });
+ });
+ newAssignment.forEach((targetPartition, members) ->
members.forEach(member ->
+ finalAssignment.computeIfAbsent(member, k -> new
HashSet<>()).add(targetPartition)));
+
+ return groupAssignment(finalAssignment, groupSpec.memberIds());
+ }
+
+ private GroupAssignment newAssignmentHeterogeneous(
+ GroupSpec groupSpec,
+ Map<String, List<TargetPartition>> memberToPartitionsSubscription,
+ Map<TargetPartition, List<String>> currentAssignment) {
+
+ // Exhaustive set of all subscribed topic partitions.
+ Set<TargetPartition> targetPartitions = new LinkedHashSet<>();
+
memberToPartitionsSubscription.values().forEach(targetPartitions::addAll);
+
+ // Create a map for topic to members subscription.
+ Map<Uuid, Set<String>> topicToMemberSubscription = new HashMap<>();
+ memberToPartitionsSubscription.forEach((member, partitions) ->
+ partitions.forEach(partition ->
topicToMemberSubscription.computeIfAbsent(partition.topicId(), k -> new
LinkedHashSet<>()).add(member)));
+
+ Map<TargetPartition, List<String>> newAssignment = new HashMap<>();
+ // Step 1: Hash member IDs to partitions.
+ memberToPartitionsSubscription.forEach((member, partitions) ->
+ memberHashAssignment(partitions,
Collections.singletonList(member), newAssignment));
+
+ // Step 2: Round-robin assignment for unassigned partitions which do
not have members already assigned in the current assignment.
+ Set<TargetPartition> assignedPartitions = new
LinkedHashSet<>(newAssignment.keySet());
+ Map<Uuid, List<TargetPartition>> unassignedPartitions = new
HashMap<>();
+ targetPartitions.forEach(targetPartition -> {
+ if (!assignedPartitions.contains(targetPartition) &&
!currentAssignment.containsKey(targetPartition))
+
unassignedPartitions.computeIfAbsent(targetPartition.topicId(), k -> new
ArrayList<>()).add(targetPartition);
+ });
+
+ unassignedPartitions.keySet().forEach(unassignedTopic ->
+
roundRobinAssignment(topicToMemberSubscription.get(unassignedTopic),
unassignedPartitions.get(unassignedTopic), newAssignment));
+
+ // Step 3: We combine current assignment and new assignment.
+ Map<String, Set<TargetPartition>> finalAssignment = new HashMap<>();
+
+ // When combining current assignment, we need to only consider the
member topic subscription in current assignment
+ // which is being subscribed in the new assignment as well.
+ currentAssignment.forEach((targetPartition, members) ->
members.forEach(member -> {
+ if
(topicToMemberSubscription.getOrDefault(targetPartition.topicId(),
Collections.emptySet()).contains(member))
+ finalAssignment.computeIfAbsent(member, k -> new
HashSet<>()).add(targetPartition);
+ }));
+ newAssignment.forEach((targetPartition, members) ->
members.forEach(member ->
+ finalAssignment.computeIfAbsent(member, k -> new
HashSet<>()).add(targetPartition)));
+
+ return groupAssignment(finalAssignment, groupSpec.memberIds());
+ }
+
+ private GroupAssignment groupAssignment(
+ Map<String, Set<TargetPartition>> assignmentByMember,
+ Collection<String> allGroupMembers) {
+ Map<String, MemberAssignment> members = new HashMap<>();
+ for (Map.Entry<String, Set<TargetPartition>> entry :
assignmentByMember.entrySet()) {
+ Map<Uuid, Set<Integer>> targetPartitions = new HashMap<>();
+ entry.getValue().forEach(targetPartition ->
targetPartitions.computeIfAbsent(targetPartition.topicId(), k -> new
HashSet<>()).add(targetPartition.partition()));
+ members.put(entry.getKey(), new
MemberAssignmentImpl(targetPartitions));
+ }
+ allGroupMembers.forEach(member -> {
+ if (!members.containsKey(member))
+ members.put(member, new MemberAssignmentImpl(new HashMap<>()));
+ });
+
return new GroupAssignment(members);
}
- private Map<Uuid, Set<Integer>> computeTargetPartitions(
+ // Visible for testing.
+ void memberHashAssignment(
+ List<TargetPartition> targetPartitions,
+ Collection<String> memberIds,
+ Map<TargetPartition, List<String>> assignment) {
+ if (!targetPartitions.isEmpty())
+ for (String memberId : memberIds) {
+ int topicPartitionIndex = Math.abs(memberId.hashCode() %
targetPartitions.size());
+ TargetPartition topicPartition =
targetPartitions.get(topicPartitionIndex);
+ assignment.computeIfAbsent(topicPartition, k -> new
ArrayList<>()).add(memberId);
+ }
+ }
+
+ // Visible for testing.
+ void roundRobinAssignment(
+ Collection<String> members,
Review Comment:
nit: Previously, you use `memberIds` and `memberId`, as opposed to `members`
and `member` in this method. I think the former is clearer because `member`
sounds like a reference to a member object, as opposed to just its id.
--
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
