Hi Boyang, sounds good to me.

-Jason

On Mon, Feb 1, 2021 at 9:32 AM Boyang Chen <reluctanthero...@gmail.com>
wrote:

> Hey Jason and Chia-Ping,
>
> looking at the discussion thread for KIP-706, we are still in discussion
> with the final API version, so I made the decision to decouple the two KIPs
> so that this one could move forward. To be specific, we are still
> introducing ProduceFailed error but as a `near-future` improvement, which
> should be implemented right after KIP-706 is finalized.
>
> Let me know your thoughts.
>
> Best,
> Boyang
>
> On Sat, Jan 30, 2021 at 11:57 AM Chia-Ping Tsai <chia7...@apache.org>
> wrote:
>
> > > a richer return type. Let me know if you are good with this, and
> whether
> > > Chia-Ping is also happy here :)
> >
> > sure. I'd like to join this bigger party :)
> >
> > On 2021/01/30 01:03:57, Boyang Chen <reluctanthero...@gmail.com> wrote:
> > > This is a great proposal Jason, I already integrated KIP-691 with
> KIP-706
> > > template to provide a new ProduceFailedException as well as a new
> > > Producer#produce API with CompletionStage as old send API replacement
> > with
> > > a richer return type. Let me know if you are good with this, and
> whether
> > > Chia-Ping is also happy here :)
> > >
> > > Best,
> > > Boyang
> > >
> > > On Fri, Jan 29, 2021 at 4:23 PM Jason Gustafson <ja...@confluent.io>
> > wrote:
> > >
> > > > Hi Boyang,
> > > >
> > > > Ok, if we are going to ask all users to update their code anyway,
> > maybe we
> > > > can get more bang out of this.
> > > >
> > > > One of the remaining problems is that a user will only get the richer
> > > > information if they are supplying a Callback. If they wait on the
> > future,
> > > > then they will get the usual exception. Our hands are somewhat tied
> > here
> > > > because of compatibility. One thing we have wanted to do for a long
> > time is
> > > > introduce a new send method which returns
> > > > `CompletableFuture<RecordMetadata>` instead. If we did so, then we
> > wouldn't
> > > > need to change `Callback` at all. Instead we could make the contract
> > of the
> > > > new api that it will always return `SendFailedException`, which could
> > > > provide a method to get the failure type.
> > > >
> > > > I think Chia-Ping has been thinking about this:
> > > > https://issues.apache.org/jira/browse/KAFKA-12227. Is there a way to
> > > > consolidate our efforts in order to avoid unnecessary noise for
> users?
> > It
> > > > would be awesome to see a more modern API which also addresses the
> > > > confusing error handling.
> > > >
> > > > -Jason
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > On Fri, Jan 29, 2021 at 3:48 PM Boyang Chen <
> > reluctanthero...@gmail.com>
> > > > wrote:
> > > >
> > > > > Thanks Jason, my hope is that we could streamline the callback
> > function
> > > > > signature, so that in the next major release (3.0),
> > > > > onCompletion(RecordMetadata metadata, SendFailure sendFailure) will
> > > > become
> > > > > the only API to be implemented.
> > > > >
> > > > > On Fri, Jan 29, 2021 at 3:42 PM Jason Gustafson <
> ja...@confluent.io>
> > > > > wrote:
> > > > >
> > > > > > Hi Boyang,
> > > > > >
> > > > > > Do you think it's necessary to deprecate the old `onCompletion`
> > > > > callback? I
> > > > > > was thinking there's probably no harm leaving it around. Users
> > might
> > > > not
> > > > > > care about the failure type. Other than that, it looks good to
> me.
> > > > > >
> > > > > > Thanks,
> > > > > > Jason
> > > > > >
> > > > > > On Thu, Jan 28, 2021 at 6:58 PM Boyang Chen <
> > > > reluctanthero...@gmail.com>
> > > > > > wrote:
> > > > > >
> > > > > > > Hey Guozhang,
> > > > > > >
> > > > > > > the CommitFailedException wrapping would cover all the
> non-fatal
> > > > > > exceptions
> > > > > > > as we listed out in the KIP, generally speaking any exception
> > that
> > > > > could
> > > > > > > recover safely by calling abortTxn should be wrapped.
> > > > > > >
> > > > > > > Best,
> > > > > > > Boyang
> > > > > > >
> > > > > > > On Thu, Jan 28, 2021 at 5:22 PM Guozhang Wang <
> > wangg...@gmail.com>
> > > > > > wrote:
> > > > > > >
> > > > > > > > Hey Boyang,
> > > > > > > >
> > > > > > > > I think maybe there's something cracking here :) I'm just
> > asking
> > > > for
> > > > > > > > clarifications that as of today, which non-fatal exceptions
> the
> > > > newly
> > > > > > > > introduced CommitFailedException would cover, and it seems to
> > be
> > > > only
> > > > > > 1)
> > > > > > > > unknown pid, 2) invalid pid mapping, and 3) concurrent
> > > > transactions.
> > > > > Is
> > > > > > > > that correct?
> > > > > > > >
> > > > > > > > On Thu, Jan 28, 2021 at 5:06 PM Boyang Chen <
> > > > > > reluctanthero...@gmail.com>
> > > > > > > > wrote:
> > > > > > > >
> > > > > > > > > Hey Guozhang, I think TimeoutException would not be covered
> > here
> > > > as
> > > > > > it
> > > > > > > > > potentially has a risk of hitting an illegal state on the
> > broker
> > > > > side
> > > > > > > > when
> > > > > > > > > the previous commit was actually successful. Users should
> > try to
> > > > > > > increase
> > > > > > > > > their max.block.ms to avoid hitting the timeout as a base
> > > > > > suggestion,
> > > > > > > > > which
> > > > > > > > > is discussed in the KIP.
> > > > > > > > >
> > > > > > > > > As for the umbrella exception, I agree it has values to
> some
> > > > > extent,
> > > > > > > but
> > > > > > > > in
> > > > > > > > > terms of adoption difficulty, additional exception types
> > usually
> > > > > make
> > > > > > > the
> > > > > > > > > error handling more complex than simplified, and we are
> > doing our
> > > > > > best
> > > > > > > to
> > > > > > > > > avoid compatibility issues caused by wrapping previously
> > thrown
> > > > raw
> > > > > > > > > exceptions that invalidates user error handling. And I'm
> > also not
> > > > > in
> > > > > > > > favor
> > > > > > > > > of our exception hierarchy today where all exceptions are
> > > > > subclasses
> > > > > > of
> > > > > > > > > KafkaException, and we already mixed its handling with
> other
> > > > > specific
> > > > > > > > > exception types that could be caught as superclass
> > > > KafkaException.
> > > > > > That
> > > > > > > > > could be addressed in a separate proposal if we see
> > necessary.
> > > > > > > > >
> > > > > > > > > Boyang
> > > > > > > > >
> > > > > > > > > On Thu, Jan 28, 2021 at 3:28 PM Guozhang Wang <
> > > > wangg...@gmail.com>
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > Thanks Jason for the suggestion, that looks good to me
> too.
> > > > > > > > > >
> > > > > > > > > > Regarding the non-fatal exceptions wrapped as
> > CommitFailed, I
> > > > > would
> > > > > > > > like
> > > > > > > > > to
> > > > > > > > > > clarify if we would cover all the following cases: 1)
> > timeout,
> > > > 2)
> > > > > > > > unknown
> > > > > > > > > > pid, 3) invalid pid mapping, 4) concurrent transactions?
> > > > > > > > > >
> > > > > > > > > > BTW I think it still makes sense to use an umbrella
> > exception
> > > > in
> > > > > > case
> > > > > > > > in
> > > > > > > > > > the future we add more non-fatal cases even though today
> we
> > > > only
> > > > > > > have a
> > > > > > > > > > few.
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Guozhang
> > > > > > > > > >
> > > > > > > > > > On Thu, Jan 28, 2021 at 1:08 PM Boyang Chen <
> > > > > > > > reluctanthero...@gmail.com>
> > > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > > Thanks Jason, I agree with the proposed solution here,
> > will
> > > > > > update
> > > > > > > > the
> > > > > > > > > > KIP.
> > > > > > > > > > >
> > > > > > > > > > > On Thu, Jan 28, 2021 at 10:52 AM Jason Gustafson <
> > > > > > > ja...@confluent.io
> > > > > > > > >
> > > > > > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > > Hi Boyang,
> > > > > > > > > > > >
> > > > > > > > > > > > It seems like a reasonable suggestion. I wonder if a
> > flag
> > > > is
> > > > > > > > > sufficient
> > > > > > > > > > > > though. The current `Callback` documentation treats
> > "fatal"
> > > > > > > errors
> > > > > > > > > from
> > > > > > > > > > > the
> > > > > > > > > > > > perspective of the individual message that was sent.
> > > > > > > > > > > >
> > > > > > > > > > > > ```
> > > > > > > > > > > >     *                  Non-Retriable exceptions
> > (fatal, the
> > > > > > > message
> > > > > > > > > > will
> > > > > > > > > > > > never be sent):
> > > > > > > > > > > > ```
> > > > > > > > > > > >
> > > > > > > > > > > > However, we also have fatal errors from the
> > perspective of
> > > > > the
> > > > > > > > > > > transaction
> > > > > > > > > > > > (e.g. when the producer gets fenced). Perhaps that
> > suggests
> > > > > we
> > > > > > > need
> > > > > > > > > > > > something richer than a boolean flag:
> > > > > > > > > > > >
> > > > > > > > > > > > At a high level, I think the following cases are
> > possible:
> > > > > > > > > > > >
> > > > > > > > > > > > - message rejected (e.g. message too large, invalid
> > topic)
> > > > > > > > > > > > - delivery failed after retries/delivery timeout
> (e.g.
> > > > > timeout,
> > > > > > > crc
> > > > > > > > > > > error,
> > > > > > > > > > > > not enough replicas)
> > > > > > > > > > > > - transaction failed (e.g. producer fenced, invalid
> > > > > transaction
> > > > > > > > > state)
> > > > > > > > > > > >
> > > > > > > > > > > > Perhaps instead we can have a type like the
> following:
> > > > > > > > > > > >
> > > > > > > > > > > > class SendFailure {
> > > > > > > > > > > >   FailureType failureType;
> > > > > > > > > > > >   Exception cause;
> > > > > > > > > > > > }
> > > > > > > > > > > >
> > > > > > > > > > > > enum FailureType {
> > > > > > > > > > > >   MESSSAGE_REJECTED, DELIVERY_FAILED,
> > TRANSACTION_FAILED
> > > > > > > > > > > > }
> > > > > > > > > > > >
> > > > > > > > > > > > (Not married to any of these names, just a starting
> > point.)
> > > > > > > > > > > >
> > > > > > > > > > > > Then we add a new `onCompletion` as you've suggested:
> > > > > > > > > > > >
> > > > > > > > > > > > default void onCompletion(RecordMetadata metadata,
> > > > > SendFailure
> > > > > > > > > > failure) {
> > > > > > > > > > > >   this.onCompletion(metadata, failure.cause());
> > > > > > > > > > > > }
> > > > > > > > > > > >
> > > > > > > > > > > > This would give streams and other applications enough
> > > > > > information
> > > > > > > > to
> > > > > > > > > > know
> > > > > > > > > > > > whether the message can be retried and whether the
> > > > > transaction
> > > > > > > can
> > > > > > > > be
> > > > > > > > > > > > aborted.
> > > > > > > > > > > >
> > > > > > > > > > > > What do you think?
> > > > > > > > > > > >
> > > > > > > > > > > > -Jason
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > On Wed, Jan 27, 2021 at 9:51 PM Boyang Chen <
> > > > > > > > > > reluctanthero...@gmail.com>
> > > > > > > > > > > > wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > > Thanks Jason for the thoughts.
> > > > > > > > > > > > >
> > > > > > > > > > > > > On Wed, Jan 27, 2021 at 11:52 AM Jason Gustafson <
> > > > > > > > > ja...@confluent.io
> > > > > > > > > > >
> > > > > > > > > > > > > wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > > Hi Boyang,
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Thanks for the iterations here. I think this is
> > > > something
> > > > > > we
> > > > > > > > > should
> > > > > > > > > > > > have
> > > > > > > > > > > > > > done a long time ago. It sounds like there are
> two
> > API
> > > > > > > changes
> > > > > > > > > > here:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > 1. We are introducing the `CommitFailedException`
> > to
> > > > wrap
> > > > > > > > > abortable
> > > > > > > > > > > > > > errors that are raised from `commitTransaction`.
> > This
> > > > > > sounds
> > > > > > > > fine
> > > > > > > > > > to
> > > > > > > > > > > > me.
> > > > > > > > > > > > > As
> > > > > > > > > > > > > > far as I know, the only case we might need this
> is
> > when
> > > > > we
> > > > > > > add
> > > > > > > > > > > support
> > > > > > > > > > > > to
> > > > > > > > > > > > > > let producers recover from coordinator timeouts.
> > Are
> > > > > there
> > > > > > > any
> > > > > > > > > > > others?
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > I think the purpose here is to ensure non-fatal
> > > > > exceptions
> > > > > > > are
> > > > > > > > > > > unified
> > > > > > > > > > > > > under the same
> > > > > > > > > > > > > exception umbrella, to make the proceeding to abort
> > any
> > > > > > ongoing
> > > > > > > > > > > > transaction
> > > > > > > > > > > > > much easier.
> > > > > > > > > > > > > I don't think `coordinator timeouts` is the only
> > case to
> > > > > > > recover
> > > > > > > > > > here,
> > > > > > > > > > > > > since we have other
> > > > > > > > > > > > > non-fatal exceptions such as UnknownPid.
> > > > > > > > > > > > >
> > > > > > > > > > > > > 2. We are wrapping non-fatal errors raised from
> > `send` as
> > > > > > > > > > > > `KafkaException`.
> > > > > > > > > > > > > > The motivation for this is less clear to me and
> it
> > > > > doesn't
> > > > > > > look
> > > > > > > > > > like
> > > > > > > > > > > > the
> > > > > > > > > > > > > > example from the KIP depends on it. My concern
> > here is
> > > > > > > > > > compatibility.
> > > > > > > > > > > > > > Currently we have the following documentation for
> > the
> > > > > > > > `Callback`
> > > > > > > > > > API:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > ```
> > > > > > > > > > > > > >      *                  Non-Retriable exceptions
> > > > (fatal,
> > > > > > the
> > > > > > > > > > message
> > > > > > > > > > > > will
> > > > > > > > > > > > > > never be sent):
> > > > > > > > > > > > > >      *
> > > > > > > > > > > > > >      *                  InvalidTopicException
> > > > > > > > > > > > > >      *
> > OffsetMetadataTooLargeException
> > > > > > > > > > > > > >      *
> > RecordBatchTooLargeException
> > > > > > > > > > > > > >      *                  RecordTooLargeException
> > > > > > > > > > > > > >      *                  UnknownServerException
> > > > > > > > > > > > > >      *
> UnknownProducerIdException
> > > > > > > > > > > > > >      *
> > InvalidProducerEpochException
> > > > > > > > > > > > > >      *
> > > > > > > > > > > > > >      *                  Retriable exceptions
> > > > (transient,
> > > > > > may
> > > > > > > be
> > > > > > > > > > > covered
> > > > > > > > > > > > > by
> > > > > > > > > > > > > > increasing #.retries):
> > > > > > > > > > > > > >      *
> > > > > > > > > > > > > >      *                  CorruptRecordException
> > > > > > > > > > > > > >      *                  InvalidMetadataException
> > > > > > > > > > > > > >      *
> > > > > > NotEnoughReplicasAfterAppendException
> > > > > > > > > > > > > >      *
> NotEnoughReplicasException
> > > > > > > > > > > > > >      *                  OffsetOutOfRangeException
> > > > > > > > > > > > > >      *                  TimeoutException
> > > > > > > > > > > > > >      *
> > > > UnknownTopicOrPartitionException
> > > > > > > > > > > > > > ```
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > If we wrap all the retriable exceptions
> documented
> > here
> > > > > as
> > > > > > > > > > > > > > `KafkaException`, wouldn't that break any error
> > > > handling
> > > > > > that
> > > > > > > > > users
> > > > > > > > > > > > might
> > > > > > > > > > > > > > already have? it's gonna introduce a
> compatibility
> > > > issue.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > The original intention was to simplify `send`
> > callback
> > > > > > error
> > > > > > > > > > handling
> > > > > > > > > > > > by
> > > > > > > > > > > > > doing exception wrapping, as on Streams level
> > > > > > > > > > > > > we have to prepare an exhausting list of exceptions
> > to
> > > > > catch
> > > > > > as
> > > > > > > > > > fatal,
> > > > > > > > > > > > and
> > > > > > > > > > > > > the same lengthy list to catch as
> > > > > > > > > > > > > non-fatal. It would be much easier if we got
> `hints`
> > from
> > > > > the
> > > > > > > > > > callback.
> > > > > > > > > > > > > However,
> > > > > > > > > > > > > I agree there is a compatibility concern, what
> about
> > > > > > > deprecating
> > > > > > > > > the
> > > > > > > > > > > > > existing:
> > > > > > > > > > > > >
> > > > > > > > > > > > > void onCompletion(RecordMetadata metadata,
> Exception
> > > > > > exception)
> > > > > > > > > > > > >
> > > > > > > > > > > > > and replace it with:
> > > > > > > > > > > > >
> > > > > > > > > > > > > default void onCompletion(RecordMetadata metadata,
> > > > > Exception
> > > > > > > > > > exception,
> > > > > > > > > > > > > boolean isFatal) {
> > > > > > > > > > > > >   this.onCompletion(metadata, exception);
> > > > > > > > > > > > > }
> > > > > > > > > > > > >
> > > > > > > > > > > > > to make sure new users get the benefit of
> > understanding
> > > > the
> > > > > > > > > fatality
> > > > > > > > > > > > based
> > > > > > > > > > > > > on the info presented by the producer?
> > > > > > > > > > > > >
> > > > > > > > > > > > > Thanks,
> > > > > > > > > > > > > > Jason
> > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > On Sat, Jan 23, 2021 at 3:31 AM Hiringuru <
> > > > > > > i...@hiringuru.com>
> > > > > > > > > > > wrote:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Why  we are receiving all emails kindly remove
> us
> > > > from
> > > > > > > > > > > > > > > dev@kafka.apache.org we don't want to receive
> > emails
> > > > > > > > anymore.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Thanks
> > > > > > > > > > > > > > > > On 01/23/2021 4:14 AM Guozhang Wang <
> > > > > > wangg...@gmail.com>
> > > > > > > > > > wrote:
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Thanks Boyang, yes I think I was confused
> > about the
> > > > > > > > different
> > > > > > > > > > > > > handling
> > > > > > > > > > > > > > of
> > > > > > > > > > > > > > > > two abortTxn calls, and now I get it was not
> > > > > > > intentional. I
> > > > > > > > > > > think I
> > > > > > > > > > > > > do
> > > > > > > > > > > > > > > not
> > > > > > > > > > > > > > > > have more concerns.
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > On Fri, Jan 22, 2021 at 1:12 PM Boyang Chen <
> > > > > > > > > > > > > > reluctanthero...@gmail.com>
> > > > > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > Thanks for the clarification Guozhang, I
> got
> > your
> > > > > > point
> > > > > > > > > that
> > > > > > > > > > we
> > > > > > > > > > > > > want
> > > > > > > > > > > > > > to
> > > > > > > > > > > > > > > > > have a consistent handling of fatal
> > exceptions
> > > > > being
> > > > > > > > thrown
> > > > > > > > > > > from
> > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > > abortTxn. I modified the current template
> to
> > move
> > > > > the
> > > > > > > > fatal
> > > > > > > > > > > > > exception
> > > > > > > > > > > > > > > > > try-catch outside of the processing loop to
> > make
> > > > > sure
> > > > > > > we
> > > > > > > > > > could
> > > > > > > > > > > > get
> > > > > > > > > > > > > a
> > > > > > > > > > > > > > > chance
> > > > > > > > > > > > > > > > > to close consumer/producer modules. Let me
> > know
> > > > > what
> > > > > > > you
> > > > > > > > > > think.
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > Best,
> > > > > > > > > > > > > > > > > Boyang
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > On Fri, Jan 22, 2021 at 11:05 AM Boyang
> Chen
> > <
> > > > > > > > > > > > > > > reluctanthero...@gmail.com>
> > > > > > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > My understanding is that abortTransaction
> > would
> > > > > > only
> > > > > > > > > throw
> > > > > > > > > > > when
> > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > > > producer is in fatal state. For
> > CommitFailed,
> > > > the
> > > > > > > > > producer
> > > > > > > > > > > > should
> > > > > > > > > > > > > > > still
> > > > > > > > > > > > > > > > > be
> > > > > > > > > > > > > > > > > > in the abortable error state, so that
> > > > > > > abortTransaction
> > > > > > > > > call
> > > > > > > > > > > > would
> > > > > > > > > > > > > > not
> > > > > > > > > > > > > > > > > throw.
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > > On Fri, Jan 22, 2021 at 11:02 AM Guozhang
> > Wang
> > > > <
> > > > > > > > > > > > > wangg...@gmail.com
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > wrote:
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > > >> Or are you going to maintain some
> internal
> > > > state
> > > > > > > such
> > > > > > > > > > that,
> > > > > > > > > > > > the
> > > > > > > > > > > > > > > > > >> `abortTransaction` in the catch block
> > would
> > > > > never
> > > > > > > > throw
> > > > > > > > > > > again?
> > > > > > > > > > > > > > > > > >>
> > > > > > > > > > > > > > > > > >> On Fri, Jan 22, 2021 at 11:01 AM
> Guozhang
> > > > Wang <
> > > > > > > > > > > > > > wangg...@gmail.com>
> > > > > > > > > > > > > > > > > >> wrote:
> > > > > > > > > > > > > > > > > >>
> > > > > > > > > > > > > > > > > >> > Hi Boyang/Jason,
> > > > > > > > > > > > > > > > > >> >
> > > > > > > > > > > > > > > > > >> > I've also thought about this (i.e.
> using
> > > > > > > > CommitFailed
> > > > > > > > > > for
> > > > > > > > > > > > all
> > > > > > > > > > > > > > > > > >> non-fatal),
> > > > > > > > > > > > > > > > > >> > but what I'm pondering is that, in the
> > catch
> > > > > > > > > > > (CommitFailed)
> > > > > > > > > > > > > > block,
> > > > > > > > > > > > > > > > > what
> > > > > > > > > > > > > > > > > >> > would happen if the
> > > > > > `producer.abortTransaction();`
> > > > > > > > > > throws
> > > > > > > > > > > > > again?
> > > > > > > > > > > > > > > > > should
> > > > > > > > > > > > > > > > > >> > that be captured as a fatal and cause
> > the
> > > > > client
> > > > > > > to
> > > > > > > > > > close
> > > > > > > > > > > > > again.
> > > > > > > > > > > > > > > > > >> >
> > > > > > > > > > > > > > > > > >> > If yes, then naively the pattern would
> > be:
> > > > > > > > > > > > > > > > > >> >
> > > > > > > > > > > > > > > > > >> > ...
> > > > > > > > > > > > > > > > > >> > catch (CommitFailedException e) {
> > > > > > > > > > > > > > > > > >> >         // Transaction commit failed
> > with
> > > > > > > abortable
> > > > > > > > > > error,
> > > > > > > > > > > > > user
> > > > > > > > > > > > > > > could
> > > > > > > > > > > > > > > > > >> reset
> > > > > > > > > > > > > > > > > >> >         // the application state and
> > resume
> > > > > > with a
> > > > > > > > new
> > > > > > > > > > > > > > > transaction.
> > > > > > > > > > > > > > > > > The
> > > > > > > > > > > > > > > > > >> > root
> > > > > > > > > > > > > > > > > >> >         // cause was wrapped in the
> > thrown
> > > > > > > > exception.
> > > > > > > > > > > > > > > > > >> >
> > > > >  resetToLastCommittedPositions(consumer);
> > > > > > > > > > > > > > > > > >> >         try {
> > > > > > > > > > > > > > > > > >> >
>  producer.abortTransaction();
> > > > > > > > > > > > > > > > > >> >         } catch (KafkaException e) {
> > > > > > > > > > > > > > > > > >> >             producer.close();
> > > > > > > > > > > > > > > > > >> >             consumer.close();
> > > > > > > > > > > > > > > > > >> >             throw e;
> > > > > > > > > > > > > > > > > >> >         }
> > > > > > > > > > > > > > > > > >> >     } catch (KafkaException e) {
> > > > > > > > > > > > > > > > > >> >         producer.close();
> > > > > > > > > > > > > > > > > >> >         consumer.close();
> > > > > > > > > > > > > > > > > >> >         throw e;
> > > > > > > > > > > > > > > > > >> >     }
> > > > > > > > > > > > > > > > > >> > ...
> > > > > > > > > > > > > > > > > >> >
> > > > > > > > > > > > > > > > > >> > Guozhang
> > > > > > > > > > > > > > > > > >> >
> > > > > > > > > > > > > > > > > >> > On Fri, Jan 22, 2021 at 10:47 AM
> Boyang
> > > > Chen <
> > > > > > > > > > > > > > > > > >> reluctanthero...@gmail.com>
> > > > > > > > > > > > > > > > > >> > wrote:
> > > > > > > > > > > > > > > > > >> >
> > > > > > > > > > > > > > > > > >> >> Hey Guozhang,
> > > > > > > > > > > > > > > > > >> >>
> > > > > > > > > > > > > > > > > >> >> Jason and I were discussing the new
> API
> > > > > offline
> > > > > > > and
> > > > > > > > > > > decided
> > > > > > > > > > > > > to
> > > > > > > > > > > > > > > take
> > > > > > > > > > > > > > > > > >> >> another
> > > > > > > > > > > > > > > > > >> >> approach. Firstly, the reason not to
> > > > invent a
> > > > > > new
> > > > > > > > API
> > > > > > > > > > > with
> > > > > > > > > > > > > > > returned
> > > > > > > > > > > > > > > > > >> >> boolean
> > > > > > > > > > > > > > > > > >> >> flag is for compatibility
> > consideration,
> > > > > since
> > > > > > > old
> > > > > > > > > EOS
> > > > > > > > > > > code
> > > > > > > > > > > > > > > would not
> > > > > > > > > > > > > > > > > >> know
> > > > > > > > > > > > > > > > > >> >> that a given transaction commit was
> > failed
> > > > > > > > internally
> > > > > > > > > > as
> > > > > > > > > > > > they
> > > > > > > > > > > > > > > don't
> > > > > > > > > > > > > > > > > >> listen
> > > > > > > > > > > > > > > > > >> >> to the output flag. Our proposed
> > > > alternative
> > > > > > > > solution
> > > > > > > > > > is
> > > > > > > > > > > to
> > > > > > > > > > > > > let
> > > > > > > > > > > > > > > > > >> >> *commitTransaction
> > > > > > > > > > > > > > > > > >> >> throw CommitFailedException whenever
> > the
> > > > > commit
> > > > > > > > > failed
> > > > > > > > > > > with
> > > > > > > > > > > > > > > non-fatal
> > > > > > > > > > > > > > > > > >> >> exception*, so that on the caller
> side
> > the
> > > > > > > handling
> > > > > > > > > > logic
> > > > > > > > > > > > > > > becomes:
> > > > > > > > > > > > > > > > > >> >>
> > > > > > > > > > > > > > > > > >> >> try {
> > > > > > > > > > > > > > > > > >> >>         if (shouldCommit) {
> > > > > > > > > > > > > > > > > >> >>
> >  producer.commitTransaction();
> > > > > > > > > > > > > > > > > >> >>         } else {
> > > > > > > > > > > > > > > > > >> >>
> > > > > > > >  resetToLastCommittedPositions(consumer);
> > > > > > > > > > > > > > > > > >> >>
> >  producer.abortTransaction();
> > > > > > > > > > > > > > > > > >> >>         }
> > > > > > > > > > > > > > > > > >> >>     } catch (CommitFailedException
> e) {
> > > > > > > > > > > > > > > > > >> >>         // Transaction commit failed
> > with
> > > > > > > abortable
> > > > > > > > > > > error,
> > > > > > > > > > > > > user
> > > > > > > > > > > > > > > could
> > > > > > > > > > > > > > > > > >> >> reset
> > > > > > > > > > > > > > > > > >> >>         // the application state and
> > resume
> > > > > > with
> > > > > > > a
> > > > > > > > > new
> > > > > > > > > > > > > > > transaction.
> > > > > > > > > > > > > > > > > The
> > > > > > > > > > > > > > > > > >> >> root
> > > > > > > > > > > > > > > > > >> >>         // cause was wrapped in the
> > thrown
> > > > > > > > exception.
> > > > > > > > > > > > > > > > > >> >>
> > > > > >  resetToLastCommittedPositions(consumer);
> > > > > > > > > > > > > > > > > >> >>         producer.abortTransaction();
> > > > > > > > > > > > > > > > > >> >>     } catch (KafkaException e) {
> > > > > > > > > > > > > > > > > >> >>         producer.close();
> > > > > > > > > > > > > > > > > >> >>         consumer.close();
> > > > > > > > > > > > > > > > > >> >>         throw e;
> > > > > > > > > > > > > > > > > >> >>     }
> > > > > > > > > > > > > > > > > >> >>
> > > > > > > > > > > > > > > > > >> >> This approach looks cleaner as all
> > > > exception
> > > > > > > types
> > > > > > > > > > other
> > > > > > > > > > > > than
> > > > > > > > > > > > > > > > > >> CommitFailed
> > > > > > > > > > > > > > > > > >> >> will doom to be fatal, which is very
> > easy
> > > > to
> > > > > > > adopt
> > > > > > > > > for
> > > > > > > > > > > > users.
> > > > > > > > > > > > > > In
> > > > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > > >> >> meantime, we still maintain the
> > commitTxn
> > > > > > > behavior
> > > > > > > > to
> > > > > > > > > > > throw
> > > > > > > > > > > > > > > instead
> > > > > > > > > > > > > > > > > of
> > > > > > > > > > > > > > > > > >> >> silently failing.
> > > > > > > > > > > > > > > > > >> >>
> > > > > > > > > > > > > > > > > >> >> In addition, we decided to drop the
> > > > > > > recommendation
> > > > > > > > to
> > > > > > > > > > > > handle
> > > > > > > > > > > > > > > > > >> >> TimeoutException and leave it to the
> > users
> > > > to
> > > > > > > make
> > > > > > > > > the
> > > > > > > > > > > > call.
> > > > > > > > > > > > > > The
> > > > > > > > > > > > > > > > > >> downside
> > > > > > > > > > > > > > > > > >> >> for blindly calling abortTxn upon
> > timeout
> > > > is
> > > > > > that
> > > > > > > > we
> > > > > > > > > > > could
> > > > > > > > > > > > > > > result in
> > > > > > > > > > > > > > > > > an
> > > > > > > > > > > > > > > > > >> >> illegal state when the commit was
> > already
> > > > > > > > successful
> > > > > > > > > on
> > > > > > > > > > > the
> > > > > > > > > > > > > > > broker
> > > > > > > > > > > > > > > > > >> >> side. Without a good guarantee on the
> > > > > outcome,
> > > > > > > > > > > > > overcomplicating
> > > > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > > >> >> template should not be encouraged
> IMHO.
> > > > > > > > > > > > > > > > > >> >>
> > > > > > > > > > > > > > > > > >> >> Let me know your thoughts on the new
> > > > approach
> > > > > > > here,
> > > > > > > > > > thank
> > > > > > > > > > > > > you!
> > > > > > > > > > > > > > > > > >> >>
> > > > > > > > > > > > > > > > > >> >> Best,
> > > > > > > > > > > > > > > > > >> >> Boyang
> > > > > > > > > > > > > > > > > >> >>
> > > > > > > > > > > > > > > > > >> >> On Tue, Jan 19, 2021 at 11:11 AM
> > Guozhang
> > > > > Wang
> > > > > > <
> > > > > > > > > > > > > > > wangg...@gmail.com>
> > > > > > > > > > > > > > > > > >> >> wrote:
> > > > > > > > > > > > > > > > > >> >>
> > > > > > > > > > > > > > > > > >> >> > Thanks for your clarification on
> > 2)/3),
> > > > > that
> > > > > > > > makes
> > > > > > > > > > > sense.
> > > > > > > > > > > > > > > > > >> >> >
> > > > > > > > > > > > > > > > > >> >> > On Tue, Jan 19, 2021 at 10:16 AM
> > Boyang
> > > > > Chen
> > > > > > <
> > > > > > > > > > > > > > > > > >> >> reluctanthero...@gmail.com>
> > > > > > > > > > > > > > > > > >> >> > wrote:
> > > > > > > > > > > > > > > > > >> >> >
> > > > > > > > > > > > > > > > > >> >> > > Thanks for the input Guozhang,
> > replied
> > > > > > > inline.
> > > > > > > > > > > > > > > > > >> >> > >
> > > > > > > > > > > > > > > > > >> >> > > On Mon, Jan 18, 2021 at 8:57 PM
> > > > Guozhang
> > > > > > > Wang <
> > > > > > > > > > > > > > > > > wangg...@gmail.com>
> > > > > > > > > > > > > > > > > >> >> > wrote:
> > > > > > > > > > > > > > > > > >> >> > >
> > > > > > > > > > > > > > > > > >> >> > > > Hello Boyang,
> > > > > > > > > > > > > > > > > >> >> > > >
> > > > > > > > > > > > > > > > > >> >> > > > Thanks for the updated KIP. I
> > read it
> > > > > > again
> > > > > > > > and
> > > > > > > > > > > have
> > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > > >> following
> > > > > > > > > > > > > > > > > >> >> > > > thoughts:
> > > > > > > > > > > > > > > > > >> >> > > >
> > > > > > > > > > > > > > > > > >> >> > > > 0. I'm a bit concerned that if
> > > > > commitTxn
> > > > > > > does
> > > > > > > > > not
> > > > > > > > > > > > throw
> > > > > > > > > > > > > > any
> > > > > > > > > > > > > > > > > >> >> non-fatal
> > > > > > > > > > > > > > > > > >> >> > > > exception, and instead we rely
> > on the
> > > > > > > > > subsequent
> > > > > > > > > > > > > beginTxn
> > > > > > > > > > > > > > > call
> > > > > > > > > > > > > > > > > to
> > > > > > > > > > > > > > > > > >> >> > throw,
> > > > > > > > > > > > > > > > > >> >> > > it
> > > > > > > > > > > > > > > > > >> >> > > > may violate some callers with a
> > > > pattern
> > > > > > > that
> > > > > > > > > > > relying
> > > > > > > > > > > > on
> > > > > > > > > > > > > > > > > >> commitTxn to
> > > > > > > > > > > > > > > > > >> >> > > > succeed to make some
> non-rollback
> > > > > > > operations.
> > > > > > > > > For
> > > > > > > > > > > > > > example:
> > > > > > > > > > > > > > > > > >> >> > > >
> > > > > > > > > > > > > > > > > >> >> > > > beginTxn()
> > > > > > > > > > > > > > > > > >> >> > > > // do some read-write on my
> > local DB
> > > > > > > > > > > > > > > > > >> >> > > > commitTxn()
> > > > > > > > > > > > > > > > > >> >> > > > // if commitTxn succeeds, then
> > commit
> > > > > the
> > > > > > > DB
> > > > > > > > > > > > > > > > > >> >> > > >
> > > > > > > > > > > > > > > > > >> >> > > > -------------
> > > > > > > > > > > > > > > > > >> >> > > >
> > > > > > > > > > > > > > > > > >> >> > > > The issue is that, committing
> DB
> > is a
> > > > > > > > > > non-rollback
> > > > > > > > > > > > > > > operation,
> > > > > > > > > > > > > > > > > and
> > > > > > > > > > > > > > > > > >> >> users
> > > > > > > > > > > > > > > > > >> >> > > may
> > > > > > > > > > > > > > > > > >> >> > > > just rely on commitTxn to
> return
> > > > > without
> > > > > > > > error
> > > > > > > > > to
> > > > > > > > > > > > make
> > > > > > > > > > > > > > this
> > > > > > > > > > > > > > > > > >> >> > non-rollback
> > > > > > > > > > > > > > > > > >> >> > > > call. Of course we can just
> claim
> > > > this
> > > > > > > > pattern
> > > > > > > > > is
> > > > > > > > > > > not
> > > > > > > > > > > > > > > > > legitimate
> > > > > > > > > > > > > > > > > >> >> and is
> > > > > > > > > > > > > > > > > >> >> > > not
> > > > > > > > > > > > > > > > > >> >> > > > the right way of doing things,
> > but
> > > > many
> > > > > > > users
> > > > > > > > > may
> > > > > > > > > > > > > > naturally
> > > > > > > > > > > > > > > > > adopt
> > > > > > > > > > > > > > > > > >> >> this
> > > > > > > > > > > > > > > > > >> >> > > > pattern.
> > > > > > > > > > > > > > > > > >> >> > > >
> > > > > > > > > > > > > > > > > >> >> > > > So maybe we should still let
> > > > commitTxn
> > > > > > also
> > > > > > > > > throw
> > > > > > > > > > > > > > non-fatal
> > > > > > > > > > > > > > > > > >> >> exceptions,
> > > > > > > > > > > > > > > > > >> >> > > in
> > > > > > > > > > > > > > > > > >> >> > > > which case we would then call
> > > > abortTxn
> > > > > > > again.
> > > > > > > > > > > > > > > > > >> >> > > >
> > > > > > > > > > > > > > > > > >> >> > > > But if we do this, the pattern
> > > > becomes:
> > > > > > > > > > > > > > > > > >> >> > > >
> > > > > > > > > > > > > > > > > >> >> > > > try {
> > > > > > > > > > > > > > > > > >> >> > > >    beginTxn()
> > > > > > > > > > > > > > > > > >> >> > > >    // do something
> > > > > > > > > > > > > > > > > >> >> > > > } catch (Exception) {
> > > > > > > > > > > > > > > > > >> >> > > >    shouldCommit = false;
> > > > > > > > > > > > > > > > > >> >> > > > }
> > > > > > > > > > > > > > > > > >> >> > > >
> > > > > > > > > > > > > > > > > >> >> > > > if (shouldCommit) {
> > > > > > > > > > > > > > > > > >> >> > > >     try {
> > > > > > > > > > > > > > > > > >> >> > > >         commitTxn()
> > > > > > > > > > > > > > > > > >> >> > > >     } catch (...) {        //
> > > > enumerate
> > > > > > all
> > > > > > > > > fatal
> > > > > > > > > > > > > > > exceptions
> > > > > > > > > > > > > > > > > >> >> > > >         shutdown()
> > > > > > > > > > > > > > > > > >> >> > > >     } catch (KafkaException) {
> > > > > > > > > > > > > > > > > >> >> > > >         // non-fatal
> > > > > > > > > > > > > > > > > >> >> > > >         shouldCommit = false;
> > > > > > > > > > > > > > > > > >> >> > > >     }
> > > > > > > > > > > > > > > > > >> >> > > > }
> > > > > > > > > > > > > > > > > >> >> > > >
> > > > > > > > > > > > > > > > > >> >> > > > if (!shouldCommit &&
> running()) {
> > > > > > > > > > > > > > > > > >> >> > > > try {
> > > > > > > > > > > > > > > > > >> >> > > >         abortTxn()
> > > > > > > > > > > > > > > > > >> >> > > >     } catch (KafkaException) {
> > > > > > > > > > > > > > > > > >> >> > > >         // only throw fatal
> > > > > > > > > > > > > > > > > >> >> > > >         shutdown()
> > > > > > > > > > > > > > > > > >> >> > > >     }
> > > > > > > > > > > > > > > > > >> >> > > > }
> > > > > > > > > > > > > > > > > >> >> > > >
> > > > > > > > > > > > > > > > > >> >> > > > ---------------------
> > > > > > > > > > > > > > > > > >> >> > > >
> > > > > > > > > > > > > > > > > >> >> > > > Which is much more complicated.
> > > > > > > > > > > > > > > > > >> >> > > >
> > > > > > > > > > > > > > > > > >> >> > > > Thank makes me think, the
> > alternative
> > > > > we
> > > > > > > have
> > > > > > > > > > > > discussed
> > > > > > > > > > > > > > > offline
> > > > > > > > > > > > > > > > > >> may
> > > > > > > > > > > > > > > > > >> >> be
> > > > > > > > > > > > > > > > > >> >> > > > better: let commitTxn() to
> > return a
> > > > > > boolean
> > > > > > > > > flag.
> > > > > > > > > > > > > > > > > >> >> > > >
> > > > > > > > > > > > > > > > > >> >> > > > * If it returns true, it means
> > the
> > > > > commit
> > > > > > > > > > > succeeded.
> > > > > > > > > > > > > > Users
> > > > > > > > > > > > > > > can
> > > > > > > > > > > > > > > > > >> >> > > comfortably
> > > > > > > > > > > > > > > > > >> >> > > > continue and do any external
> > > > > non-rollback
> > > > > > > > > > > operations
> > > > > > > > > > > > if
> > > > > > > > > > > > > > > they
> > > > > > > > > > > > > > > > > >> like.
> > > > > > > > > > > > > > > > > >> >> > > > * If it returns false, it means
> > the
> > > > > > commit
> > > > > > > > > failed
> > > > > > > > > > > > with
> > > > > > > > > > > > > > > > > non-fatal
> > > > > > > > > > > > > > > > > >> >> error
> > > > > > > > > > > > > > > > > >> >> > > *AND
> > > > > > > > > > > > > > > > > >> >> > > > the txn has been aborted*.
> Users
> > do
> > > > not
> > > > > > > need
> > > > > > > > to
> > > > > > > > > > > call
> > > > > > > > > > > > > > > abortTxn
> > > > > > > > > > > > > > > > > >> again.
> > > > > > > > > > > > > > > > > >> >> > > > * If it throws, then it means
> > fatal
> > > > > > errors.
> > > > > > > > > Users
> > > > > > > > > > > > > should
> > > > > > > > > > > > > > > shut
> > > > > > > > > > > > > > > > > >> down
> > > > > > > > > > > > > > > > > >> >> the
> > > > > > > > > > > > > > > > > >> >> > > > client.
> > > > > > > > > > > > > > > > > >> >> > > >
> > > > > > > > > > > > > > > > > >> >> > > > In this case, the pattern
> > becomes:
> > > > > > > > > > > > > > > > > >> >> > > >
> > > > > > > > > > > > > > > > > >> >> > > > try {
> > > > > > > > > > > > > > > > > >> >> > > >    beginTxn()
> > > > > > > > > > > > > > > > > >> >> > > >    // do something
> > > > > > > > > > > > > > > > > >> >> > > > } catch (Exception) {
> > > > > > > > > > > > > > > > > >> >> > > >    shouldCommit = false;
> > > > > > > > > > > > > > > > > >> >> > > > }
> > > > > > > > > > > > > > > > > >> >> > > >
> > > > > > > > > > > > > > > > > >> >> > > > try {
> > > > > > > > > > > > > > > > > >> >> > > >     if (shouldCommit) {
> > > > > > > > > > > > > > > > > >> >> > > >         commitSucceeded =
> > commitTxn()
> > > > > > > > > > > > > > > > > >> >> > > >     } else {
> > > > > > > > > > > > > > > > > >> >> > > >         // reset offsets,
> > rollback
> > > > > > > > operations,
> > > > > > > > > > etc
> > > > > > > > > > > > > > > > > >> >> > > >         abortTxn()
> > > > > > > > > > > > > > > > > >> >> > > >     }
> > > > > > > > > > > > > > > > > >> >> > > > } catch (KafkaException) {
> > > > > > > > > > > > > > > > > >> >> > > >     // only throw fatal
> > > > > > > > > > > > > > > > > >> >> > > >     shutdown()
> > > > > > > > > > > > > > > > > >> >> > > > }
> > > > > > > > > > > > > > > > > >> >> > > >
> > > > > > > > > > > > > > > > > >> >> > > > if (commitSucceeded)
> > > > > > > > > > > > > > > > > >> >> > > >    // do other non-rollbackable
> > > > things
> > > > > > > > > > > > > > > > > >> >> > > > else
> > > > > > > > > > > > > > > > > >> >> > > >    // reset offsets, rollback
> > > > > operations,
> > > > > > > etc
> > > > > > > > > > > > > > > > > >> >> > > >
> > > > > > > > > > > > > > > > > >> >> > > > -------------------------
> > > > > > > > > > > > > > > > > >> >> > > >
> > > > > > > > > > > > > > > > > >> >> > > > Of course, if we want to have
> > better
> > > > > > > > visibility
> > > > > > > > > > > into
> > > > > > > > > > > > > what
> > > > > > > > > > > > > > > > > caused
> > > > > > > > > > > > > > > > > >> the
> > > > > > > > > > > > > > > > > >> >> > > commit
> > > > > > > > > > > > > > > > > >> >> > > > to fail and txn to abort. We
> can
> > let
> > > > > the
> > > > > > > > return
> > > > > > > > > > > type
> > > > > > > > > > > > be
> > > > > > > > > > > > > > an
> > > > > > > > > > > > > > > enum
> > > > > > > > > > > > > > > > > >> >> instead
> > > > > > > > > > > > > > > > > >> >> > > of
> > > > > > > > > > > > > > > > > >> >> > > > a flag. But my main idea is to
> > still
> > > > > let
> > > > > > > the
> > > > > > > > > > > > commitTxn
> > > > > > > > > > > > > be
> > > > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > > >> final
> > > > > > > > > > > > > > > > > >> >> > point
> > > > > > > > > > > > > > > > > >> >> > > > users can tell whether this txn
> > > > > succeeded
> > > > > > > or
> > > > > > > > > not,
> > > > > > > > > > > > > instead
> > > > > > > > > > > > > > > of
> > > > > > > > > > > > > > > > > >> >> relying on
> > > > > > > > > > > > > > > > > >> >> > > the
> > > > > > > > > > > > > > > > > >> >> > > > next beginTxn() call.
> > > > > > > > > > > > > > > > > >> >> > > >
> > > > > > > > > > > > > > > > > >> >> > > > I agree that adding a boolean
> > flag is
> > > > > > > indeed
> > > > > > > > > > useful
> > > > > > > > > > > > in
> > > > > > > > > > > > > > this
> > > > > > > > > > > > > > > > > case.
> > > > > > > > > > > > > > > > > >> >> Will
> > > > > > > > > > > > > > > > > >> >> > > update the KIP.
> > > > > > > > > > > > > > > > > >> >> > >
> > > > > > > > > > > > > > > > > >> >> > > 1. Re: "while maintaining the
> > behavior
> > > > to
> > > > > > > throw
> > > > > > > > > > fatal
> > > > > > > > > > > > > > > exception
> > > > > > > > > > > > > > > > > in
> > > > > > > > > > > > > > > > > >> >> raw"
> > > > > > > > > > > > > > > > > >> >> > not
> > > > > > > > > > > > > > > > > >> >> > > > sure what do you mean by
> "throw"
> > > > here.
> > > > > > Are
> > > > > > > > you
> > > > > > > > > > > > > proposing
> > > > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > > >> >> callback
> > > > > > > > > > > > > > > > > >> >> > > would
> > > > > > > > > > > > > > > > > >> >> > > > *pass* (not throw) in any fatal
> > > > > > exceptions
> > > > > > > > when
> > > > > > > > > > > > > triggered
> > > > > > > > > > > > > > > > > without
> > > > > > > > > > > > > > > > > >> >> > > wrapping?
> > > > > > > > > > > > > > > > > >> >> > > >
> > > > > > > > > > > > > > > > > >> >> > > > Yes, I want to say *pass*, the
> > > > benefit
> > > > > is
> > > > > > > to
> > > > > > > > > make
> > > > > > > > > > > the
> > > > > > > > > > > > > end
> > > > > > > > > > > > > > > > > user's
> > > > > > > > > > > > > > > > > >> >> > > expectation consistent
> > > > > > > > > > > > > > > > > >> >> > > regarding exception handling.
> > > > > > > > > > > > > > > > > >> >> > >
> > > > > > > > > > > > > > > > > >> >> > >
> > > > > > > > > > > > > > > > > >> >> > > > 2. I'm not sure I understand
> the
> > > > claim
> > > > > > > > > regarding
> > > > > > > > > > > the
> > > > > > > > > > > > > > > callback
> > > > > > > > > > > > > > > > > >> that
> > > > > > > > > > > > > > > > > >> >> "In
> > > > > > > > > > > > > > > > > >> >> > > EOS
> > > > > > > > > > > > > > > > > >> >> > > > setup, it is not required to
> > handle
> > > > the
> > > > > > > > > > exception".
> > > > > > > > > > > > Are
> > > > > > > > > > > > > > you
> > > > > > > > > > > > > > > > > >> >> proposing
> > > > > > > > > > > > > > > > > >> >> > > that,
> > > > > > > > > > > > > > > > > >> >> > > > e.g. in Streams, we do not try
> to
> > > > > handle
> > > > > > > any
> > > > > > > > > > > > exceptions
> > > > > > > > > > > > > > if
> > > > > > > > > > > > > > > EOS
> > > > > > > > > > > > > > > > > is
> > > > > > > > > > > > > > > > > >> >> > enabled
> > > > > > > > > > > > > > > > > >> >> > > > in the callback anymore, but
> > just let
> > > > > > > > > commitTxn()
> > > > > > > > > > > > > itself
> > > > > > > > > > > > > > to
> > > > > > > > > > > > > > > > > fail
> > > > > > > > > > > > > > > > > >> to
> > > > > > > > > > > > > > > > > >> >> be
> > > > > > > > > > > > > > > > > >> >> > > > notified about the problem?
> > > > Personally
> > > > > I
> > > > > > > > think
> > > > > > > > > in
> > > > > > > > > > > > > Streams
> > > > > > > > > > > > > > > we
> > > > > > > > > > > > > > > > > >> should
> > > > > > > > > > > > > > > > > >> >> > just
> > > > > > > > > > > > > > > > > >> >> > > > make the handling logic of the
> > > > callback
> > > > > > to
> > > > > > > be
> > > > > > > > > > > > > consistent
> > > > > > > > > > > > > > > > > >> regardless
> > > > > > > > > > > > > > > > > >> >> of
> > > > > > > > > > > > > > > > > >> >> > > the
> > > > > > > > > > > > > > > > > >> >> > > > EOS settings (today we have
> > different
> > > > > > logic
> > > > > > > > > > > depending
> > > > > > > > > > > > > on
> > > > > > > > > > > > > > > this
> > > > > > > > > > > > > > > > > >> logic,
> > > > > > > > > > > > > > > > > >> >> > > which
> > > > > > > > > > > > > > > > > >> >> > > > I think could be unified as
> > well).
> > > > > > > > > > > > > > > > > >> >> > > >
> > > > > > > > > > > > > > > > > >> >> > > > My idea originates from the
> > claim on
> > > > > send
> > > > > > > > API:
> > > > > > > > > > > > > > > > > >> >> > > "When used as part of a
> > transaction, it
> > > > > is
> > > > > > > not
> > > > > > > > > > > > necessary
> > > > > > > > > > > > > to
> > > > > > > > > > > > > > > > > define
> > > > > > > > > > > > > > > > > >> a
> > > > > > > > > > > > > > > > > >> >> > > callback or check the result of
> the
> > > > > future
> > > > > > > in
> > > > > > > > > > order
> > > > > > > > > > > to
> > > > > > > > > > > > > > > detect
> > > > > > > > > > > > > > > > > >> errors
> > > > > > > > > > > > > > > > > >> >> > from
> > > > > > > > > > > > > > > > > >> >> > > <code>send</code>. "
> > > > > > > > > > > > > > > > > >> >> > > My understanding is that for EOS,
> > the
> > > > > > > exception
> > > > > > > > > > will
> > > > > > > > > > > be
> > > > > > > > > > > > > > > detected
> > > > > > > > > > > > > > > > > by
> > > > > > > > > > > > > > > > > >> >> > calling
> > > > > > > > > > > > > > > > > >> >> > > transactional APIs either way, so
> > it
> > > > is a
> > > > > > > > > duplicate
> > > > > > > > > > > > > > handling
> > > > > > > > > > > > > > > to
> > > > > > > > > > > > > > > > > >> track
> > > > > > > > > > > > > > > > > >> >> > > all the sendExceptions in
> > > > > RecordCollector.
> > > > > > > > > > However, I
> > > > > > > > > > > > > > looked
> > > > > > > > > > > > > > > up
> > > > > > > > > > > > > > > > > >> >> > > sendException is being used today
> > as
> > > > > > follow:
> > > > > > > > > > > > > > > > > >> >> > >
> > > > > > > > > > > > > > > > > >> >> > > 1. Pass to
> > "ProductionExceptionHandler"
> > > > > for
> > > > > > > any
> > > > > > > > > > > default
> > > > > > > > > > > > > or
> > > > > > > > > > > > > > > > > >> customized
> > > > > > > > > > > > > > > > > >> >> > > exception handler to handle
> > > > > > > > > > > > > > > > > >> >> > > 2. Stop collecting offset info or
> > new
> > > > > > > > exceptions
> > > > > > > > > > > > > > > > > >> >> > > 3. Check and rethrow exceptions
> in
> > > > > close(),
> > > > > > > > > flush()
> > > > > > > > > > > or
> > > > > > > > > > > > > new
> > > > > > > > > > > > > > > send()
> > > > > > > > > > > > > > > > > >> >> calls
> > > > > > > > > > > > > > > > > >> >> > >
> > > > > > > > > > > > > > > > > >> >> > > To my understanding, we should
> > still
> > > > > honor
> > > > > > > the
> > > > > > > > > > > > commitment
> > > > > > > > > > > > > > to
> > > > > > > > > > > > > > > #1
> > > > > > > > > > > > > > > > > for
> > > > > > > > > > > > > > > > > >> >> any
> > > > > > > > > > > > > > > > > >> >> > > user customized implementation.
> > The #2
> > > > > does
> > > > > > > not
> > > > > > > > > > > really
> > > > > > > > > > > > > > > affect our
> > > > > > > > > > > > > > > > > >> >> > decision
> > > > > > > > > > > > > > > > > >> >> > > upon EOS. The #3 here is still
> > valuable
> > > > > as
> > > > > > it
> > > > > > > > > could
> > > > > > > > > > > > help
> > > > > > > > > > > > > us
> > > > > > > > > > > > > > > fail
> > > > > > > > > > > > > > > > > >> fast
> > > > > > > > > > > > > > > > > >> >> in
> > > > > > > > > > > > > > > > > >> >> > > new send() instead of waiting to
> > later
> > > > > > stage
> > > > > > > of
> > > > > > > > > > > > > processing.
> > > > > > > > > > > > > > > In
> > > > > > > > > > > > > > > > > that
> > > > > > > > > > > > > > > > > >> >> > sense,
> > > > > > > > > > > > > > > > > >> >> > > I agree we should continue to
> > record
> > > > send
> > > > > > > > > > exceptions
> > > > > > > > > > > > even
> > > > > > > > > > > > > > > under
> > > > > > > > > > > > > > > > > EOS
> > > > > > > > > > > > > > > > > >> >> case
> > > > > > > > > > > > > > > > > >> >> > to
> > > > > > > > > > > > > > > > > >> >> > > ensure the strength of stream
> side
> > > > > Producer
> > > > > > > > > logic.
> > > > > > > > > > On
> > > > > > > > > > > > the
> > > > > > > > > > > > > > > safer
> > > > > > > > > > > > > > > > > >> side,
> > > > > > > > > > > > > > > > > >> >> we
> > > > > > > > > > > > > > > > > >> >> > no
> > > > > > > > > > > > > > > > > >> >> > > longer need to wrap certain fatal
> > > > > > exceptions
> > > > > > > > like
> > > > > > > > > > > > > > > ProducerFenced
> > > > > > > > > > > > > > > > > as
> > > > > > > > > > > > > > > > > >> >> > > TaskMigrated, since they should
> not
> > > > crash
> > > > > > the
> > > > > > > > > > stream
> > > > > > > > > > > > > thread
> > > > > > > > > > > > > > > if
> > > > > > > > > > > > > > > > > >> thrown
> > > > > > > > > > > > > > > > > >> >> in
> > > > > > > > > > > > > > > > > >> >> > > raw format, once we adopt the new
> > > > > > processing
> > > > > > > > > model
> > > > > > > > > > in
> > > > > > > > > > > > the
> > > > > > > > > > > > > > > send
> > > > > > > > > > > > > > > > > >> phase.
> > > > > > > > > > > > > > > > > >> >> > >
> > > > > > > > > > > > > > > > > >> >> > >
> > > > > > > > > > > > > > > > > >> >> > > >
> > > > > > > > > > > > > > > > > >> >> > > > Guozhang
> > > > > > > > > > > > > > > > > >> >> > > >
> > > > > > > > > > > > > > > > > >> >> > > >
> > > > > > > > > > > > > > > > > >> >> > > >
> > > > > > > > > > > > > > > > > >> >> > > > On Thu, Dec 17, 2020 at 8:42 PM
> > > > Boyang
> > > > > > > Chen <
> > > > > > > > > > > > > > > > > >> >> > reluctanthero...@gmail.com>
> > > > > > > > > > > > > > > > > >> >> > > > wrote:
> > > > > > > > > > > > > > > > > >> >> > > >
> > > > > > > > > > > > > > > > > >> >> > > > > Thanks for everyone's
> feedback
> > so
> > > > > far.
> > > > > > I
> > > > > > > > have
> > > > > > > > > > > > > polished
> > > > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > > KIP
> > > > > > > > > > > > > > > > > >> >> after
> > > > > > > > > > > > > > > > > >> >> > > > > offline discussion with some
> > folks
> > > > > > > working
> > > > > > > > on
> > > > > > > > > > EOS
> > > > > > > > > > > > to
> > > > > > > > > > > > > > > make the
> > > > > > > > > > > > > > > > > >> >> > exception
> > > > > > > > > > > > > > > > > >> >> > > > > handling more lightweight.
> The
> > > > > > essential
> > > > > > > > > change
> > > > > > > > > > > is
> > > > > > > > > > > > > that
> > > > > > > > > > > > > > > we
> > > > > > > > > > > > > > > > > are
> > > > > > > > > > > > > > > > > >> not
> > > > > > > > > > > > > > > > > >> >> > > > > inventing a new intermediate
> > > > > exception
> > > > > > > > type,
> > > > > > > > > > but
> > > > > > > > > > > > > > instead
> > > > > > > > > > > > > > > > > >> >> separating a
> > > > > > > > > > > > > > > > > >> >> > > > full
> > > > > > > > > > > > > > > > > >> >> > > > > transaction into two phases:
> > > > > > > > > > > > > > > > > >> >> > > > >
> > > > > > > > > > > > > > > > > >> >> > > > > 1. The data transmission
> phase
> > > > > > > > > > > > > > > > > >> >> > > > > 2. The commit phase
> > > > > > > > > > > > > > > > > >> >> > > > >
> > > > > > > > > > > > > > > > > >> >> > > > > This way for any exception
> > thrown
> > > > > from
> > > > > > > > phase
> > > > > > > > > 1,
> > > > > > > > > > > > will
> > > > > > > > > > > > > be
> > > > > > > > > > > > > > > an
> > > > > > > > > > > > > > > > > >> >> indicator
> > > > > > > > > > > > > > > > > >> >> > in
> > > > > > > > > > > > > > > > > >> >> > > > > phase 2 whether we should do
> > commit
> > > > > or
> > > > > > > > abort,
> > > > > > > > > > and
> > > > > > > > > > > > > from
> > > > > > > > > > > > > > > now on
> > > > > > > > > > > > > > > > > >> >> > > > > `commitTransaction` should
> only
> > > > throw
> > > > > > > fatal
> > > > > > > > > > > > > exceptions,
> > > > > > > > > > > > > > > > > >> similar to
> > > > > > > > > > > > > > > > > >> >> > > > > `abortTransaction`, so that
> any
> > > > > > > > > KafkaException
> > > > > > > > > > > > caught
> > > > > > > > > > > > > > in
> > > > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > > >> >> commit
> > > > > > > > > > > > > > > > > >> >> > > phase
> > > > > > > > > > > > > > > > > >> >> > > > > will be definitely fatal to
> > crash
> > > > the
> > > > > > > app.
> > > > > > > > > For
> > > > > > > > > > > more
> > > > > > > > > > > > > > > advanced
> > > > > > > > > > > > > > > > > >> users
> > > > > > > > > > > > > > > > > >> >> > such
> > > > > > > > > > > > > > > > > >> >> > > > as
> > > > > > > > > > > > > > > > > >> >> > > > > Streams, we have the ability
> to
> > > > > further
> > > > > > > > wrap
> > > > > > > > > > > > selected
> > > > > > > > > > > > > > > types
> > > > > > > > > > > > > > > > > of
> > > > > > > > > > > > > > > > > >> >> fatal
> > > > > > > > > > > > > > > > > >> >> > > > > exceptions to trigger task
> > > > migration
> > > > > if
> > > > > > > > > > > necessary.
> > > > > > > > > > > > > > > > > >> >> > > > >
> > > > > > > > > > > > > > > > > >> >> > > > > More details in the KIP, feel
> > free
> > > > to
> > > > > > > take
> > > > > > > > > > > another
> > > > > > > > > > > > > > look,
> > > > > > > > > > > > > > > > > >> thanks!
> > > > > > > > > > > > > > > > > >> >> > > > >
> > > > > > > > > > > > > > > > > >> >> > > > > On Thu, Dec 17, 2020 at 8:36
> PM
> > > > > Boyang
> > > > > > > > Chen <
> > > > > > > > > > > > > > > > > >> >> > > reluctanthero...@gmail.com>
> > > > > > > > > > > > > > > > > >> >> > > > > wrote:
> > > > > > > > > > > > > > > > > >> >> > > > >
> > > > > > > > > > > > > > > > > >> >> > > > > > Thanks Bruno for the
> > feedback.
> > > > > > > > > > > > > > > > > >> >> > > > > >
> > > > > > > > > > > > > > > > > >> >> > > > > > On Mon, Dec 7, 2020 at 5:26
> > AM
> > > > > Bruno
> > > > > > > > > Cadonna
> > > > > > > > > > <
> > > > > > > > > > > > > > > > > >> >> br...@confluent.io>
> > > > > > > > > > > > > > > > > >> >> > > > wrote:
> > > > > > > > > > > > > > > > > >> >> > > > > >
> > > > > > > > > > > > > > > > > >> >> > > > > >> Thanks Boyang for the KIP!
> > > > > > > > > > > > > > > > > >> >> > > > > >>
> > > > > > > > > > > > > > > > > >> >> > > > > >> Like Matthias, I do also
> not
> > > > know
> > > > > > the
> > > > > > > > > > producer
> > > > > > > > > > > > > > > internal
> > > > > > > > > > > > > > > > > well
> > > > > > > > > > > > > > > > > >> >> > enough
> > > > > > > > > > > > > > > > > >> >> > > to
> > > > > > > > > > > > > > > > > >> >> > > > > >> comment on the
> > categorization.
> > > > > > > However,
> > > > > > > > I
> > > > > > > > > > > think
> > > > > > > > > > > > > > > having a
> > > > > > > > > > > > > > > > > >> super
> > > > > > > > > > > > > > > > > >> >> > > > exception
> > > > > > > > > > > > > > > > > >> >> > > > > >> (e.g. RetriableException)
> > that
> > > > > > > encodes
> > > > > > > > if
> > > > > > > > > > an
> > > > > > > > > > > > > > > exception is
> > > > > > > > > > > > > > > > > >> >> fatal
> > > > > > > > > > > > > > > > > >> >> > or
> > > > > > > > > > > > > > > > > >> >> > > > not
> > > > > > > > > > > > > > > > > >> >> > > > > >> is cleaner, better
> > > > understandable
> > > > > > and
> > > > > > > > less
> > > > > > > > > > > > > > > error-prone,
> > > > > > > > > > > > > > > > > >> because
> > > > > > > > > > > > > > > > > >> >> > > > ideally
> > > > > > > > > > > > > > > > > >> >> > > > > >> when you add a new
> non-fatal
> > > > > > exception
> > > > > > > > in
> > > > > > > > > > > future
> > > > > > > > > > > > > you
> > > > > > > > > > > > > > > just
> > > > > > > > > > > > > > > > > >> need
> > > > > > > > > > > > > > > > > >> >> to
> > > > > > > > > > > > > > > > > >> >> > > > think
> > > > > > > > > > > > > > > > > >> >> > > > > >> about letting it inherit
> > from
> > > > the
> > > > > > > super
> > > > > > > > > > > > exception
> > > > > > > > > > > > > > and
> > > > > > > > > > > > > > > all
> > > > > > > > > > > > > > > > > >> the
> > > > > > > > > > > > > > > > > >> >> rest
> > > > > > > > > > > > > > > > > >> >> > > of
> > > > > > > > > > > > > > > > > >> >> > > > > >> the code will just behave
> > > > > correctly
> > > > > > > > > without
> > > > > > > > > > > the
> > > > > > > > > > > > > need
> > > > > > > > > > > > > > > to
> > > > > > > > > > > > > > > > > wrap
> > > > > > > > > > > > > > > > > >> >> the
> > > > > > > > > > > > > > > > > >> >> > new
> > > > > > > > > > > > > > > > > >> >> > > > > >> exception into another
> > exception
> > > > > > each
> > > > > > > > time
> > > > > > > > > > it
> > > > > > > > > > > is
> > > > > > > > > > > > > > > thrown
> > > > > > > > > > > > > > > > > >> (maybe
> > > > > > > > > > > > > > > > > >> >> it
> > > > > > > > > > > > > > > > > >> >> > is
> > > > > > > > > > > > > > > > > >> >> > > > > >> thrown at different
> > location in
> > > > > the
> > > > > > > > code).
> > > > > > > > > > > > > > > > > >> >> > > > > >>
> > > > > > > > > > > > > > > > > >> >> > > > > >> As far as I understand the
> > > > > following
> > > > > > > > > > statement
> > > > > > > > > > > > > from
> > > > > > > > > > > > > > > your
> > > > > > > > > > > > > > > > > >> >> previous
> > > > > > > > > > > > > > > > > >> >> > > > e-mail
> > > > > > > > > > > > > > > > > >> >> > > > > >> is the reason that
> currently
> > > > such
> > > > > a
> > > > > > > > super
> > > > > > > > > > > > > exception
> > > > > > > > > > > > > > > is not
> > > > > > > > > > > > > > > > > >> >> > possible:
> > > > > > > > > > > > > > > > > >> >> > > > > >>
> > > > > > > > > > > > > > > > > >> >> > > > > >> "Right now we have
> > > > > > RetriableException
> > > > > > > > > type,
> > > > > > > > > > if
> > > > > > > > > > > > we
> > > > > > > > > > > > > > are
> > > > > > > > > > > > > > > > > going
> > > > > > > > > > > > > > > > > >> to
> > > > > > > > > > > > > > > > > >> >> > add a
> > > > > > > > > > > > > > > > > >> >> > > > > >>
> `ProducerRetriableException`
> > > > type,
> > > > > > we
> > > > > > > > have
> > > > > > > > > > to
> > > > > > > > > > > > put
> > > > > > > > > > > > > > > this new
> > > > > > > > > > > > > > > > > >> >> > interface
> > > > > > > > > > > > > > > > > >> >> > > > as
> > > > > > > > > > > > > > > > > >> >> > > > > >> the parent of the
> > > > > > RetriableException,
> > > > > > > > > > because
> > > > > > > > > > > > not
> > > > > > > > > > > > > > all
> > > > > > > > > > > > > > > > > thrown
> > > > > > > > > > > > > > > > > >> >> > > non-fatal
> > > > > > > > > > > > > > > > > >> >> > > > > >> exceptions are `retriable`
> > in
> > > > > > general
> > > > > > > > for
> > > > > > > > > > > > > producer"
> > > > > > > > > > > > > > > > > >> >> > > > > >>
> > > > > > > > > > > > > > > > > >> >> > > > > >>
> > > > > > > > > > > > > > > > > >> >> > > > > >> In the list of exceptions
> in
> > > > your
> > > > > > > KIP, I
> > > > > > > > > > found
> > > > > > > > > > > > > > > non-fatal
> > > > > > > > > > > > > > > > > >> >> > exceptions
> > > > > > > > > > > > > > > > > >> >> > > > that
> > > > > > > > > > > > > > > > > >> >> > > > > >> do not inherit from
> > > > > > > RetriableException.
> > > > > > > > I
> > > > > > > > > > > guess
> > > > > > > > > > > > > > those
> > > > > > > > > > > > > > > are
> > > > > > > > > > > > > > > > > >> the
> > > > > > > > > > > > > > > > > >> >> ones
> > > > > > > > > > > > > > > > > >> >> > > you
> > > > > > > > > > > > > > > > > >> >> > > > > >> are referring to in your
> > > > > statement:
> > > > > > > > > > > > > > > > > >> >> > > > > >>
> > > > > > > > > > > > > > > > > >> >> > > > > >>
> > InvalidProducerEpochException
> > > > > > > > > > > > > > > > > >> >> > > > > >> InvalidPidMappingException
> > > > > > > > > > > > > > > > > >> >> > > > > >>
> TransactionAbortedException
> > > > > > > > > > > > > > > > > >> >> > > > > >>
> > > > > > > > > > > > > > > > > >> >> > > > > >> All of those exceptions
> are
> > > > > > non-fatal
> > > > > > > > and
> > > > > > > > > do
> > > > > > > > > > > not
> > > > > > > > > > > > > > > inherit
> > > > > > > > > > > > > > > > > >> from
> > > > > > > > > > > > > > > > > >> >> > > > > >> RetriableException. Is
> > there a
> > > > > > reason
> > > > > > > > for
> > > > > > > > > > > that?
> > > > > > > > > > > > If
> > > > > > > > > > > > > > > they
> > > > > > > > > > > > > > > > > >> >> depended
> > > > > > > > > > > > > > > > > >> >> > > from
> > > > > > > > > > > > > > > > > >> >> > > > > >> RetriableException we
> would
> > be a
> > > > > bit
> > > > > > > > > closer
> > > > > > > > > > > to a
> > > > > > > > > > > > > > super
> > > > > > > > > > > > > > > > > >> >> exception I
> > > > > > > > > > > > > > > > > >> >> > > > > >> mention above.
> > > > > > > > > > > > > > > > > >> >> > > > > >>
> > > > > > > > > > > > > > > > > >> >> > > > > >> The reason is that sender
> > may
> > > > > catch
> > > > > > > > those
> > > > > > > > > > > > > exceptions
> > > > > > > > > > > > > > > in
> > > > > > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > > >> >> > > > > > ProduceResponse, and it
> > currently
> > > > > > does
> > > > > > > > > > infinite
> > > > > > > > > > > > > > > > > >> >> > > > > > retries on
> > RetriableException. To
> > > > > > make
> > > > > > > > sure
> > > > > > > > > > we
> > > > > > > > > > > > > could
> > > > > > > > > > > > > > > > > trigger
> > > > > > > > > > > > > > > > > >> the
> > > > > > > > > > > > > > > > > >> >> > > > > > abortTransaction() by
> > catching
> > > > > > > non-fatal
> > > > > > > > > > thrown
> > > > > > > > > > > > > > > > > >> >> > > > > > exceptions and reinitialize
> > the
> > > > txn
> > > > > > > > state,
> > > > > > > > > we
> > > > > > > > > > > > chose
> > > > > > > > > > > > > > > not to
> > > > > > > > > > > > > > > > > >> let
> > > > > > > > > > > > > > > > > >> >> > those
> > > > > > > > > > > > > > > > > >> >> > > > > > exceptions inherit
> > > > > RetriableException
> > > > > > > so
> > > > > > > > > that
> > > > > > > > > > > > > > > > > >> >> > > > > > they won't cause infinite
> > retry
> > > > on
> > > > > > > > sender.
> > > > > > > > > > > > > > > > > >> >> > > > > >
> > > > > > > > > > > > > > > > > >> >> > > > > >
> > > > > > > > > > > > > > > > > >> >> > > > > >> With
> > OutOfOrderSequenceException
> > > > > and
> > > > > > > > > > > > > > > > > >> >> UnknownProducerIdException, I
> > > > > > > > > > > > > > > > > >> >> > > > think
> > > > > > > > > > > > > > > > > >> >> > > > > >> to understand that their
> > > > fatality
> > > > > > > > depends
> > > > > > > > > on
> > > > > > > > > > > the
> > > > > > > > > > > > > > type
> > > > > > > > > > > > > > > > > (i.e.
> > > > > > > > > > > > > > > > > >> >> > > > > >> configuration) of the
> > producer.
> > > > > That
> > > > > > > > makes
> > > > > > > > > > it
> > > > > > > > > > > > > > > difficult to
> > > > > > > > > > > > > > > > > >> >> have a
> > > > > > > > > > > > > > > > > >> >> > > > super
> > > > > > > > > > > > > > > > > >> >> > > > > >> exception that encodes the
> > > > > > > retriability
> > > > > > > > as
> > > > > > > > > > > > > mentioned
> > > > > > > > > > > > > > > > > above.
> > > > > > > > > > > > > > > > > >> >> Would
> > > > > > > > > > > > > > > > > >> >> > it
> > > > > > > > > > > > > > > > > >> >> > > > be
> > > > > > > > > > > > > > > > > >> >> > > > > >> possible to introduce
> > exceptions
> > > > > > that
> > > > > > > > > > inherit
> > > > > > > > > > > > from
> > > > > > > > > > > > > > > > > >> >> > > RetriableException
> > > > > > > > > > > > > > > > > >> >> > > > > >> and that are thrown when
> > those
> > > > > > > > exceptions
> > > > > > > > > > are
> > > > > > > > > > > > > caught
> > > > > > > > > > > > > > > from
> > > > > > > > > > > > > > > > > >> the
> > > > > > > > > > > > > > > > > >> >> > > brokers
> > > > > > > > > > > > > > > > > >> >> > > > > >> and the type of the
> > producer is
> > > > > such
> > > > > > > > that
> > > > > > > > > > the
> > > > > > > > > > > > > > > exceptions
> > > > > > > > > > > > > > > > > are
> > > > > > > > > > > > > > > > > >> >> > > > retriable?
> > > > > > > > > > > > > > > > > >> >> > > > > >>
> > > > > > > > > > > > > > > > > >> >> > > > > >> Yea, I think in general
> the
> > > > > > exception
> > > > > > > > type
> > > > > > > > > > > > mixing
> > > > > > > > > > > > > is
> > > > > > > > > > > > > > > > > >> difficult
> > > > > > > > > > > > > > > > > >> >> to
> > > > > > > > > > > > > > > > > >> >> > > get
> > > > > > > > > > > > > > > > > >> >> > > > > > them all right. I have
> > already
> > > > > > proposed
> > > > > > > > > > another
> > > > > > > > > > > > > > > solution
> > > > > > > > > > > > > > > > > >> based
> > > > > > > > > > > > > > > > > >> >> on
> > > > > > > > > > > > > > > > > >> >> > my
> > > > > > > > > > > > > > > > > >> >> > > > > > offline discussion with
> some
> > > > folks
> > > > > > > > working
> > > > > > > > > on
> > > > > > > > > > > EOS
> > > > > > > > > > > > > > > > > >> >> > > > > > to make the handling more
> > > > > > > straightforward
> > > > > > > > > for
> > > > > > > > > > > end
> > > > > > > > > > > > > > users
> > > > > > > > > > > > > > > > > >> without
> > > > > > > > > > > > > > > > > >> >> the
> > > > > > > > > > > > > > > > > >> >> > > > need
> > > > > > > > > > > > > > > > > >> >> > > > > > to distinguish exception
> > > > fatality.
> > > > > > > > > > > > > > > > > >> >> > > > > >
> > > > > > > > > > > > > > > > > >> >> > > > > >> Best,
> > > > > > > > > > > > > > > > > >> >> > > > > >> Bruno
> > > > > > > > > > > > > > > > > >> >> > > > > >>
> > > > > > > > > > > > > > > > > >> >> > > > > >>
> > > > > > > > > > > > > > > > > >> >> > > > > >> On 04.12.20 19:34,
> Guozhang
> > Wang
> > > > > > > wrote:
> > > > > > > > > > > > > > > > > >> >> > > > > >> > Thanks Boyang for the
> > > > proposal!
> > > > > I
> > > > > > > > made a
> > > > > > > > > > > pass
> > > > > > > > > > > > > over
> > > > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > > >> list
> > > > > > > > > > > > > > > > > >> >> and
> > > > > > > > > > > > > > > > > >> >> > > here
> > > > > > > > > > > > > > > > > >> >> > > > > are
> > > > > > > > > > > > > > > > > >> >> > > > > >> > some thoughts:
> > > > > > > > > > > > > > > > > >> >> > > > > >> >
> > > > > > > > > > > > > > > > > >> >> > > > > >> > 0) Although this is not
> > part
> > > > of
> > > > > > the
> > > > > > > > > public
> > > > > > > > > > > > API,
> > > > > > > > > > > > > I
> > > > > > > > > > > > > > > think
> > > > > > > > > > > > > > > > > we
> > > > > > > > > > > > > > > > > >> >> > should
> > > > > > > > > > > > > > > > > >> >> > > > make
> > > > > > > > > > > > > > > > > >> >> > > > > >> sure
> > > > > > > > > > > > > > > > > >> >> > > > > >> > that the suggested
> pattern
> > > > (i.e.
> > > > > > > user
> > > > > > > > > can
> > > > > > > > > > > > always
> > > > > > > > > > > > > > > call
> > > > > > > > > > > > > > > > > >> >> abortTxn()
> > > > > > > > > > > > > > > > > >> >> > > > when
> > > > > > > > > > > > > > > > > >> >> > > > > >> > handling non-fatal
> > errors) are
> > > > > > > indeed
> > > > > > > > > > > > supported.
> > > > > > > > > > > > > > > E.g. if
> > > > > > > > > > > > > > > > > >> the
> > > > > > > > > > > > > > > > > >> >> txn
> > > > > > > > > > > > > > > > > >> >> > > is
> > > > > > > > > > > > > > > > > >> >> > > > > >> already
> > > > > > > > > > > > > > > > > >> >> > > > > >> > aborted by the broker
> > side,
> > > > then
> > > > > > > users
> > > > > > > > > can
> > > > > > > > > > > > still
> > > > > > > > > > > > > > > call
> > > > > > > > > > > > > > > > > >> >> abortTxn
> > > > > > > > > > > > > > > > > >> >> > > which
> > > > > > > > > > > > > > > > > >> >> > > > > >> would
> > > > > > > > > > > > > > > > > >> >> > > > > >> > not throw another
> > exception
> > > > but
> > > > > > just
> > > > > > > > be
> > > > > > > > > > > > treated
> > > > > > > > > > > > > > as a
> > > > > > > > > > > > > > > > > >> no-op.
> > > > > > > > > > > > > > > > > >> >> > > > > >> >
> > > > > > > > > > > > > > > > > >> >> > > > > >> > 1)
> > > > > > > *ConcurrentTransactionsException*:
> > > > > > > > I
> > > > > > > > > > > think
> > > > > > > > > > > > > this
> > > > > > > > > > > > > > > error
> > > > > > > > > > > > > > > > > >> can
> > > > > > > > > > > > > > > > > >> >> > also
> > > > > > > > > > > > > > > > > >> >> > > be
> > > > > > > > > > > > > > > > > >> >> > > > > >> > returned but not
> > documented
> > > > yet.
> > > > > > > This
> > > > > > > > > > should
> > > > > > > > > > > > be
> > > > > > > > > > > > > a
> > > > > > > > > > > > > > > > > >> non-fatal
> > > > > > > > > > > > > > > > > >> >> > error.
> > > > > > > > > > > > > > > > > >> >> > > > > >> >
> > > > > > > > > > > > > > > > > >> >> > > > > >> > 2)
> > *InvalidTxnStateException*:
> > > > > > this
> > > > > > > > > error
> > > > > > > > > > is
> > > > > > > > > > > > > > > returned
> > > > > > > > > > > > > > > > > from
> > > > > > > > > > > > > > > > > >> >> > broker
> > > > > > > > > > > > > > > > > >> >> > > > when
> > > > > > > > > > > > > > > > > >> >> > > > > >> txn
> > > > > > > > > > > > > > > > > >> >> > > > > >> > state transition failed
> > (e.g.
> > > > it
> > > > > > is
> > > > > > > > > trying
> > > > > > > > > > > to
> > > > > > > > > > > > > > > transit to
> > > > > > > > > > > > > > > > > >> >> > > > > complete-commit
> > > > > > > > > > > > > > > > > >> >> > > > > >> > while the current state
> > is not
> > > > > > > > > > > > prepare-commit).
> > > > > > > > > > > > > > This
> > > > > > > > > > > > > > > > > error
> > > > > > > > > > > > > > > > > >> >> could
> > > > > > > > > > > > > > > > > >> >> > > > > >> indicates
> > > > > > > > > > > > > > > > > >> >> > > > > >> > a bug on the client
> > internal
> > > > > code
> > > > > > or
> > > > > > > > the
> > > > > > > > > > > > broker
> > > > > > > > > > > > > > > code,
> > > > > > > > > > > > > > > > > OR a
> > > > > > > > > > > > > > > > > >> >> user
> > > > > > > > > > > > > > > > > >> >> > > > error
> > > > > > > > > > > > > > > > > >> >> > > > > >> --- a
> > > > > > > > > > > > > > > > > >> >> > > > > >> > similar error is
> > > > > > > > > > > > > ConcurrentTransactionsException,
> > > > > > > > > > > > > > > i.e.
> > > > > > > > > > > > > > > > > if
> > > > > > > > > > > > > > > > > >> >> Kafka
> > > > > > > > > > > > > > > > > >> >> > is
> > > > > > > > > > > > > > > > > >> >> > > > > >> bug-free
> > > > > > > > > > > > > > > > > >> >> > > > > >> > these exceptions would
> > only be
> > > > > > > > returned
> > > > > > > > > if
> > > > > > > > > > > > users
> > > > > > > > > > > > > > > try to
> > > > > > > > > > > > > > > > > do
> > > > > > > > > > > > > > > > > >> >> > > something
> > > > > > > > > > > > > > > > > >> >> > > > > >> wrong,
> > > > > > > > > > > > > > > > > >> >> > > > > >> > e.g. calling abortTxn
> > right
> > > > > after
> > > > > > a
> > > > > > > > > > > commitTxn,
> > > > > > > > > > > > > > etc.
> > > > > > > > > > > > > > > So
> > > > > > > > > > > > > > > > > I'm
> > > > > > > > > > > > > > > > > >> >> > > thinking
> > > > > > > > > > > > > > > > > >> >> > > > it
> > > > > > > > > > > > > > > > > >> >> > > > > >> > should be a non-fatal
> > error
> > > > > > instead
> > > > > > > > of a
> > > > > > > > > > > fatal
> > > > > > > > > > > > > > > error,
> > > > > > > > > > > > > > > > > >> wdyt?
> > > > > > > > > > > > > > > > > >> >> > > > > >> >
> > > > > > > > > > > > > > > > > >> >> > > > > >> > 3) *KafkaException*:
> case
> > i
> > > > > > > "indicates
> > > > > > > > > > fatal
> > > > > > > > > > > > > > > > > transactional
> > > > > > > > > > > > > > > > > >> >> > > sequence
> > > > > > > > > > > > > > > > > >> >> > > > > >> > (Fatal)", this is a bit
> > > > > > conflicting
> > > > > > > > with
> > > > > > > > > > the
> > > > > > > > > > > > > > > > > >> >> > > > *OutOfSequenceException*
> > > > > > > > > > > > > > > > > >> >> > > > > >> that
> > > > > > > > > > > > > > > > > >> >> > > > > >> > is treated as
> non-fatal. I
> > > > guess
> > > > > > > your
> > > > > > > > > > > proposal
> > > > > > > > > > > > > is
> > > > > > > > > > > > > > > that
> > > > > > > > > > > > > > > > > >> >> > > > > >> >
> > OutOfOrderSequenceException
> > > > > would
> > > > > > be
> > > > > > > > > > treated
> > > > > > > > > > > > > > either
> > > > > > > > > > > > > > > as
> > > > > > > > > > > > > > > > > >> fatal
> > > > > > > > > > > > > > > > > >> >> > with
> > > > > > > > > > > > > > > > > >> >> > > > > >> > transactional producer,
> or
> > > > > > non-fatal
> > > > > > > > > with
> > > > > > > > > > > > > > idempotent
> > > > > > > > > > > > > > > > > >> >> producer,
> > > > > > > > > > > > > > > > > >> >> > is
> > > > > > > > > > > > > > > > > >> >> > > > that
> > > > > > > > > > > > > > > > > >> >> > > > > >> > right? If the producer
> is
> > only
> > > > > > > > > configured
> > > > > > > > > > > with
> > > > > > > > > > > > > > > > > idempotency
> > > > > > > > > > > > > > > > > >> >> but
> > > > > > > > > > > > > > > > > >> >> > not
> > > > > > > > > > > > > > > > > >> >> > > > > >> > transaction, then
> > throwing a
> > > > > > > > > > > > > > > > > >> >> TransactionStateCorruptedException
> > > > > > > > > > > > > > > > > >> >> > > for
> > > > > > > > > > > > > > > > > >> >> > > > > >> > non-fatal errors would
> be
> > > > > > confusing
> > > > > > > > > since
> > > > > > > > > > > > users
> > > > > > > > > > > > > > are
> > > > > > > > > > > > > > > not
> > > > > > > > > > > > > > > > > >> using
> > > > > > > > > > > > > > > > > >> >> > > > > >> transactions
> > > > > > > > > > > > > > > > > >> >> > > > > >> > at all.. So I suggest we
> > > > always
> > > > > > > throw
> > > > > > > > > > > > > > > > > >> OutOfSequenceException
> > > > > > > > > > > > > > > > > >> >> > as-is
> > > > > > > > > > > > > > > > > >> >> > > > > (i.e.
> > > > > > > > > > > > > > > > > >> >> > > > > >> > not wrapped) no matter
> > how the
> > > > > > > > producer
> > > > > > > > > is
> > > > > > > > > > > > > > > configured,
> > > > > > > > > > > > > > > > > and
> > > > > > > > > > > > > > > > > >> >> let
> > > > > > > > > > > > > > > > > >> >> > the
> > > > > > > > > > > > > > > > > >> >> > > > > >> caller
> > > > > > > > > > > > > > > > > >> >> > > > > >> > decide how to handle it
> > based
> > > > on
> > > > > > > > whether
> > > > > > > > > > it
> > > > > > > > > > > is
> > > > > > > > > > > > > > only
> > > > > > > > > > > > > > > > > >> >> idempotent
> > > > > > > > > > > > > > > > > >> >> > or
> > > > > > > > > > > > > > > > > >> >> > > > > >> > transactional itself.
> > > > > > > > > > > > > > > > > >> >> > > > > >> >
> > > > > > > > > > > > > > > > > >> >> > > > > >> > 4) Besides all the txn
> > APIs,
> > > > the
> > > > > > > > > `send()`
> > > > > > > > > > > > > > callback /
> > > > > > > > > > > > > > > > > >> future
> > > > > > > > > > > > > > > > > >> >> can
> > > > > > > > > > > > > > > > > >> >> > > also
> > > > > > > > > > > > > > > > > >> >> > > > > >> throw
> > > > > > > > > > > > > > > > > >> >> > > > > >> > txn-related exceptions,
> I
> > > > think
> > > > > > this
> > > > > > > > KIP
> > > > > > > > > > > > should
> > > > > > > > > > > > > > also
> > > > > > > > > > > > > > > > > cover
> > > > > > > > > > > > > > > > > >> >> this
> > > > > > > > > > > > > > > > > >> >> > > API
> > > > > > > > > > > > > > > > > >> >> > > > as
> > > > > > > > > > > > > > > > > >> >> > > > > >> well?
> > > > > > > > > > > > > > > > > >> >> > > > > >> >
> > > > > > > > > > > > > > > > > >> >> > > > > >> > 5) This is related to
> 1/2)
> > > > > above:
> > > > > > > > > > sometimes
> > > > > > > > > > > > > those
> > > > > > > > > > > > > > > > > >> non-fatal
> > > > > > > > > > > > > > > > > >> >> > errors
> > > > > > > > > > > > > > > > > >> >> > > > > like
> > > > > > > > > > > > > > > > > >> >> > > > > >> > ConcurrentTxn or
> > > > InvalidTxnState
> > > > > > are
> > > > > > > > not
> > > > > > > > > > due
> > > > > > > > > > > > to
> > > > > > > > > > > > > > the
> > > > > > > > > > > > > > > > > state
> > > > > > > > > > > > > > > > > >> >> being
> > > > > > > > > > > > > > > > > >> >> > > > > >> corrupted
> > > > > > > > > > > > > > > > > >> >> > > > > >> > at the broker side, but
> > maybe
> > > > > > users
> > > > > > > > are
> > > > > > > > > > > doing
> > > > > > > > > > > > > > > something
> > > > > > > > > > > > > > > > > >> >> wrong.
> > > > > > > > > > > > > > > > > >> >> > So
> > > > > > > > > > > > > > > > > >> >> > > > I'm
> > > > > > > > > > > > > > > > > >> >> > > > > >> > wondering if we should
> > further
> > > > > > > > > distinguish
> > > > > > > > > > > > those
> > > > > > > > > > > > > > > > > non-fatal
> > > > > > > > > > > > > > > > > >> >> > errors
> > > > > > > > > > > > > > > > > >> >> > > > > >> between
> > > > > > > > > > > > > > > > > >> >> > > > > >> > a) those that are caused
> > by
> > > > > Kafka
> > > > > > > > > itself,
> > > > > > > > > > > > e.g. a
> > > > > > > > > > > > > > > broker
> > > > > > > > > > > > > > > > > >> timed
> > > > > > > > > > > > > > > > > >> >> > out
> > > > > > > > > > > > > > > > > >> >> > > > and
> > > > > > > > > > > > > > > > > >> >> > > > > >> > aborted a txn and later
> an
> > > > > endTxn
> > > > > > > > > request
> > > > > > > > > > is
> > > > > > > > > > > > > > > received,
> > > > > > > > > > > > > > > > > >> and b)
> > > > > > > > > > > > > > > > > >> >> > the
> > > > > > > > > > > > > > > > > >> >> > > > > user's
> > > > > > > > > > > > > > > > > >> >> > > > > >> > API call pattern is
> > incorrect,
> > > > > > > causing
> > > > > > > > > the
> > > > > > > > > > > > > request
> > > > > > > > > > > > > > > to be
> > > > > > > > > > > > > > > > > >> >> > rejected
> > > > > > > > > > > > > > > > > >> >> > > > with
> > > > > > > > > > > > > > > > > >> >> > > > > >> an
> > > > > > > > > > > > > > > > > >> >> > > > > >> > error code from the
> > broker.
> > > > > > > > > > > > > > > > > >> >> *TransactionStateCorruptedException*
> > > > > > > > > > > > > > > > > >> >> > > > feels
> > > > > > > > > > > > > > > > > >> >> > > > > >> to
> > > > > > > > > > > > > > > > > >> >> > > > > >> > me more like for case
> a),
> > but
> > > > > not
> > > > > > > case
> > > > > > > > > b).
> > > > > > > > > > > > > > > > > >> >> > > > > >> >
> > > > > > > > > > > > > > > > > >> >> > > > > >> >
> > > > > > > > > > > > > > > > > >> >> > > > > >> > Guozhang
> > > > > > > > > > > > > > > > > >> >> > > > > >> >
> > > > > > > > > > > > > > > > > >> >> > > > > >> >
> > > > > > > > > > > > > > > > > >> >> > > > > >> > On Wed, Dec 2, 2020 at
> > 4:50 PM
> > > > > > > Boyang
> > > > > > > > > > Chen <
> > > > > > > > > > > > > > > > > >> >> > > > > reluctanthero...@gmail.com>
> > > > > > > > > > > > > > > > > >> >> > > > > >> > wrote:
> > > > > > > > > > > > > > > > > >> >> > > > > >> >
> > > > > > > > > > > > > > > > > >> >> > > > > >> >> Thanks Matthias, I
> think
> > your
> > > > > > > > proposal
> > > > > > > > > > > makes
> > > > > > > > > > > > > > sense
> > > > > > > > > > > > > > > as
> > > > > > > > > > > > > > > > > >> well,
> > > > > > > > > > > > > > > > > >> >> on
> > > > > > > > > > > > > > > > > >> >> > > the
> > > > > > > > > > > > > > > > > >> >> > > > > pro
> > > > > > > > > > > > > > > > > >> >> > > > > >> side
> > > > > > > > > > > > > > > > > >> >> > > > > >> >> we could have a
> > universally
> > > > > > agreed
> > > > > > > > > > > exception
> > > > > > > > > > > > > type
> > > > > > > > > > > > > > > to be
> > > > > > > > > > > > > > > > > >> >> caught
> > > > > > > > > > > > > > > > > >> >> > by
> > > > > > > > > > > > > > > > > >> >> > > > the
> > > > > > > > > > > > > > > > > >> >> > > > > >> user,
> > > > > > > > > > > > > > > > > >> >> > > > > >> >> without having an extra
> > layer
> > > > > on
> > > > > > > top
> > > > > > > > of
> > > > > > > > > > the
> > > > > > > > > > > > > > actual
> > > > > > > > > > > > > > > > > >> >> exceptions.
> > > > > > > > > > > > > > > > > >> >> > I
> > > > > > > > > > > > > > > > > >> >> > > > > could
> > > > > > > > > > > > > > > > > >> >> > > > > >> see
> > > > > > > > > > > > > > > > > >> >> > > > > >> >> some issue on
> downsides:
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>
> > > > > > > > > > > > > > > > > >> >> > > > > >> >> 1. The exception
> > hierarchy
> > > > will
> > > > > > be
> > > > > > > > more
> > > > > > > > > > > > > complex.
> > > > > > > > > > > > > > > Right
> > > > > > > > > > > > > > > > > >> now
> > > > > > > > > > > > > > > > > >> >> we
> > > > > > > > > > > > > > > > > >> >> > > have
> > > > > > > > > > > > > > > > > >> >> > > > > >> >> RetriableException
> type,
> > if
> > > > we
> > > > > > are
> > > > > > > > > going
> > > > > > > > > > to
> > > > > > > > > > > > > add a
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>
> > `ProducerRetriableException`
> > > > > > type,
> > > > > > > we
> > > > > > > > > > have
> > > > > > > > > > > to
> > > > > > > > > > > > > put
> > > > > > > > > > > > > > > this
> > > > > > > > > > > > > > > > > >> new
> > > > > > > > > > > > > > > > > >> >> > > > interface
> > > > > > > > > > > > > > > > > >> >> > > > > >> as the
> > > > > > > > > > > > > > > > > >> >> > > > > >> >> parent of the
> > > > > RetriableException,
> > > > > > > > > because
> > > > > > > > > > > not
> > > > > > > > > > > > > all
> > > > > > > > > > > > > > > > > thrown
> > > > > > > > > > > > > > > > > >> >> > > non-fatal
> > > > > > > > > > > > > > > > > >> >> > > > > >> >> exceptions are
> > `retriable` in
> > > > > > > general
> > > > > > > > > for
> > > > > > > > > > > > > > > producer, for
> > > > > > > > > > > > > > > > > >> >> example
> > > > > > > > > > > > > > > > > >> >> > > > > >> >> <
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>
> > > > > > > > > > > > > > > > > >> >> > > > > >>
> > > > > > > > > > > > > > > > > >> >> > > > >
> > > > > > > > > > > > > > > > > >> >> > > >
> > > > > > > > > > > > > > > > > >> >> > >
> > > > > > > > > > > > > > > > > >> >> >
> > > > > > > > > > > > > > > > > >> >>
> > > > > > > > > > > > > > > > > >>
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> >
> https://github.com/apache/kafka/blob/e275742f850af4a1b79b0d1bd1ac9a1d2e89c64e/clients/src/main/java/org/apache/kafka/clients/producer/internals/Sender.java#L745
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>> .
> > > > > > > > > > > > > > > > > >> >> > > > > >> >> We could have an empty
> > > > > interface
> > > > > > > > > > > > > > > > > >> >> `ProducerRetriableException`
> > > > > > > > > > > > > > > > > >> >> > to
> > > > > > > > > > > > > > > > > >> >> > > > let
> > > > > > > > > > > > > > > > > >> >> > > > > >> all
> > > > > > > > > > > > > > > > > >> >> > > > > >> >> the thrown exceptions
> > > > implement
> > > > > > for
> > > > > > > > > sure,
> > > > > > > > > > > but
> > > > > > > > > > > > > > it's
> > > > > > > > > > > > > > > a
> > > > > > > > > > > > > > > > > bit
> > > > > > > > > > > > > > > > > >> >> > > unorthodox
> > > > > > > > > > > > > > > > > >> >> > > > > in
> > > > > > > > > > > > > > > > > >> >> > > > > >> the
> > > > > > > > > > > > > > > > > >> >> > > > > >> >> way we deal with
> > exceptions
> > > > in
> > > > > > > > general.
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>
> > > > > > > > > > > > > > > > > >> >> > > > > >> >> 2. There are cases
> where
> > we
> > > > > > throw a
> > > > > > > > > > > > > > KafkaException
> > > > > > > > > > > > > > > > > >> wrapping
> > > > > > > > > > > > > > > > > >> >> > > another
> > > > > > > > > > > > > > > > > >> >> > > > > >> >> KafkaException as
> either
> > > > fatal
> > > > > or
> > > > > > > > > > > non-fatal.
> > > > > > > > > > > > If
> > > > > > > > > > > > > > we
> > > > > > > > > > > > > > > use
> > > > > > > > > > > > > > > > > an
> > > > > > > > > > > > > > > > > >> >> > > interface
> > > > > > > > > > > > > > > > > >> >> > > > > to
> > > > > > > > > > > > > > > > > >> >> > > > > >> >> solve #1, it is also
> > required
> > > > > to
> > > > > > > > > > implement
> > > > > > > > > > > > > > another
> > > > > > > > > > > > > > > > > >> bloated
> > > > > > > > > > > > > > > > > >> >> > > > exception
> > > > > > > > > > > > > > > > > >> >> > > > > >> class
> > > > > > > > > > > > > > > > > >> >> > > > > >> >> which could replace
> > > > > > KafkaException
> > > > > > > > > type,
> > > > > > > > > > as
> > > > > > > > > > > > we
> > > > > > > > > > > > > > > couldn't
> > > > > > > > > > > > > > > > > >> mark
> > > > > > > > > > > > > > > > > >> >> > > > > >> KafkaException
> > > > > > > > > > > > > > > > > >> >> > > > > >> >> as retriable for sure.
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>
> > > > > > > > > > > > > > > > > >> >> > > > > >> >> 3. In terms of the
> > > > > encapsulation,
> > > > > > > > > > wrapping
> > > > > > > > > > > > > means
> > > > > > > > > > > > > > we
> > > > > > > > > > > > > > > > > could
> > > > > > > > > > > > > > > > > >> >> limit
> > > > > > > > > > > > > > > > > >> >> > > the
> > > > > > > > > > > > > > > > > >> >> > > > > >> scope
> > > > > > > > > > > > > > > > > >> >> > > > > >> >> of affection to the
> > producer
> > > > > > only,
> > > > > > > > > which
> > > > > > > > > > is
> > > > > > > > > > > > > > > important
> > > > > > > > > > > > > > > > > >> since
> > > > > > > > > > > > > > > > > >> >> we
> > > > > > > > > > > > > > > > > >> >> > > > don't
> > > > > > > > > > > > > > > > > >> >> > > > > >> want
> > > > > > > > > > > > > > > > > >> >> > > > > >> >> shared exception types
> to
> > > > > > > implement a
> > > > > > > > > > > > > > > producer-related
> > > > > > > > > > > > > > > > > >> >> > interface,
> > > > > > > > > > > > > > > > > >> >> > > > > such
> > > > > > > > > > > > > > > > > >> >> > > > > >> >> as
> > > > > > > UnknownTopicOrPartitionException.
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>
> > > > > > > > > > > > > > > > > >> >> > > > > >> >> Best,
> > > > > > > > > > > > > > > > > >> >> > > > > >> >> Boyang
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>
> > > > > > > > > > > > > > > > > >> >> > > > > >> >> On Wed, Dec 2, 2020 at
> > 3:38
> > > > PM
> > > > > > > > Matthias
> > > > > > > > > > J.
> > > > > > > > > > > > Sax
> > > > > > > > > > > > > <
> > > > > > > > > > > > > > > > > >> >> > mj...@apache.org
> > > > > > > > > > > > > > > > > >> >> > > >
> > > > > > > > > > > > > > > > > >> >> > > > > >> wrote:
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>> Thanks for the KIP
> > Boyang!
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>>
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>> Overall, categorizing
> > > > > exceptions
> > > > > > > > > makes a
> > > > > > > > > > > lot
> > > > > > > > > > > > > of
> > > > > > > > > > > > > > > sense.
> > > > > > > > > > > > > > > > > >> As I
> > > > > > > > > > > > > > > > > >> >> > > don't
> > > > > > > > > > > > > > > > > >> >> > > > > know
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>> the producer internals
> > well
> > > > > > > enough,
> > > > > > > > I
> > > > > > > > > > > cannot
> > > > > > > > > > > > > > > comment
> > > > > > > > > > > > > > > > > on
> > > > > > > > > > > > > > > > > >> the
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>> categorization in
> detail
> > > > > though.
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>>
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>> What I am wondering
> is,
> > if
> > > > we
> > > > > > > should
> > > > > > > > > > > > introduce
> > > > > > > > > > > > > > an
> > > > > > > > > > > > > > > > > >> exception
> > > > > > > > > > > > > > > > > >> >> > > > > interface
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>> that non-fatal
> exception
> > > > would
> > > > > > > > > implement
> > > > > > > > > > > > > instead
> > > > > > > > > > > > > > > of
> > > > > > > > > > > > > > > > > >> >> creating a
> > > > > > > > > > > > > > > > > >> >> > > new
> > > > > > > > > > > > > > > > > >> >> > > > > >> class
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>> that will wrap
> non-fatal
> > > > > > > exceptions?
> > > > > > > > > > What
> > > > > > > > > > > > > would
> > > > > > > > > > > > > > > be the
> > > > > > > > > > > > > > > > > >> >> > pros/cons
> > > > > > > > > > > > > > > > > >> >> > > > for
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>> both designs?
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>>
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>>
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>> -Matthias
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>>
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>>
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>> On 12/2/20 11:35 AM,
> > Boyang
> > > > > Chen
> > > > > > > > > wrote:
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>>> Hey there,
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>>>
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>>> I would like to
> start a
> > > > > > > discussion
> > > > > > > > > > thread
> > > > > > > > > > > > for
> > > > > > > > > > > > > > > > > KIP-691:
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>>>
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>>
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>
> > > > > > > > > > > > > > > > > >> >> > > > > >>
> > > > > > > > > > > > > > > > > >> >> > > > >
> > > > > > > > > > > > > > > > > >> >> > > >
> > > > > > > > > > > > > > > > > >> >> > >
> > > > > > > > > > > > > > > > > >> >> >
> > > > > > > > > > > > > > > > > >> >>
> > > > > > > > > > > > > > > > > >>
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-691%3A+Enhance+Transactional+Producer+Exception+Handling
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>>>
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>>> The KIP is aiming to
> > > > simplify
> > > > > > the
> > > > > > > > > > > exception
> > > > > > > > > > > > > > > handling
> > > > > > > > > > > > > > > > > >> logic
> > > > > > > > > > > > > > > > > >> >> > for
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>>> transactional
> Producer
> > > > users
> > > > > by
> > > > > > > > > > > classifying
> > > > > > > > > > > > > > > fatal and
> > > > > > > > > > > > > > > > > >> >> > non-fatal
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>> exceptions
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>>> and throw them
> > > > > correspondingly
> > > > > > > for
> > > > > > > > > > easier
> > > > > > > > > > > > > catch
> > > > > > > > > > > > > > > and
> > > > > > > > > > > > > > > > > >> retry.
> > > > > > > > > > > > > > > > > >> >> > Let
> > > > > > > > > > > > > > > > > >> >> > > me
> > > > > > > > > > > > > > > > > >> >> > > > > >> know
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>> what
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>>> you think.
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>>>
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>>> Best,
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>>> Boyang
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>>>
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>>
> > > > > > > > > > > > > > > > > >> >> > > > > >> >>
> > > > > > > > > > > > > > > > > >> >> > > > > >> >
> > > > > > > > > > > > > > > > > >> >> > > > > >> >
> > > > > > > > > > > > > > > > > >> >> > > > > >>
> > > > > > > > > > > > > > > > > >> >> > > > > >
> > > > > > > > > > > > > > > > > >> >> > > > >
> > > > > > > > > > > > > > > > > >> >> > > >
> > > > > > > > > > > > > > > > > >> >> > > >
> > > > > > > > > > > > > > > > > >> >> > > > --
> > > > > > > > > > > > > > > > > >> >> > > > -- Guozhang
> > > > > > > > > > > > > > > > > >> >> > > >
> > > > > > > > > > > > > > > > > >> >> > >
> > > > > > > > > > > > > > > > > >> >> >
> > > > > > > > > > > > > > > > > >> >> >
> > > > > > > > > > > > > > > > > >> >> > --
> > > > > > > > > > > > > > > > > >> >> > -- Guozhang
> > > > > > > > > > > > > > > > > >> >> >
> > > > > > > > > > > > > > > > > >> >>
> > > > > > > > > > > > > > > > > >> >
> > > > > > > > > > > > > > > > > >> >
> > > > > > > > > > > > > > > > > >> > --
> > > > > > > > > > > > > > > > > >> > -- Guozhang
> > > > > > > > > > > > > > > > > >> >
> > > > > > > > > > > > > > > > > >>
> > > > > > > > > > > > > > > > > >>
> > > > > > > > > > > > > > > > > >> --
> > > > > > > > > > > > > > > > > >> -- Guozhang
> > > > > > > > > > > > > > > > > >>
> > > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > --
> > > > > > > > > > > > > > > > -- Guozhang
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > --
> > > > > > > > > > -- Guozhang
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > --
> > > > > > > > -- Guozhang
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Reply via email to