-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[fix][broker] Prevent unexpected recycle failure in dispatcher's read callback #24741
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][broker] Prevent unexpected recycle failure in dispatcher's read callback #24741
Conversation
|
While the reason of #24697 is still in investigation, this PR aims at the hard-to-debug recycling issue that did happen in real world by eliminating the recycler in read path |
...va/org/apache/pulsar/broker/service/persistent/PersistentDispatcherSingleActiveConsumer.java
Outdated
Show resolved
Hide resolved
...va/org/apache/pulsar/broker/service/persistent/PersistentDispatcherSingleActiveConsumer.java
Outdated
Show resolved
Hide resolved
nodece
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
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, great job @BewareMyPower.
|
Cherry-picking to branch-3.0 causes merge conflicts mainly due to #22285 being missing in branch-3.0, perhaps others. It was fairly easy to backport to branch-3.3. @BewareMyPower can you please handle cherry-picking to branch-3.0? I already resolved other merge conflicts in branch-3.3 commit 65ddd44 so you should be able to cherry-pick that to branch-3.0 once the preconditions in branch-3.0 have been addressed. |
|
Okay, I will do that |
… callback (apache#24741) (cherry picked from commit ccbd245) (cherry picked from commit 215a1fd)
… callback (apache#24741) (cherry picked from commit ccbd245) (cherry picked from commit 215a1fd)
… callback (apache#24741) (cherry picked from commit ccbd245)
Main Issue: #24697
Motivation
Using callback-based read API with a Netty recyclable object is buggy that once the
ReadEntriesCtxobject has been recycled twice.Modifications
asyncReadEntriesWithSkipOrWaittestConcurrentReadto verify concurrent read will failCompactedTopicUtils#asyncReadCompactedEntriesAPI to return a future as wellReadEntriesCtxIn extreme cases like #24697, it's safe to deliver messages to the consumer after that.
Documentation
docdoc-requireddoc-not-neededdoc-completeMatching PR in forked repository
PR in forked repository: