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
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
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
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:
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
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
>
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
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
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
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
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
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
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
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
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
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
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
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.
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
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
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
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
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
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
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
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
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
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
28 matches
Mail list logo