[improve][broker] Choose random thread for consumerFlow in PersistentDispatcherSingleActiveConsumer #20522
Conversation
tisonkun
left a comment
There was a problem hiding this comment.
The reason we chooseThread(topicName) is that we'd like to order all tasks of the same topics.
This patch can introduce new race conditions that different consumers of the same topics now submit tasks that can be run concurrently. Please provide more details on how this patch won't cause such regressions.
why we need to use the same thread with the different sub in the same topic? #1522 only to mmon thread for frequence operations, don't need to promise the different dispatcher with the same topic need to use the same thread |
|
@congbobo184 Thank you! This is the evidence I ask for fulfilling the description :D |
725d1b9 to
2c8bc30
Compare
michaeljmarshall
left a comment
There was a problem hiding this comment.
@AnonHxy - do you have any metrics to show how this affects performance?
For what it's worth, I see that the PersistentDispatcherMultipleConsumers uses this same logic and that the primary synchronization withinPersistentDispatcherSingleActiveConsumer is on the object itself, so I don't see any reason that we shouldn't make this change.
There is no specific metrcs for this, but this patch solves the problem that too many subscriptions in one topic will consume slow in my in production environment. @michaeljmarshall |
…DispatcherSingleActiveConsumer (#20522) ### Motivation Currently, all subscriptions of one topic will do `consuemrFlow` action in a single thread, which is chosen by topicName: ``` this.topicExecutor = topic.getBrokerService().getTopicOrderedExecutor().chooseThread(topicName); ``` If there is a large number of subscriptions in a topic, all the work will focus on one thread ---- the chosen thread, which will reduce the consume performance. So this this patch , I'd like to choose a ramdom thread for `consumerFlow` in `PersistentDispatcherSingleActiveConsumer` to improve the consume performance. ### Modifications * `topic.getBrokerService().getTopicOrderedExecutor().chooseThread(topicName);` -> `topic.getBrokerService().getTopicOrderedExecutor().chooseThread();` * `this.topicExecutor ` -> `this.executor` (cherry picked from commit 51c2bb4)
…DispatcherSingleActiveConsumer (apache#20522) ### Motivation Currently, all subscriptions of one topic will do `consuemrFlow` action in a single thread, which is chosen by topicName: ``` this.topicExecutor = topic.getBrokerService().getTopicOrderedExecutor().chooseThread(topicName); ``` If there is a large number of subscriptions in a topic, all the work will focus on one thread ---- the chosen thread, which will reduce the consume performance. So this this patch , I'd like to choose a ramdom thread for `consumerFlow` in `PersistentDispatcherSingleActiveConsumer` to improve the consume performance. ### Modifications * `topic.getBrokerService().getTopicOrderedExecutor().chooseThread(topicName);` -> `topic.getBrokerService().getTopicOrderedExecutor().chooseThread();` * `this.topicExecutor ` -> `this.executor` (cherry picked from commit 51c2bb4)
Motivation
Currently, all subscriptions of one topic will do
consuemrFlowaction in a single thread, which is chosen by topicName:If there is a large number of subscriptions in a topic, all the work will focus on one thread ---- the chosen thread, which will reduce the consume performance. So this this patch , I'd like to choose a ramdom thread for
consumerFlowinPersistentDispatcherSingleActiveConsumerto improve the consume performance.Modifications
topic.getBrokerService().getTopicOrderedExecutor().chooseThread(topicName);->topic.getBrokerService().getTopicOrderedExecutor().chooseThread();this.topicExecutor->this.executorVerifying this change
Documentation
docdoc-requireddoc-not-neededdoc-completeMatching PR in forked repository
PR in forked repository: AnonHxy#42