[improve][broker] Choose random thread for consumerFlow in PersistentDispatcherSingleActiveConsumer

[AnonHxy]

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

Verifying this change

• Make sure that the change passes the CI checks.

Documentation

• doc
• doc-required
• doc-not-needed
• doc-complete

Matching PR in forked repository

PR in forked repository: AnonHxy#42

[tisonkun]

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.

[congbobo184]

The reason we chooseThread(topicName) is that we'd like to order all tasks of the same topics.

why we need to use the same thread with the different sub in the same topic?
https://github.com/apache/pulsar/blob/master/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/persistent/PersistentDispatcherMultipleConsumers.java#L544
It does not use this thread with the same sub in a 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

[tisonkun]

@congbobo184 Thank you! This is the evidence I ask for fulfilling the description :D

resolved

[AnonHxy]

@codelipenghui @eolivelli @Jason918 @merlimat @HQebupt PTAL

[gaoran10]

LGTM

[HQebupt]

LGTM

[congbobo184]

LGTM!

[michaeljmarshall]

@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.

[AnonHxy]

do you have any metrics to show how this affects performance?

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