-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[fix][ml] Fix the possibility of message loss or disorder when ML PayloadProcessor processing fails #24522
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][ml] Fix the possibility of message loss or disorder when ML PayloadProcessor processing fails #24522
Conversation
…ailed before write to bookies.
…ailed before write to bookies.
BewareMyPower
left a comment
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.
We should take care of the test time when adding tests.
pulsar-broker/src/test/java/org/apache/pulsar/broker/PublishWithMLPayloadProcessorTest.java
Outdated
Show resolved
Hide resolved
pulsar-broker/src/test/java/org/apache/pulsar/broker/PublishWithMLPayloadProcessorTest.java
Outdated
Show resolved
Hide resolved
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpAddEntry.java
Outdated
Show resolved
Hide resolved
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpAddEntry.java
Outdated
Show resolved
Hide resolved
pulsar-broker/src/test/java/org/apache/pulsar/broker/PublishWithMLPayloadProcessorTest.java
Outdated
Show resolved
Hide resolved
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/ManagedLedger.java
Outdated
Show resolved
Hide resolved
BewareMyPower
left a comment
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.
Improve the docs via Gemini
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/ManagedLedger.java
Show resolved
Hide resolved
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpAddEntry.java
Outdated
Show resolved
Hide resolved
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/ManagedLedger.java
Show resolved
Hide resolved
pulsar-broker/src/test/java/org/apache/pulsar/broker/PublishWithMLPayloadProcessorTest.java
Outdated
Show resolved
Hide resolved
pulsar-broker/src/test/java/org/apache/pulsar/broker/stats/MetricsAuthenticationTest.java
Outdated
Show resolved
Hide resolved
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpAddEntry.java
Show resolved
Hide resolved
managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/OpAddEntry.java
Outdated
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #24522 +/- ##
============================================
+ Coverage 73.57% 74.28% +0.71%
- Complexity 32624 32958 +334
============================================
Files 1877 1874 -3
Lines 139502 146244 +6742
Branches 15299 16773 +1474
============================================
+ Hits 102638 108640 +6002
- Misses 28908 28968 +60
- Partials 7956 8636 +680
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…loadProcessor processing fails (apache#24522) Co-authored-by: Yunze Xu <xyzinfernity@163.com> (cherry picked from commit a14794a)
…loadProcessor processing fails (apache#24522) Co-authored-by: Yunze Xu <xyzinfernity@163.com> (cherry picked from commit a14794a)
…loadProcessor processing fails (apache#24522) Co-authored-by: Yunze Xu <xyzinfernity@163.com> (cherry picked from commit a14794a) (cherry picked from commit 4f156b5)
…loadProcessor processing fails (apache#24522) Co-authored-by: Yunze Xu <xyzinfernity@163.com> (cherry picked from commit a14794a) (cherry picked from commit a378bdc)
…loadProcessor processing fails (apache#24522) Co-authored-by: Yunze Xu <xyzinfernity@163.com> (cherry picked from commit a14794a) (cherry picked from commit a378bdc)
…loadProcessor processing fails (apache#24522) Co-authored-by: Yunze Xu <xyzinfernity@163.com> (cherry picked from commit a14794a) (cherry picked from commit a378bdc)
…loadProcessor processing fails (apache#24522) Co-authored-by: Yunze Xu <xyzinfernity@163.com> (cherry picked from commit a14794a) (cherry picked from commit 4f156b5)
…loadProcessor processing fails (apache#24522) Co-authored-by: Yunze Xu <xyzinfernity@163.com>
…loadProcessor processing fails (apache#24522) Co-authored-by: Yunze Xu <xyzinfernity@163.com>
Motivation
In #23683 and #23817, we handled the failure of ML PayloadProcessor.
But it maybe lead to message lost or out-of-order.
You can reproduce the issue via
PublishWithMLPayloadProcessorTest:message-0~message-9to the topic and consume.The
message-3is lost3. When disable message deduplication, we can consume the following messages:
** The
message-3is aftermessage-4, message is out-of-order.The key-point is we didn't fail all the incoming messages until
Topicun-fenced.Modifications
Verifying this change
(Please pick either of the following options)
This change is a trivial rework / code cleanup without any test coverage.
(or)
This change is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(example:)
Does this pull request potentially affect one of the following parts:
If the box was checked, please highlight the changes
Documentation
docdoc-requireddoc-not-neededdoc-complete