[Branch-2.8] Fix ConsumerBuilderImpl#subscribeAsync blocks calling thread. - #14614
Merged
Merged
Conversation
codelipenghui
approved these changes
Mar 9, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cherry-pick #14433
Fixes #14413
Motivation
When Retry topics are enabled, ConsumerBuilderImpl performs a backwards-compatibility check to look for DLQ or Retry topics that were created on previous version of Pulsar using a different naming scheme.
It does this by calling
getPartitionedTopicMetadataand then using.get()to block while waiting for the resultsThis was implemented in #10129
A partial fix to add a Timeout was implemented in #11597
However this fix does still block the calling thread during the lookup.
This can be an issue for code that attempt to call
subscribeAsyncon a non-blocking Pool (such as when using Netty). The signature ofsubscribeAsyncimplies that it's non-blocking. (And it seems somewhat pointless to havesubscribeAsyncif it blocks anyway)Note that this is an undocumented breaking change in behavior. Up until 2.9, it was safe to call this from a non-blocking pool.
In our case, we were running a custom SLF4J log exporter on the same thread pool, which resulted in a deadlock when the number of concurrent
subscribeAsynccalls exceeded the pool size. (This is probably an extreme example and a poor decision on our part, but perhaps a good example of why unexpected blocking can be dangerous)Note that blocking call is still made even if the retry and DLQ names are explicitly specified in DeadLetterPolicy. In that scenario the check should not be needed. Aside from blocking the calling Thread, this also results in unneeded lookup requests.
Modifications
Documentation
no-need-doc