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,

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,

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

2021-11-16 Thread Arjun Satish
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 >

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

2021-11-16 Thread Arjun Satish
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

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

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

2021-11-11 Thread Chris Egerton
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

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

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

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

2021-11-01 Thread Arjun Satish
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

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

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

2021-10-28 Thread Arjun Satish
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 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

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

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

2021-10-28 Thread Arjun Satish
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

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:

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

2021-10-28 Thread Arjun Satish
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?

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

2021-10-27 Thread Chris Egerton
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

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

2021-10-27 Thread John Roesler
Hi Knowles, Thanks for the reply! That all sounds reasonable to me, and that's a good catch regarding the SourceRecord. Thanks, -John On Wed, 2021-10-27 at 15:32 -0400, Knowles Atchison Jr wrote: > John, > > Thank you for the response and feedback! > > I originally started my first pass with

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

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

2021-10-27 Thread John Roesler
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

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

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

2021-10-18 Thread Christopher Shannon
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

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: > > >

[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