Hi David, Thanks a lot for the detailed review!
*DJ_01:* That's a fair point. I'll update the KIP to use group.streams.assignors to align with KIP-848 and remove the misleading motivation section. *DJ_02:* Thanks for the suggestion. I went through the KIP-848 interfaces in org.apache.kafka.coordinator.group.api.assignor as well as the existing implementations (UniformAssignor, RangeAssignor), and the pattern makes total sense now. I'll update the input types in the KIP accordingly: - GroupSpec will be changed to an interface with per-member accessors, instead of returning a raw Map<String, AssignmentMemberSpec>. - AssignmentMemberSpec will be replaced by two distinct interfaces: MemberSubscription and MemberAssignment. This will separate the member's static metadata (processId, instanceId, rackId, clientTags) from its runtime task assignment (active/standby/warmup tasks), mirroring the clean split in KIP-848. *DJ_03:* Thank you for the suggestion. I'll rename it to configs() and clarify in the text that it will only return configurations relevant to the assignment. *DJ_04:*Moving all dynamic group config validation to the broker makes sense, but that would be a bit beyond the scope of this KIP. In this case, maybe we should just keep the config like this? Best, Gabriella On Fri, Jun 12, 2026 at 10:54 AM David Jacot <[email protected]> wrote: > Hi Gabriella, > > Thanks for the KIP. I have a few high level comments: > > DJ_01: The motivation to not use `group.streams.assignors` and follow > the pattern introduced by KIP-848 is pretty weak in my opinion, > especially the "avoids leaking implementation class names into > per-group dynamic configuration" part. The class names wont leak into > the group config as the group config requires the name of the > assignor. From a user perspective, it is exactly the same concept so > using a different way to express it is wrong in my opinion. Should we > just use `group.streams.assignors` to be consistent in our configs? > > DJ_02: I would suggest reconsidering the interface of the assignor. We > started with a similar interface in KIP-848 and we realized during the > implementation that using POJOS and returning Maps (e.g. for > members()) was really inflexible. Moreover, I am not sure if using > records is good from an evolutionary point of view. Using interfaces > may be better. It also allows us to wrap internal objects to expose > them to the assignor. Have you looked into the KIP-848's interface and > the various assignors? > > DJ_03: Regarding the `assignmentConfigs()`, should we call it > `configs()` as it is already clear that it is for the assignment. > Moreover, I wonder if it is going to return all group configs or only > a subset of them. Could you please clarify in the KIP? > > DJ_04: The dynamic group configs validation makes sense to me. Note > that this pattern is not used today. I also wonder whether we should > move the validation of all dynamic group configs there. To be > discussed. > > Best, > David > > On Thu, Jun 11, 2026 at 8:37 PM Gabriella Fu via dev > <[email protected]> wrote: > > > > Hi all, > > > > I’d like to start the discussion for KIP-1357: Add broker side custom > > assignors for "streams" groups > > > > KIP: > > > https://urldefense.com/v3/__https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=430408758__;!!Ayb5sqE7!t7TDJ8XzjCLooFy9FhJLiHKHu5c7RJf8-41tQuqRykmMLMB2yl4ApQsRMSEVKCIIl0FuvbznfZcDww$ > > JIRA: > https://urldefense.com/v3/__https://issues.apache.org/jira/browse/KAFKA-20683__;!!Ayb5sqE7!t7TDJ8XzjCLooFy9FhJLiHKHu5c7RJf8-41tQuqRykmMLMB2yl4ApQsRMSEVKCIIl0Fuvbzx-7tAbA$ > > > > Summary: > > The Streams Rebalance Protocol (KIP-1071) moves task assignment from the > > client to the broker, but unlike the classic protocol and KIP-848 > consumer > > groups, it offers no way to plug in a custom assignor. This KIP closes > that > > gap by making the existing streams task assignor interfaces public API > and > > adding broker and group configurations so operators can load custom > > assignor implementations and select them per group by short name, with no > > client-side involvement. > > > > Please let me know your feedback > > > > Thanks, > > Gabriella Fu >
