[ISSUE #3585] [Part K] move execution of notifyMessageArriving() from ReputMessageService thread to PullRequestHoldService thread

[areyouok]

This commit speed up consume qps greatly, in our test up to 200,000 qps.

Make sure set the target branch to develop

What is the purpose of the change

XXXXX

Brief changelog

XX

Verifying this change

XXXX

Follow this checklist to help us incorporate your contribution quickly and easily. Notice, it would be helpful if you could finish the following 5 checklist(the last one is not necessary)before request the community to review your PR.

• [x] Make sure there is a Github issue filed for the change (usually before you start working on it). Trivial changes like typos do not require a Github issue. Your pull request should address just this issue, without pulling in other changes - one PR resolves one issue.
• [x] Format the pull request title like [ISSUE #123] Fix UnknownException when host config not exist. Each commit in the pull request should have a meaningful subject line and body.
• [x] Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
• [x] Write necessary unit-test(over 80% coverage) to verify your logic correction, more mock a little better when cross module dependency exist. If the new feature or significant change is committed, please remember to add integration-test in test module.
• [x] Run mvn -B clean apache-rat:check findbugs:findbugs checkstyle:checkstyle to make sure basic checks pass. Run mvn clean install -DskipITs to make sure unit-test pass. Run mvn clean test-compile failsafe:integration-test to make sure integration-test pass.
• [ ] If this contribution is large, please file an Apache Individual Contributor License Agreement.

[coveralls]

Coverage decreased (-0.09%) to 51.183% when pulling aa69a9371ca2d50b0c773abc0fe4d9d8745ce59e on areyouok:492_PartK into ca92d367fda6030adde4ce87ee09b335047857ae on apache:develop.

[codecov-commenter]

Codecov Report

Merging #3659 (9d4b981) into develop (ecb061a) will decrease coverage by 2.35%. The diff coverage is 46.95%.

@@              Coverage Diff              @@
##             develop    #3659      +/-   ##
=============================================
- Coverage      49.69%   47.33%   -2.36%     
- Complexity      4725     5050     +325     
=============================================
  Files            555      628      +73     
  Lines          36798    41496    +4698     
  Branches        4853     5395     +542     
=============================================
+ Hits           18286    19643    +1357     
- Misses         16214    19429    +3215     
- Partials        2298     2424     +126     

Impacted Files	Coverage Δ
...a/org/apache/rocketmq/store/StoreStatsService.java	29.96% <ø> (-0.20%)	:arrow_down:
...tmq/broker/longpolling/PullRequestHoldService.java	17.19% <17.77%> (-3.29%)	:arrow_down:
.../java/org/apache/rocketmq/common/BrokerConfig.java	32.55% <25.00%> (-0.38%)	:arrow_down:
...e/rocketmq/broker/longpolling/PullNotifyQueue.java	98.27% <98.27%> (ø)
.../rocketmq/broker/filter/ConsumerFilterManager.java	72.19% <0.00%> (-0.90%)	:arrow_down:
...he/rocketmq/client/impl/consumer/ProcessQueue.java	60.55% <0.00%> (-0.85%)	:arrow_down:
...a/org/apache/rocketmq/client/impl/MQAdminImpl.java	5.09% <0.00%> (-0.03%)	:arrow_down:
...ain/java/org/apache/rocketmq/store/MappedFile.java	50.18% <0.00%> (-0.01%)	:arrow_down:
... and 88 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update ecb061a...9d4b981. Read the comment docs.

[areyouok]

Flame chart before this commit (only of the ReputMessageService thread):

[caigy]

I found System.nanoTime() was used instead of System.currentTimeMillis(), will it bring benefits to performance improvement? It also brings some conversion between nanosecond variables and config values defined in millisecond.

[areyouok]

I found System.nanoTime() was used instead of System.currentTimeMillis(), will it bring benefits to performance improvement? It also brings some conversion between nanosecond variables and config values defined in millisecond.

There is no performance benefit.

nanoTime() are more strictly than currentTimeMillis() for measuring time elapse. For example, NTP service may adjust system clock, the nanoTime() method will not affect by it. See javadoc of the method.

[areyouok]

I found the current commit may increase consume lantency in performance test (abount 45ms).

I'm working on it.

[areyouok]

finished

[dongeforever]

is there any impact on the "end to end" latency?

Maybe a performance test report is needed.

[vongosling]

Any update?

[areyouok]

Any update?

There is no update. But we have run it in out product environment for 3 months.

[github-actions[bot]]

This PR is stale because it has been open for 365 days with no activity. It will be closed in 3 days if no further activity occurs. If you wish not to mark it as stale, please leave a comment in this PR.