Sorry, but this KIP is still open to discussion, any comments and ideas
would be appreciated, Thanks.

张祥 <xiangzhang1...@gmail.com> 于2020年4月17日周五 下午1:04写道:

> Guozhang, thanks for the valuable suggestion.
>
> A new part called "suggested coding pattern" has been added and I copy the
> core code here:
>
> 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 (Exception e) {
>     producer.abortTransaction();
>     if(e instanceof IllegalStateException ||
>         e instanceof ProducerFencedException ||
>         e instanceof UnsupportedVersionException ||
>         e instanceof AuthorizationException ||
>         e instanceof OutOfOrderSequenceException) {
>         producer.close();
>     }
> }
>
> As you can see, in the catch block,  all fatal exceptions need to be
> listed, I am not sure I have listed all of them and I wonder if there is a
> better way to do this.
>
>
> Guozhang Wang <wangg...@gmail.com> 于2020年4月17日周五 上午8:50写道:
>
>> 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
>>
>

Reply via email to