Xiang, thanks for the written KIP. I just have one meta comment and
otherwise it looks good to me: could you also add a section about suggested
coding patterns (especially how try - catch should be implemented) as we
discussed on the JIRA to the wiki page as well?
And please also note that besides the javadoc of the function, on top of
the KafkaProducer class there are also comments regarding example snippet:
```
* <pre>
* {@code
* Properties props = new Properties();
* props.put("bootstrap.servers", "localhost:9092");
* props.put("transactional.id", "my-transactional-id");
* Producer<String, String> producer = new KafkaProducer<>(props, new
StringSerializer(), new StringSerializer());
*
* producer.initTransactions();
*
* try {
* producer.beginTransaction();
* for (int i = 0; i < 100; i++)
* producer.send(new ProducerRecord<>("my-topic",
Integer.toString(i), Integer.toString(i)));
* producer.commitTransaction();
* } catch (ProducerFencedException | OutOfOrderSequenceException |
AuthorizationException e) {
* // We can't recover from these exceptions, so our only option is
to close the producer and exit.
* producer.close();
* } catch (KafkaException e) {
* // For all other exceptions, just abort the transaction and try again.
* producer.abortTransaction();
* }
* producer.close();
* } </pre>
```
I think with this change we do not need to educate users that they should
distinguish the types of exceptions when calling `abortTxn`, instead they
only need to depend on the exception to decide whether to `close` the
producer, so the above recommendation could look like:
try {
} catch {Exception e} {
producer.abortTxn;
if (e instanceof /*fatal exceptions*/) {
producer.close();
}
}
Guozhang
On Thu, Apr 16, 2020 at 12:14 AM 张祥 <xiangzhang1...@gmail.com> wrote:
> Thanks for the structure change Boyang. And I agree with you on the weak
> proposal part, I have adjusted it according to your suggestion. Thanks
> again!
>
> Boyang Chen <reluctanthero...@gmail.com> 于2020年4月16日周四 下午2:39写道：
>
> > Thanks for the KIP Xiang!
> >
> > I think the motivation looks good, and I just did a slight structure
> change
> > to separate "Proposed Changes" and "Public Interfaces", hope you don't
> > mind.
> >
> > However, "we can determine whether the producer client is already in
> error
> > state in abortTransaction" sounds a bit weak about the actual proposal,
> > instead we could propose something as "we would remember whether a fatal
> > exception has already been thrown to the application level, so that in
> > abort transaction we will not throw again, thus making the function safe
> to
> > be called in an error state".
> >
> > Other than that, I think the KIP is in pretty good shape.
> >
> > Boyang
> >
> > On Wed, Apr 15, 2020 at 7:07 PM 张祥 <xiangzhang1...@gmail.com> wrote:
> >
> > > Hi everyone,
> > >
> > > I have opened a small KIP about safely aborting transaction during
> > > shutdown. I'd like to use this thread to discuss about it and any
> > feedback
> > > is appreciated (sorry for earlier KIP number mistake). Here is a link
> to
> > > KIP-596 :
> > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-596%3A+Safely+abort+Producer+transactions+during+application+shutdown
> > >
> > > Thank you!
> > >
> >
>
--
-- Guozhang
