Re: [DISCUSS] KIP-654 Aborting Transaction with non-flushed data should throw a non-fatal Exception

2020-09-11 Thread Gokul Srinivas
All, As the vote has passed, I have raised a PR here: https://github.com/apache/kafka/pull/9280 Please help review. Thanks, Gokul On 04-09-2020 00:48, Gokul Srinivas wrote: Appreciate the help! On 04-09-2020 00:46, Sophie Blee-Goldman wrote: Yep, you can go ahead and call for a vote on the

Re: [DISCUSS] KIP-654 Aborting Transaction with non-flushed data should throw a non-fatal Exception

2020-09-03 Thread Gokul Srinivas
Appreciate the help! On 04-09-2020 00:46, Sophie Blee-Goldman wrote: Yep, you can go ahead and call for a vote on the KIP On Thu, Sep 3, 2020 at 12:09 PM Gokul Srinivas wrote: Sophie, That sounds fair. I've updated the KIP to state the same message for backward compatibility to

Re: [DISCUSS] KIP-654 Aborting Transaction with non-flushed data should throw a non-fatal Exception

2020-09-03 Thread Sophie Blee-Goldman
Yep, you can go ahead and call for a vote on the KIP On Thu, Sep 3, 2020 at 12:09 PM Gokul Srinivas wrote: > Sophie, > > That sounds fair. I've updated the KIP to state the same message for > backward compatibility to existing (albeit hacky) solutions. > > As this is my first ever contribution

Re: [DISCUSS] KIP-654 Aborting Transaction with non-flushed data should throw a non-fatal Exception

2020-09-03 Thread Gokul Srinivas
Sophie, That sounds fair. I've updated the KIP to state the same message for backward compatibility to existing (albeit hacky) solutions. As this is my first ever contribution - is the next step to initiate the voting on this KIP? -Gokul On 04-09-2020 00:34, Sophie Blee-Goldman wrote: I

Re: [DISCUSS] KIP-654 Aborting Transaction with non-flushed data should throw a non-fatal Exception

2020-09-03 Thread Sophie Blee-Goldman
I think the current proposal looks good to me. One minor suggestion I have is to consider keeping the same error message: Failing batch since transaction was aborted When we were running into this issue in Streams and accidentally rethrowing the KafkaException as fatal, we ended up checking the

Re: [DISCUSS] KIP-654 Aborting Transaction with non-flushed data should throw a non-fatal Exception

2020-09-03 Thread Gokul Srinivas
All, Gentle reminder - any comments on the line of thinking I mentioned in the last email? I've updated the Exception to be named "TransactionAbortedException" on the KIP confluence page. -Gokul On 01-09-2020 18:34, Gokul Srinivas wrote: Matthias, Sophie, Jason, Took another pass at

Re: [DISCUSS] KIP-654 Aborting Transaction with non-flushed data should throw a non-fatal Exception

2020-09-01 Thread Gokul Srinivas
Matthias, Sophie, Jason, Took another pass at understanding the internals and it seems to me like we should be extending the `ApiException` rather than the `RetriableException`. The check in question = Do we abort any undrained batches that are present on this

Re: [DISCUSS] KIP-654 Aborting Transaction with non-flushed data should throw a non-fatal Exception

2020-08-28 Thread Jason Gustafson
Hi Gokul, Thanks, I think it makes sense to use a separate exception type. +1 on Sophie's suggestion for `TransactionAbortedException`. Extending from `RetriableException` seems reasonable as well. I guess the only question is whether it's safe to catch it as a `RetriableException` and apply

Re: [DISCUSS] KIP-654 Aborting Transaction with non-flushed data should throw a non-fatal Exception

2020-08-27 Thread Sophie Blee-Goldman
Hey Gokul, thanks for taking up this KIP! I agree with Matthias that directly extending KafkaException may not be ideal, and we should instead extend APIException or RetriableException. Of the two, I think APIException would be more appropriate. My understanding is that RetriableException is

Re: [DISCUSS] KIP-654 Aborting Transaction with non-flushed data should throw a non-fatal Exception

2020-08-27 Thread Matthias J. Sax
Thanks for the KIP. Looks good overall. However, I am wondering if the new exception should extend `KafkaException`? It seems, extending `ApiException` or maybe even `RetriableException` might be better? About the name itself. I would prefer something simpler like `AbortedTransactionException`.

[DISCUSS] KIP-654 Aborting Transaction with non-flushed data should throw a non-fatal Exception

2020-08-27 Thread Gokul Srinivas
Hello all, I would like to propose the following KIP to throw a new non-fatal exception whilst aborting transactions with non-flushed data. This will help users distinguish non-fatal errors and potentially retry the batch. *Issue *- https://issues.apache.org/jira/browse/KAFKA-10186