Re: [DISCUSS] KIP-875: First-class offsets support in Kafka Connect

2023-01-19 Thread Chris Egerton
Hi Greg, Thanks for your thoughts. Responses inline: > Does this mean that a connector may be assigned to a non-leader worker in the cluster, an alter request comes in, and a connector instance is temporarily started on the leader to service the request? > While the alterOffsets method is being

Re: [DISCUSS] KIP-875: First-class offsets support in Kafka Connect

2023-01-18 Thread Greg Harris
Hi Chris, I had some clarifying questions about the alterOffsets hooks. The KIP includes these elements of the design: * The Javadoc for the methods mentions that the alterOffsets methods are only called on started and initialized connector objects. * The 'Altering' and 'Resetting' offsets

Re: [DISCUSS] KIP-875: First-class offsets support in Kafka Connect

2022-12-16 Thread Yash Mayya
Hi Chris, Thanks for clarifying, I had missed that update in the KIP (the bit about altering/resetting offsets response). I think your arguments for not going with an additional method or a custom return type make sense. Thanks, Yash On Sat, Dec 10, 2022 at 12:28 AM Chris Egerton wrote: > Hi

Re: [DISCUSS] KIP-875: First-class offsets support in Kafka Connect

2022-12-09 Thread Chris Egerton
Hi Yash, The idea with the boolean is to just signify that a connector has overridden this method, which allows us to issue a definitive response in the REST API when servicing offset alter/reset requests (described more in detail here:

Re: [DISCUSS] KIP-875: First-class offsets support in Kafka Connect

2022-12-07 Thread Yash Mayya
Hi Chris, Sorry for the late reply. > I don't believe logging an error message is sufficient for > handling failures to reset-after-delete. IMO it's highly > likely that users will either shoot themselves in the foot > by not reading the fine print and realizing that the offset > request may

Re: [DISCUSS] KIP-875: First-class offsets support in Kafka Connect

2022-11-21 Thread Mickael Maison
Hi Chris, Thanks for all the updates, yes that seems good! Mickael On Thu, Nov 17, 2022 at 8:41 PM Chris Egerton wrote: > > Hi Mickael, > > Thanks for your thoughts! IMO it's most intuitive to use a null value in > the PATCH API to signify that an offset should be reset, since it aligns >

Re: [DISCUSS] KIP-875: First-class offsets support in Kafka Connect

2022-11-17 Thread Chris Egerton
Hi Mickael, Thanks for your thoughts! IMO it's most intuitive to use a null value in the PATCH API to signify that an offset should be reset, since it aligns nicely with the API we provide to source connectors, where null offsets are translated under the hood to tombstone records in the internal

Re: [DISCUSS] KIP-875: First-class offsets support in Kafka Connect

2022-11-17 Thread Chris Egerton
Hi Yash, I've updated the KIP with the correct "kafka_topic", "kafka_partition", and "kafka_offset" keys in the JSON examples (settled on those instead of prefixing with "Kafka " for better interactions with tooling like JQ). I've also added a note about sink offset requests failing if there are

Re: [DISCUSS] KIP-875: First-class offsets support in Kafka Connect

2022-11-14 Thread Mickael Maison
Hi Chris, Thanks for the update! It's relatively common to only want to reset offsets for a specific resource (for example with MirrorMaker for one or a group of topics). Could it be possible to add a way to do so? Either by providing a payload to DELETE or by setting the offset field to an

Re: [DISCUSS] KIP-875: First-class offsets support in Kafka Connect

2022-11-12 Thread Yash Mayya
Hi Chris, Thanks for pointing out that the consumer group deletion step itself will fail in case of zombie sink tasks. Since we can't get any stronger guarantees from consumers (unlike with transactional producers), I think it makes perfect sense to fail the offset reset attempt in such scenarios

Re: [DISCUSS] KIP-875: First-class offsets support in Kafka Connect

2022-11-11 Thread Chris Egerton
Hi Mickael, Thanks for your feedback. This has been on my TODO list as well :) 1. That's fair! Support for altering offsets is easy enough to design, so I've added it to the KIP. The reset-after-delete feature, on the other hand, is actually pretty tricky to design; I've updated the rationale in

Re: [DISCUSS] KIP-875: First-class offsets support in Kafka Connect

2022-11-09 Thread Mickael Maison
Hi Chris, Thanks for the KIP, you're picking something that has been in my todo list for a while ;) It looks good overall, I just have a couple of questions: 1) I consider both features listed in Future Work pretty important. In both cases you mention the reason for not addressing them now is

Re: [DISCUSS] KIP-875: First-class offsets support in Kafka Connect

2022-11-08 Thread Chris Egerton
Hi Yash, Good question! This is actually a subtle source of asymmetry in the current proposal. Requests to delete a consumer group with active members will fail, so if there are zombie sink tasks that are still communicating with Kafka, offset reset requests for that connector will also fail. It

Re: [DISCUSS] KIP-875: First-class offsets support in Kafka Connect

2022-11-08 Thread Yash Mayya
Hi Chris, Thanks for the response and the explanations, I think you've answered pretty much all the questions I had meticulously! > if something goes wrong while resetting offsets, there's no > immediate impact--the connector will still be in the STOPPED > state. The REST response for requests

Re: [DISCUSS] KIP-875: First-class offsets support in Kafka Connect

2022-11-04 Thread Chris Egerton
Hi Yash, Thanks again for your thoughts! Responses to ongoing discussions inline (easier to track context than referencing comment numbers): > However, this then leads me to wonder if we can make that explicit by including "connect" or "connector" in the higher level field names? Or do you think

Re: [DISCUSS] KIP-875: First-class offsets support in Kafka Connect

2022-10-18 Thread Yash Mayya
Hi Chris, 1. Thanks a lot for elaborating on this, I'm now convinced about the usefulness of the new offset reset endpoint. Regarding the follow-up KIP for a fine-grained offset write API, I'd be happy to take that on once this KIP is finalized and I will definitely look forward to your feedback

Re: [DISCUSS] KIP-875: First-class offsets support in Kafka Connect

2022-10-17 Thread Chris Egerton
Hi Yash, Thanks for your detailed thoughts. 1. In KAFKA-4107 [1], the primary request is exactly what's proposed in the KIP right now: a way to reset offsets for connectors. Sure, there's an extra step of stopping the connector, but renaming a connector isn't as convenient of an alternative as

Re: [DISCUSS] KIP-875: First-class offsets support in Kafka Connect

2022-10-17 Thread Matthew Benedict de Detrich
I realised that I mistook the KPI and didn't realize this wasn't part of the REST API so you can ignore what I previously wrote. On Mon, 17 Oct 2022, 12:18 Matthew Benedict de Detrich, < matthew.dedetr...@aiven.io> wrote: > So my only 2 cents on this KIP is mainly from a REST perspective, i.e.

Re: Re: [DISCUSS] KIP-875: First-class offsets support in Kafka Connect

2022-10-17 Thread Chris Egerton
Thanks Greg! I think sorting results (at least w/r/t pagination) is only valuable if it's part of a formal contract; otherwise, a REST extension wouldn't really be able to take advantage of that behavior without becoming brittle across non-major version changes. I'd opt on the side of leaving it

Re: [DISCUSS] KIP-875: First-class offsets support in Kafka Connect

2022-10-17 Thread Matthew Benedict de Detrich
So my only 2 cents on this KIP is mainly from a REST perspective, i.e. in the KIP you mention that you need to add a new field "state.v2": “STOPPED” due to maintaining backwards compatibility with older brokers. My main qualm here is that putting versioning into JSON field names isn’t typically

Re: [DISCUSS] KIP-875: First-class offsets support in Kafka Connect

2022-10-16 Thread Yash Mayya
One more tiny thing, the Jira linked in the KIP seems to be a circular link to the KIP itself. Should it link to https://issues.apache.org/jira/browse/KAFKA-4107 instead? On Sun, Oct 16, 2022 at 8:12 PM Yash Mayya wrote: > Hi Chris, > > Thanks a lot for this KIP, I think something like this has

Re: [DISCUSS] KIP-875: First-class offsets support in Kafka Connect

2022-10-16 Thread Yash Mayya
Hi Chris, Thanks a lot for this KIP, I think something like this has been long overdue for Kafka Connect :) Some thoughts and questions that I had - 1. I'm wondering if you could elaborate a little more on the use case for the `DELETE /connectors/{connector}/offsets` API. I think we can all

Re: Re: [DISCUSS] KIP-875: First-class offsets support in Kafka Connect

2022-10-14 Thread Greg Harris
Hey Chris, Thanks for your quick reply! Design 1: I think this is a completely reasonable way to implement a V1, without any more complicated pagination. I had not considered that a REST extension would be able to add pagination, but upon reflection that makes a lot of sense. I like the

Re: [DISCUSS] KIP-875: First-class offsets support in Kafka Connect

2022-10-14 Thread Chris Egerton
Hi Ashwin, Thanks for your thoughts. Regarding your questions: 1. The response would show the offsets that are visible to the source connector, so it would combine the contents of the two topics, giving priority to offsets present in the connector-specific topic. I'm imagining a follow-up

Re: Re: [DISCUSS] KIP-875: First-class offsets support in Kafka Connect

2022-10-14 Thread Chris Egerton
Hi Greg, Thanks for your thoughts. RE your design questions: 1. The responses in the REST API may grow fairly large for sink connectors that consume from a large number of Kafka topic partitions, and source connectors that store a wide range of source partitions. If there is a large amount of

Re: [DISCUSS] KIP-875: First-class offsets support in Kafka Connect

2022-10-13 Thread Ashwin
Thanks for KIP Chris - I think this is a useful feature. Can you please elaborate on the following in the KIP - 1. How would the response of GET /connectors/{connector}/offsets look like if the worker has both global and connector specific offsets topic ? 2. How can we pass the reset options

RE: Re: [DISCUSS] KIP-875: First-class offsets support in Kafka Connect

2022-10-13 Thread Greg Harris
Hey Chris, Thanks for the KIP! I think this is an important feature for both development and operations use-cases, and it's an obvious gap in the REST feature set. I also appreciate the incremental nature of the KIP and the future extensions that will now be possible. I had a couple of

Re: [DISCUSS] KIP-875: First-class offsets support in Kafka Connect

2022-10-13 Thread Chris Egerton
Hi all, I noticed a fairly large gap in the first version of this KIP that I published last Friday, which has to do with accommodating connectors that target different Kafka clusters than the one that the Kafka Connect cluster uses for its internal topics and source connectors with dedicated