-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix PartitionedProducerImpl flushAsync always fail when one partition send TimeOutException #14602
Conversation
… send TimeOutException
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerImpl.java
Outdated
Show resolved
Hide resolved
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerImpl.java
Outdated
Show resolved
Hide resolved
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerImpl.java
Outdated
Show resolved
Hide resolved
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerImpl.java
Outdated
Show resolved
Hide resolved
pulsar-client/src/test/java/org/apache/pulsar/client/impl/PartitionedProducerImplTest.java
Outdated
Show resolved
Hide resolved
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerImpl.java
Outdated
Show resolved
Hide resolved
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerImpl.java
Outdated
Show resolved
Hide resolved
pulsar-client/src/test/java/org/apache/pulsar/client/impl/PartitionedProducerImplTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I support @lhotari 's suggestions.
pulsar-client/src/test/java/org/apache/pulsar/client/impl/PartitionedProducerImplTest.java
Outdated
Show resolved
Hide resolved
@lhotari @michaeljmarshall @eolivelli Thanks much for your review and help. I add a test |
/pulsarbot run-failure-checks |
pulsar-broker/src/test/java/org/apache/pulsar/client/api/SimpleProducerConsumerTest.java
Outdated
Show resolved
Hide resolved
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerImpl.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Good work @wenbingshen. Thank you for the contribution.
@lhotari Thanks for your review and great comments. :) |
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerImpl.java
Show resolved
Hide resolved
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerImpl.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, just left some minor comments.
pulsar-client-api/src/main/java/org/apache/pulsar/client/api/Producer.java
Outdated
Show resolved
Hide resolved
pulsar-client/src/main/java/org/apache/pulsar/client/impl/ProducerImpl.java
Outdated
Show resolved
Hide resolved
pulsar-broker/src/test/java/org/apache/pulsar/client/api/SimpleProducerConsumerTest.java
Show resolved
Hide resolved
pulsar-broker/src/test/java/org/apache/pulsar/client/api/SimpleProducerConsumerTest.java
Show resolved
Hide resolved
/pulsarbot run-failure-checks |
… send TimeOutException (#14602) Fixes #14598 Master Issue: #14598 Detailed issue description can be found at #14598 After the `lastSendFuture` returned to application from in `org.apache.pulsar.client.impl.ProducerImpl#flushAsync` acquired , it should not continue to be thrown to the application, the `lastSendFuture` whether an exception occurs or completed, and the application is only allowed to acquire it once, otherwise it will cause `org. apache.pulsar.client.impl.PartitionedProducerImpl#flushAsync ` cannot continue to send data until data is sent again to the abnormal `ProducerImpl`. (cherry picked from commit ddca852)
… send TimeOutException (#14602) Fixes #14598 Master Issue: #14598 ### Motivation Detailed issue description can be found at #14598 ### Modifications After the `lastSendFuture` returned to application from in `org.apache.pulsar.client.impl.ProducerImpl#flushAsync` acquired , it should not continue to be thrown to the application, the `lastSendFuture` whether an exception occurs or completed, and the application is only allowed to acquire it once, otherwise it will cause `org. apache.pulsar.client.impl.PartitionedProducerImpl#flushAsync ` cannot continue to send data until data is sent again to the abnormal `ProducerImpl`. (cherry picked from commit ddca852)
… send TimeOutException (apache#14602) Fixes apache#14598 Master Issue: apache#14598 ### Motivation Detailed issue description can be found at apache#14598 ### Modifications After the `lastSendFuture` returned to application from in `org.apache.pulsar.client.impl.ProducerImpl#flushAsync` acquired , it should not continue to be thrown to the application, the `lastSendFuture` whether an exception occurs or completed, and the application is only allowed to acquire it once, otherwise it will cause `org. apache.pulsar.client.impl.PartitionedProducerImpl#flushAsync ` cannot continue to send data until data is sent again to the abnormal `ProducerImpl`.
Fixes #14598
Master Issue: #14598
Motivation
Detailed issue description can be found at #14598
Modifications
After the
lastSendFuture
returned to application from inorg.apache.pulsar.client.impl.ProducerImpl#flushAsync
acquired , it should not continue to be thrown to the application, thelastSendFuture
whether an exception occurs or completed, and the application is only allowed to acquire it once, otherwise it will causeorg. apache.pulsar.client.impl.PartitionedProducerImpl#flushAsync
cannot continue to send data until data is sent again to the abnormalProducerImpl
.Documentation
Check the box below or label this PR directly (if you have committer privilege).
Need to update docs?
no-need-doc