dajac commented on code in PR #19677:
URL: https://github.com/apache/kafka/pull/19677#discussion_r2086234212
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/assignor/UniformHomogeneousAssignmentBuilder.java:
##########
@@ -218,6 +234,15 @@ private void maybeRevokePartitions() {
targetAssignment.put(memberId, new
MemberAssignmentImpl(newAssignment));
}
}
+
+ // Distribute the remaining extra partitions to the members that are
at or under the minimum
+ // quota. When there are remaining extra partitions, we can't run out
of unfilledMembers
+ // here because all members either subtracted from
remainingMembersToGetAnExtraPartition or
+ // went into the unfilledMembers list.
+ for (int i = 0; i < remainingMembersToGetAnExtraPartition; i++) {
+ unfilledMembers.get(i).remainingQuota++;
+ }
+ remainingMembersToGetAnExtraPartition = 0;
Review Comment:
I think that we could avoid this extra loop. Let's say that we track the
index of the current member while we iterate on the memberIds. When the
memberIds.size() - index <= remainingMembersToGetAnExtraPartition, we know that
we must keep the remaining members with the adjusted quota. The idea is to
basically put extra partitions at the end, unless that have been taken during
the iteration. Would something like this work? I am not sure if it is really
simpler than your current solution though. The benefit is that we would not
need the `(remainingQuota == 0)` condition during the assignment and we could
keep `MemberWithRemainingQuota` with final attributes.
--
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
