adixitconfluent commented on code in PR #18864:
URL: https://github.com/apache/kafka/pull/18864#discussion_r1960964532


##########
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());

Review Comment:
   you're right, we don't need 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]

Reply via email to