[ https://issues.apache.org/jira/browse/KAFKA-14386?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Andrew Grant updated KAFKA-14386: --------------------------------- Description: In KRaft mode, a new interface was introduced, [ReplicaPlacer|https://github.com/apache/kafka/blob/trunk/metadata/src/main/java/org/apache/kafka/metadata/placement/ReplicaPlacer.java], that is used by ReplicationControlManager to create partition assignments during both CreateTopics and CreatePartitions RPCs. Right now it has one implementation, StripedReplicaPlacer. Currently, it has a method called place that returns a list of list of integers: {code:java} List<List<Integer>> place( PlacementSpec placement, ClusterDescriber cluster ) throws InvalidReplicationFactorException;{code} The index corresponds to the partition ID and the integers are the replicas of the assignment. The suggestion is to update the interface so that it models topic and partitions more explicitly. I'm thinking something like: {code:java} TopicAssignment place( PlacementSpec placement, ClusterDescriber cluster ) throws InvalidReplicationFactorException;{code} where we have {code:java} public class TopicAssignment { private List<PartitionAssignment> assignments; public TopicAssignment(List<PartitionAssignment> assignments) { this.assignments = assignments; } public List<PartitionAssignment> assignments() { return assignments; } }{code} and {code:java} public class PartitionAssignment { private List<Integer> replicas; public PartitionAssignment(List<Integer> replicas) { this.replicas = replicas; } public List<Integer> replicas() { return replicas; } }{code} There are two reasons for the suggestion. First, as mentioned above, it will make the interface, arguably, a bit more readable and understandable by explicitly modeling the idea of topic and partition. Second and more importantly, it makes the interface more extendable in the future. Right now it would be challenging to add more metadata to the response. By having classes, we can easily add fields to them without breaking/changing the interface. For example, in the CreatePartitions RPC we are adding partitions to an existing topic and we might want to add some metadata to response making it clear which partition the assignment starts at which could look something like: {code:java} public class TopicAssignment { private List<PartitionAssignment> assignments; private Integer firstPartitionId; public TopicAssignment(List<PartitionAssignment> assignments, Integer firstPartitionId) { this.assignments = assignments; this.firstPartitionId = firstPartitionId; } public List<PartitionAssignment> assignments() { return assignments; } ... } {code} Curious to hear other folks thoughts on this. was: In KRaft mode, a new interface was introduced, [ReplicaPlacer|https://github.com/apache/kafka/blob/trunk/metadata/src/main/java/org/apache/kafka/metadata/placement/ReplicaPlacer.java], that is used by ReplicationControlManager to create partition assignments during both CreateTopics and CreatePartitions RPCs. Right now it has one implementation, StripedReplicaPlacer. Currently, it has a method called place that returns a list of list of integers: {code:java} List<List<Integer>> place( PlacementSpec placement, ClusterDescriber cluster ) throws InvalidReplicationFactorException;{code} The index corresponds to the partition ID and the integers are the replicas of the assignment. The suggestion is to update the interface so that it models topic and partitions more explicitly. I'm thinking something like: {code:java} TopicAssignment place( PlacementSpec placement, ClusterDescriber cluster ) throws InvalidReplicationFactorException;{code} where we have {code:java} public class TopicAssignment { private List<PartitionAssignment> assignments; public TopicAssignment(List<PartitionAssignment> assignments) { this.assignments = assignments; } public List<PartitionAssignment> assignments() { return assignments; } }{code} and {code:java} public class PartitionAssignment { private List<Integer> replicas; public PartitionAssignment(List<Integer> replicas) { this.replicas = replicas; } public List<Integer> replicas() { return replicas; } }{code} There are two reasons for the suggestion. First, as mentioned above, it will make the interface, arguably, a bit more readable and understandable by explicitly modeling the idea of topic and partition. Second and more importantly, it makes the interface more extendable in the future. Right now it would be challenging to add more metadata to the response. By having classes, we can easily add fields to them without breaking/changing the interface. For example, in the CreatePartitions RPC we are adding partitions to an existing topic and we might want to add some metadata to response making it clear which partition the assignment starts at which could look something like: {code:java} public class TopicAssignment { private List<PartitionAssignment> assignments; private Integer firstPartitionId; public TopicAssignment(List<PartitionAssignment> assignments, Integer firstPartitionId) { this.assignments = assignments; this.firstPartitionId = firstPartitionId; } public List<PartitionAssignment> assignments() { return assignments; } ... } {code} Curious to hear other folks thoughts on this. > Change ReplicaPlacer place method to return a class instead of list of list > of integers > --------------------------------------------------------------------------------------- > > Key: KAFKA-14386 > URL: https://issues.apache.org/jira/browse/KAFKA-14386 > Project: Kafka > Issue Type: Improvement > Reporter: Andrew Grant > Assignee: Andrew Grant > Priority: Major > > In KRaft mode, a new interface was introduced, > [ReplicaPlacer|https://github.com/apache/kafka/blob/trunk/metadata/src/main/java/org/apache/kafka/metadata/placement/ReplicaPlacer.java], > that is used by ReplicationControlManager to create partition assignments > during both CreateTopics and CreatePartitions RPCs. Right now it has one > implementation, StripedReplicaPlacer. > Currently, it has a method called place that returns a list of list of > integers: > {code:java} > List<List<Integer>> place( > PlacementSpec placement, > ClusterDescriber cluster > ) throws InvalidReplicationFactorException;{code} > The index corresponds to the partition ID and the integers are the replicas > of the assignment. The suggestion is to update the interface so that it > models topic and partitions more explicitly. I'm thinking something like: > {code:java} > TopicAssignment place( > PlacementSpec placement, > ClusterDescriber cluster > ) throws InvalidReplicationFactorException;{code} > where we have > {code:java} > public class TopicAssignment { > private List<PartitionAssignment> assignments; > public TopicAssignment(List<PartitionAssignment> assignments) { > this.assignments = assignments; > } > public List<PartitionAssignment> assignments() { > return assignments; > } > }{code} > and > {code:java} > public class PartitionAssignment { > private List<Integer> replicas; > public PartitionAssignment(List<Integer> replicas) { > this.replicas = replicas; > } > public List<Integer> replicas() { > return replicas; > } > }{code} > There are two reasons for the suggestion. First, as mentioned above, it will > make the interface, arguably, a bit more readable and understandable by > explicitly modeling the idea of topic and partition. Second and more > importantly, it makes the interface more extendable in the future. Right now > it would be challenging to add more metadata to the response. By having > classes, we can easily add fields to them without breaking/changing the > interface. For example, in the CreatePartitions RPC we are adding partitions > to an existing topic and we might want to add some metadata to response > making it clear which partition the assignment starts at which could look > something like: > {code:java} > public class TopicAssignment { > private List<PartitionAssignment> assignments; > private Integer firstPartitionId; > > public TopicAssignment(List<PartitionAssignment> assignments, Integer > firstPartitionId) { > this.assignments = assignments; > this.firstPartitionId = firstPartitionId; > } > public List<PartitionAssignment> assignments() { > return assignments; > } > ... > } > {code} > > Curious to hear other folks thoughts on this. -- This message was sent by Atlassian Jira (v8.20.10#820010)