Hi Matthias,
I like this compromise a lot. +1 from me
Best,
Chris
On Mon, Jun 17, 2024, 16:15 Justine Olshan
wrote:
> Makes sense to me.
>
> Thanks Matthias for summarizing the state.
>
> Justine
>
> On Mon, Jun 17, 2024 at 1:12 PM Matthias J. Sax wrote:
>
> > Hey,
> >
> > seems this KIP is
Makes sense to me.
Thanks Matthias for summarizing the state.
Justine
On Mon, Jun 17, 2024 at 1:12 PM Matthias J. Sax wrote:
> Hey,
>
> seems this KIP is very difficult, and we actually had a lot of
> background discussion about it in the last weeks. I believe the problem
> with this KIP is, t
Hey,
seems this KIP is very difficult, and we actually had a lot of
background discussion about it in the last weeks. I believe the problem
with this KIP is, that we have 3 starting points to look at a problem,
but these 3 starting points don't align:
1. Producer API: we want a clean API d
Hello Alieh, thanks for this useful KIP.
There is a typo in the motivation when you talk about the
UnknownTopicOrPartitionException. It's delivery.timeout.ms, not
deliver.timeout.ms.
In the past, I did some work to improve and clean the official Kafka
examples, which I think are useful for new Ka
Hi Alieh,
Thank you for all the updates! One final question--how will the retry
timeout for unknown topic partition errors be implemented? I think it would
be best if this could be done with an implementation of the error handler,
but I don't see a way to track the necessary information with the
c
Thanks Andrew. Done :)
@Chris: I changed the config parameter type from boolean to integer, which
defines the timeout for retrying. I thought reusing `max.block.ms` was not
reasonable as you mentioned.
So if the KIP looks good, let 's skip to the good part ;-) VOTING :)
Bests,
Alieh
On Tue,
Hi Alieh,
Just one final comment.
[AJS5] Existing classes use Retriable, not Retryable. For example:
https://kafka.apache.org/21/javadoc/org/apache/kafka/common/errors/RetriableException.html
I suggest RetriableResponse and NonRetriableResponse.
Thanks,
Andrew
> On 13 May 2024, at 23:17, Alieh
Hi all,
Thanks for all the valid points you listed.
KIP updates and addressing concerns:
1) The KIP now suggests two Response types: `RetryableResponse` and
`NonRetryableResponse`
2) `custom.exception.handler` is changed to `custom.exception.handler.class`
3) The KIP clarifies that `In th
Oh I see. The type isn't the error type but a newly defined type for the
response. Makes sense and works for me.
Justine
On Mon, May 13, 2024 at 9:13 AM Chris Egerton
wrote:
> If we have dedicated methods for each kind of exception
> (handleRecordTooLarge, handleUnknownTopicOrPartition, etc.),
If we have dedicated methods for each kind of exception
(handleRecordTooLarge, handleUnknownTopicOrPartition, etc.), doesn't that
provide sufficient constraint? I'm not suggesting we eliminate these
methods, just that we change their return types to something more flexible.
On Mon, May 13, 2024, 1
I'm not sure I agree with the Retriable and NonRetriableResponse comment.
This doesn't limit the blast radius or enforce certain errors are used.
I think we might disagree on how controlled these interfaces can be...
Justine
On Mon, May 13, 2024 at 8:40 AM Chris Egerton
wrote:
> Hi Alieh,
>
> T
Hi Alieh,
Thanks for the updates! I just have a few more thoughts:
- I don't think a boolean property is sufficient to dictate retries for
unknown topic partitions, though. These errors can occur if a topic has
just been created, which can occur if, for example, automatic topic
creation is enable
Hi Alieh,
Just a few more comments on the KIP. It is looking much less risky now the scope
is tighter.
[AJS1] It would be nice to have default implementations of the handle methods
so an implementor would not need to implement both themselves.
[AJS2] Producer configurations which are class names
Hi all,
Thanks for the very interesting discussion during my PTO.
KIP updates and addressing concerns:
1) Two handle() methods are defined in ProducerExceptionHandler for the two
exceptions with different input parameters so that we have
handle(RecordTooLargeException e, ProducerRecord record
Hi Mathias,
> [AL1] While I see the point, I would think having a different callback
for every exception might not really be elegant?
I'm not sure how to assess the level of elegance of the proposal, but I can
comment on the technical characteristics:
1. Having specific interfaces that codify th
My concern with respect to it being fragile: the code that ensures the
error type is internal to the producer. Someone may see it and say, I want
to add such and such error. This looks like internal code, so I don't need
a KIP, and then they can change it to whatever they want thinking it is
within
Hi Alieh,
Continuing prior discussions:
1) Regarding the "flexibility" discussion, my overarching point is that I
don't see the point in allowing for this kind of pluggable logic without
also covering more scenarios. Take example 2 in the KIP: if we're going to
implement retries only on "importan
Very interesting discussion.
Seems a central point is the question about "how generic" we approach
this, and some people think we need to be conservative and others think
we should try to be as generic as possible.
Personally, I think following a limited scope for this KIP (by
explicitly say
Hi Alieh,
Thanks for the KIP. The motivation talks about very specific cases, but
the interface is generic.
[AL1]
If the interface evolves in the future I think we could have the following
confusion:
1. A user implemented SWALLOW action for both RecordTooLargeException and
UnknownTopicOrPartiti
Alieh and Chris,
Thanks for clarifying 1) but I saw the motivation. I guess I just didn't
understand how that would be ensured on the producer side. I saw the sample
code -- is it just an if statement checking for the error before the
handler is invoked? That seems a bit fragile.
Can you clarify
Hi,
Thank you, Chris and Justine, for the feedback.
@Chris
1) Flexibility: it has two meanings. The first meaning is the one you
mentioned. We are going to cover more exceptions in the future, but as
Justine mentioned, we must be very conservative about adding more
exceptions. Additionally, fl
Hi Justine,
The method signatures for the interface are indeed open-ended, but the KIP
states that its uses will be limited. See the motivation section:
> We believe that the user should be able to develop custom exception
handlers for managing producer exceptions. On the other hand, this will be
Hi Alieh,
I was out for KSB and then was also sick. :(
To your point 1) Chris, I don't think it is limited to two specific
scenarios, since the interface accepts a generic Exception e and can be
implemented to check if that e is an instanceof any exception. I didn't see
anywhere that specific err
Hi Alieh,
Sorry for the delay, I've been out sick. I still have some thoughts that
I'd like to see addressed before voting.
1) If flexibility is the motivation for a pluggable interface, why are we
only limiting the uses for this interface to two very specific scenarios?
Why not also allow, e.g.,
Hi all,
A summary of the KIP and the discussions:
The KIP introduces a handler interface for Producer in order to handle two
exceptions: RecordTooLargeException and UnknownTopicOrPartitionException.
The handler handles the exceptions per-record.
- Do we need this handler? [Motivation and Exa
Hi Alieh,
Thanks for the updates!
Comments inline...
> On Apr 25, 2024, at 1:10 PM, Alieh Saeedi
> wrote:
>
> Hi all,
>
> Thanks a lot for the constructive feedbacks!
>
>
>
> Addressing some of the main concerns:
>
>
> - The `RecordTooLargeException` can be thrown by broker, producer a
Hi all,
Thanks a lot for the constructive feedbacks!
Addressing some of the main concerns:
- The `RecordTooLargeException` can be thrown by broker, producer and
consumer. Of course, the `ProducerExceptionHandler` interface is introduced
to affect only the exceptions thrown from the producer.
Thanks Alieh for the updates.
I'm a little concerned about the design pattern here. It seems like we want
specific usages, but we are packaging it as a generic handler.
I think we tried to narrow down on the specific errors we want to handle,
but it feels a little clunky as we have a generic thing
Hi Alieh,
Thanks for the KIP!
A few questions:
K1. What is the expected behavior for the producer if it generates a
RecordTooLargeException, but the handler returns RETRY?
K2. How do we determine which Record was responsible for the
UnknownTopicOrPartitionException since we get that response w
Alieh,
Having run into issues with not being able to handle producer failures, I
think this is good functionality to have.
With this new functionality proposed at the Producer level, how would
ecosystems that sit on top of it function? Specifically, Connect was
updated a few years ago to allow So
Thanks Matthias. I changed it to `custom.exception.handler`
Alieh
On Tue, Apr 23, 2024 at 8:47 AM Matthias J. Sax wrote:
> Thanks Alieh!
>
> A few nits:
>
>
> 1) The new config we add for the producer should be mentioned in the
> "Public Interfaces" section.
>
> 2) Why do we use `producer.` pr
Thanks Alieh!
A few nits:
1) The new config we add for the producer should be mentioned in the
"Public Interfaces" section.
2) Why do we use `producer.` prefix for a *producer* config? Should it
be `exception.handler` only?
-Matthias
On 4/22/24 7:38 AM, Alieh Saeedi wrote:
Thank you al
Thank you all for the feedback!
Addressing the main concern: The KIP is about giving the user the ability
to handle producer exceptions, but to be more conservative and avoid future
issues, we decided to be limited to a short list of exceptions. I included
*RecordTooLargeExceptin* and *UnknownTopi
Thanks for the KIP Alieh! It addresses an important case for error handling.
I agree that using this handler would be an expert API, as mentioned by
a few people. But I don't think it would be a reason to not add it. It's
always a tricky tradeoff what to expose to users and to avoid foot guns,
Hi Alieh,
Thanks for the KIP.
Exception handling in the Kafka producer and consumer is really not ideal.
It’s even harder working out what’s going on with the consumer.
I’m a bit nervous about this KIP and I agree with Chris that it could do with
additional
motivation. This would be an expert-le
Hi Alieh,
Thanks for the KIP! The issue with writing to non-existent topics is
particularly frustrating for users of Kafka Connect and has been the source
of a handful of Jira tickets over the years. My thoughts:
1. An additional detail we can add to the motivation (or possibly rejected
alternati
Hey Alieh,
some comments:
* "Compatibility" section wasn't clear to me. Are we just introducing
the interfaces or are we changing the default behavior? If so, that
should be explained in more detail.
* If we are introducing a new interface `ClientExceptionHandler`, what
is it going to be used for
Hey Alieh,
I echo what Omnia says, I'm not sure I understand the implications of the
change and I think more detail is needed.
This comment also confused me a bit:
* {@code ClientExceptionHandler} that continues the transaction even if a
record is too large.
* Otherwise, it makes the transaction
Hi Alieh,
Thanks for the KIP! I have couple of comments
- You mentioned in the KIP motivation,
> Another example for which a production exception handler could be useful is
> if a user tries to write into a non-existing topic, which returns a retryable
> error code; with infinite retries, the p
39 matches
Mail list logo