[improve][broker] Remove the atomicity on active consumer of a dispatcher #22285
Conversation
This file contains 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
…cher ### Motivation The modifications on `AbstractDispatcherSingleActiveConsumer#activeConsumer` only happen on `pickAndScheduleActiveConsumer`, which is only called in `addConsumer` and `removeConsumer`. However, both `addConsumer` and `removeConsumer` are synchronized methods so that we don't have to perform an atomic operation on `activeConsumer`. The composite operations on `activeConsumer` also happen on synchronized methods like `disconnectActiveConsumers`. The only access to `activeConsumer` that is not synchronized is the read on `activeConsumer` like `getActiveConsumer()` and `isConsumerConnected()`. ### Modifications Remove the `ACTIVE_CONSUMER_UPDATER` and note some methods are not thread safe or there is no need to make them thread safe. Besides, since `pickAndScheduleActiveConsumer` is never called concurrently, we don't need a double-checked locking on `readOnActiveConsumerTask`. Instead, just synchronize the modification in the scheduled task.
BewareMyPower
added
type/enhancement
BewareMyPower
requested review from
merlimat,
lhotari,
Technoboy-,
codelipenghui,
poorbarcode and
mattisonchao
Merged
4 tasks
poorbarcode
approved these changes
Mar 16, 2024
Technoboy-
approved these changes
Mar 18, 2024
hanmz
approved these changes
Mar 19, 2024
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.
Motivation
The modifications on
AbstractDispatcherSingleActiveConsumer#activeConsumeronly happen onpickAndScheduleActiveConsumer, which is only called inaddConsumerandremoveConsumer. However, bothaddConsumerandremoveConsumerare synchronized methods so that we don't have to perform an atomic operation onactiveConsumer.The composite operations on
activeConsumeralso happen on synchronized methods likedisconnectActiveConsumers. The only access toactiveConsumerthat is not synchronized is the read onactiveConsumerlikegetActiveConsumer()andisConsumerConnected().Modifications
Remove the
ACTIVE_CONSUMER_UPDATERand note some methods are not thread safe or there is no need to make them thread safe.Besides, since
pickAndScheduleActiveConsumeris never called concurrently, we don't need a double-checked locking onreadOnActiveConsumerTask. Instead, just synchronize the modification in the scheduled task.Documentation
docdoc-requireddoc-not-neededdoc-completeMatching PR in forked repository
PR in forked repository: