Hi kafka-devs, I would like a second review to the proposed changes on KIP-795: Add public APIs for AbstractCoordinator [ https://cwiki.apache.org/confluence/display/KAFKA/KIP-795%3A+Add+public+APIs+for +AbstractCoordinator ]
I've amended the KIP addressing Tom's feedback, and also opened a PR with the proposed changes [ https://github.com/apache/kafka/pull/11515 ]. There's also a PR for the kafka-monitor tool [ https://github.com/linkedin/kafka-monitor/pull/355 ] that demonstrates how we leveraged AbstractCoordinator to implement a High-Availability mode for it, with the intention of showing how this feature can be leveraged by other services within the Kafka ecosystem and outside it. Thanks for your time and consideration. Hector From: dev@kafka.apache.org At: 11/29/21 13:31:26 UTC-5:00To: dev@kafka.apache.org Subject: Re: [DISCUSS] KIP-795: Add public APIs for AbstractCoordinator Hello again Tom, kafka devs First, congrats on becoming a PMC member! That's so cool. Since your last reply I've updated the KIP trying to address some of your suggestions. A few more details have been added to the motivation section, and also went ahead and opened a draft pull-request with the changes I think are needed for this KIP, with the hope that it makes it easier to discuss the general approach and any other concerns the community may have. KIP: https://cwiki.apache.org/confluence/display/KAFKA/KIP-795%3A+Add+public+APIs+for +AbstractCoordinator PR: https://github.com/apache/kafka/pull/11515 Looking forward for some community feedback. Regards, Hector From: dev@kafka.apache.org At: 11/11/21 17:15:17 UTC-5:00To: dev@kafka.apache.org Subject: Re: [DISCUSS] KIP-795: Add public APIs for AbstractCoordinator Hi Tom, Thanks for taking time reviewing the KIP. I think it's reasonable to ask if Kafka's Group Coordination protocol should be used for use cases other than the distributed event log. This was actually briefly addressed by Gwen Shapira during her presentation at the strangeloop conference in '18 (a link to the video is included in the KIP), in which she explain in greater details the protocol internals. We should also keep in mind that this protocol is already being used for other use cases outside of core Kafka: Confluent Schema Registry uses it to determine leadership between members of a cluster, Kafka Connect uses it for task assignments, same with Kafka Stream for partition and task distributions, and so on. So having a public, stable API not just for new use cases (like ours) but existing ones is IMHO a good thing to have. I'll amend the KIP and add a bit more details to the motivation and alternatives sections, so the usefulness of this KIP is better understood. Now, for the first point of your technical observations (regarding protocolTypes()), I don't think it matters in this context, as the protocol name and subtype are only relevant in the context of a consumer group and group rebalance. It really doesn't matter if two different libraries decide to name their protocols the same. For item #2, I was under the impression that, because these classes all implement the org.apache.kafka.common.protocol.[Message, ApiMessage] interface, they are implicitly part of the Kafka protocol and the top-level API. Isn't that really the case? And finally, for #3, the goal I had in mind when creating this KPI was a small one: to provide an interface that users can rely on when extending the AbstactCoordinator. So my thought was that, while the AbstractCoordinator itself uses some internal APIs (like ConsumerNetworkClient, ConsumerMetadata and so on) those can remain internal. But it probably makes sense to at least explore the possibility of moving the whole AbstractCoordinator class to be part of the public API. I'll do that exercise, see what it entails, and update the KIP with my findings. Thanks again! Hector From: dev@kafka.apache.org At: 11/10/21 06:43:59 UTC-5:00To: Hector Geraldino (BLOOMBERG/ 919 3RD A ) , dev@kafka.apache.org Subject: Re: [DISCUSS] KIP-795: Add public APIs for AbstractCoordinator Hi Hector, Thanks for the KIP. At a high level, I think the question to be answered by the community is "Should Kafka really be providing this kind of cluster management API?". While Kafka clients need this to provide their functionality it's a different thing to expose that as a public API of the project, which is otherwise about providing a distributed event log/data streaming platform/<insert your favourite definition of Kafka here>. Having a public API brings a significant commitment for API compatibility, which could impair the ability of the project to change the API in order to make improvements to the Kafka clients. The current AbstractCoordinator not being a supported API means we don't currently have to reason about compatibility here. So I think it would help the motivation section of the KIP to describe in a bit more detail the use case(s) you have for implementing your own coordinators. For example, are these applications using Kafka otherwise, or just to leverage this API? And what alternatives to implementing your own coordinators did you consider, and why did you reject them? From a technical point of view, there are a number of issues I think would need addressing in order to do something like this: 1. There probably ought to be a way to ensure that protocolTypes() don't collide, or at least reduce the chances of a collision. While probably unlikely in practice the consequences of different protocols having the same name could be pretty confusing to debug. 2. JoinGroupRequestData and JoinGroupResponseData are not public classes (none of the *RequestData or *ResponseData classes are, intentionally), so there would have to be an abstraction for them. 3. It's all well and good having an interface that anyone can implement, but there is no supported Kafka API which takes an instance as a parameter (i.e. where do you plug your implementation in without having to use a bunch of other non-public Kafka APIs?) I assume in your current usage you're having to make use of other non-supported client APIs to make use of your coordinator. The KIP isn't really a complete piece of work without a way to use a custom implementation, in my opinion. It would be confusing if it looked like we were encouraging people to use those other non-supported APIs by making this coordinator public. Kind regards, Tom On Mon, Nov 8, 2021 at 2:01 PM Hector Geraldino (BLOOMBERG/ 919 3RD A) < hgerald...@bloomberg.net> wrote: > Hi Kafka devs, > > I would like to start the discussion of KIP-795: Add public APIs for > AbstractCoordinator > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-795%3A+Add+public+APIs+for +AbstractCoordinator > > Looking forward for some feedback from the community. > > Regards, > Hector
