dajac commented on code in PR #23574:
URL: https://github.com/apache/kafka/pull/23574#discussion_r4104298941
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/modern/TargetAssignmentBuilder.java:
##########
Review Comment:
nit: We need to update the doc here.
##########
group-coordinator/src/main/java/org/apache/kafka/coordinator/group/modern/TargetAssignmentBuilder.java:
##########
@@ -31,66 +30,31 @@
import org.apache.kafka.coordinator.group.modern.share.ShareGroupMember;
import org.apache.kafka.coordinator.group.util.UnionSet;
-import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
-import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
/**
- * Build a new Target Assignment based on the provided parameters. As a result,
- * it yields the records that must be persisted to the log and the new member
- * assignments as a map.
- *
- * Records are only created for members which have a new target assignment. If
- * their assignment did not change, no new record is needed.
- *
- * When a member is deleted, it is assumed that its target assignment record
- * is deleted as part of the member deletion process. In other words, this
class
- * does not yield a tombstone for removed members.
+ * Build a new Target Assignment based on the provided parameters.
*/
public abstract class TargetAssignmentBuilder<T extends ModernGroupMember, U
extends TargetAssignmentBuilder<T, U>> {
/**
- * The assignment result returned by {{@link
TargetAssignmentBuilder#build()}}.
+ * The assignment result returned by {@link
TargetAssignmentBuilder#build()}.
+ *
+ * @param targetAssignment The new target assignment for the group.
+ * @param targetAssignmentMetadata The new target assignment metadata.
*/
- public static class TargetAssignmentResult {
- /**
- * The records that must be applied to the __consumer_offsets
- * topics to persist the new target assignment.
- */
- private final List<CoordinatorRecord> records;
-
- /**
- * The new target assignment for the group.
- */
- private final Map<String, MemberAssignment> targetAssignment;
-
- TargetAssignmentResult(
- List<CoordinatorRecord> records,
- Map<String, MemberAssignment> targetAssignment
- ) {
- Objects.requireNonNull(records);
+ public record TargetAssignmentResult(
+ Map<String, Assignment> targetAssignment,
Review Comment:
Why did you chose to use `Assignment` instead of keeping `MemberAssignment`?
As `Assignment` implements `MemberAssignment`, we may also have an opportunity
here as we may be able to avoid allocating a new `Assignment` for each member.
--
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]
