Re: [DISCUSS] KIP-1059: Enable the Producer flush() method to clear the latest send() error
Hi all, thanks for the KIP Alieh! LM1. Totally agree with Artem's point about the config not being the most explicit/flexible way to express this capability. Getting then to Matthias 4 options, what I don't like about 3 and 4 is that it seems they might not age very well? Aren't we going to be wanting some other twist to the flush semantics that will have us adding yet another param to it, or another overloaded method? I truly don't have the context to answer that, but if it feels like a realistic future maybe adding some kind FlushOptions params to the flush would be better from an extensibility point of view. It would only have the clearErrors option available for now but could accept any other we may need. I find that this would remove the "ugliness" Matthias pointed out for 3. and 4. LM2. No matter how we end up expressing the different semantics for flush, let's make sure we update the KIP on the flush and commitTransaction java docs. It currently states that flush "clears the last exception" and commitTransaction "will NOT throw" if called after flush, but it really all depends on the config/options/method used. LM3. I find it would be helpful to include an example to show the new flow that we're unblocking (I see this as the great gain here): flush with clear error option enabled -> catch and do whatever error handling we want -> commitTransaction successfully Thanks! Lianet On Wed, Jun 19, 2024 at 11:26 PM Chris Egerton wrote: > Hi Matthias, > > I like the alternatives you've listed. One more that might help is if, > instead of overloading flush(), we overloaded commitTransaction() to > something like commitTransaction(boolean tolerateRecordErrors). This seems > slightly cleaner in that it takes the behavioral change we want, which only > applies to transactional producers, to an API method that is only used for > transactional producers. It would also avoid the issue of whether or not > flush() (or a new variant of it with altered semantics) should throw or > not. Thoughts? > > Hi Alieh, > > Thanks for the KIP, I like this direction a lot more than the pluggable > handler! > > I share Artem's concerns that enabling this behavior via configuration > doesn't seem like a great fit. It's likely that application code will be > written in a style that only works with one type of behavior from > transactional producers, so requiring that application code to declare its > expectations for the behavior of its producer seems more appropriate than, > e.g., allowing users deploying that application to tweak a configuration > file that gets fed to producers spun up inside it. > > Cheers, > > Chris > > On Wed, Jun 19, 2024 at 10:32 PM Matthias J. Sax wrote: > > > Thanks for the KIP Alieh. I actually like the KIP as-is, but think > > Arthem raises very good points... > > > > Seems we have four options on how to move forward? > > > > 1. add config to allow "silent error clearance" as the KIP proposes > > 2. change flush() to clear error and let it throw > > 3. add new flushAndThrow()` (or better name) which clears error and > > throws > > 4. add `flush(boolean clearAndThrow)` and let user pick (and deprecate > > existing `flush()`) > > > > For (2), given that it would be a behavior change, we might also need a > > public "feature flag" config. > > > > It seems, both (1) and (2) have the issue Artem mentioned. (3) and (4) > > would be safer to this end, however, for both we kinda get an ugly API? > > > > Not sure right now if I have any preference. Seems we need to pick some > > evil and that there is no clear best solution? Would be good to her from > > others what they think > > > > > > -Matthias > > > > > > On 6/18/24 8:39 PM, Artem Livshits wrote: > > > Hi Alieh, > > > > > > Thank you for the KIP. I have a couple of suggestions: > > > > > > AL1. We should throw an error from flush after we clear it. This > would > > > make it so that both "send + commit" and "send + flush + commit" (the > > > latter looks like just a more verbose way to express the former, and it > > > would be intuitive if it behaves the same) would throw if the > transaction > > > has an error (so if the code is written either way it's going be > > correct). > > > At the same time, the latter could be extended by the caller to > intercept > > > exceptions from flush, ignore as needed, and commit the transaction. > > This > > > solution would keep basic things simple (if someone has code that > doesn't > > > require advanced error handling, then basic "send + flush + commit" > would > > > do the right thing) and advanced things possible, an application can > add > > > try + catch around flush and ignore some errors. > > > > > > AL2. I'm not sure if config is the best way to express the > modification > > of > > > the "flush" semantics -- the application logic that calls "flush" needs > > to > > > match the "flush" semantics and configuring semantics in a detached > place > > > creates a room for bugs due to
Re: [VOTE] KIP-1033: Add Kafka Streams exception handler for exceptions occurring during processing
+1 (non-binding) Thanks for the KIP and updates! Lianet On Tue, May 14, 2024, 12:03 a.m. Matthias J. Sax wrote: > +1 (binding) > > On 5/13/24 5:54 PM, Sophie Blee-Goldman wrote: > > Thanks for the KIP guys! > > > > +1 (binding) > > > > On Mon, May 13, 2024 at 6:02 AM Bill Bejeck wrote: > > > >> Thanks for the KIP, this will be a great addition! > >> > >> +1(binding) > >> > >> -Bill > >> > >> On Fri, May 3, 2024 at 4:48 AM Bruno Cadonna > wrote: > >> > >>> Hi Damien, Sébastien, and Loïc, > >>> > >>> Thanks for the KIP! > >>> > >>> +1 (binding) > >>> > >>> Best, > >>> Bruno > >>> > >>> > >>> On 4/26/24 4:00 PM, Damien Gasparina wrote: > Hi all, > > We would like to start a vote for KIP-1033: Add Kafka Streams > exception handler for exceptions occurring during processing > > The KIP is available on > > >>> > >> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-1033%3A+Add+Kafka+Streams+exception+handler+for+exceptions+occurring+during+processing > > If you have any suggestions or feedback, feel free to participate to > the discussion thread: > https://lists.apache.org/thread/1nhhsrogmmv15o7mk9nj4kvkb5k2bx9s > > Best regards, > Damien Sebastien and Loic > >>> > >> > > >
Re: [DISCUSS] KIP-1033: Add Kafka Streams exception handler for exceptions occuring during processing
Hi all, thanks Damien for the KIP!
After looking into the KIP and comments, my only concern is aligned with
one of Matthias comments, around the ImmutableHeaders introduction, with
the motivation not being clear enough. The existing handlers already expose
the headers (indirectly). Ex.
ProductionExceptionHandler.handleSerializationException provides the
ProducerRecord as an argument, so they are already exposed in those
callbacks through record.headers(). Is there a reason to think that it
would be a problem to expose the headers in the
new ProcessingExceptionHandler, but that it's not a problem for the
existing handler?
If there is no real concern about the KS engine requiring those headers, it
feels hard to mentally justify the complexity we transfer to the user by
exposing a new concept into the callbacks to represent the headers. In the
end, it strays aways from the simple/consistent representation of Headers
used all over. Even if eventually the KS engine needs to use the headers
after the callbacks with certainty that they were not altered, still feels
like it's something we could attempt to solve internally, without having to
transfer "new concepts" into the user (ex. the deep-copy as it was
suggested, seems like the kind of trade-off that would maybe be acceptable
here to gain simplicity and consistency among the handlers with a single
existing representation of Headers).
Best!
Lianet
On Tue, Apr 30, 2024 at 9:36 PM Matthias J. Sax wrote:
> Thanks for the update.
>
> I am wondering if we should use `ReadOnlyHeaders` instead of
> `ImmutableHeaders` as interface name?
>
> Also, the returned `Header` interface is technically not immutable
> either, because `Header#key()` returns a mutable byte-array... Would we
> need a `ReadOnlyHeader` interface?
>
> If yes, it seems that `ReadOnlyHeaders` should not be a super-interface
> of `Headers` but it would rather be a standalone interface, and a
> wrapper for a `Headers` instance? And `ReadOnlyHeader` would return some
> immutable type instead of `byte[]` for the value()?
>
> An alternative would be to deep-copy the value byte-array what would not
> be free, but given that we are talking about exception handling, it
> would not be on the hot code path, and thus might be acceptable?
>
>
> The above seems to increase the complexity significantly though. Hence,
> I have seconds thoughts on the immutability question:
>
> Do we really need to worry about mutability after all, because in the
> end, KS runtime won't read the Headers instance after the handler was
> called, and if a user modifies the passed in headers, there won't be any
> actual damage (ie, no side effects)? For this case, it might even be ok
> to also not add `ImmutableHeaders` to begin with?
>
>
>
> Sorry for the forth and back (yes, forth and back, because back and
> forth does not make sense -- it's not logical -- just trying to fix
> English :D) as I did bring up the immutability question in the first
> place...
>
>
>
> -Matthias
>
> On 4/25/24 5:56 AM, Loic Greffier wrote:
> > Hi Matthias,
> >
> > I have updated the KIP regarding points 103 and 108.
> >
> > 103.
> > I have suggested a new `ImmutableHeaders` interface to deal with the
> > immutability concern of the headers, which is basically the `Headers`
> > interface without the write accesses.
> >
> > public interface ImmutableHeaders {
> > Header lastHeader(String key);
> > Iterable headers(String key);
> > Header[] toArray();
> > }
> >
> > The `Headers` interface can be updated accordingly:
> >
> > public interface Headers extends ImmutableHeaders, Iterable {
> > //…
> > }
> >
> > Loïc
>
Re: [VOTE] 3.6.2 RC2
Hi Manikumar, I did the following checks: - downloaded and built from src - ran all unit test and integration test for clients - ran quickstart with Kraft mode - ran simple workloads with the console consumer/producer - checked all links All looks good to me with this. +1 (non-binding) Thanks! On Wed, Apr 3, 2024, 2:19 a.m. Manikumar wrote: > Gentle reminder. Please download, test and vote for the release. > > Thanks, > > On Fri, Mar 29, 2024 at 4:57 PM Manikumar wrote: > > > Hi All, > > > > System test runs are green. There were 13 test failures in the first run. > > All the failed tests passed in the second run. > > > > System test results: > > https://gist.github.com/omkreddy/17d23d3eb36ef840011f2494d65bbd4f > > > > Thanks, > > > > On Thu, Mar 28, 2024 at 3:21 PM Manikumar wrote: > > > >> Hello Kafka users, developers and client-developers, > >> > >> This is the second candidate we are considering for the release of > Apache > >> Kafka 3.6.2. > >> > >> This is a bugfix release with several fixes, including dependency > >> version bumps for CVEs. > >> > >> Release notes for the 3.6.2 release: > >> https://home.apache.org/~manikumar/kafka-3.6.2-rc2/RELEASE_NOTES.html > >> > >> *** Please download, test and vote by by Wednesday, April 3rd > >> > >> Kafka's KEYS file containing PGP keys we use to sign the release: > >> https://kafka.apache.org/KEYS > >> > >> * Release artifacts to be voted upon (source and binary): > >> https://home.apache.org/~manikumar/kafka-3.6.2-rc2/ > >> > >> > >> * Maven artifacts to be voted upon: > >> https://repository.apache.org/content/groups/staging/org/apache/kafka/ > >> > >> * Javadoc: > >> https://home.apache.org/~manikumar/kafka-3.6.2-rc2/javadoc/ > >> > >> * Tag to be voted upon (off 3.6 branch) is the 3.6.2 tag: > >> https://github.com/apache/kafka/releases/tag/3.6.2-rc2 > >> > >> * Documentation: > >> https://kafka.apache.org/36/documentation.html > >> > >> * Protocol: > >> https://kafka.apache.org/36/protocol.html > >> > >> * Successful Jenkins builds for the 3.6 branch: > >> Unit/integration tests: > >> https://ci-builds.apache.org/job/Kafka/job/kafka/job/3.6/167/ (test > >> failures passed in local runs) > >> System tests: I will update system test results soon. > >> > >> > >> Thanks, > >> Manikumar > >> > > >
Re: [VOTE] KIP-932: Queues for Kafka
Hi Andrew, Happy to see this KIP unleashing the capabilities of Kafka, from the core up, to elegantly bring in a new, long wanted consumption pattern. +1 (non-binding) Lianet On Sun, Mar 17, 2024, 7:06 p.m. Andrew Schofield < andrew_schofield_j...@outlook.com> wrote: > Hi, > I’ve been working to complete KIP-932 over the past few months and > discussions have quietened down. > > I’d like to open the voting for KIP-932: > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-932%3A+Queues+for+Kafka > > Thanks, > Andrew
Re: [ANNOUNCE] New committer: Lucas Brutschy
Congratulations Lucas! On Thu, Sept 21, 2023, 11:45 a.m. Bruno Cadonna wrote: > Hi all, > > The PMC of Apache Kafka is pleased to announce a new Kafka committer > Lucas Brutschy. > > Lucas' major contributions are around Kafka Streams. > > Lucas' significantly contributed to the state updater > (https://issues.apache.org/jira/browse/KAFKA-10199) and he drives the > implementation of the new threading model for Kafka Streams > (https://issues.apache.org/jira/browse/KAFKA-15326). > > Lucas' contributions to KIP discussions and PR reviews are very thoughtful. > > Congratulations, Lucas! > > Thanks, > > Bruno (on behalf of the Apache Kafka PMC) >
Request to contribute to Kafka
Wiki ID: lianetmr Jira ID: lianetm Both on email liane...@gmail.com Thanks! Lianet Magrans
