[jira] [Comment Edited] (KAFKA-10186) Aborting transaction with pending data should throw non-fatal exception
[
https://issues.apache.org/jira/browse/KAFKA-10186?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17172514#comment-17172514
]
Gokul Srinivas edited comment on KAFKA-10186 at 8/6/20, 4:25 PM:
-
[~mjsax] [~ableegoldman]
I've created a work-in-progress KIP page. Should I also take a pass at the
code, look at how best I can create the new exception and then add that to the
KIP as well? I'll send an email to the dev mailing list once I have run the
initial draft by you.
[KIP-654|https://cwiki.apache.org/confluence/display/KAFKA/KIP-654%3A+Aborted+transaction+with+non-flushed+data+should+throw+a+non-fatal+exception]
was (Author: nym3r0s):
[~mjsax] [~ableegoldman]
I've created a work-in-progress KIP page. Should I also take a pass at the
code, look at how best I can create the new exception and then add that to the
KIP as well?
[KIP-654|https://cwiki.apache.org/confluence/display/KAFKA/KIP-654%3A+Aborted+transaction+with+non-flushed+data+should+throw+a+non-fatal+exception]
> Aborting transaction with pending data should throw non-fatal exception
> ---
>
> Key: KAFKA-10186
> URL: https://issues.apache.org/jira/browse/KAFKA-10186
> Project: Kafka
> Issue Type: Improvement
> Components: producer
>Reporter: Sophie Blee-Goldman
>Assignee: Gokul Srinivas
>Priority: Major
> Labels: needs-kip, newbie, newbie++
>
> Currently if you try to abort a transaction with any pending (non-flushed)
> data, the send exception is set to
> {code:java}
> KafkaException("Failing batch since transaction was aborted"){code}
> This exception type is generally considered fatal, but this is a valid state
> to be in -- the point of throwing the exception is to alert that the records
> will not be sent, not that you are in an unrecoverable error state.
> We should throw a different (possibly new) type of exception here to
> distinguish from fatal and recoverable errors.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Comment Edited] (KAFKA-10186) Aborting transaction with pending data should throw non-fatal exception
[
https://issues.apache.org/jira/browse/KAFKA-10186?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17169052#comment-17169052
]
John Thomas edited comment on KAFKA-10186 at 8/1/20, 7:08 AM:
--
[~ableegoldman] My understanding after reading the description is that, If we
abort a transaction with any non-flushed data, we want to throw a different
exception, since we know its non-fatal.
Going through the code I see that , {color:#172b4d}In
Sender#maybeSendAndPollTransactionalRequest :
transactionManaer.hasAbortableError() -> This is fatal, {color}
{color:#172b4d}transactionManager.isAborting() - > This is something we know
that its aborted, and is recoverable.{color}
{color:#172b4d}You are suggesting we should put in a new exception message to
differentiate ?Or a whole new Exception ?
{color}
{code:java}
if (transactionManager.hasAbortableError() || transactionManager.isAborting()) {
if (accumulator.hasIncomplete()) {
RuntimeException exception = transactionManager.lastError();
if (exception == null) {
exception = new KafkaException("Failing batch since transaction was
aborted");
}
accumulator.abortUndrainedBatches(exception);
}
}{code}
PS : #newbie !
was (Author: johnthotekat):
[~ableegoldman] For my understanding, If we abort a transaction with any
non-flushed data, we want to throw a different exception, since we know its
non-fatal ?
{color:#172b4d}In Sender#maybeSendAndPollTransactionalRequest :
transactionManaer.hasAbortableError() -> This is fatal, {color}
{color:#172b4d}transactionManager.isAborting() - > This is something we know
that its aborted, and is recoverable.
{color}
{color:#172b4d}You are suggesting we should put in a new exception message ? or
a whole new exception class ? {color}
{code:java}
if (transactionManager.hasAbortableError() || transactionManager.isAborting()) {
if (accumulator.hasIncomplete()) {
RuntimeException exception = transactionManager.lastError();
if (exception == null) {
exception = new KafkaException("Failing batch since transaction was
aborted");
}
accumulator.abortUndrainedBatches(exception);
}
}{code}
PS : #newbie !
> Aborting transaction with pending data should throw non-fatal exception
> ---
>
> Key: KAFKA-10186
> URL: https://issues.apache.org/jira/browse/KAFKA-10186
> Project: Kafka
> Issue Type: Improvement
> Components: producer
>Reporter: Sophie Blee-Goldman
>Priority: Major
> Labels: needs-kip, newbie, newbie++
>
> Currently if you try to abort a transaction with any pending (non-flushed)
> data, the send exception is set to
> {code:java}
> KafkaException("Failing batch since transaction was aborted"){code}
> This exception type is generally considered fatal, but this is a valid state
> to be in -- the point of throwing the exception is to alert that the records
> will not be sent, not that you are in an unrecoverable error state.
> We should throw a different (possibly new) type of exception here to
> distinguish from fatal and recoverable errors.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Comment Edited] (KAFKA-10186) Aborting transaction with pending data should throw non-fatal exception
[
https://issues.apache.org/jira/browse/KAFKA-10186?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17169052#comment-17169052
]
John Thomas edited comment on KAFKA-10186 at 8/1/20, 7:02 AM:
--
[~ableegoldman] For my understanding, If we abort a transaction with any
non-flushed data, we want to throw a different exception, since we know its
non-fatal ?
{color:#172b4d}In Sender#maybeSendAndPollTransactionalRequest :
transactionManaer.hasAbortableError() -> This is fatal, {color}
{color:#172b4d}transactionManager.isAborting() - > This is something we know
that its aborted, and is recoverable.
{color}
{color:#172b4d}You are suggesting we should put in a new exception message ? or
a whole new exception class ? {color}
{code:java}
if (transactionManager.hasAbortableError() || transactionManager.isAborting()) {
if (accumulator.hasIncomplete()) {
RuntimeException exception = transactionManager.lastError();
if (exception == null) {
exception = new KafkaException("Failing batch since transaction was
aborted");
}
accumulator.abortUndrainedBatches(exception);
}
}{code}
PS : #newbie !
was (Author: johnthotekat):
[~ableegoldman] If we abort a transaction with any non-flushed data, we want to
throw a different exception, since we know its non-fatal ?
{color:#172b4d}If my understanding is correct, In
Sender#maybeSendAndPollTransactionalRequest :
transactionManaer.hasAbortableError() -> This is fatal, {color}
{color:#172b4d}transactionManager.isAborting() - > This is something we know
that its aborted, and is recoverable. --
{color}
{code:java}
if (transactionManager.hasAbortableError() || transactionManager.isAborting()) {
if (accumulator.hasIncomplete()) {
RuntimeException exception = transactionManager.lastError();
if (exception == null) {
exception = new KafkaException("Failing batch since transaction was
aborted");
}
accumulator.abortUndrainedBatches(exception);
}
}{code}
PS : #newbie !
> Aborting transaction with pending data should throw non-fatal exception
> ---
>
> Key: KAFKA-10186
> URL: https://issues.apache.org/jira/browse/KAFKA-10186
> Project: Kafka
> Issue Type: Improvement
> Components: producer
>Reporter: Sophie Blee-Goldman
>Priority: Major
> Labels: needs-kip, newbie, newbie++
>
> Currently if you try to abort a transaction with any pending (non-flushed)
> data, the send exception is set to
> {code:java}
> KafkaException("Failing batch since transaction was aborted"){code}
> This exception type is generally considered fatal, but this is a valid state
> to be in -- the point of throwing the exception is to alert that the records
> will not be sent, not that you are in an unrecoverable error state.
> We should throw a different (possibly new) type of exception here to
> distinguish from fatal and recoverable errors.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
