-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[improve][broker]Remove block calling that named cursor.asyncGetNth when expiring messages #24606
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
[improve][broker]Remove block calling that named cursor.asyncGetNth when expiring messages #24606
Conversation
…hen expiring messages
dao-jun
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.
I‘m +1 with the change, but do we need to add tests for the new methods?
I have added the same test cases that call |
...rc/main/java/org/apache/pulsar/broker/service/persistent/PersistentMessageExpiryMonitor.java
Outdated
Show resolved
Hide resolved
...broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentSubscription.java
Outdated
Show resolved
Hide resolved
...rc/main/java/org/apache/pulsar/broker/service/persistent/PersistentMessageExpiryMonitor.java
Outdated
Show resolved
Hide resolved
...rc/main/java/org/apache/pulsar/broker/service/persistent/PersistentMessageExpiryMonitor.java
Outdated
Show resolved
Hide resolved
lhotari
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.
LGTM
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #24606 +/- ##
============================================
+ Coverage 74.21% 74.32% +0.11%
+ Complexity 33142 32789 -353
============================================
Files 1881 1881
Lines 146770 146814 +44
Branches 16859 16862 +3
============================================
+ Hits 108922 109125 +203
+ Misses 29181 29022 -159
Partials 8667 8667
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
…hen expiring messages (apache#24606)
…hen expiring messages (apache#24606)
…hen expiring messages (apache#24606)
…hen expiring messages (#24606)
…hen expiring messages (apache#24606) (cherry picked from commit 4a2dde2)
…hen expiring messages (apache#24606) (cherry picked from commit 4a2dde2)
…hen expiring messages (apache#24606)
…hen expiring messages (apache#24606)
Motivation
Broker checks TTL policies in the background per 5 minutes in a single thread. It calls a block method named
topic.isOldestMessageExpired, which causes a prolonged thread blockage.Modifications
Instead of calling a block method, call a non-blocked method
Documentation
docdoc-requireddoc-not-neededdoc-completeMatching PR in forked repository
PR in forked repository: x