Hi Justine, I did not update the KIP with `TxnSendOption` since I thought it'd be better discussed here beforehand. right now, there are 2 PRs: - the PR that implements the current version of the KIP: https://github.com/apache/kafka/pull/16332 - the POC PR that clarifies the `TxnSendOption`: https://github.com/apache/kafka/pull/16465
Bests, Alieh On Thu, Jun 27, 2024 at 12:42 AM Justine Olshan <jols...@confluent.io.invalid> wrote: > Hey Alieh, > > I think I am a little confused. Are the 3 points above addressed by the KIP > or did something change? The PR seems to not include this change and still > has the CommitOption as well. > > Thanks, > Justine > > On Wed, Jun 26, 2024 at 2:15 PM Alieh Saeedi <asae...@confluent.io.invalid > > > wrote: > > > Hi all, > > > > > > Looking at the PR <https://github.com/apache/kafka/pull/16332> > > corresponding to the KIP, there are some points worthy of mention: > > > > > > 1) clearing the error ends up dirty/messy code in `TransactionManager`. > > > > 2) By clearing the error, we are actually doing an illegal transition > from > > `ABORTABLE_ERROR` to `IN_TRANSACTION` which is conceptually not > acceptable. > > This can be the root cause of some issues, with perhaps further future > > changes by others. > > > > 3) If the poison pill record `r1` causes a transition to the error state > > and then the next record `r2` requires adding a partition to the > > transaction, the action fails due to being in the error state. In this > > case, clearing errors during `commitTnx(CLEAR_SEND_ERROR)` is too late. > > However, this case can NOT be the main concern as soon as KIP-890 is > fully > > implemented. > > > > > > My suggestion is to solve the problem where it arises. If the transition > to > > the error state does not happen during `send()`, we do not need to clear > > the error later. Therefore, instead of `CommitOption`, we can define a > > `TxnSendOption` and prevent the `send()` method from going to the error > > state in case 1) we're in a transaction and 2) the user asked for > > IGONRE_SEND_ERRORS. For more clarity, you can take a look at the POC PR > > <https://github.com/apache/kafka/pull/16465>. > > > > Cheers, > > Alieh > > >
