You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2022/06/08 04:27:49 UTC
[GitHub] [kafka] C0urante opened a new pull request, #12264: KAFKA-13967: Document guarantees for producer callbacks on transaction commit
C0urante opened a new pull request, #12264:
URL: https://github.com/apache/kafka/pull/12264
[Jira](https://issues.apache.org/jira/browse/KAFKA-13967)
Also added some `<p>` tags to help organize the rendered Javadocs.
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] hachikuji commented on a diff in pull request #12264: KAFKA-13967: Document guarantees for producer callbacks on transaction commit
Posted by GitBox <gi...@apache.org>.
hachikuji commented on code in PR #12264:
URL: https://github.com/apache/kafka/pull/12264#discussion_r892886145
##########
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java:
##########
@@ -754,11 +754,16 @@ public void sendOffsetsToTransaction(Map<TopicPartition, OffsetAndMetadata> offs
/**
* Commits the ongoing transaction. This method will flush any unsent records before actually committing the transaction.
- *
+ * <p>
* Further, if any of the {@link #send(ProducerRecord)} calls which were part of the transaction hit irrecoverable
* errors, this method will throw the last received exception immediately and the transaction will not be committed.
* So all {@link #send(ProducerRecord)} calls in a transaction must succeed in order for this method to succeed.
- *
+ * <p>
+ * If the transaction is committed successfully and this method returns without throwing an exception, it is guaranteed
+ * that all {@link Callback callbacks} for records in the transaction will have been invoked and completed, either
+ * successfully or by raising an exception. Exceptions thrown by callbacks are ignored; the producer proceeds to commit
Review Comment:
I don't think we allow a commit to proceed if any of the sends fail.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] hachikuji commented on a diff in pull request #12264: KAFKA-13967: Document guarantees for producer callbacks on transaction commit
Posted by GitBox <gi...@apache.org>.
hachikuji commented on code in PR #12264:
URL: https://github.com/apache/kafka/pull/12264#discussion_r893780149
##########
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java:
##########
@@ -754,11 +754,16 @@ public void sendOffsetsToTransaction(Map<TopicPartition, OffsetAndMetadata> offs
/**
* Commits the ongoing transaction. This method will flush any unsent records before actually committing the transaction.
- *
+ * <p>
* Further, if any of the {@link #send(ProducerRecord)} calls which were part of the transaction hit irrecoverable
* errors, this method will throw the last received exception immediately and the transaction will not be committed.
* So all {@link #send(ProducerRecord)} calls in a transaction must succeed in order for this method to succeed.
- *
+ * <p>
+ * If the transaction is committed successfully and this method returns without throwing an exception, it is guaranteed
+ * that all {@link Callback callbacks} for records in the transaction will have been invoked and completed, either
+ * successfully or by raising an exception. Exceptions thrown by callbacks are ignored; the producer proceeds to commit
Review Comment:
Hmm, I think my confusion came from the phrase "either successfully or by raising an exception." I thought this was referring to the callback result (i.e. what we are passing to `Callback.onCompletion`). Perhaps we could just leave that part out since the next sentence is clarifying anyway?
> If the transaction is committed successfully and this method returns without throwing an exception, it is guaranteed that all {@link Callback callbacks} for records in the transaction will have been invoked and completed. Note that exceptions thrown by callbacks are ignored; the producer proceeds to commit the transaction in any case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] hachikuji merged pull request #12264: KAFKA-13967: Document guarantees for producer callbacks on transaction commit
Posted by GitBox <gi...@apache.org>.
hachikuji merged PR #12264:
URL: https://github.com/apache/kafka/pull/12264
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] C0urante commented on a diff in pull request #12264: KAFKA-13967: Document guarantees for producer callbacks on transaction commit
Posted by GitBox <gi...@apache.org>.
C0urante commented on code in PR #12264:
URL: https://github.com/apache/kafka/pull/12264#discussion_r892901863
##########
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java:
##########
@@ -754,11 +754,16 @@ public void sendOffsetsToTransaction(Map<TopicPartition, OffsetAndMetadata> offs
/**
* Commits the ongoing transaction. This method will flush any unsent records before actually committing the transaction.
- *
+ * <p>
* Further, if any of the {@link #send(ProducerRecord)} calls which were part of the transaction hit irrecoverable
* errors, this method will throw the last received exception immediately and the transaction will not be committed.
* So all {@link #send(ProducerRecord)} calls in a transaction must succeed in order for this method to succeed.
- *
+ * <p>
+ * If the transaction is committed successfully and this method returns without throwing an exception, it is guaranteed
+ * that all {@link Callback callbacks} for records in the transaction will have been invoked and completed, either
+ * successfully or by raising an exception. Exceptions thrown by callbacks are ignored; the producer proceeds to commit
Review Comment:
Does a failing callback qualify as a failing send? Otherwise I'm not sure what part of this implies that a transaction will proceed if a send fails, and the previous paragraph explicitly calls that out already.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] C0urante commented on a diff in pull request #12264: KAFKA-13967: Document guarantees for producer callbacks on transaction commit
Posted by GitBox <gi...@apache.org>.
C0urante commented on code in PR #12264:
URL: https://github.com/apache/kafka/pull/12264#discussion_r892549850
##########
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java:
##########
@@ -754,11 +754,16 @@ public void sendOffsetsToTransaction(Map<TopicPartition, OffsetAndMetadata> offs
/**
* Commits the ongoing transaction. This method will flush any unsent records before actually committing the transaction.
- *
+ * <p>
* Further, if any of the {@link #send(ProducerRecord)} calls which were part of the transaction hit irrecoverable
* errors, this method will throw the last received exception immediately and the transaction will not be committed.
* So all {@link #send(ProducerRecord)} calls in a transaction must succeed in order for this method to succeed.
- *
+ * <p>
+ * If the transaction is committed successfully and this method returns without throwing an exception, it is guaranteed
+ * that all {@link Callback callbacks} for records in the transaction will have been invoked and completed, either
+ * successfully or by raising an exception. In the event that a callback fails (i.e., raises an exception), the producer
+ * will still proceed with the transaction commit.
Review Comment:
To me, "invoked" means that the method has been called but not necessarily that it has completed (i.e., returned or thrown an exception), and given the multi-threaded nature of the producer, I'd like it if we could make that distinction so that there's no longer any room for doubt in users' minds on that front.
Fine with the new language in the last sentence though 👍
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] C0urante commented on a diff in pull request #12264: KAFKA-13967: Document guarantees for producer callbacks on transaction commit
Posted by GitBox <gi...@apache.org>.
C0urante commented on code in PR #12264:
URL: https://github.com/apache/kafka/pull/12264#discussion_r894119117
##########
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java:
##########
@@ -754,11 +754,16 @@ public void sendOffsetsToTransaction(Map<TopicPartition, OffsetAndMetadata> offs
/**
* Commits the ongoing transaction. This method will flush any unsent records before actually committing the transaction.
- *
+ * <p>
* Further, if any of the {@link #send(ProducerRecord)} calls which were part of the transaction hit irrecoverable
* errors, this method will throw the last received exception immediately and the transaction will not be committed.
* So all {@link #send(ProducerRecord)} calls in a transaction must succeed in order for this method to succeed.
- *
+ * <p>
+ * If the transaction is committed successfully and this method returns without throwing an exception, it is guaranteed
+ * that all {@link Callback callbacks} for records in the transaction will have been invoked and completed, either
+ * successfully or by raising an exception. Exceptions thrown by callbacks are ignored; the producer proceeds to commit
Review Comment:
Sounds good, done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] hachikuji commented on a diff in pull request #12264: KAFKA-13967: Document guarantees for producer callbacks on transaction commit
Posted by GitBox <gi...@apache.org>.
hachikuji commented on code in PR #12264:
URL: https://github.com/apache/kafka/pull/12264#discussion_r892886145
##########
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java:
##########
@@ -754,11 +754,16 @@ public void sendOffsetsToTransaction(Map<TopicPartition, OffsetAndMetadata> offs
/**
* Commits the ongoing transaction. This method will flush any unsent records before actually committing the transaction.
- *
+ * <p>
* Further, if any of the {@link #send(ProducerRecord)} calls which were part of the transaction hit irrecoverable
* errors, this method will throw the last received exception immediately and the transaction will not be committed.
* So all {@link #send(ProducerRecord)} calls in a transaction must succeed in order for this method to succeed.
- *
+ * <p>
+ * If the transaction is committed successfully and this method returns without throwing an exception, it is guaranteed
+ * that all {@link Callback callbacks} for records in the transaction will have been invoked and completed, either
+ * successfully or by raising an exception. Exceptions thrown by callbacks are ignored; the producer proceeds to commit
Review Comment:
I don't think we allow a commit to proceed if any of the sends fail. The user has to abort.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] tombentley commented on a diff in pull request #12264: KAFKA-13967: Document guarantees for producer callbacks on transaction commit
Posted by GitBox <gi...@apache.org>.
tombentley commented on code in PR #12264:
URL: https://github.com/apache/kafka/pull/12264#discussion_r892246131
##########
clients/src/main/java/org/apache/kafka/clients/producer/KafkaProducer.java:
##########
@@ -754,11 +754,16 @@ public void sendOffsetsToTransaction(Map<TopicPartition, OffsetAndMetadata> offs
/**
* Commits the ongoing transaction. This method will flush any unsent records before actually committing the transaction.
- *
+ * <p>
* Further, if any of the {@link #send(ProducerRecord)} calls which were part of the transaction hit irrecoverable
* errors, this method will throw the last received exception immediately and the transaction will not be committed.
* So all {@link #send(ProducerRecord)} calls in a transaction must succeed in order for this method to succeed.
- *
+ * <p>
+ * If the transaction is committed successfully and this method returns without throwing an exception, it is guaranteed
+ * that all {@link Callback callbacks} for records in the transaction will have been invoked and completed, either
+ * successfully or by raising an exception. In the event that a callback fails (i.e., raises an exception), the producer
+ * will still proceed with the transaction commit.
Review Comment:
```suggestion
* If the transaction is committed successfully and this method returns without throwing an exception, it is guaranteed
* that all {@link Callback callbacks} for records in the transaction will have been invoked. Exceptions thrown by
* callbacks are ignored; the producer proceeds to commit the transaction in any case.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org