Thanks Jason!
On Tue, Feb 2, 2021 at 11:10 PM Jason Gustafson wrote:
> Hi Boyang, sounds good to me.
>
> -Jason
>
> On Mon, Feb 1, 2021 at 9:32 AM Boyang Chen
> wrote:
>
> > Hey Jason and Chia-Ping,
> >
> > looking at the discussion thread for KIP-706, we are still in discussion
> > with the
Hi Boyang, sounds good to me.
-Jason
On Mon, Feb 1, 2021 at 9:32 AM Boyang Chen
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
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`
> 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 wrote:
> This is a great proposal Jason, I already integrated KIP-691 with KIP-706
> template to provide
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
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
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 wrote:
> Hi
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
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 wrote:
> Hey
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
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
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
Thanks Jason, I agree with the proposed solution here, will update the KIP.
On Thu, Jan 28, 2021 at 10:52 AM Jason Gustafson 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
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
Thanks Jason for the thoughts.
On Wed, Jan 27, 2021 at 11:52 AM Jason Gustafson 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
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
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 wrote:
>
>
> Thanks Boyang, yes I think I was confused about the different handling of
> two abortTxn calls, and now I get it was
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
wrote:
> Thanks for the clarification Guozhang, I got your point that we want to
>
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
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 wrote:
> Or are you going to
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 wrote:
> Hi Boyang/Jason,
>
> I've also thought about this (i.e. using CommitFailed for all non-fatal),
> but what I'm
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
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
Thanks for your clarification on 2)/3), that makes sense.
On Tue, Jan 19, 2021 at 10:16 AM Boyang Chen
wrote:
> Thanks for the input Guozhang, replied inline.
>
> On Mon, Jan 18, 2021 at 8:57 PM Guozhang Wang wrote:
>
> > Hello Boyang,
> >
> > Thanks for the updated KIP. I read it again and
Thanks for the input Guozhang, replied inline.
On Mon, Jan 18, 2021 at 8:57 PM Guozhang Wang 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
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
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
Thanks Bruno for the feedback.
On Mon, Dec 7, 2020 at 5:26 AM Bruno Cadonna 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
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
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
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
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
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
33 matches
Mail list logo
