Thanks for the clearly written KIP, no further comments from my end. -Bill
On Wed, Dec 6, 2017 at 9:52 AM, Matt Farmer <m...@frmr.me> wrote: > There is already a vote thread for this KIP. I can bump it so that it’s > towards the top of your inbox. > > With regard to your concerns: > > 1) We do not have the "ProductionExceptionHandler" interface defined in the > wiki page, thought it is sort of clear that it is a one-function interface > with record and exception. Could you add it? > > > It is defined, it’s just not defined using a code snippet. The KIP reads as > follows: > > === > > A public interface named ProductionExceptionHandler with a single method, > handle, that has the following signature: > > - ProductionExceptionHandlerResponse handle(ProducerRecord<byte[], > byte[]> record, Exception exception) > > > === > > If you’d like me to add a code snippet illustrating this that’s simple for > me to do, but it seemed superfluous. > > 2) A quick question about your example code: where would be the "logger" > object be created? > > > SLF4J loggers are typically created as a class member in the class. Such > as: > > private Logger logger = LoggerFactory.getLogger(HelloWorld.class); > > I omit that in my implementation examples for brevity. > > On December 6, 2017 at 2:14:58 AM, Guozhang Wang (wangg...@gmail.com) > wrote: > > Hello Matt, > > Thanks for writing up the KIP. I made a pass over it and here is a few > minor comments. I think you can consider starting a voting thread for this > KIP while addressing them. > > 1) We do not have the "ProductionExceptionHandler" interface defined in the > wiki page, thought it is sort of clear that it is a one-function interface > with record and exception. Could you add it? > > 2) A quick question about your example code: where would be the "logger" > object be created? Note that the implementation of this interface have to > give a non-param constructor, or as a static field of the class but in that > case you would not be able to log which instance is throwing this error (we > may have multiple producers within a single instance, even within a > thread). Just a reminder to consider in your implementation. > > > Guozhang > > On Tue, Dec 5, 2017 at 3:15 PM, Matthias J. Sax <matth...@confluent.io> > wrote: > > > Thanks a lot for the update! Great write-up! Very clearly explained what > > the change will look like! > > > > Looks good to me. No further comments from my side. > > > > > > -Matthias > > > > > > On 12/5/17 9:14 AM, Matt Farmer wrote: > > > I have updated this KIP accordingly. > > > > > > Can you please take a look and let me know if what I wrote looks > correct > > to > > > you? > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP- > > 210+-+Provide+for+custom+error+handling++when+Kafka+ > > Streams+fails+to+produce > > > > > > Thanks! > > > > > > Matt > > > > > > > > > On December 4, 2017 at 9:39:13 PM, Matt Farmer (m...@frmr.me) wrote: > > > > > > Hey Matthias, thanks for getting back to me. > > > > > > That's fine. But if we add it to `test` package, we don't need to talk > > > about it in the KIP. `test` is not public API. > > > > > > Yes, that makes sense. It was in the KIP originally because I was, at > one > > > point, planning on including it. We can remove it now that we’ve > decided > > we > > > won’t include it in the public API. > > > > > > Understood. That makes sense. We should explain this clearly in the KIP > > > and maybe log all other following exceptions at DEBUG level? > > > > > > > > > I thought it was clear in the KIP, but I can go back and double check > my > > > wording and revise it to try and make it clearer. > > > > > > I’ll take a look at doing more work on the KIP and the Pull Request > > > tomorrow. > > > > > > Thanks again! > > > > > > On December 4, 2017 at 5:50:33 PM, Matthias J. Sax ( > > matth...@confluent.io) > > > wrote: > > > > > > Hey, > > > > > > About your questions: > > > > > >>>> Acknowledged, so is ProducerFencedException the only kind of > > exception I > > >>>> need to change my behavior on? Or are there other types I need to > > > check? Is > > >>>> there a comprehensive list somewhere? > > > > > > I cannot think if any other atm. We should list all fatal exceptions > for > > > which we don't call the handler and explain why (exception is "global" > > > and will affect all other records, too | ProducerFenced is > self-healing). > > > > > > We started to collect and categorize exception here (not completed > yet): > > > https://cwiki.apache.org/confluence/display/KAFKA/ > > Kafka+Streams+Architecture#KafkaStreamsArchitecture-TypesofExceptions > > > : > > > > > > This list should be a good starting point though. > > > > > >> I include it in the test package because I have tests that assert that > > if > > >> the record collector impl encounters an Exception and receives a > > CONTINUE > > >> that it actually does CONTINUE. > > > > > > That's fine. But if we add it to `test` package, we don't need to talk > > > about it in the KIP. `test` is not public API. > > > > > >> I didn't want to invoke the handler in places where the CONTINUE or > FAIL > > >> result would just be ignored. Presumably, after a FAIL has been > > returned, > > >> subsequent exceptions are likely to be repeats or noise from my > > >> understanding of the code paths. If this is incorrect we can revisit. > > > > > > Understood. That makes sense. We should explain this clearly in the KIP > > > and maybe log all other following exceptions at DEBUG level? > > > > > > > > > -Matthias > > > > > > > > > On 12/1/17 11:43 AM, Matt Farmer wrote: > > >> Bump! It's been three days here and I haven't seen any further > feedback. > > >> Eager to get this resolved, approved, and merged. =) > > >> > > >> On Tue, Nov 28, 2017 at 9:53 AM Matt Farmer <m...@frmr.me> wrote: > > >> > > >>> Hi there, sorry for the delay in responding. Last week had a holiday > > and > > >>> several busy work days in it so I'm just now getting around to > > > responding. > > >>> > > >>>> We would only exclude > > >>>> exception Streams can handle itself (like ProducerFencedException) > -- > > >>>> thus, if the handler has code to react to this, it would not be bad, > > as > > >>>> this code is just never called. > > >>> [...] > > >>>> Thus, I am still in favor of not calling the > > ProductionExceptionHandler > > >>>> for fatal exception. > > >>> > > >>> Acknowledged, so is ProducerFencedException the only kind of > exception > > I > > >>> need to change my behavior on? Or are there other types I need to > > check? > > > Is > > >>> there a comprehensive list somewhere? > > >>> > > >>>> About the "always continue" case. Sounds good to me to remove it > (not > > >>>> sure why we need it in test package?) > > >>> > > >>> I include it in the test package because I have tests that assert > that > > if > > >>> the record collector impl encounters an Exception and receives a > > CONTINUE > > >>> that it actually does CONTINUE. > > >>> > > >>>> What is there reasoning for invoking the handler only for the first > > >>>> exception? > > >>> > > >>> I didn't want to invoke the handler in places where the CONTINUE or > > FAIL > > >>> result would just be ignored. Presumably, after a FAIL has been > > returned, > > >>> subsequent exceptions are likely to be repeats or noise from my > > >>> understanding of the code paths. If this is incorrect we can revisit. > > >>> > > >>> Once I get the answers to these questions I can make another pass on > > the > > >>> pull request! > > >>> > > >>> Matt > > >>> > > >>> On Mon, Nov 20, 2017 at 4:07 PM Matthias J. Sax < > matth...@confluent.io > > > > > >>> wrote: > > >>> > > >>>> Thanks for following up! > > >>>> > > >>>> One thought about an older reply from you: > > >>>> > > >>>>>>>> I strongly disagree here. The purpose of this handler isn't > *just* > > > to > > >>>>>>>> make a decision for streams. There may also be desirable side > > >>>> effects that > > >>>>>>>> users wish to cause when production exceptions occur. There may > be > > >>>>>>>> side-effects that they wish to cause when > AuthenticationExceptions > > >>>> occur, > > >>>>>>>> as well. We should still give them the hooks to preform those > side > > >>>> effects. > > >>>> > > >>>> And your follow up: > > >>>> > > >>>>>> - I think I would rather invoke it for all exceptions that could > > >>>> occur > > >>>>>> from attempting to produce - even those exceptions were returning > > >>>> CONTINUE > > >>>>>> may not be a good idea (e.g. Authorization exception). Until there > > >>>> is a > > >>>>>> different type for exceptions that are totally fatal (for example > a > > >>>>>> FatalStreamsException or some sort), maintaining a list of > > >>>> exceptions that > > >>>>>> you can intercept with this handler and exceptions you cannot > would > > >>>> be > > >>>>>> really error-prone and hard to keep correct. > > >>>> > > >>>> I understand what you are saying, however, consider that Streams > needs > > >>>> to die for a fatal exception. Thus, if you call the handler for a > > fatal > > >>>> exception, we would need to ignore the return value and fail -- this > > >>>> seems to be rather intuitive. Furthermore, users can register an > > >>>> uncaught-exception-handler and side effects for fatal errors can be > > >>>> triggered there. > > >>>> > > >>>> Btw: an AuthorizationException is fatal -- not sure what you mean by > > an > > >>>> "totally fatal" exception -- there is no superlative to fatal from > my > > >>>> understanding. > > >>>> > > >>>> About maintaining a list of exceptions: I don't think this is too > > hard, > > >>>> and users also don't need to worry about it IMHO. We would only > > exclude > > >>>> exception Streams can handle itself (like ProducerFencedException) > -- > > >>>> thus, if the handler has code to react to this, it would not be bad, > > as > > >>>> this code is just never called. In case that there is an exception > > >>>> Streams could actually handle but we still call the handler (ie, > bug), > > >>>> there should be no harm either -- the handler needs to return either > > >>>> CONTINUE or FAIL and we would obey. It could only happen, that > Streams > > >>>> dies---as request by the user(!)---even if Streams could actually > > handle > > >>>> the error and move on. But this seems to be not a issue from my > point > > of > > >>>> view. > > >>>> > > >>>> Thus, I am still in favor of not calling the > > ProductionExceptionHandler > > >>>> for fatal exception. > > >>>> > > >>>> > > >>>> > > >>>> About the "always continue" case. Sounds good to me to remove it > (not > > >>>> sure why we need it in test package?) and to rename the "failing" > > >>>> handler to "Default" (even if "default" is less descriptive and I > > would > > >>>> still prefer "Fail" in the name). > > >>>> > > >>>> > > >>>> Last question: > > >>>> > > >>>>>> - Continue to *only* invoke it on the first exception that we > > >>>>>> encounter (before sendException is set) > > >>>> > > >>>> > > >>>> What is there reasoning for invoking the handler only for the first > > >>>> exception? > > >>>> > > >>>> > > >>>> > > >>>> > > >>>> -Matthias > > >>>> > > >>>> On 11/20/17 10:43 AM, Matt Farmer wrote: > > >>>>> Alright, here are some updates I'm planning to make after thinking > on > > >>>> this > > >>>>> for awhile: > > >>>>> > > >>>>> - Given that the "always continue" handler isn't something I'd > > >>>> recommend > > >>>>> for production use as is, I'm going to move it into the test > > >>>> namespace and > > >>>>> remove it from mention in the public API. > > >>>>> - I'm going to rename the "AlwaysFailProductionExceptionHandler" > to > > >>>>> "DefaultProductionExceptionHandler" > > >>>>> - Given that the API for the exception handler involves being > > >>>> invoked by > > >>>>> streams to make a decision about whether or not to continue — I > > >>>> think that > > >>>>> we should: > > >>>>> - Continue to *only* invoke it on the first exception that we > > >>>>> encounter (before sendException is set) > > >>>>> - Stop invoking it for the self-healing fenced exceptions. > > >>>>> - I think I would rather invoke it for all exceptions that could > > >>>> occur > > >>>>> from attempting to produce - even those exceptions were returning > > >>>> CONTINUE > > >>>>> may not be a good idea (e.g. Authorization exception). Until there > > >>>> is a > > >>>>> different type for exceptions that are totally fatal (for example a > > >>>>> FatalStreamsException or some sort), maintaining a list of > > >>>> exceptions that > > >>>>> you can intercept with this handler and exceptions you cannot would > > >>>> be > > >>>>> really error-prone and hard to keep correct. > > >>>>> - I'm happy to file a KIP for the creation of this new Exception > > >>>> type > > >>>>> if there is interest. > > >>>>> > > >>>>> @ Matthias — What do you think about the above? > > >>>>> > > >>>>> On Tue, Nov 14, 2017 at 9:44 AM Matt Farmer <m...@frmr.me> wrote: > > >>>>> > > >>>>>> I responded before reading your code review and didn't see the bit > > >>>> about > > >>>>>> how ProducerFencedException is self-healing. This error handling > > logic > > >>>> is > > >>>>>> *quite* confusing to reason about... I think I'm going to sit down > > > with > > >>>>>> the code a bit more today, but I'm inclined to think that if the > > > fenced > > >>>>>> exceptions are, indeed, self healing that we still invoke the > > handler > > >>>> but > > >>>>>> ignore its result for those exceptions. > > >>>>>> > > >>>>>> On Tue, Nov 14, 2017 at 9:37 AM Matt Farmer <m...@frmr.me> wrote: > > >>>>>> > > >>>>>>> Hi there, > > >>>>>>> > > >>>>>>> Following up here... > > >>>>>>> > > >>>>>>>> One tiny comment: I would prefer to remove the "Always" from the > > >>>>>>> handler implementation names -- it sounds "cleaner" to me without > > it. > > >>>>>>> > > >>>>>>> Let me think on this. I generally prefer expressiveness to > > > clean-ness, > > >>>>>>> and I think that comes out in the names I chose for things. The > > straw > > >>>> man > > >>>>>>> in my head is the person that tried to substitute in the > > >>>> "AlwaysContinue" > > >>>>>>> variant and the "Always" is a trigger to really consider whether > or > > >>>> not > > >>>>>>> they *always* want to try to continue. > > >>>>>>> > > >>>>>>> To be truthful, after some thought, using the "AlwaysContinue" > > > variant > > >>>>>>> isn't something I'd recommend generally in a production system. > > >>>> Ideally > > >>>>>>> these handlers are targeted at handling a specific series of > > >>>> exceptions > > >>>>>>> that a user wants to ignore, and not ignoring all exceptions. > More > > on > > >>>> this > > >>>>>>> in a minute. > > >>>>>>> > > >>>>>>>> For the first category, it seems to not make sense to call the > > >>>> handle but > > >>>>>>> Streams should always fail. If we follow this design, the KIP > > should > > >>>> list > > >>>>>>> all exceptions for which the handler is not called. > > >>>>>>> > > >>>>>>> I strongly disagree here. The purpose of this handler isn't > *just* > > to > > >>>>>>> make a decision for streams. There may also be desirable side > > effects > > >>>> that > > >>>>>>> users wish to cause when production exceptions occur. There may > be > > >>>>>>> side-effects that they wish to cause when > AuthenticationExceptions > > >>>> occur, > > >>>>>>> as well. We should still give them the hooks to preform those > side > > >>>> effects. > > >>>>>>> > > >>>>>>> In light of the above, I'm thinking that the > > >>>>>>> "AlwaysContinueProductionExceptionHandler" variant should > > probably be > > >>>>>>> removed from the public API and moved into tests since that's > where > > >>>> it's > > >>>>>>> most useful. The more I think on it, the more I feel that having > > that > > >>>> in > > >>>>>>> the public API is a landmine. If you agree, then, we can rename > the > > >>>>>>> "AlwaysFailProductionExceptionHandler" to > > >>>>>>> "DefaultProductionExceptionHandler". > > >>>>>>> > > >>>>>>> Thoughts? > > >>>>>>> > > >>>>>>> On Fri, Nov 10, 2017 at 6:13 PM Matthias J. Sax < > > >>>> matth...@confluent.io> > > >>>>>>> wrote: > > >>>>>>> > > >>>>>>>> I just review the PR, and there is one thing we should discuss. > > >>>>>>>> > > >>>>>>>> There are different types of exceptions that could occur. Some > > apply > > >>>> to > > >>>>>>>> all records (like Authorization exception) while others are for > > >>>> single > > >>>>>>>> records only (like record too large). > > >>>>>>>> > > >>>>>>>> For the first category, it seems to not make sense to call the > > > handle > > >>>>>>>> but Streams should always fail. If we follow this design, the > KIP > > >>>> should > > >>>>>>>> list all exceptions for which the handler is not called. > > >>>>>>>> > > >>>>>>>> WDYT? > > >>>>>>>> > > >>>>>>>> > > >>>>>>>> -Matthias > > >>>>>>>> > > >>>>>>>> > > >>>>>>>> On 11/10/17 2:56 PM, Matthias J. Sax wrote: > > >>>>>>>>> Just catching up on this KIP. > > >>>>>>>>> > > >>>>>>>>> One tiny comment: I would prefer to remove the "Always" from > the > > >>>>>>>> handler > > >>>>>>>>> implementation names -- it sounds "cleaner" to me without it. > > >>>>>>>>> > > >>>>>>>>> > > >>>>>>>>> -Matthias > > >>>>>>>>> > > >>>>>>>>> On 11/5/17 12:57 PM, Matt Farmer wrote: > > >>>>>>>>>> It is agreed, then. I've updated the pull request. I'm trying > to > > >>>> also > > >>>>>>>>>> update the KIP accordingly, but cwiki is being slow and > dropping > > >>>>>>>>>> connections..... I'll try again a bit later but please > consider > > > the > > >>>>>>>> KIP > > >>>>>>>>>> updated for all intents and purposes. heh. > > >>>>>>>>>> > > >>>>>>>>>> On Sun, Nov 5, 2017 at 3:45 PM Guozhang Wang < > > wangg...@gmail.com> > > >>>>>>>> wrote: > > >>>>>>>>>> > > >>>>>>>>>>> That makes sense. > > >>>>>>>>>>> > > >>>>>>>>>>> > > >>>>>>>>>>> Guozhang > > >>>>>>>>>>> > > >>>>>>>>>>> On Sun, Nov 5, 2017 at 12:33 PM, Matt Farmer <m...@frmr.me> > > >>>> wrote: > > >>>>>>>>>>> > > >>>>>>>>>>>> Interesting. I'm not sure I agree. I've been bitten many > times > > > by > > >>>>>>>>>>>> unintentionally shipping code that fails to properly > implement > > >>>>>>>> logging. I > > >>>>>>>>>>>> always discover this at the exact *worst* moment, too. > > (Normally > > >>>> at > > >>>>>>>> 3 AM > > >>>>>>>>>>>> during an on-call shift. Hah.) However, if others feel the > > same > > >>>> way > > >>>>>>>> I > > >>>>>>>>>>> could > > >>>>>>>>>>>> probably be convinced to remove it. > > >>>>>>>>>>>> > > >>>>>>>>>>>> We could also meet halfway and say that when a customized > > >>>>>>>>>>>> ProductionExceptionHandler instructs Streams to CONTINUE, we > > log > > >>>> at > > >>>>>>>> DEBUG > > >>>>>>>>>>>> level instead of WARN level. Would that alternative be > > appealing > > >>>> to > > >>>>>>>> you? > > >>>>>>>>>>>> > > >>>>>>>>>>>> On Sun, Nov 5, 2017 at 12:32 PM Guozhang Wang < > > >>>> wangg...@gmail.com> > > >>>>>>>>>>> wrote: > > >>>>>>>>>>>> > > >>>>>>>>>>>>> Thanks for the updates. I made a pass over the wiki again > and > > > it > > >>>>>>>> looks > > >>>>>>>>>>>>> good. > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> About whether record collector should still internally log > > the > > >>>>>>>> error in > > >>>>>>>>>>>>> addition to what the customized ProductionExceptionHandler > > >>>> does. I > > >>>>>>>>>>>>> personally would prefer only to log if the returned value > is > > >>>> FAIL > > >>>>>>>> to > > >>>>>>>>>>>>> indicate that this thread is going to shutdown and trigger > > the > > >>>>>>>>>>> exception > > >>>>>>>>>>>>> handler. > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> Guozhang > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> On Sun, Nov 5, 2017 at 6:09 AM, Matt Farmer <m...@frmr.me> > > >>>> wrote: > > >>>>>>>>>>>>> > > >>>>>>>>>>>>>> Hello, a bit later than I'd anticipated, but I've updated > > this > > >>>>>>>> KIP as > > >>>>>>>>>>>>>> outlined above. The updated KIP is now ready for review > > again! > > >>>>>>>>>>>>>> > > >>>>>>>>>>>>>> On Sat, Nov 4, 2017 at 1:03 PM Matt Farmer <m...@frmr.me> > > >>>> wrote: > > >>>>>>>>>>>>>> > > >>>>>>>>>>>>>>> Ah. I actually created both of those in the PR and forgot > > to > > >>>>>>>>>>> mention > > >>>>>>>>>>>>> them > > >>>>>>>>>>>>>>> by name in the KIP! Thanks for pointing out the > oversight. > > >>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>> I’ll revise the KIP this afternoon accordingly. > > >>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>> The logging is actually provided for in the record > > collector. > > >>>>>>>>>>>> Whenever > > >>>>>>>>>>>>> a > > >>>>>>>>>>>>>>> handler continues it’ll log a warning to ensure that it’s > > >>>>>>>>>>>> *impossible* > > >>>>>>>>>>>>> to > > >>>>>>>>>>>>>>> write a handler that totally suppresses production > > exceptions > > >>>>>>>> from > > >>>>>>>>>>>> the > > >>>>>>>>>>>>>> log. > > >>>>>>>>>>>>>>> As such, the default continue handler just returns the > > >>>> continue > > >>>>>>>>>>>> value. > > >>>>>>>>>>>>> I > > >>>>>>>>>>>>>>> can add details about those semantics to the KIP as well. > > >>>>>>>>>>>>>>> On Sat, Nov 4, 2017 at 12:46 PM Matthias J. Sax < > > >>>>>>>>>>>> matth...@confluent.io > > >>>>>>>>>>>>>> > > >>>>>>>>>>>>>>> wrote: > > >>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> One more comment. > > >>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> You mention a default implementation for the handler > that > > >>>>>>>> fails. I > > >>>>>>>>>>>>>>>> think, this should be part of the public API and thus > > should > > >>>>>>>> have > > >>>>>>>>>>> a > > >>>>>>>>>>>>>>>> proper defined named that is mentioned in the KIP. > > >>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> We could also add a second implementation for the > > >>>>>>>> log-and-move-on > > >>>>>>>>>>>>>>>> strategy, as both are the two most common cases. This > > class > > >>>>>>>> should > > >>>>>>>>>>>>> also > > >>>>>>>>>>>>>>>> be part of public API (so users can just set in the > > config) > > >>>>>>>> with a > > >>>>>>>>>>>>>>>> proper name. > > >>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> Otherwise, I like the KIP a lot! Thanks. > > >>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> -Matthias > > >>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> On 11/1/17 12:23 AM, Matt Farmer wrote: > > >>>>>>>>>>>>>>>>> Thanks for the heads up. Yes, I think my changes are > > >>>> compatible > > >>>>>>>>>>>> with > > >>>>>>>>>>>>>>>> that > > >>>>>>>>>>>>>>>>> PR, but there will be a merge conflict that happens > > > whenever > > >>>>>>>> one > > >>>>>>>>>>>> of > > >>>>>>>>>>>>>> the > > >>>>>>>>>>>>>>>> PRs > > >>>>>>>>>>>>>>>>> is merged. Happy to reconcile the changes in my PR if > > 4148 > > >>>> goes > > >>>>>>>>>>> in > > >>>>>>>>>>>>>>>> first. :) > > >>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>> On Tue, Oct 31, 2017 at 6:44 PM Guozhang Wang < > > >>>>>>>>>>> wangg...@gmail.com > > >>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> wrote: > > >>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> That sounds reasonable, thanks Matt. > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> As for the implementation, please note that there is > > >>>> another > > >>>>>>>>>>>>> ongoing > > >>>>>>>>>>>>>> PR > > >>>>>>>>>>>>>>>>>> that may touch the same classes that you are working > on: > > >>>>>>>>>>>>>>>>>> https://github.com/apache/kafka/pull/4148 > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> So it may help if you can also take a look at that PR > > and > > >>>> see > > >>>>>>>>>>> if > > >>>>>>>>>>>> it > > >>>>>>>>>>>>>> is > > >>>>>>>>>>>>>>>>>> compatible with your changes. > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> Guozhang > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> On Tue, Oct 31, 2017 at 10:59 AM, Matt Farmer < > > >>>> m...@frmr.me> > > >>>>>>>>>>>>> wrote: > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>> I've opened this pull request to implement the KIP as > > >>>>>>>>>>> currently > > >>>>>>>>>>>>>>>> written: > > >>>>>>>>>>>>>>>>>>> https://github.com/apache/kafka/pull/4165. It still > > needs > > >>>>>>>>>>> some > > >>>>>>>>>>>>>> tests > > >>>>>>>>>>>>>>>>>>> added, > > >>>>>>>>>>>>>>>>>>> but largely represents the shape I was going for. > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>> If there are more points that folks would like to > > > discuss, > > >>>>>>>>>>>> please > > >>>>>>>>>>>>>> let > > >>>>>>>>>>>>>>>> me > > >>>>>>>>>>>>>>>>>>> know. If I don't hear anything by tomorrow afternoon > > I'll > > >>>>>>>>>>>> probably > > >>>>>>>>>>>>>>>> start > > >>>>>>>>>>>>>>>>>> a > > >>>>>>>>>>>>>>>>>>> [VOTE] thread. > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>> Thanks, > > >>>>>>>>>>>>>>>>>>> Matt > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>> On Fri, Oct 27, 2017 at 7:33 PM Matt Farmer < > > m...@frmr.me > > >>>>> > > >>>>>>>>>>>> wrote: > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>> I can’t think of a reason that would be problematic. > > >>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>> Most of the time I would write a handler like this, > I > > >>>> either > > >>>>>>>>>>>> want > > >>>>>>>>>>>>>> to > > >>>>>>>>>>>>>>>>>>>> ignore the error or fail and bring everything down > so > > >>>> that I > > >>>>>>>>>>>> can > > >>>>>>>>>>>>>> spin > > >>>>>>>>>>>>>>>>>> it > > >>>>>>>>>>>>>>>>>>>> back up later and resume from earlier offsets. When > we > > >>>> start > > >>>>>>>>>>> up > > >>>>>>>>>>>>>> after > > >>>>>>>>>>>>>>>>>>>> crashing we’ll eventually try to process the message > > we > > >>>>>>>>>>> failed > > >>>>>>>>>>>> to > > >>>>>>>>>>>>>>>>>> produce > > >>>>>>>>>>>>>>>>>>>> again. > > >>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>> I’m concerned that “putting in a queue for later” > > opens > > >>>> you > > >>>>>>>>>>> up > > >>>>>>>>>>>> to > > >>>>>>>>>>>>>>>>>> putting > > >>>>>>>>>>>>>>>>>>>> messages into the destination topic in an unexpected > > >>>> order. > > >>>>>>>>>>>>> However > > >>>>>>>>>>>>>>>> if > > >>>>>>>>>>>>>>>>>>>> others feel differently, I’m happy to talk about it. > > >>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>> On Fri, Oct 27, 2017 at 7:10 PM Guozhang Wang < > > >>>>>>>>>>>>> wangg...@gmail.com> > > >>>>>>>>>>>>>>>>>>> wrote: > > >>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>> Please correct me if I'm wrong, but my > understanding > > > is > > >>>>>>>>>>> that > > >>>>>>>>>>>>> the > > >>>>>>>>>>>>>>>>>>> record > > >>>>>>>>>>>>>>>>>>>>>> metadata is always null if an exception occurred > > while > > >>>>>>>>>>> trying > > >>>>>>>>>>>>> to > > >>>>>>>>>>>>>>>>>>>>> produce. > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>> That is right. Thanks. > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>> I looked at the example code, and one thing I > > realized > > >>>> that > > >>>>>>>>>>>>> since > > >>>>>>>>>>>>>> we > > >>>>>>>>>>>>>>>>>> are > > >>>>>>>>>>>>>>>>>>>>> not passing the context in the handle function, we > > may > > >>>> not > > >>>>>>>>>>> be > > >>>>>>>>>>>>>>>>>> implement > > >>>>>>>>>>>>>>>>>>>>> the > > >>>>>>>>>>>>>>>>>>>>> logic to send the fail records into another queue > for > > >>>>>>>> future > > >>>>>>>>>>>>>>>>>> processing. > > >>>>>>>>>>>>>>>>>>>>> Would people think that would be a big issue? > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>> Guozhang > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>> On Thu, Oct 26, 2017 at 12:14 PM, Matt Farmer < > > >>>>>>>> m...@frmr.me > > >>>>>>>>>>>> > > >>>>>>>>>>>>>> wrote: > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>> Hello all, > > >>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>> I've updated the KIP based on this conversation, > and > > >>>> made > > >>>>>>>>>>> it > > >>>>>>>>>>>> so > > >>>>>>>>>>>>>>>> that > > >>>>>>>>>>>>>>>>>>> its > > >>>>>>>>>>>>>>>>>>>>>> interface, config setting, and parameters line up > > more > > >>>>>>>>>>>> closely > > >>>>>>>>>>>>>> with > > >>>>>>>>>>>>>>>>>>> the > > >>>>>>>>>>>>>>>>>>>>>> interface in KIP-161 (deserialization handler). > > >>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>> I believe there are a few specific questions I > need > > to > > >>>>>>>>>>> reply > > >>>>>>>>>>>>>>>> to..... > > >>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>> The question I had about then handle parameters > are > > >>>>>>>> around > > >>>>>>>>>>>> the > > >>>>>>>>>>>>>>>>>>> record, > > >>>>>>>>>>>>>>>>>>>>>>> should it be `ProducerRecord<byte[], byte[]>`, or > > be > > >>>>>>>>>>>> generics > > >>>>>>>>>>>>> of > > >>>>>>>>>>>>>>>>>>>>>>> `ProducerRecord<? extends K, ? extends V>` or > > >>>>>>>>>>>>> `ProducerRecord<? > > >>>>>>>>>>>>>>>>>>>>> extends > > >>>>>>>>>>>>>>>>>>>>>>> Object, ? extends Object>`? > > >>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>> At this point in the code we're guaranteed that > this > > >>>> is a > > >>>>>>>>>>>>>>>>>>>>>> ProducerRecord<byte[], byte[]>, so the generics > > would > > >>>> just > > >>>>>>>>>>>> make > > >>>>>>>>>>>>>> it > > >>>>>>>>>>>>>>>>>>>>> harder > > >>>>>>>>>>>>>>>>>>>>>> to work with the key and value. > > >>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>> Also, should the handle function include the > > >>>>>>>>>>>> `RecordMetadata` > > >>>>>>>>>>>>> as > > >>>>>>>>>>>>>>>>>>> well > > >>>>>>>>>>>>>>>>>>>>> in > > >>>>>>>>>>>>>>>>>>>>>>> case it is not null? > > >>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>> Please correct me if I'm wrong, but my > understanding > > > is > > >>>>>>>>>>> that > > >>>>>>>>>>>>> the > > >>>>>>>>>>>>>>>>>>> record > > >>>>>>>>>>>>>>>>>>>>>> metadata is always null if an exception occurred > > while > > >>>>>>>>>>> trying > > >>>>>>>>>>>>> to > > >>>>>>>>>>>>>>>>>>>>> produce. > > >>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>> We may probably try to write down at least the > > >>>> following > > >>>>>>>>>>>>>> handling > > >>>>>>>>>>>>>>>>>>>>> logic > > >>>>>>>>>>>>>>>>>>>>>> and > > >>>>>>>>>>>>>>>>>>>>>>> see if the given API is sufficient for it > > >>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>> I've added some examples to the KIP. Let me know > > what > > >>>> you > > >>>>>>>>>>>>> think. > > >>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>> Cheers, > > >>>>>>>>>>>>>>>>>>>>>> Matt > > >>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>> On Mon, Oct 23, 2017 at 9:00 PM Matt Farmer < > > >>>> m...@frmr.me > > >>>>>>>>> > > >>>>>>>>>>>>>> wrote: > > >>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>> Thanks for this feedback. I’m at a conference > right > > >>>> now > > >>>>>>>>>>> and > > >>>>>>>>>>>> am > > >>>>>>>>>>>>>>>>>>>>> planning > > >>>>>>>>>>>>>>>>>>>>>> on > > >>>>>>>>>>>>>>>>>>>>>>> updating the KIP again with details from this > > >>>>>>>> conversation > > >>>>>>>>>>>>> later > > >>>>>>>>>>>>>>>>>>> this > > >>>>>>>>>>>>>>>>>>>>>> week. > > >>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>> I’ll shoot you a more detailed response then! :) > > >>>>>>>>>>>>>>>>>>>>>>> On Mon, Oct 23, 2017 at 8:16 PM Guozhang Wang < > > >>>>>>>>>>>>>> wangg...@gmail.com > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>> wrote: > > >>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>> Thanks for the KIP Matt. > > >>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>> Regarding the handle interface of > > >>>>>>>>>>>>>> ProductionExceptionHandlerResp > > >>>>>>>>>>>>>>>>>>> onse, > > >>>>>>>>>>>>>>>>>>>>>>>> could > > >>>>>>>>>>>>>>>>>>>>>>>> you write it on the wiki also, along with the > > actual > > >>>>>>>>>>> added > > >>>>>>>>>>>>>> config > > >>>>>>>>>>>>>>>>>>>>> names > > >>>>>>>>>>>>>>>>>>>>>>>> (e.g. what > > >>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP- > > >>>>>>>>>>>> 161%3A+streams+ > > >>>>>>>>>>>>>>>>>>>>>> deserialization+exception+handlers > > >>>>>>>>>>>>>>>>>>>>>>>> described). > > >>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>> The question I had about then handle parameters > > are > > >>>>>>>>>>> around > > >>>>>>>>>>>>> the > > >>>>>>>>>>>>>>>>>>>>> record, > > >>>>>>>>>>>>>>>>>>>>>>>> should it be `ProducerRecord<byte[], byte[]>`, > or > > be > > >>>>>>>>>>>> generics > > >>>>>>>>>>>>>> of > > >>>>>>>>>>>>>>>>>>>>>>>> `ProducerRecord<? extends K, ? extends V>` or > > >>>>>>>>>>>>> `ProducerRecord<? > > >>>>>>>>>>>>>>>>>>>>> extends > > >>>>>>>>>>>>>>>>>>>>>>>> Object, ? extends Object>`? > > >>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>> Also, should the handle function include the > > >>>>>>>>>>>> `RecordMetadata` > > >>>>>>>>>>>>>> as > > >>>>>>>>>>>>>>>>>>>>> well in > > >>>>>>>>>>>>>>>>>>>>>>>> case it is not null? > > >>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>> We may probably try to write down at least the > > >>>> following > > >>>>>>>>>>>>>> handling > > >>>>>>>>>>>>>>>>>>>>> logic > > >>>>>>>>>>>>>>>>>>>>>>>> and > > >>>>>>>>>>>>>>>>>>>>>>>> see if the given API is sufficient for it: 1) > > throw > > >>>>>>>>>>>> exception > > >>>>>>>>>>>>>>>>>>>>>> immediately > > >>>>>>>>>>>>>>>>>>>>>>>> to fail fast and stop the world, 2) log the > error > > > and > > >>>>>>>>>>> drop > > >>>>>>>>>>>>>> record > > >>>>>>>>>>>>>>>>>>> and > > >>>>>>>>>>>>>>>>>>>>>>>> proceed silently, 3) send such errors to a > > specific > > >>>>>>>>>>> "error" > > >>>>>>>>>>>>>> Kafka > > >>>>>>>>>>>>>>>>>>>>> topic, > > >>>>>>>>>>>>>>>>>>>>>>>> or > > >>>>>>>>>>>>>>>>>>>>>>>> record it as an app-level metrics ( > > >>>>>>>>>>>>>>>>>>>>>>>> > > >>>> https://kafka.apache.org/documentation/#kafka_streams_ > > >>>>>>>>>>>>>> monitoring > > >>>>>>>>>>>>>>>>>> ) > > >>>>>>>>>>>>>>>>>>>>> for > > >>>>>>>>>>>>>>>>>>>>>>>> monitoring. > > >>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>> Guozhang > > >>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>> On Fri, Oct 20, 2017 at 5:47 PM, Matt Farmer < > > >>>>>>>>>>> m...@frmr.me > > >>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> wrote: > > >>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>> I did some more digging tonight. > > >>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>> @Ted: It looks like the deserialization handler > > > uses > > >>>>>>>>>>>>>>>>>>>>>>>>> "default.deserialization.exception.handler" > for > > the > > >>>>>>>>>>>> config > > >>>>>>>>>>>>>>>>>>> name. No > > >>>>>>>>>>>>>>>>>>>>>>>>> ".class" on the end. I'm inclined to think this > > >>>> should > > >>>>>>>>>>> use > > >>>>>>>>>>>>>>>>>>>>>>>>> "default.production.exception.handler". > > >>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>> On Fri, Oct 20, 2017 at 8:22 PM Matt Farmer < > > >>>>>>>>>>> m...@frmr.me > > >>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>> wrote: > > >>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>> Okay, I've dug into this a little bit. > > >>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>> I think getting access to the serialized > record > > is > > >>>>>>>>>>>>> possible, > > >>>>>>>>>>>>>>>>>>> and > > >>>>>>>>>>>>>>>>>>>>>>>> changing > > >>>>>>>>>>>>>>>>>>>>>>>>>> the naming and return type is certainly > doable. > > >>>>>>>>>>> However, > > >>>>>>>>>>>>>>>>>>> because > > >>>>>>>>>>>>>>>>>>>>>> we're > > >>>>>>>>>>>>>>>>>>>>>>>>>> hooking into the onCompletion callback we have > > no > > >>>>>>>>>>>> guarantee > > >>>>>>>>>>>>>>>>>>> that > > >>>>>>>>>>>>>>>>>>>>> the > > >>>>>>>>>>>>>>>>>>>>>>>>>> ProcessorContext state hasn't changed by the > > time > > >>>> this > > >>>>>>>>>>>>>>>>>>> particular > > >>>>>>>>>>>>>>>>>>>>>>>> handler > > >>>>>>>>>>>>>>>>>>>>>>>>>> runs. So I think the signature would change to > > >>>>>>>>>>> something > > >>>>>>>>>>>>>>>>>> like: > > >>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>> ProductionExceptionHandlerResponse > handle(final > > >>>>>>>>>>>>>>>>>>>>> ProducerRecord<..> > > >>>>>>>>>>>>>>>>>>>>>>>>> record, > > >>>>>>>>>>>>>>>>>>>>>>>>>> final Exception exception) > > >>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>> Would this be acceptable? > > >>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>> On Thu, Oct 19, 2017 at 7:33 PM Matt Farmer < > > >>>>>>>>>>>> m...@frmr.me> > > >>>>>>>>>>>>>>>>>>>>> wrote: > > >>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>> Ah good idea. Hmmm. I can line up the naming > > and > > >>>>>>>>>>> return > > >>>>>>>>>>>>> type > > >>>>>>>>>>>>>>>>>>> but > > >>>>>>>>>>>>>>>>>>>>>> I’m > > >>>>>>>>>>>>>>>>>>>>>>>> not > > >>>>>>>>>>>>>>>>>>>>>>>>>>> sure if I can get my hands on the context and > > the > > >>>>>>>>>>> record > > >>>>>>>>>>>>>>>>>>> itself > > >>>>>>>>>>>>>>>>>>>>>>>> without > > >>>>>>>>>>>>>>>>>>>>>>>>>>> other changes. > > >>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>> Let me dig in and follow up here tomorrow. > > >>>>>>>>>>>>>>>>>>>>>>>>>>> On Thu, Oct 19, 2017 at 7:14 PM Matthias J. > > Sax < > > >>>>>>>>>>>>>>>>>>>>>>>> matth...@confluent.io> > > >>>>>>>>>>>>>>>>>>>>>>>>>>> wrote: > > >>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> Thanks for the KIP. > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> Are you familiar with KIP-161? > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP- > > >>>>>>>>>>>> 161%3A+streams+ > > >>>>>>>>>>>>>>>>>>>>>>>>> deserialization+exception+handlers > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> I thinks, we should align the design > > (parameter > > >>>>>>>>>>> naming, > > >>>>>>>>>>>>>>>>>>> return > > >>>>>>>>>>>>>>>>>>>>>>>> types, > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> class names etc) of KIP-210 to KIP-161 to > get > > a > > >>>>>>>>>>> unified > > >>>>>>>>>>>>>>>>>> user > > >>>>>>>>>>>>>>>>>>>>>>>>> experience. > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> -Matthias > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> On 10/18/17 4:20 PM, Matt Farmer wrote: > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> I’ll create the JIRA ticket. > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> I think that config name will work. I’ll > > update > > >>>> the > > >>>>>>>>>>>> KIP > > >>>>>>>>>>>>>>>>>>>>>>>> accordingly. > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Wed, Oct 18, 2017 at 6:09 PM Ted Yu < > > >>>>>>>>>>>>>>>>>>> yuzhih...@gmail.com> > > >>>>>>>>>>>>>>>>>>>>>>>> wrote: > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Can you create JIRA that corresponds to > the > > >>>> KIP ? > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> For the new config, how about naming it > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> production.exception.processor.class > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ? This way it is clear that class name > > should > > >>>> be > > >>>>>>>>>>>>>>>>>>> specified. > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Cheers > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Wed, Oct 18, 2017 at 2:40 PM, Matt > > Farmer < > > >>>>>>>>>>>>>>>>>>> m...@frmr.me> > > >>>>>>>>>>>>>>>>>>>>>>>> wrote: > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Hello everyone, > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> This is the discussion thread for the > KIP > > >>>> that I > > >>>>>>>>>>>> just > > >>>>>>>>>>>>>>>>>>> filed > > >>>>>>>>>>>>>>>>>>>>>>>> here: > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>> https://cwiki.apache.org/confluence/display/KAFKA/KIP- > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> 210+-+Provide+for+custom+ > > >>>>>>>>>>>> error+handling++when+Kafka+ > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Streams+fails+to+produce > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Looking forward to getting some feedback > > from > > >>>>>>>>>>> folks > > >>>>>>>>>>>>>>>>>> about > > >>>>>>>>>>>>>>>>>>>>> this > > >>>>>>>>>>>>>>>>>>>>>>>> idea > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> and > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> working toward a solution we can > > contribute > > >>>> back. > > >>>>>>>>>>> :) > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Cheers, > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> Matt Farmer > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>>> -- > > >>>>>>>>>>>>>>>>>>>>>>>> -- Guozhang > > >>>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>>> -- > > >>>>>>>>>>>>>>>>>>>>> -- Guozhang > > >>>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>>> -- > > >>>>>>>>>>>>>>>>>> -- Guozhang > > >>>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>>>> > > >>>>>>>>>>>>>> > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> > > >>>>>>>>>>>>> -- > > >>>>>>>>>>>>> -- Guozhang > > >>>>>>>>>>>>> > > >>>>>>>>>>>> > > >>>>>>>>>>> > > >>>>>>>>>>> > > >>>>>>>>>>> > > >>>>>>>>>>> -- > > >>>>>>>>>>> -- Guozhang > > >>>>>>>>>>> > > >>>>>>>>>> > > >>>>>>>>> > > >>>>>>>> > > >>>>>>>> > > >>>>> > > >>>> > > >>>> > > >> > > > > > > > > > > -- > -- Guozhang >
