KAFKA-14386: Return TopicAssignment from the ReplicaPlacer

[andrewgrantcflt]

JIRA

https://issues.apache.org/jira/browse/KAFKA-14386

Summary

This changes the ReplicaPlacer interface to return a class instead of a list of list of integers. There are two reasons for the suggestion. First, as mentioned in the JIRA, 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:

Committer Checklist (excluded from commit message)

• Verify design and implementation
• Verify test coverage and CI build status
• Verify documentation (including upgrade notes)

[jsancio]

Thanks for the changes @andymg3 .

Should we also improve the PartitionRegistration type to use a PartitionAssignment instead of int[] replicas?

[andrewgrantcflt]

Thanks for the changes @andymg3 .

Should we also improve the PartitionRegistration type to use a PartitionAssignment instead of int[] replicas?

Thanks for the review @jsancio. As of now I dont see much benefit in changing that type. In PartitionRegistration we also have []int isr, int[] removingReplicas, and int[] addingReplicas. I'm inclined to leave replicas as is for consistency - I think it reads a bit better when they're all the same type. They're often processed at the same time as well, so having to deal with both types may be a bit cumbersome. There are quite a few places in the codebase that use the replicas field and in practice if we were to use PartitionAssignment all callers would mostly just call PartitionAssignment.replicas() anyways. So overall I don't see much benefit in introducing the type.

What do you think?

[jsancio]

There are quite a few places in the codebase that use the replicas field and in practice if we were to use PartitionAssignment all callers would mostly just call PartitionAssignment.replicas() anyways. So overall I don't see much benefit in introducing the type.

Sounds good.

[jsancio]

LGTM