Mailing-List: contact dev-help@kafka.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@kafka.apache.org
From: Matt Farmer <matt@frmr.me>
In-Reply-To: <CAHwHRrUBAsct+rGEdVrs=DPCMf6MrMzZs44eRLo9AfU3G+YJ2Q@mail.gmail.com>
References: <CAM5dya-HUqi5TQjMY=VXeTs2O+yQ85VArHUVUie-X3xj_gPN=g@mail.gmail.com>
 <CAM5dya-hXnS5ZAra0kSiOQr4c48+YB-6cU_0LJrEFvABp7GTog@mail.gmail.com>
 <CAHwHRrVOF=_5CC0nvTu1yWXxj7OQyjkRNn=dtRt-YkfQDc=jbw@mail.gmail.com>
 <CAM5dya-3kB8vtPY-n0r7WUAdYYjdhdxuznBaWJ98TV5t-KwnLg@mail.gmail.com>
 <ed3bd542-1d32-46ba-389c-a1ca80d8585e@confluent.io> <aa1bd7a5-8425-3a92-c080-2cf359674913@confluent.io>
 <CAM5dya_f33DhvmdXMgh0___jQ0=oY6xDaUXQ1S_ANWHkABjVFQ@mail.gmail.com>
 <CAM5dya86jmORWbS_7KfWcuKNH-LZgkoAfcJBxGN7sY2S5fdpHw@mail.gmail.com>
 <CAM5dya8kvG8eUiuDpyngWY8_nTEpRFGKfCRh6CTkd6mPDc1HXQ@mail.gmail.com>
 <3c92bb5a-a8bc-5c48-46f5-677f1f35b68b@confluent.io> <CAM5dya9fxagLJ+dPuTg7RW2PgB5CKX9B0DEbmgD6N9Rewm7K_Q@mail.gmail.com>
 <CAM5dya-xgYsJ65ySEu314vRWPSFtyDKQjkLzvP7ZBi0ogNWJ=g@mail.gmail.com>
 <67749cf4-4afc-442e-f5d4-2fd71e569d65@confluent.io> <CAM5dya9LCEfD1ye+fqfQ3tbO6_0iXuH9+rY_tBtxxxiSJ+nfwQ@mail.gmail.com>
 <CAM5dya8qqQ+7kHZ9W2ce56UQ115+NfJcOfBk_jwHwfaNodqYKw@mail.gmail.com>
 <1e2311fb-9e86-4094-ebd7-b3db3e2e49f3@confluent.io> <CAHwHRrUBAsct+rGEdVrs=DPCMf6MrMzZs44eRLo9AfU3G+YJ2Q@mail.gmail.com>
MIME-Version: 1.0
Date: Wed, 6 Dec 2017 09:52:55 -0500
Message-ID: <CAM5dya8D6PrJ5=gRhSCZc37QdWO87Wp39vwyfKiNDa13yN_z9w@mail.gmail.com>
Subject: Re: [DISCUSS] KIP-210: Provide for custom error handling when Kafka
 Streams fails to produce
To: dev@kafka.apache.org
Content-Type: multipart/alternative; boundary="001a1143195061340b055fad1b31"
archived-at: Wed, 06 Dec 2017 14:53:22 -0000

--001a1143195061340b055fad1b31
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

There is already a vote thread for this KIP. I can bump it so that it=E2=80=
=99s
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=E2=80=99s just not defined using a code snippet. The KIP =
reads as
follows:

=3D=3D=3D

A public interface named ProductionExceptionHandler with a single method,
handle, that has the following signature:

   - ProductionExceptionHandlerResponse handle(ProducerRecord<byte[],
   byte[]> record, Exception exception)


=3D=3D=3D

If you=E2=80=99d like me to add a code snippet illustrating this that=E2=80=
=99s 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 =3D LoggerFactory.getLogger(HelloWorld.class);

I omit that in my implementation examples for brevity.

On December 6, 2017 at 2:14:58 AM, Guozhang Wang (wangguoz@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 <matthias@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 (matt@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=E2=80=99v=
e
decided
> we
> > won=E2=80=99t 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=E2=80=99ll take a look at doing more work on the KIP and the Pull Req=
uest
> > tomorrow.
> >
> > Thanks again!
> >
> > On December 4, 2017 at 5:50:33 PM, Matthias J. Sax (
> matthias@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. =3D)
> >>
> >> On Tue, Nov 28, 2017 at 9:53 AM Matt Farmer <matt@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)
--=20
> >>>> 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 <matthias@confluent.i=
o
> >
> >>> 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)
--=20
> >>>> 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 =E2=80=
=94 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 =E2=80=94 What do you think about the above?
> >>>>>
> >>>>> On Tue, Nov 14, 2017 at 9:44 AM Matt Farmer <matt@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 <matt@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 <
> >>>> matthias@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 <
> wangguoz@gmail.com>
> >>>>>>>> wrote:
> >>>>>>>>>>
> >>>>>>>>>>> That makes sense.
> >>>>>>>>>>>
> >>>>>>>>>>>
> >>>>>>>>>>> Guozhang
> >>>>>>>>>>>
> >>>>>>>>>>> On Sun, Nov 5, 2017 at 12:33 PM, Matt Farmer <matt@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 <
> >>>> wangguoz@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 <matt@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 <matt@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=E2=80=99ll revise the KIP this afternoon accordingly.
> >>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>> The logging is actually provided for in the record
> collector.
> >>>>>>>>>>>> Whenever
> >>>>>>>>>>>>> a
> >>>>>>>>>>>>>>> handler continues it=E2=80=99ll log a warning to ensure t=
hat it=E2=80=99s
> >>>>>>>>>>>> *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 <
> >>>>>>>>>>>> matthias@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 <
> >>>>>>>>>>> wangguoz@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 <
> >>>> matt@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 <
> matt@frmr.me
> >>>>>
> >>>>>>>>>>>> wrote:
> >>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>> I can=E2=80=99t think of a reason that would be prob=
lematic.
> >>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>> 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=E2=80=99ll eventually try to process the=
 message
> we
> >>>>>>>>>>> failed
> >>>>>>>>>>>> to
> >>>>>>>>>>>>>>>>>> produce
> >>>>>>>>>>>>>>>>>>>> again.
> >>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>> I=E2=80=99m concerned that =E2=80=9Cputting in a que=
ue for later=E2=80=9D
> opens
> >>>> you
> >>>>>>>>>>> up
> >>>>>>>>>>>> to
> >>>>>>>>>>>>>>>>>> putting
> >>>>>>>>>>>>>>>>>>>> messages into the destination topic in an unexpected
> >>>> order.
> >>>>>>>>>>>>> However
> >>>>>>>>>>>>>>>> if
> >>>>>>>>>>>>>>>>>>>> others feel differently, I=E2=80=99m happy to talk a=
bout it.
> >>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>> On Fri, Oct 27, 2017 at 7:10 PM Guozhang Wang <
> >>>>>>>>>>>>> wangguoz@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 <
> >>>>>>>> matt@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 <
> >>>> matt@frmr.me
> >>>>>>>>>
> >>>>>>>>>>>>>> wrote:
> >>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>> Thanks for this feedback. I=E2=80=99m at a confer=
ence
right
> >>>> now
> >>>>>>>>>>> and
> >>>>>>>>>>>> am
> >>>>>>>>>>>>>>>>>>>>> planning
> >>>>>>>>>>>>>>>>>>>>>> on
> >>>>>>>>>>>>>>>>>>>>>>> updating the KIP again with details from this
> >>>>>>>> conversation
> >>>>>>>>>>>>> later
> >>>>>>>>>>>>>>>>>>> this
> >>>>>>>>>>>>>>>>>>>>>> week.
> >>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>> I=E2=80=99ll shoot you a more detailed response t=
hen! :)
> >>>>>>>>>>>>>>>>>>>>>>> On Mon, Oct 23, 2017 at 8:16 PM Guozhang Wang <
> >>>>>>>>>>>>>> wangguoz@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 <
> >>>>>>>>>>> matt@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 <
> >>>>>>>>>>> matt@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 <
> >>>>>>>>>>>> matt@frmr.me>
> >>>>>>>>>>>>>>>>>>>>> wrote:
> >>>>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>>>> Ah good idea. Hmmm. I can line up the naming
> and
> >>>>>>>>>>> return
> >>>>>>>>>>>>> type
> >>>>>>>>>>>>>>>>>>> but
> >>>>>>>>>>>>>>>>>>>>>> I=E2=80=99m
> >>>>>>>>>>>>>>>>>>>>>>>> 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 <
> >>>>>>>>>>>>>>>>>>>>>>>> matthias@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=E2=80=99ll create the JIRA ticket.
> >>>>>>>>>>>>>>>>>>>>>>>>>>>>>
> >>>>>>>>>>>>>>>>>>>>>>>>>>>>> I think that config name will work. I=E2=80=
=99ll
> update
> >>>> the
> >>>>>>>>>>>> KIP
> >>>>>>>>>>>>>>>>>>>>>>>> accordingly.
> >>>>>>>>>>>>>>>>>>>>>>>>>>>>> On Wed, Oct 18, 2017 at 6:09 PM Ted Yu <
> >>>>>>>>>>>>>>>>>>> yuzhihong@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 <
> >>>>>>>>>>>>>>>>>>> matt@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
> >>>>>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>>
> >>>>>
> >>>>
> >>>>
> >>
> >
>
>


--=20
-- Guozhang

--001a1143195061340b055fad1b31--