[ 
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)

Reply via email to