KAFKA-17047: Refactored group coordinator classes to modern package (KIP-932) #16474
Conversation
|
Build passed with no new tests failure, only existing non-related tests failure. |
| @@ -28,7 +28,7 @@ | |||
| import java.util.Objects; | |||
|
|
|||
| /** | |||
| * The assignment specification for a consumer group. | |||
| * The assignment specification for a modern group. | |||
There was a problem hiding this comment.
not sure whether this is necessary change. Maybe we should align the term: "next gen", "modern", or "CONSUMER".
There was a problem hiding this comment.
Thanks for your review @chia7712. I have aligned the changes to have the common functionality as modern. As this spec class will be used in target assignment which will be also utilized by upcoming share groups. Please let me know if there is correction needed elsewhere as well.
There was a problem hiding this comment.
sorry for raising one more question
As this spec class will be used in target assignment which will be also utilized by upcoming share groups
it seems those share/consumer groups are on top of the new protocol, and the root package of those classes is called "modern". Hence, should we rename the value of group.coordinator.rebalance.protocols from CONSUMER to MODERN?
There was a problem hiding this comment.
I don't think that we shall change the group.coordinator.rebalance.protocols itself. There shall remain 2 for those as classic and consumer. Package modern is more to share the common target assignment code between consumer and share groups. Share groups will be be specifically used for a consumption pattern where partitions get shared between clients to facilitate co-operative consumption.
There was a problem hiding this comment.
I raise the "naming" issue, because it is hard to distinguish the term of "consumer protocol" and "consumer instance" in introducing the "AsyncConsumer" and "CONSUMER protocol" to users. I normally give up the term used by source code and then using term "next generation" be the replacement :_
Following the discussion and suggestion by @dajac, #16054 (comment), the PR refactors the common classes to build TargetAssignment in
modernpackage.consumerpackage has been moved insidemodernpackage with classes exclusive toconsumer group.This PR completes the refactoring and base to introduce
sharepackage insidemodern. The subsequent PRs will define the implementation specific to Share Groups while re-using the common functionality frommodernpackage classes.Committer Checklist (excluded from commit message)