John, Thank you for the response and feedback!
I originally started my first pass with the ProducerRecord<byte[], byte[]>. 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 <vvcep...@apache.org> 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 > > > 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 > > > > > > > > > > > > > > >
