Re: [DISCUSS] KIP-1038: Add Custom Error Handler to Producer

2024-04-23 Thread Knowles Atchison Jr
Alieh,

Having run into issues with not being able to handle producer failures, I
think this is good functionality to have.

With this new functionality proposed at the Producer level, how would
ecosystems that sit on top of it function? Specifically, Connect was
updated a few years ago to allow Source Connect Workers to handle producer
exceptions that would never succeed when the source data was bad.

Knowles

On Tue, Apr 23, 2024 at 5:23 AM Alieh Saeedi 
wrote:

> Thanks Matthias. I changed it to `custom.exception.handler`
>
> Alieh
>
>
> On Tue, Apr 23, 2024 at 8:47 AM Matthias J. Sax  wrote:
>
> > Thanks Alieh!
> >
> > A few nits:
> >
> >
> > 1) The new config we add for the producer should be mentioned in the
> > "Public Interfaces" section.
> >
> > 2) Why do we use `producer.` prefix for a *producer* config? Should it
> > be `exception.handler` only?
> >
> >
> > -Matthias
> >
> > On 4/22/24 7:38 AM, Alieh Saeedi wrote:
> > > Thank you all for the feedback!
> > >
> > > Addressing the main concern: The KIP is about giving the user the
> ability
> > > to handle producer exceptions, but to be more conservative and avoid
> > future
> > > issues, we decided to be limited to a short list of exceptions. I
> > included
> > > *RecordTooLargeExceptin* and *UnknownTopicOrPartitionException. *Open
> to
> > > suggestion for adding some more ;-)
> > >
> > > KIP Updates:
> > > - clarified the way that the user should configure the Producer to use
> > the
> > > custom handler. I think adding a producer config property is the
> cleanest
> > > one.
> > > - changed the *ClientExceptionHandler* to *ProducerExceptionHandler* to
> > be
> > > closer to what we are changing.
> > > - added the ProducerRecord as the input parameter of the handle()
> method
> > as
> > > well.
> > > - increased the response types to 3 to have fail and two types of
> > continue.
> > > - The default behaviour is having no custom handler, having the
> > > corresponding config parameter set to null. Therefore, the KIP provides
> > no
> > > default implementation of the interface.
> > > - We follow the interface solution as described in the
> > > Rejected Alternetives section.
> > >
> > >
> > > Cheers,
> > > Alieh
> > >
> > >
> > > On Thu, Apr 18, 2024 at 8:11 PM Matthias J. Sax 
> > wrote:
> > >
> > >> Thanks for the KIP Alieh! It addresses an important case for error
> > >> handling.
> > >>
> > >> I agree that using this handler would be an expert API, as mentioned
> by
> > >> a few people. But I don't think it would be a reason to not add it.
> It's
> > >> always a tricky tradeoff what to expose to users and to avoid foot
> guns,
> > >> but we added similar handlers to Kafka Streams, and have good
> experience
> > >> with it. Hence, I understand, but don't share the concern raised.
> > >>
> > >> I also agree that there is some responsibility by the user to
> understand
> > >> how such a handler should be implemented to not drop data by accident.
> > >> But it seem unavoidable and acceptable.
> > >>
> > >> While I understand that a "simpler / reduced" API (eg via configs)
> might
> > >> also work, I personally prefer a full handler. Configs have the same
> > >> issue that they could be miss-used potentially leading to incorrectly
> > >> dropped data, but at the same time are less flexible (and thus maybe
> > >> ever harder to use correctly...?). Base on my experience, there is
> also
> > >> often weird corner case for which it make sense to also drop records
> for
> > >> other exceptions, and a full handler has the advantage of full
> > >> flexibility and "absolute power!".
> > >>
> > >> To be fair: I don't know the exact code paths of the producer in
> > >> details, so please keep me honest. But my understanding is, that the
> KIP
> > >> aims to allow users to react to internal exception, and decide to keep
> > >> retrying internally, swallow the error and drop the record, or raise
> the
> > >> error?
> > >>
> > >> Maybe the KIP would need to be a little bit more precises what error
> we
> > >> want to cover -- I don't think this list must be exhaustive, as we can
> > >> always do follow up KIP to also apply the handler to other errors to
> > >> expand the scope of the handler. The KIP does mention examples, but it
> > >> might be good to explicitly state for what cases the handler gets
> > applied?
> > >>
> > >> I am also not sure if CONTINUE and FAIL are enough options? Don't we
> > >> need three options? Or would `CONTINUE` have different meaning
> depending
> > >> on the type of error? Ie, for a retryable error `CONTINUE` would mean
> > >> keep retrying internally, but for a non-retryable error `CONTINUE`
> means
> > >> swallow the error and drop the record? This semantic overload seems
> > >> tricky to reason about by users, so it might better to split
> `CONTINUE`
> > >> into two cases -> `RETRY` and `SWALLOW` (or some better names).
> > >>
> > >> Additionally, should we just ship a `DefaultClientExceptionHandler`
> > >> which would return 

Re: [VOTE] KIP-477: Add PATCH method for connector config in Connect REST API

2024-04-08 Thread Knowles Atchison Jr
+1 (non binding)

On Mon, Apr 8, 2024, 3:30 PM Chris Egerton  wrote:

> Thanks Ivan! +1 (binding) from me.
>
> On Mon, Apr 8, 2024, 06:59 Ivan Yurchenko  wrote:
>
> > Hello!
> >
> > I'd like to put the subj KIP[1] to a vote. Thank you.
> >
> > Best regards,
> > Ivan
> >
> > [1]
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-477%3A+Add+PATCH+method+for+connector+config+in+Connect+REST+API
> >
>


Re: [VOTE] KIP-980: Allow creating connectors in a stopped state

2023-10-09 Thread Knowles Atchison Jr
This is super useful for pipeline setup!

+1 (non binding)

On Mon, Oct 9, 2023, 7:57 AM Chris Egerton  wrote:

> Thanks for the KIP, Yash!
>
> +1 (binding)
>
> On Mon, Oct 9, 2023, 01:12 Yash Mayya  wrote:
>
> > Hi all,
> >
> > I'd like to start a vote on KIP-980 which proposes allowing the creation
> of
> > connectors in a stopped (or paused) state.
> >
> > KIP -
> >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-980%3A+Allow+creating+connectors+in+a+stopped+state
> >
> > Discussion Thread -
> > https://lists.apache.org/thread/om803vl191ysf711qm7czv94285rtt5d
> >
> > Thanks,
> > Yash
> >
>


Re: [VOTE] KIP-864: Add End-To-End Latency Metrics to Connectors

2023-02-27 Thread Knowles Atchison Jr
+1 (non binding)

On Mon, Feb 27, 2023 at 11:21 AM Chris Egerton 
wrote:

> Hi all,
>
> I could have sworn I +1'd this but I can't seem to find a record of that.
>
> In the hopes that this action is idempotent, +1 (binding). Thanks for the
> KIP!
>
> Cheers,
>
> Chris
>
> On Mon, Feb 27, 2023 at 6:28 AM Mickael Maison 
> wrote:
>
> > Thanks for the KIP
> >
> > +1 (binding)
> >
> > On Thu, Jan 26, 2023 at 4:36 PM Jorge Esteban Quilcate Otoya
> >  wrote:
> > >
> > > Hi all,
> > >
> > > I'd like to call for a vote on KIP-864, which proposes to add metrics
> to
> > > measure end-to-end latency in source and sink connectors.
> > >
> > > KIP:
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-864%3A+Add+End-To-End+Latency+Metrics+to+Connectors
> > >
> > > Discussion thread:
> > > https://lists.apache.org/thread/k6rh2mr7pg94935fgpqw8b5fj308f2n7
> > >
> > > Many thanks,
> > > Jorge.
> >
>


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

2023-01-24 Thread Knowles Atchison Jr
+1 (non binding)

On Tue, Jan 24, 2023 at 5:24 AM Yash Mayya  wrote:

> Hi Chris,
>
> I'm +1 (non-binding). Thanks again for proposing this extremely
> valuable addition to Kafka Connect!
>
> Thanks,
> Yash
>
> On Thu, Jan 19, 2023 at 12:11 AM Chris Egerton 
> wrote:
>
> > Hi all,
> >
> > I'd like to call for a vote on KIP-875, which adds support for viewing
> and
> > manipulating the offsets of connectors to the Kafka Connect REST API.
> >
> > The KIP:
> >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-875%3A+First-class+offsets+support+in+Kafka+Connect
> >
> > The discussion thread:
> > https://lists.apache.org/thread/m5bklnh5w4mwr9nbzrmfk0pftpxfjd02
> >
> > Cheers,
> >
> > Chris
> >
>


Re: [DISCUSS] KIP-886 Add Client Producer and Consumer Builders

2022-11-10 Thread Knowles Atchison Jr
This would be helpful. For our own client library wrappers we implemented
this functionality for any type with defaults for  and
 consumers/producers.

On Thu, Nov 10, 2022, 6:35 PM Dan S  wrote:

> Hello all,
>
> I think that adding builders for the producer and the consumer in kafka
> client would make it much easier for developers to instantiate new
> producers and consumers, especially if they are using an IDE with
> intellisense, and using the IDE to navigate to the documentation which
> could be added to the builder's withXYZ methods.
>
> Please let me know if you have any comments, questions, or suggestions!
>
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-886%3A+Add+Client+Producer+and+Consumer+Builders
>
> Thanks,
>
> Dan
>


Re: [DISCUSS] KIP-779: Allow Source Tasks to Handle Producer Exceptions

2021-12-10 Thread Knowles Atchison Jr
Good morning,

Any additional feedback and/or review on the PR for this change would be
greatly appreciated:

https://github.com/apache/kafka/pull/11382

Knowles

On Tue, Nov 16, 2021 at 4:02 PM Knowles Atchison Jr 
wrote:

> Thank you all for the feedback, the KIP has been updated.
>
> On Tue, Nov 16, 2021 at 10:46 AM Arjun Satish 
> wrote:
>
>> One more nit: the RetryWithToleranceOperator class is not a public
>> interface. So we do not have to call the changes in them out in the Public
>> Interfaces section.
>>
>>
>> On Tue, Nov 16, 2021 at 10:42 AM Arjun Satish 
>> wrote:
>>
>> > Chris' point about upgrades is valid. An existing configuration will now
>> > have additional behavior. We should clearly call this out in the kip,
>> and
>> > whenever they are prepared -- the release notes. It's a bit crummy when
>> > upgrading, but I do think it's better than introducing a new
>> configuration
>> > in the long term.
>> >
>> > On Mon, Nov 15, 2021 at 2:52 PM Knowles Atchison Jr <
>> katchiso...@gmail.com>
>> > wrote:
>> >
>> >> Chris,
>> >>
>> >> Thank you for the feedback. I can certainly update the KIP to state
>> that
>> >> once exactly one support is in place, the task would be failed even if
>> >> error.tolerance were set to all. Programmatically it would still
>> require
>> >> PRs to be merged to build on top of. I also liked my original
>> >> implementation of the hook as it gave the connector writers the most
>> >> flexibility in handling producer errors. I changed the original
>> >> implementation as the progression/changes still supported my use case
>> and
>> >> I
>> >> thought it would move this process along faster.
>> >>
>> >> Knowles
>> >>
>> >> On Thu, Nov 11, 2021 at 3:43 PM Chris Egerton
>> > >> >
>> >> wrote:
>> >>
>> >> > Hi Knowles,
>> >> >
>> >> > I think this looks good for the most part but I'd still like to see
>> an
>> >> > explicit mention in the KIP (and proposed doc/Javadoc changes) that
>> >> states
>> >> > that, with exactly-once support enabled, producer exceptions that
>> result
>> >> > from failures related to exactly-once support (including but not
>> >> limited to
>> >> > ProducerFencedExcecption instances (
>> >> >
>> >> >
>> >>
>> https://kafka.apache.org/30/javadoc/org/apache/kafka/common/errors/ProducerFencedException.html
>> >> > ))
>> >> > will not be skipped even with "errors.tolerance" set to "all", and
>> will
>> >> > instead unconditionally cause the task to fail. Your proposal that
>> >> > "WorkerSourceTask could check the configuration before handing off
>> the
>> >> > records and exception to this function" seems great as long as we
>> update
>> >> > "handing off the records and exceptions to this function" to the
>> >> > newly-proposed behavior of "logging the exception and continuing to
>> poll
>> >> > the task for data".
>> >> >
>> >> > I'm also a little bit wary of updating the existing
>> "errors.tolerance"
>> >> > configuration to have new behavior that users can't opt out of
>> without
>> >> also
>> >> > opting out of the current behavior they get with "errors.tolerance"
>> set
>> >> to
>> >> > "all", but I think I've found a decent argument in favor of it. One
>> >> thought
>> >> > that came to mind is whether this use case was originally considered
>> >> when
>> >> > KIP-298 was being discussed. However, it appears that KAFKA-8586 (
>> >> > https://issues.apache.org/jira/browse/KAFKA-8586), the fix for which
>> >> > caused
>> >> > tasks to fail on non-retriable, asynchronous producer exceptions
>> >> instead of
>> >> > logging them and continuing, was discovered over a full year after
>> the
>> >> > changes for KIP-298 (https://github.com/apache/kafka/pull/5065) were
>> >> > merged. I suspect that the current proposal aligns nicely with the
>> >> original
>> >> > design intent of KIP-298, and that if KAFKA-8586 were discovered
>> befo

Re: [VOTE] KIP-779: Allow Source Tasks to Handle Producer Exceptions

2021-11-29 Thread Knowles Atchison Jr
Thank you all for voting!

KIP-779 has been approved:

3 binding votes (John, Mickael, Tom)
4 non-binding votes (Knowles, Chris S., Chris E., Arjun)

The vote is now closed. Other than modifying the wiki, is anything
additional I need to do vote wise?

Knowles

On Mon, Nov 29, 2021 at 10:49 AM Tom Bentley  wrote:

> Hi Knowles,
>
> Thanks for the KIP, +1 (binding)
>
> Kind regards,
>
> Tom
>
> On 11/29/21, Mickael Maison  wrote:
> > Hi Knowles,
> >
> > +1 (binding)
> >
> > Thanks for the KIP!
> >
> > On Mon, Nov 29, 2021 at 12:56 PM Knowles Atchison Jr
> >  wrote:
> >>
> >> Good morning,
> >>
> >> Bringing this back to the top.
> >>
> >> We currently have
> >>
> >> 1 binding
> >> 4 non-binding
> >>
> >> Knowles
> >>
> >> On Fri, Nov 19, 2021 at 10:02 AM Knowles Atchison Jr
> >> 
> >> wrote:
> >>
> >> > Thank you all for voting. We still need two more binding votes.
> >> >
> >> > I have rebased and updated the PR to be ready to go once this vote
> >> > passes:
> >> >
> >> > https://github.com/apache/kafka/pull/11382
> >> >
> >> > Knowles
> >> >
> >> > On Tue, Nov 16, 2021 at 3:43 PM Chris Egerton
> >> > 
> >> > wrote:
> >> >
> >> >> +1 (non-binding). Thanks Knowles!
> >> >>
> >> >> On Tue, Nov 16, 2021 at 10:48 AM Arjun Satish <
> arjun.sat...@gmail.com>
> >> >> wrote:
> >> >>
> >> >> > +1 (non-binding). Thanks for the KIP, Knowles! and appreciate the
> >> >> > follow-ups!
> >> >> >
> >> >> > On Thu, Nov 11, 2021 at 2:55 PM John Roesler 
> >> >> wrote:
> >> >> >
> >> >> > > Thanks, Knowles!
> >> >> > >
> >> >> > > I'm +1 (binding)
> >> >> > >
> >> >> > > -John
> >> >> > >
> >> >> > > On Wed, 2021-11-10 at 12:42 -0500, Christopher Shannon
> >> >> > > wrote:
> >> >> > > > +1 (non-binding). This looks good to me and will be useful as a
> >> >> > > > way
> >> >> to
> >> >> > > > handle producer errors.
> >> >> > > >
> >> >> > > > On Mon, Nov 8, 2021 at 8:55 AM Knowles Atchison Jr <
> >> >> > > katchiso...@gmail.com>
> >> >> > > > wrote:
> >> >> > > >
> >> >> > > > > Good morning,
> >> >> > > > >
> >> >> > > > > I'd like to start a vote for KIP-779: Allow Source Tasks to
> >> >> > > > > Handle
> >> >> > > Producer
> >> >> > > > > Exceptions:
> >> >> > > > >
> >> >> > > > >
> >> >> > > > >
> >> >> > >
> >> >> >
> >> >>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-779%3A+Allow+Source+Tasks+to+Handle+Producer+Exceptions
> >> >> > > > >
> >> >> > > > > The purpose of this KIP is to allow Source Tasks the option
> to
> >> >> > "ignore"
> >> >> > > > > kafka producer exceptions. After a few iterations, this is
> now
> >> >> part
> >> >> > of
> >> >> > > the
> >> >> > > > > "errors.tolerance" configuration and provides a null
> >> >> RecordMetadata
> >> >> > to
> >> >> > > > > commitRecord() in lieu of a new SourceTask interface method
> or
> >> >> worker
> >> >> > > > > configuration item.
> >> >> > > > >
> >> >> > > > > PR is here:
> >> >> > > > >
> >> >> > > > > https://github.com/apache/kafka/pull/11382
> >> >> > > > >
> >> >> > > > > Any comments and feedback are welcome.
> >> >> > > > >
> >> >> > > > > Knowles
> >> >> > > > >
> >> >> > >
> >> >> > >
> >> >> > >
> >> >> >
> >> >>
> >> >
> >
> >
>
>


Re: [VOTE] KIP-779: Allow Source Tasks to Handle Producer Exceptions

2021-11-29 Thread Knowles Atchison Jr
Good morning,

Bringing this back to the top.

We currently have

1 binding
4 non-binding

Knowles

On Fri, Nov 19, 2021 at 10:02 AM Knowles Atchison Jr 
wrote:

> Thank you all for voting. We still need two more binding votes.
>
> I have rebased and updated the PR to be ready to go once this vote passes:
>
> https://github.com/apache/kafka/pull/11382
>
> Knowles
>
> On Tue, Nov 16, 2021 at 3:43 PM Chris Egerton 
> wrote:
>
>> +1 (non-binding). Thanks Knowles!
>>
>> On Tue, Nov 16, 2021 at 10:48 AM Arjun Satish 
>> wrote:
>>
>> > +1 (non-binding). Thanks for the KIP, Knowles! and appreciate the
>> > follow-ups!
>> >
>> > On Thu, Nov 11, 2021 at 2:55 PM John Roesler 
>> wrote:
>> >
>> > > Thanks, Knowles!
>> > >
>> > > I'm +1 (binding)
>> > >
>> > > -John
>> > >
>> > > On Wed, 2021-11-10 at 12:42 -0500, Christopher Shannon
>> > > wrote:
>> > > > +1 (non-binding). This looks good to me and will be useful as a way
>> to
>> > > > handle producer errors.
>> > > >
>> > > > On Mon, Nov 8, 2021 at 8:55 AM Knowles Atchison Jr <
>> > > katchiso...@gmail.com>
>> > > > wrote:
>> > > >
>> > > > > Good morning,
>> > > > >
>> > > > > I'd like to start a vote for KIP-779: Allow Source Tasks to Handle
>> > > Producer
>> > > > > Exceptions:
>> > > > >
>> > > > >
>> > > > >
>> > >
>> >
>> https://cwiki.apache.org/confluence/display/KAFKA/KIP-779%3A+Allow+Source+Tasks+to+Handle+Producer+Exceptions
>> > > > >
>> > > > > The purpose of this KIP is to allow Source Tasks the option to
>> > "ignore"
>> > > > > kafka producer exceptions. After a few iterations, this is now
>> part
>> > of
>> > > the
>> > > > > "errors.tolerance" configuration and provides a null
>> RecordMetadata
>> > to
>> > > > > commitRecord() in lieu of a new SourceTask interface method or
>> worker
>> > > > > configuration item.
>> > > > >
>> > > > > PR is here:
>> > > > >
>> > > > > https://github.com/apache/kafka/pull/11382
>> > > > >
>> > > > > Any comments and feedback are welcome.
>> > > > >
>> > > > > Knowles
>> > > > >
>> > >
>> > >
>> > >
>> >
>>
>


Re: [VOTE] KIP-779: Allow Source Tasks to Handle Producer Exceptions

2021-11-19 Thread Knowles Atchison Jr
Thank you all for voting. We still need two more binding votes.

I have rebased and updated the PR to be ready to go once this vote passes:

https://github.com/apache/kafka/pull/11382

Knowles

On Tue, Nov 16, 2021 at 3:43 PM Chris Egerton 
wrote:

> +1 (non-binding). Thanks Knowles!
>
> On Tue, Nov 16, 2021 at 10:48 AM Arjun Satish 
> wrote:
>
> > +1 (non-binding). Thanks for the KIP, Knowles! and appreciate the
> > follow-ups!
> >
> > On Thu, Nov 11, 2021 at 2:55 PM John Roesler 
> wrote:
> >
> > > Thanks, Knowles!
> > >
> > > I'm +1 (binding)
> > >
> > > -John
> > >
> > > On Wed, 2021-11-10 at 12:42 -0500, Christopher Shannon
> > > wrote:
> > > > +1 (non-binding). This looks good to me and will be useful as a way
> to
> > > > handle producer errors.
> > > >
> > > > On Mon, Nov 8, 2021 at 8:55 AM Knowles Atchison Jr <
> > > katchiso...@gmail.com>
> > > > wrote:
> > > >
> > > > > Good morning,
> > > > >
> > > > > I'd like to start a vote for KIP-779: Allow Source Tasks to Handle
> > > Producer
> > > > > Exceptions:
> > > > >
> > > > >
> > > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-779%3A+Allow+Source+Tasks+to+Handle+Producer+Exceptions
> > > > >
> > > > > The purpose of this KIP is to allow Source Tasks the option to
> > "ignore"
> > > > > kafka producer exceptions. After a few iterations, this is now part
> > of
> > > the
> > > > > "errors.tolerance" configuration and provides a null RecordMetadata
> > to
> > > > > commitRecord() in lieu of a new SourceTask interface method or
> worker
> > > > > configuration item.
> > > > >
> > > > > PR is here:
> > > > >
> > > > > https://github.com/apache/kafka/pull/11382
> > > > >
> > > > > Any comments and feedback are welcome.
> > > > >
> > > > > Knowles
> > > > >
> > >
> > >
> > >
> >
>


Re: [DISCUSS] KIP-779: Allow Source Tasks to Handle Producer Exceptions

2021-11-16 Thread Knowles Atchison Jr
Thank you all for the feedback, the KIP has been updated.

On Tue, Nov 16, 2021 at 10:46 AM Arjun Satish 
wrote:

> One more nit: the RetryWithToleranceOperator class is not a public
> interface. So we do not have to call the changes in them out in the Public
> Interfaces section.
>
>
> On Tue, Nov 16, 2021 at 10:42 AM Arjun Satish 
> wrote:
>
> > Chris' point about upgrades is valid. An existing configuration will now
> > have additional behavior. We should clearly call this out in the kip, and
> > whenever they are prepared -- the release notes. It's a bit crummy when
> > upgrading, but I do think it's better than introducing a new
> configuration
> > in the long term.
> >
> > On Mon, Nov 15, 2021 at 2:52 PM Knowles Atchison Jr <
> katchiso...@gmail.com>
> > wrote:
> >
> >> Chris,
> >>
> >> Thank you for the feedback. I can certainly update the KIP to state that
> >> once exactly one support is in place, the task would be failed even if
> >> error.tolerance were set to all. Programmatically it would still require
> >> PRs to be merged to build on top of. I also liked my original
> >> implementation of the hook as it gave the connector writers the most
> >> flexibility in handling producer errors. I changed the original
> >> implementation as the progression/changes still supported my use case
> and
> >> I
> >> thought it would move this process along faster.
> >>
> >> Knowles
> >>
> >> On Thu, Nov 11, 2021 at 3:43 PM Chris Egerton
>  >> >
> >> wrote:
> >>
> >> > Hi Knowles,
> >> >
> >> > I think this looks good for the most part but I'd still like to see an
> >> > explicit mention in the KIP (and proposed doc/Javadoc changes) that
> >> states
> >> > that, with exactly-once support enabled, producer exceptions that
> result
> >> > from failures related to exactly-once support (including but not
> >> limited to
> >> > ProducerFencedExcecption instances (
> >> >
> >> >
> >>
> https://kafka.apache.org/30/javadoc/org/apache/kafka/common/errors/ProducerFencedException.html
> >> > ))
> >> > will not be skipped even with "errors.tolerance" set to "all", and
> will
> >> > instead unconditionally cause the task to fail. Your proposal that
> >> > "WorkerSourceTask could check the configuration before handing off the
> >> > records and exception to this function" seems great as long as we
> update
> >> > "handing off the records and exceptions to this function" to the
> >> > newly-proposed behavior of "logging the exception and continuing to
> poll
> >> > the task for data".
> >> >
> >> > I'm also a little bit wary of updating the existing "errors.tolerance"
> >> > configuration to have new behavior that users can't opt out of without
> >> also
> >> > opting out of the current behavior they get with "errors.tolerance"
> set
> >> to
> >> > "all", but I think I've found a decent argument in favor of it. One
> >> thought
> >> > that came to mind is whether this use case was originally considered
> >> when
> >> > KIP-298 was being discussed. However, it appears that KAFKA-8586 (
> >> > https://issues.apache.org/jira/browse/KAFKA-8586), the fix for which
> >> > caused
> >> > tasks to fail on non-retriable, asynchronous producer exceptions
> >> instead of
> >> > logging them and continuing, was discovered over a full year after the
> >> > changes for KIP-298 (https://github.com/apache/kafka/pull/5065) were
> >> > merged. I suspect that the current proposal aligns nicely with the
> >> original
> >> > design intent of KIP-298, and that if KAFKA-8586 were discovered
> before
> >> or
> >> > during discussion for KIP-298, non-retriable, asynchronous producer
> >> > exceptions would have been included in its scope. With that in mind,
> >> > although it may cause issues for some niche use cases, I think that
> >> this is
> >> > a valid change and would be worth the tradeoff of potentially
> >> complicating
> >> > life for a small number of users. I'd be interested in Arjun's
> thoughts
> >> on
> >> > this though (as he designed and implemented KIP-298), and if this
> >> analysis
> >> > is agreeable

Re: [DISCUSS] KIP-779: Allow Source Tasks to Handle Producer Exceptions

2021-11-15 Thread Knowles Atchison Jr
Chris,

Thank you for the feedback. I can certainly update the KIP to state that
once exactly one support is in place, the task would be failed even if
error.tolerance were set to all. Programmatically it would still require
PRs to be merged to build on top of. I also liked my original
implementation of the hook as it gave the connector writers the most
flexibility in handling producer errors. I changed the original
implementation as the progression/changes still supported my use case and I
thought it would move this process along faster.

Knowles

On Thu, Nov 11, 2021 at 3:43 PM Chris Egerton 
wrote:

> Hi Knowles,
>
> I think this looks good for the most part but I'd still like to see an
> explicit mention in the KIP (and proposed doc/Javadoc changes) that states
> that, with exactly-once support enabled, producer exceptions that result
> from failures related to exactly-once support (including but not limited to
> ProducerFencedExcecption instances (
>
> https://kafka.apache.org/30/javadoc/org/apache/kafka/common/errors/ProducerFencedException.html
> ))
> will not be skipped even with "errors.tolerance" set to "all", and will
> instead unconditionally cause the task to fail. Your proposal that
> "WorkerSourceTask could check the configuration before handing off the
> records and exception to this function" seems great as long as we update
> "handing off the records and exceptions to this function" to the
> newly-proposed behavior of "logging the exception and continuing to poll
> the task for data".
>
> I'm also a little bit wary of updating the existing "errors.tolerance"
> configuration to have new behavior that users can't opt out of without also
> opting out of the current behavior they get with "errors.tolerance" set to
> "all", but I think I've found a decent argument in favor of it. One thought
> that came to mind is whether this use case was originally considered when
> KIP-298 was being discussed. However, it appears that KAFKA-8586 (
> https://issues.apache.org/jira/browse/KAFKA-8586), the fix for which
> caused
> tasks to fail on non-retriable, asynchronous producer exceptions instead of
> logging them and continuing, was discovered over a full year after the
> changes for KIP-298 (https://github.com/apache/kafka/pull/5065) were
> merged. I suspect that the current proposal aligns nicely with the original
> design intent of KIP-298, and that if KAFKA-8586 were discovered before or
> during discussion for KIP-298, non-retriable, asynchronous producer
> exceptions would have been included in its scope. With that in mind,
> although it may cause issues for some niche use cases, I think that this is
> a valid change and would be worth the tradeoff of potentially complicating
> life for a small number of users. I'd be interested in Arjun's thoughts on
> this though (as he designed and implemented KIP-298), and if this analysis
> is agreeable, we may want to document that information in the KIP as well
> to strengthen our case for not introducing a new configuration property and
> instead making this behavior tied to the existing "errors.tolerance"
> property with no opt-out besides using a new value for that property.
>
> My last thought is that, although it may be outside the scope of this KIP,
> I believe your original proposal of giving tasks a hook to handle
> downstream exceptions is actually quite valid. The DLQ feature for sink
> connectors is an extremely valuable one as it prevents data loss when
> "errors.tolerance" is set to "all" by allowing users to reprocess
> problematic records at a later date without stopping the flow of data in
> their connector entirely. As others have noted, it's difficult if not
> outright impossible to provide a Kafka DLQ topic for source connectors with
> the same guarantees, and so allowing source connectors the option of
> storing problematic records back in the system that they came from seems
> like a reasonable alternative. I think we're probably past the point of
> making that happen in this KIP, but I don't believe the changes you've
> proposed make that any harder in the future than it is now (which is
> great!), and I wanted to voice my general support for a mechanism like this
> in case you or someone following along think it'd be worth it to pursue at
> a later date.
>
> Thanks for your KIP and thanks for your patience with the process!
>
> Cheers,
>
> Chris
>
> On Fri, Nov 5, 2021 at 8:26 AM Knowles Atchison Jr 
> wrote:
>
> > Good morning,
> >
> > If there is no additional feedback, I am going to call a vote for this
> KIP
> > on Monday.
> >
> > Knowles
> >
> > On Tue, Nov 2, 20

[VOTE] KIP-779: Allow Source Tasks to Handle Producer Exceptions

2021-11-08 Thread Knowles Atchison Jr
Good morning,

I'd like to start a vote for KIP-779: Allow Source Tasks to Handle Producer
Exceptions:

https://cwiki.apache.org/confluence/display/KAFKA/KIP-779%3A+Allow+Source+Tasks+to+Handle+Producer+Exceptions

The purpose of this KIP is to allow Source Tasks the option to "ignore"
kafka producer exceptions. After a few iterations, this is now part of the
"errors.tolerance" configuration and provides a null RecordMetadata to
commitRecord() in lieu of a new SourceTask interface method or worker
configuration item.

PR is here:

https://github.com/apache/kafka/pull/11382

Any comments and feedback are welcome.

Knowles


Re: [DISCUSS] KIP-779: Allow Source Tasks to Handle Producer Exceptions

2021-11-05 Thread Knowles Atchison Jr
Good morning,

If there is no additional feedback, I am going to call a vote for this KIP
on Monday.

Knowles

On Tue, Nov 2, 2021 at 10:00 AM Knowles Atchison Jr 
wrote:

> Third time's the charm.
>
> I've added a getter for the RetryWithToleranceOperator to get the
> ToleranceType. I've updated WorkerSourceTask to check this setting to see
> if it is ToleranceType.ALL.
>
> Setting "errors.tolerance" to "all" solves both problems:
>
> 1. Use an existing configuration
> 2. Moves the configuration back to the connector/task level instead of at
> the connect worker level.
>
> I've updated the KIP and PR.
>
> Additional thoughts and feedback are welcome.
>
> Knowles
>
> On Mon, Nov 1, 2021 at 2:00 AM Arjun Satish 
> wrote:
>
>> Looks really nice. Thanks for the changes. Couple of suggestions:
>>
>> 1. Can we reuse any of the existing configs, instead of introducing a new
>> one? I’m wondering if the error.tolerance configuration’s scope can be
>> increased to include produce errors as well. That’ll help us keep number
>> of
>> configs in check. Effectively, if error.tolerance is set to all, then the
>> behavior would be like how you describe the worker would ignore producer
>> errors.
>>
>> 2. If we do choose to have a new config, could you please call out the
>> possible values it can take in the kip?
>>
>> Thanks again!
>>
>> Best,
>>
>>
>> On Fri, Oct 29, 2021 at 9:53 AM Knowles Atchison Jr <
>> katchiso...@gmail.com>
>> wrote:
>>
>> > Arjun,
>> >
>> > Thank you for your feedback, I have updated the KIP.
>> >
>> > This solution is more elegant than my original proposal; however, after
>> > working on the implementation, we have now pushed the configuration from
>> > the connector/task itself back to the connect worker. All tasks running
>> on
>> > the worker would share this ignore producer exception configuration
>> flag.
>> > This works for my use cases where I cannot envision setting this for
>> only
>> > one type of connector we have, but this does take the choice out of the
>> > hands of the connector developer. I suppose that is for the best, in a
>> > vacuum only the worker should have a say in how it handles message
>> > production.
>> >
>> > Additional thoughts and feedback are welcome.
>> >
>> > Knowles
>> >
>> > On Thu, Oct 28, 2021 at 10:54 AM Arjun Satish 
>> > wrote:
>> >
>> > > Yes, that makes sense. And it fits in very nicely with the current
>> error
>> > > handling framework.
>> > >
>> > > On Thu, Oct 28, 2021 at 10:39 AM Knowles Atchison Jr <
>> > > katchiso...@gmail.com>
>> > > wrote:
>> > >
>> > > > That would work. I originally thought that it would be confusing to
>> > > > overload that function when a Record that wasn't actually written,
>> but
>> > > > looking at SourceTask more closely, in commitRecord(SourceRecord,
>> > > > RecordMetadata), the RecordMetadata is set to null in the event of a
>> > > > filtered transformation so the framework is already doing this in a
>> > > certain
>> > > > regard.
>> > > >
>> > > > Knowles
>> > > >
>> > > > On Thu, Oct 28, 2021 at 10:29 AM Arjun Satish <
>> arjun.sat...@gmail.com>
>> > > > wrote:
>> > > >
>> > > > > To ack the message back to the source system, we already have a
>> > > > > commitRecord method. Once the bad record is handled by skip/dlq,
>> we
>> > > could
>> > > > > just call commitRecord() on it?
>> > > > >
>> > > > > On Thu, Oct 28, 2021 at 9:35 AM Knowles Atchison Jr <
>> > > > katchiso...@gmail.com
>> > > > > >
>> > > > > wrote:
>> > > > >
>> > > > > > Hi Chris,
>> > > > > >
>> > > > > > Thank you for your reply!
>> > > > > >
>> > > > > > It is a clarity error regarding the javadoc. I am not
>> operationally
>> > > > > > familiar with all of the exceptions Kafka considers
>> non-retriable,
>> > > so I
>> > > > > > pulled the list from Callback.java:
>> > > > > >
>> > > > > >
>> > > > >
>> &

Re: [DISCUSS] KIP-779: Allow Source Tasks to Handle Producer Exceptions

2021-11-02 Thread Knowles Atchison Jr
Third time's the charm.

I've added a getter for the RetryWithToleranceOperator to get the
ToleranceType. I've updated WorkerSourceTask to check this setting to see
if it is ToleranceType.ALL.

Setting "errors.tolerance" to "all" solves both problems:

1. Use an existing configuration
2. Moves the configuration back to the connector/task level instead of at
the connect worker level.

I've updated the KIP and PR.

Additional thoughts and feedback are welcome.

Knowles

On Mon, Nov 1, 2021 at 2:00 AM Arjun Satish  wrote:

> Looks really nice. Thanks for the changes. Couple of suggestions:
>
> 1. Can we reuse any of the existing configs, instead of introducing a new
> one? I’m wondering if the error.tolerance configuration’s scope can be
> increased to include produce errors as well. That’ll help us keep number of
> configs in check. Effectively, if error.tolerance is set to all, then the
> behavior would be like how you describe the worker would ignore producer
> errors.
>
> 2. If we do choose to have a new config, could you please call out the
> possible values it can take in the kip?
>
> Thanks again!
>
> Best,
>
>
> On Fri, Oct 29, 2021 at 9:53 AM Knowles Atchison Jr  >
> wrote:
>
> > Arjun,
> >
> > Thank you for your feedback, I have updated the KIP.
> >
> > This solution is more elegant than my original proposal; however, after
> > working on the implementation, we have now pushed the configuration from
> > the connector/task itself back to the connect worker. All tasks running
> on
> > the worker would share this ignore producer exception configuration flag.
> > This works for my use cases where I cannot envision setting this for only
> > one type of connector we have, but this does take the choice out of the
> > hands of the connector developer. I suppose that is for the best, in a
> > vacuum only the worker should have a say in how it handles message
> > production.
> >
> > Additional thoughts and feedback are welcome.
> >
> > Knowles
> >
> > On Thu, Oct 28, 2021 at 10:54 AM Arjun Satish 
> > wrote:
> >
> > > Yes, that makes sense. And it fits in very nicely with the current
> error
> > > handling framework.
> > >
> > > On Thu, Oct 28, 2021 at 10:39 AM Knowles Atchison Jr <
> > > katchiso...@gmail.com>
> > > wrote:
> > >
> > > > That would work. I originally thought that it would be confusing to
> > > > overload that function when a Record that wasn't actually written,
> but
> > > > looking at SourceTask more closely, in commitRecord(SourceRecord,
> > > > RecordMetadata), the RecordMetadata is set to null in the event of a
> > > > filtered transformation so the framework is already doing this in a
> > > certain
> > > > regard.
> > > >
> > > > Knowles
> > > >
> > > > On Thu, Oct 28, 2021 at 10:29 AM Arjun Satish <
> arjun.sat...@gmail.com>
> > > > wrote:
> > > >
> > > > > To ack the message back to the source system, we already have a
> > > > > commitRecord method. Once the bad record is handled by skip/dlq, we
> > > could
> > > > > just call commitRecord() on it?
> > > > >
> > > > > On Thu, Oct 28, 2021 at 9:35 AM Knowles Atchison Jr <
> > > > katchiso...@gmail.com
> > > > > >
> > > > > wrote:
> > > > >
> > > > > > Hi Chris,
> > > > > >
> > > > > > Thank you for your reply!
> > > > > >
> > > > > > It is a clarity error regarding the javadoc. I am not
> operationally
> > > > > > familiar with all of the exceptions Kafka considers
> non-retriable,
> > > so I
> > > > > > pulled the list from Callback.java:
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> https://github.com/apache/kafka/blob/1afe2a5190e9c98e38c84dc793f4303ea51bc19b/clients/src/main/java/org/apache/kafka/clients/producer/Callback.java#L35
> > > > > > to be an illustrative example of the types of exceptions that
> would
> > > > kill
> > > > > > the connector outright. Any exception thrown during the producer
> > > write
> > > > > will
> > > > > > be passed to this handler. I will update the KIP/PR to be more
> > clear
> > > on
> > > > > > this matter.
> > > > > >
> > > > > 

Re: [DISCUSS] KIP-779: Allow Source Tasks to Handle Producer Exceptions

2021-10-29 Thread Knowles Atchison Jr
Arjun,

Thank you for your feedback, I have updated the KIP.

This solution is more elegant than my original proposal; however, after
working on the implementation, we have now pushed the configuration from
the connector/task itself back to the connect worker. All tasks running on
the worker would share this ignore producer exception configuration flag.
This works for my use cases where I cannot envision setting this for only
one type of connector we have, but this does take the choice out of the
hands of the connector developer. I suppose that is for the best, in a
vacuum only the worker should have a say in how it handles message
production.

Additional thoughts and feedback are welcome.

Knowles

On Thu, Oct 28, 2021 at 10:54 AM Arjun Satish 
wrote:

> Yes, that makes sense. And it fits in very nicely with the current error
> handling framework.
>
> On Thu, Oct 28, 2021 at 10:39 AM Knowles Atchison Jr <
> katchiso...@gmail.com>
> wrote:
>
> > That would work. I originally thought that it would be confusing to
> > overload that function when a Record that wasn't actually written, but
> > looking at SourceTask more closely, in commitRecord(SourceRecord,
> > RecordMetadata), the RecordMetadata is set to null in the event of a
> > filtered transformation so the framework is already doing this in a
> certain
> > regard.
> >
> > Knowles
> >
> > On Thu, Oct 28, 2021 at 10:29 AM Arjun Satish 
> > wrote:
> >
> > > To ack the message back to the source system, we already have a
> > > commitRecord method. Once the bad record is handled by skip/dlq, we
> could
> > > just call commitRecord() on it?
> > >
> > > On Thu, Oct 28, 2021 at 9:35 AM Knowles Atchison Jr <
> > katchiso...@gmail.com
> > > >
> > > wrote:
> > >
> > > > Hi Chris,
> > > >
> > > > Thank you for your reply!
> > > >
> > > > It is a clarity error regarding the javadoc. I am not operationally
> > > > familiar with all of the exceptions Kafka considers non-retriable,
> so I
> > > > pulled the list from Callback.java:
> > > >
> > > >
> > >
> >
> https://github.com/apache/kafka/blob/1afe2a5190e9c98e38c84dc793f4303ea51bc19b/clients/src/main/java/org/apache/kafka/clients/producer/Callback.java#L35
> > > > to be an illustrative example of the types of exceptions that would
> > kill
> > > > the connector outright. Any exception thrown during the producer
> write
> > > will
> > > > be passed to this handler. I will update the KIP/PR to be more clear
> on
> > > > this matter.
> > > >
> > > > You raise an excellent point, how should the framework protect the
> > > > connector or developer from themselves? If a connector enables
> > > exactly-once
> > > > semantics, it would make sense to me to have the task killed. The
> > > framework
> > > > should enforce this type of misconfiguration that would break the
> > > internal
> > > > semantics of KIP-618. WorkerSourceTask could check the configuration
> > > before
> > > > handing off the records and exception to this function, fail initial
> > > > configuration check, or something of that nature.
> > > >
> > > > Hi Arjun,
> > > >
> > > > Thank you for your response!
> > > >
> > > > My specific use case is our custom JMS connector. We ack back to the
> > jms
> > > > broker once Kafka commits the record. We thread out our JMS consumer
> > such
> > > > that I would need access to the SourceRecord to confirm we are going
> to
> > > > throw away the message.
> > > >
> > > > Skipping such records, writing some log messages, and/or writing some
> > > error
> > > > context to a DLQ would cover most if not all of the use cases I
> > envision.
> > > >
> > > > "discard.message.on.producer.exception": "true"
> > > >
> > > > or some equivalent would get my personal use case 99% of the way
> > there. I
> > > > would still need some kind of callback from inside the connector with
> > the
> > > > Source Record to successfully ack back to my source system.
> > > >
> > > > I have updated the KIP regarding the callback being executed in a
> > > different
> > > > thread than poll().
> > > >
> > > > Knowles
> > > >
> > > > On Thu, Oct 28, 2021 at 2:02 AM A

Re: [DISCUSS] KIP-779: Allow Source Tasks to Handle Producer Exceptions

2021-10-28 Thread Knowles Atchison Jr
That would work. I originally thought that it would be confusing to
overload that function when a Record that wasn't actually written, but
looking at SourceTask more closely, in commitRecord(SourceRecord,
RecordMetadata), the RecordMetadata is set to null in the event of a
filtered transformation so the framework is already doing this in a certain
regard.

Knowles

On Thu, Oct 28, 2021 at 10:29 AM Arjun Satish 
wrote:

> To ack the message back to the source system, we already have a
> commitRecord method. Once the bad record is handled by skip/dlq, we could
> just call commitRecord() on it?
>
> On Thu, Oct 28, 2021 at 9:35 AM Knowles Atchison Jr  >
> wrote:
>
> > Hi Chris,
> >
> > Thank you for your reply!
> >
> > It is a clarity error regarding the javadoc. I am not operationally
> > familiar with all of the exceptions Kafka considers non-retriable, so I
> > pulled the list from Callback.java:
> >
> >
> https://github.com/apache/kafka/blob/1afe2a5190e9c98e38c84dc793f4303ea51bc19b/clients/src/main/java/org/apache/kafka/clients/producer/Callback.java#L35
> > to be an illustrative example of the types of exceptions that would kill
> > the connector outright. Any exception thrown during the producer write
> will
> > be passed to this handler. I will update the KIP/PR to be more clear on
> > this matter.
> >
> > You raise an excellent point, how should the framework protect the
> > connector or developer from themselves? If a connector enables
> exactly-once
> > semantics, it would make sense to me to have the task killed. The
> framework
> > should enforce this type of misconfiguration that would break the
> internal
> > semantics of KIP-618. WorkerSourceTask could check the configuration
> before
> > handing off the records and exception to this function, fail initial
> > configuration check, or something of that nature.
> >
> > Hi Arjun,
> >
> > Thank you for your response!
> >
> > My specific use case is our custom JMS connector. We ack back to the jms
> > broker once Kafka commits the record. We thread out our JMS consumer such
> > that I would need access to the SourceRecord to confirm we are going to
> > throw away the message.
> >
> > Skipping such records, writing some log messages, and/or writing some
> error
> > context to a DLQ would cover most if not all of the use cases I envision.
> >
> > "discard.message.on.producer.exception": "true"
> >
> > or some equivalent would get my personal use case 99% of the way there. I
> > would still need some kind of callback from inside the connector with the
> > Source Record to successfully ack back to my source system.
> >
> > I have updated the KIP regarding the callback being executed in a
> different
> > thread than poll().
> >
> > Knowles
> >
> > On Thu, Oct 28, 2021 at 2:02 AM Arjun Satish 
> > wrote:
> >
> > > Hi Knowles,
> > >
> > > Thanks for the KIP!
> > >
> > > Could you please call out some use-cases on what the source connectors
> > > would do when they hit such exceptions? I'm wondering if we would need
> to
> > > do anything other than skipping such records, writing some log
> messages,
> > > and/or writing some error context to a DLQ?
> > >
> > > One of the goals for Connect was to abstract away intricacies of Kafka
> > > topics, clients etc, so that connectors could focus on the external
> > systems
> > > themselves. Ideally, we'd want to see if we could call out the most
> > common
> > > cases and handle them in the framework itself, instead of delegating
> them
> > > back to the connector. This way, instead of the new API, we'd probably
> > > introduce some more configuration options, but they could be applicable
> > to
> > > all the connectors that are out there.
> > >
> > > Also, If the above mentioned are the most common uses, then we could
> > apply
> > > KIP-298 (with some adjustments) to source connectors for non-retriable
> > > producer errors.
> > >
> > > If we decide to go with the API you are referring to though, would the
> > > preTransformation record suffice? SMTs can be causing the actual issues
> > > (for example, changing the topic name) that cause these non-retriable
> > > exceptions. The new callback might be receiving insufficient context to
> > do
> > > any corrective action.
> > >
> > > In the documentation for the new API, we might want to specify that
> this
> > > call

Re: [DISCUSS] KIP-779: Allow Source Tasks to Handle Producer Exceptions

2021-10-28 Thread Knowles Atchison Jr
Hi Chris,

Thank you for your reply!

It is a clarity error regarding the javadoc. I am not operationally
familiar with all of the exceptions Kafka considers non-retriable, so I
pulled the list from Callback.java:
https://github.com/apache/kafka/blob/1afe2a5190e9c98e38c84dc793f4303ea51bc19b/clients/src/main/java/org/apache/kafka/clients/producer/Callback.java#L35
to be an illustrative example of the types of exceptions that would kill
the connector outright. Any exception thrown during the producer write will
be passed to this handler. I will update the KIP/PR to be more clear on
this matter.

You raise an excellent point, how should the framework protect the
connector or developer from themselves? If a connector enables exactly-once
semantics, it would make sense to me to have the task killed. The framework
should enforce this type of misconfiguration that would break the internal
semantics of KIP-618. WorkerSourceTask could check the configuration before
handing off the records and exception to this function, fail initial
configuration check, or something of that nature.

Hi Arjun,

Thank you for your response!

My specific use case is our custom JMS connector. We ack back to the jms
broker once Kafka commits the record. We thread out our JMS consumer such
that I would need access to the SourceRecord to confirm we are going to
throw away the message.

Skipping such records, writing some log messages, and/or writing some error
context to a DLQ would cover most if not all of the use cases I envision.

"discard.message.on.producer.exception": "true"

or some equivalent would get my personal use case 99% of the way there. I
would still need some kind of callback from inside the connector with the
Source Record to successfully ack back to my source system.

I have updated the KIP regarding the callback being executed in a different
thread than poll().

Knowles

On Thu, Oct 28, 2021 at 2:02 AM Arjun Satish  wrote:

> Hi Knowles,
>
> Thanks for the KIP!
>
> Could you please call out some use-cases on what the source connectors
> would do when they hit such exceptions? I'm wondering if we would need to
> do anything other than skipping such records, writing some log messages,
> and/or writing some error context to a DLQ?
>
> One of the goals for Connect was to abstract away intricacies of Kafka
> topics, clients etc, so that connectors could focus on the external systems
> themselves. Ideally, we'd want to see if we could call out the most common
> cases and handle them in the framework itself, instead of delegating them
> back to the connector. This way, instead of the new API, we'd probably
> introduce some more configuration options, but they could be applicable to
> all the connectors that are out there.
>
> Also, If the above mentioned are the most common uses, then we could apply
> KIP-298 (with some adjustments) to source connectors for non-retriable
> producer errors.
>
> If we decide to go with the API you are referring to though, would the
> preTransformation record suffice? SMTs can be causing the actual issues
> (for example, changing the topic name) that cause these non-retriable
> exceptions. The new callback might be receiving insufficient context to do
> any corrective action.
>
> In the documentation for the new API, we might want to specify that this
> callback will be called from a different thread than the ones calling
> poll(). So any shared objects must be protected appropriately.
>
> Cheers,
>
> On Wed, Oct 27, 2021 at 7:01 PM Chris Egerton  >
> wrote:
>
> > Hi Knowles,
> >
> > Thanks for the KIP. I may have more to say later but there's one thing
> I'd
> > like to make sure to share now. In the Javadocs for the proposed
> > SourceTask::ignoreNonRetriableProducerException method,
> > the InvalidProducerEpochException exception class is included as an
> example
> > of a non-retriable exception that may cause the new SourceTask method to
> be
> > invoked. This exception should only arise if the source task's producer
> is
> > a transactional producer, which is currently never the case and, once
> > KIP-618 (https://cwiki.apache.org/confluence/display/KAFKA/KIP-618) is
> > merged, will only be the case when the task is running with exactly-once
> > support. I wonder if it's safe to allow connectors to discard this
> > exception when they're running with exactly-once support, or if the task
> > should still be unconditionally failed in that case?
> >
> > Cheers,
> >
> > Chris
> >
> > On Wed, Oct 27, 2021 at 5:39 PM John Roesler 
> wrote:
> >
> > > Hi Knowles,
> > >
> > > Thanks for the reply! That all sounds reasonable to me, and
> > > that's a good catch regarding the SourceRecord.
&g

Re: [DISCUSS] KIP-779: Allow Source Tasks to Handle Producer Exceptions

2021-10-27 Thread Knowles Atchison Jr
John,

Thank you for the response and feedback!

I originally started my first pass with the ProducerRecord.
For our connector, we need some of the information out of the SourceRecord
to ack our source system. If I had the actual ProducerRecord, I would have
to convert it back before I would be able to do anything useful with it. I
think there is merit in providing both records as parameters to this
callback. Then connector writers can decide which of the representations of
the data is most useful to them. I also noticed that in my PR I was sending
the SourceRecord post transformation, when we really should be sending the
preTransformRecord.

The Streams solution to this is very interesting. Given the nature of a
connector, to me it makes the most sense for the api call to be part of
that task rather than an external class that is configurable. This allows
the connector to use state it may have at the time to inform decisions on
what to do with these producer exceptions.

I have updated the KIP and PR.

Knowles

On Wed, Oct 27, 2021 at 1:03 PM John Roesler  wrote:

> Good morning, Knowles,
>
> Thanks for the KIP!
>
> To address your latest questions, it is fine to call for a
> vote if a KIP doesn't generate much discussion. Either the
> KIP was just not controversial enough for anyone to comment,
> in which case a vote is appropriate; or no one had time to
> review it, in which case, calling for a vote might be more
> provacative and elicit a response.
>
> As far as pinging people directly, one idea would be to look
> at the git history (git blame/praise) for the files you're
> changing to see which committers have recently been
> involved. Those are the folks who are most likely to have
> valuable feedback on your proposal. It might not be
> appropriate to directly email them, but I have seen KIP
> discussions before that requested feedback from people by
> name. It's probably not best to lead with that, but since no
> one has responded so far, it might not hurt. I'm sure that
> the reason they haven't noticed your KIP is just that they
> are so busy it slipped their radar. They might actually
> appreciate a more direct ping at this point.
>
> I'm happy to review, but as a caveat, I don't have much
> experience with using or maintaining Connect, so caveat
> emptor as far as my review goes.
>
> First of all, thanks for the well written KIP. Without much
> context, I was able to understand the motivation and
> proposal easily just by reading your document.
>
> I think your proposal is a good one. It seems like it would
> be pretty obvious as a user what (if anything) to do with
> the proposed method.
>
> For your reference, this proposal reminds me of these
> capabilities in Streams:
>
> https://github.com/apache/kafka/blob/trunk/streams/src/main/java/org/apache/kafka/streams/errors/DeserializationExceptionHandler.java
> and
>
> https://github.com/apache/kafka/blob/trunk/streams/src/main/java/org/apache/kafka/streams/errors/ProductionExceptionHandler.java
> .
>
> I'm not sure if there's value in bringing your proposed
> interface closer to that pattern or not. Streams and Connect
> are quite different domains after all. At least, I wanted
> you to be aware of them so you could consider the
> alternative API strategy they present.
>
> Regardless, I do wonder if it would be helpful to also
> include the actual ProducerRecord we tried to send, since
> there's a non-trivial transformation that takes place to
> convert the SourceRecord into a ProducerRecord. I'm not sure
> what people would do with it, exactly, but it might be
> helpful in deciding what to do about the exception, or maybe
> even in understanding the exception.
>
> Those are the only thoughts that come to my mind! Thanks
> again,
> -John
>
> On Wed, 2021-10-27 at 09:16 -0400, Knowles Atchison Jr
> wrote:
> > Good morning,
> >
> > Bumping this thread. Is there someone specific on the Connect framework
> > team that I should ping? Is it appropriate to just call a vote? All
> source
> > connectors are dead in the water without a way to handle producer write
> > exceptions. Thank you.
> >
> > Knowles
> >
> > On Mon, Oct 18, 2021 at 8:33 AM Christopher Shannon <
> > christopher.l.shan...@gmail.com> wrote:
> >
> > > I also would find this feature useful to handle errors better, does
> anyone
> > > have any comments or feedback?
> > >
> > >
> > > On Mon, Oct 11, 2021 at 8:52 AM Knowles Atchison Jr <
> katchiso...@gmail.com
> > > >
> > > wrote:
> > >
> > > > Good morning,
> > > >
> > > > Bumping this for visibility. I would like this to go into the next
>

Re: [DISCUSS] KIP-779: Allow Source Tasks to Handle Producer Exceptions

2021-10-27 Thread Knowles Atchison Jr
Good morning,

Bumping this thread. Is there someone specific on the Connect framework
team that I should ping? Is it appropriate to just call a vote? All source
connectors are dead in the water without a way to handle producer write
exceptions. Thank you.

Knowles

On Mon, Oct 18, 2021 at 8:33 AM Christopher Shannon <
christopher.l.shan...@gmail.com> wrote:

> I also would find this feature useful to handle errors better, does anyone
> have any comments or feedback?
>
>
> On Mon, Oct 11, 2021 at 8:52 AM Knowles Atchison Jr  >
> wrote:
>
> > Good morning,
> >
> > Bumping this for visibility. I would like this to go into the next
> release.
> > KIP freeze is Friday.
> >
> > Any comments and feedback are welcome.
> >
> > Knowles
> >
> > On Tue, Oct 5, 2021 at 4:24 PM Knowles Atchison Jr <
> katchiso...@gmail.com>
> > wrote:
> >
> > > Hello all,
> > >
> > > I would like to discuss the following KIP:
> > >
> > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-779%3A+Allow+Source+Tasks+to+Handle+Producer+Exceptions
> > >
> > > The main purpose is to allow Source Tasks the ability to see underlying
> > > Producer Exceptions and decide what to do rather than being killed. In
> > our
> > > use cases we would want to log/write off some information and continue
> > > processing.
> > >
> > > PR is here:
> > >
> > > https://github.com/apache/kafka/pull/11382
> > >
> > > Any comments and feedback are welcome.
> > >
> > >
> > > Knowles
> > >
> >
>


KIP Process Needs Improvement

2021-10-19 Thread Knowles Atchison Jr
Good morning,

The current process of KIPs needs to be improved. There are at least a
handful of open KIPs with existing PRs that are in a purgatory state. I
understand that people are busy, but if you are going to gatekeep Kafka
with this process, then it must be responsive. Even if the community
decides they do not want the change, the KIP should be addressed and closed
out.

The entire wiki page is a graveyard of unresponded KIPs. For some changes,
it takes a nontrivial amount of effort to put together the wiki page and
one has to essentially write the code implementation hoping that it will be
pulled into the codebase. This is very frustrating as an external developer
to have put in the work and then effectively be ignored.

We have to maintain a custom build because KIPs are not debated, voted on,
or merged in a timely manner.

Knowles


Re: [DISCUSS] KIP-779: Allow Source Tasks to Handle Producer Exceptions

2021-10-11 Thread Knowles Atchison Jr
Good morning,

Bumping this for visibility. I would like this to go into the next release.
KIP freeze is Friday.

Any comments and feedback are welcome.

Knowles

On Tue, Oct 5, 2021 at 4:24 PM Knowles Atchison Jr 
wrote:

> Hello all,
>
> I would like to discuss the following KIP:
>
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-779%3A+Allow+Source+Tasks+to+Handle+Producer+Exceptions
>
> The main purpose is to allow Source Tasks the ability to see underlying
> Producer Exceptions and decide what to do rather than being killed. In our
> use cases we would want to log/write off some information and continue
> processing.
>
> PR is here:
>
> https://github.com/apache/kafka/pull/11382
>
> Any comments and feedback are welcome.
>
>
> Knowles
>


[DISCUSS] KIP-779: Allow Source Tasks to Handle Producer Exceptions

2021-10-05 Thread Knowles Atchison Jr
Hello all,

I would like to discuss the following KIP:

https://cwiki.apache.org/confluence/display/KAFKA/KIP-779%3A+Allow+Source+Tasks+to+Handle+Producer+Exceptions

The main purpose is to allow Source Tasks the ability to see underlying
Producer Exceptions and decide what to do rather than being killed. In our
use cases we would want to log/write off some information and continue
processing.

PR is here:

https://github.com/apache/kafka/pull/11382

Any comments and feedback are welcome.


Knowles


Re: Wiki Permissions

2021-10-05 Thread Knowles Atchison Jr
Bill,

Thank you. I'm still seeing the "Sorry, you don't have permission to create
content. Contact your space administrator to request access." on the Create
KIP button. Am I not looking in the right place?

Knowles

On Tue, Oct 5, 2021 at 2:35 PM Bill Bejeck  wrote:

> Done.  Thanks for your interest in Apache Kafka.
>
> -Bill
>
> On Tue, Oct 5, 2021 at 10:54 AM Knowles Atchison Jr  >
> wrote:
>
> > Good morning,
> >
> > I would like to author a KIP.
> >
> > May I please have permissions granted for the wiki?
> >
> > username: katchison
> >
> > Thank you.
> >
> > Knowles
> >
>


[jira] [Created] (KAFKA-13348) Allow Source Tasks to Handle Producer Exceptions

2021-10-05 Thread Knowles Atchison Jr (Jira)
Knowles Atchison Jr created KAFKA-13348:
---

 Summary: Allow Source Tasks to Handle Producer Exceptions
 Key: KAFKA-13348
 URL: https://issues.apache.org/jira/browse/KAFKA-13348
 Project: Kafka
  Issue Type: Improvement
  Components: KafkaConnect
Affects Versions: 3.0.0, 2.8.1, 3.1.0
Reporter: Knowles Atchison Jr


KAFKA-8586 added capture of Producer Exceptions which will kill the connector.

There is a need to allow the connector itself to be aware of these errors, 
handle it in some manner, and continuing processing records.

The proposed change is to add a function to SourceTask that allows handling of 
the SourceRecord and Exception as thrown from the Producer. The SourceTask can 
examine these items and determine if it is appropriate to die (current 
behavior) or let the record be thrown away and continue processing.

The current behavior will be maintained by defaulting to returning false from 
this function. If the implementing SourceTask override of this function returns 
true, Kafka Connect will ignore this error record and continue processing.



--
This message was sent by Atlassian Jira
(v8.3.4#803005)


Wiki Permissions

2021-10-05 Thread Knowles Atchison Jr
Good morning,

I would like to author a KIP.

May I please have permissions granted for the wiki?

username: katchison

Thank you.

Knowles


Jira contributor access

2021-07-22 Thread Knowles Atchison Jr
Requesting Jira contributor access.

Username: katchison


Re: Kafka Advisory Topic

2021-01-26 Thread Knowles Atchison Jr
Gwen,

Yes, Chris and I work together; we can put together a KIP.

If someone would please grant me wiki permissions, username is katchison.

Thank you.

On Mon, Jan 25, 2021 at 2:58 PM Gwen Shapira  wrote:

> Agree that this sounds like a good idea.
>
> Would be good to have a more formal proposal (i.e. a KIP) with the details.
> I can think of about 100 different questions (will there be "levels"
> like in logs, what type of events are in or out of scope, rate
> limiting, data formats, etc).
> I am also curious on whether the notifications are intended for
> humans, automated processes or even the Kafka client applications
> themselves. I hope the proposal can include a few example scenarios to
> help us reason about the experience.
>
> Knowlton, is this something you want to pick up?
>
> Gwen
>
> On Thu, Jan 21, 2021 at 6:05 AM Christopher Shannon
>  wrote:
> >
> > Hi,
> >
> > I am on the ActiveMQ PMC and I think this is a very good idea to have a
> way
> > to do advisories/notifications/events (whatever you want to call it). In
> > ActiveMQ classic you have advisories and in Artemis you have
> notifications.
> > Having management messages that can be subscribed to in real time is
> > actually a major feature that is missing from Kafka that many other
> brokers
> > have.
> >
> > The idea here would be to publish notifications of different configurable
> > events when something important happens so a consumer can listen in on
> > things it cares about and be able to do something instead of having to
> poll
> > the admin API. There are many events that happen in a broker that would
> be
> > useful to be notified about. Events such as new connections to the
> cluster,
> > new topics created or destroyed, consumer group creation, authorization
> > errors, new leader election, etc. The list is pretty much endless.
> >
> > The metadata topic that will exist is probably not going to have all of
> > this information so some other mechanism would be needed to handle
> > publishing these messages to a specific management topic that would be
> > useful for a consumer.
> >
> > Chris
> >
> >
> > On Wed, Jan 20, 2021 at 4:12 PM Boyang Chen 
> > wrote:
> >
> > > Hey Knowles,
> > >
> > > in Kafka people normally use admin clients to get those metadata. I'm
> not
> > > sure why you mentioned specifically that having a topic to manage these
> > > information is useful, but a good news is that in KIP-500
> > > <
> > >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-500%3A+Replace+ZooKeeper+with+a+Self-Managed+Metadata+Quorum
> > > >
> > > we
> > > are trying to deprecate Zookeeper and migrate to a self-managed
> metadata
> > > topic quorum. At the time this feature is fully done, you should be
> able to
> > > use consumers to pull the metadata log.
> > >
> > > Best,
> > > Boyang
> > >
> > > On Wed, Jan 20, 2021 at 11:22 AM Knowles Atchison Jr <
> > > katchiso...@gmail.com>
> > > wrote:
> > >
> > > > Good afternoon all,
> > > >
> > > > In our Kafka clusters we have a need to know when certain activities
> are
> > > > performed, mainly topics being created, but brokers coming up/down is
> > > also
> > > > useful. This would be akin to what ActiveMQ does via advisory
> messages (
> > > > https://activemq.apache.org/advisory-message).
> > > >
> > > > Since there did not appear to be anything in the ecosystem
> currently, I
> > > > wrote a standalone Java program that watches the various ZooKeeper
> > > > locations that the Kafka broker writes to and deltas can tell us
> > > > topic/broker actions etc... and writes to a kafka topic for
> downstream
> > > > consumption.
> > > >
> > > > Ideally, we would rather have the broker handle this internally
> rather
> > > > than yet another service stood up in our systems. I began digging
> through
> > > > the broker source (my Scala is basically hello world level) and there
> > > does
> > > > not appear to be any mechanism in which this could be easily patched
> into
> > > > the broker.
> > > >
> > > > Specifically, a producer or consumer acting upon an nonexistent
> topic or
> > > a
> > > > manual CreateTopic would trigger a Produce to this advisory topic
> and the
> > > > KafkaApis framework would handle it like any other request. However,
> by
> > > the
> > > > time we are inside the getTopicMetadata call there doesn't seem to
> be a
> > > > clean way to fire off another message that would make its way through
> > > > KafkaApis. Perhaps another XManager type object is required?
> > > >
> > > > Looking for alternative ideas or guidance (or I missed something in
> the
> > > > broker).
> > > >
> > > > Thank you.
> > > >
> > > > Knowles
> > > >
> > >
>
>
>
> --
> Gwen Shapira
> Engineering Manager | Confluent
> 650.450.2760 | @gwenshap
> Follow us: Twitter | blog
>


Kafka Advisory Topic

2021-01-20 Thread Knowles Atchison Jr
Good afternoon all,

In our Kafka clusters we have a need to know when certain activities are
performed, mainly topics being created, but brokers coming up/down is also
useful. This would be akin to what ActiveMQ does via advisory messages (
https://activemq.apache.org/advisory-message).

Since there did not appear to be anything in the ecosystem currently, I
wrote a standalone Java program that watches the various ZooKeeper
locations that the Kafka broker writes to and deltas can tell us
topic/broker actions etc... and writes to a kafka topic for downstream
consumption.

Ideally, we would rather have the broker handle this internally rather
than yet another service stood up in our systems. I began digging through
the broker source (my Scala is basically hello world level) and there does
not appear to be any mechanism in which this could be easily patched into
the broker.

Specifically, a producer or consumer acting upon an nonexistent topic or a
manual CreateTopic would trigger a Produce to this advisory topic and the
KafkaApis framework would handle it like any other request. However, by the
time we are inside the getTopicMetadata call there doesn't seem to be a
clean way to fire off another message that would make its way through
KafkaApis. Perhaps another XManager type object is required?

Looking for alternative ideas or guidance (or I missed something in the
broker).

Thank you.

Knowles