You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@rocketmq.apache.org by GitBox <gi...@apache.org> on 2021/12/16 10:35:33 UTC

[GitHub] [rocketmq] areyouok opened a new pull request #3659: [ISSUE #3585] move execution of notifyMessageArriving() from ReputMessageService thread to PullRequestHoldService thread

areyouok opened a new pull request #3659:
URL: https://github.com/apache/rocketmq/pull/3659


   
   
   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](https://github.com/apache/rocketmq/issues) 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](https://github.com/apache/rocketmq/tree/master/test).
   - [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](http://www.apache.org/licenses/#clas).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] codecov-commenter commented on pull request #3659: [ISSUE #3585] [Part K] move execution of notifyMessageArriving() from ReputMessageService thread to PullRequestHoldService thread

Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #3659:
URL: https://github.com/apache/rocketmq/pull/3659#issuecomment-995700050


   # [Codecov](https://codecov.io/gh/apache/rocketmq/pull/3659?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#3659](https://codecov.io/gh/apache/rocketmq/pull/3659?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9885f99) into [develop](https://codecov.io/gh/apache/rocketmq/commit/ecb061ae425f76ad28a597293b98f374ceba14ec?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ecb061a) will **increase** coverage by `0.02%`.
   > The diff coverage is `30.76%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/rocketmq/pull/3659/graphs/tree.svg?width=650&height=150&src=pr&token=4w0sxP1wZv&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/rocketmq/pull/3659?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@              Coverage Diff              @@
   ##             develop    #3659      +/-   ##
   =============================================
   + Coverage      49.69%   49.71%   +0.02%     
   - Complexity      4725     4731       +6     
   =============================================
     Files            555      555              
     Lines          36798    36815      +17     
     Branches        4853     4856       +3     
   =============================================
   + Hits           18286    18304      +18     
   + Misses         16214    16211       -3     
   - Partials        2298     2300       +2     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/rocketmq/pull/3659?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...tmq/broker/longpolling/PullRequestHoldService.java](https://codecov.io/gh/apache/rocketmq/pull/3659/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvbG9uZ3BvbGxpbmcvUHVsbFJlcXVlc3RIb2xkU2VydmljZS5qYXZh) | `20.00% <30.76%> (-0.49%)` | :arrow_down: |
   | [...ava/org/apache/rocketmq/filter/util/BitsArray.java](https://codecov.io/gh/apache/rocketmq/pull/3659/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZmlsdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9maWx0ZXIvdXRpbC9CaXRzQXJyYXkuamF2YQ==) | `58.11% <0.00%> (-1.71%)` | :arrow_down: |
   | [.../rocketmq/broker/filter/ConsumerFilterManager.java](https://codecov.io/gh/apache/rocketmq/pull/3659/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvZmlsdGVyL0NvbnN1bWVyRmlsdGVyTWFuYWdlci5qYXZh) | `72.19% <0.00%> (-0.90%)` | :arrow_down: |
   | [...a/org/apache/rocketmq/store/StoreStatsService.java](https://codecov.io/gh/apache/rocketmq/pull/3659/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL1N0b3JlU3RhdHNTZXJ2aWNlLmphdmE=) | `29.50% <0.00%> (-0.66%)` | :arrow_down: |
   | [...etmq/client/latency/LatencyFaultToleranceImpl.java](https://codecov.io/gh/apache/rocketmq/pull/3659/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvbGF0ZW5jeS9MYXRlbmN5RmF1bHRUb2xlcmFuY2VJbXBsLmphdmE=) | `50.00% <0.00%> (ø)` | |
   | [...main/java/org/apache/rocketmq/store/CommitLog.java](https://codecov.io/gh/apache/rocketmq/pull/3659/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL0NvbW1pdExvZy5qYXZh) | `74.51% <0.00%> (+0.11%)` | :arrow_up: |
   | [...mq/client/impl/producer/DefaultMQProducerImpl.java](https://codecov.io/gh/apache/rocketmq/pull/3659/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9wcm9kdWNlci9EZWZhdWx0TVFQcm9kdWNlckltcGwuamF2YQ==) | `46.07% <0.00%> (+0.12%)` | :arrow_up: |
   | [.../apache/rocketmq/logging/inner/LoggingBuilder.java](https://codecov.io/gh/apache/rocketmq/pull/3659/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-bG9nZ2luZy9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvbG9nZ2luZy9pbm5lci9Mb2dnaW5nQnVpbGRlci5qYXZh) | `63.92% <0.00%> (+0.31%)` | :arrow_up: |
   | [...he/rocketmq/client/impl/consumer/ProcessQueue.java](https://codecov.io/gh/apache/rocketmq/pull/3659/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9jb25zdW1lci9Qcm9jZXNzUXVldWUuamF2YQ==) | `61.86% <0.00%> (+0.46%)` | :arrow_up: |
   | ... and [4 more](https://codecov.io/gh/apache/rocketmq/pull/3659/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/rocketmq/pull/3659?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/rocketmq/pull/3659?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [ecb061a...9885f99](https://codecov.io/gh/apache/rocketmq/pull/3659?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] areyouok commented on a change in pull request #3659: [ISSUE #3585] [Part K] move execution of notifyMessageArriving() from ReputMessageService thread to PullRequestHoldService thread

Posted by GitBox <gi...@apache.org>.
areyouok commented on a change in pull request #3659:
URL: https://github.com/apache/rocketmq/pull/3659#discussion_r788714807



##########
File path: broker/src/main/java/org/apache/rocketmq/broker/longpolling/PullNotifyQueue.java
##########
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.rocketmq.broker.longpolling;
+
+import java.util.LinkedList;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.TimeUnit;
+
+class PullNotifyQueue<T> {
+    private PullNotifyQueueConfig config;
+
+    private final LinkedBlockingQueue<T> queue;
+
+    private int activeConsumeQueueCount = 1;
+    private long activeConsumeQueueEpochTime = System.nanoTime();
+    // only change in unit test
+    private long activeConsumeQueueRefreshTime = 1_000_000_000;
+
+    private long lastBatchDrainTime;
+
+    public PullNotifyQueue(PullNotifyQueueConfig config) {
+        queue = new LinkedBlockingQueue<>(500_000);
+        this.config = config;
+    }
+
+    public void put(T data) throws InterruptedException {
+        queue.put(data);
+    }
+
+    public LinkedList<T> drain() throws InterruptedException {
+        LinkedList<T> result = new LinkedList<>();
+        long tps = config.getTps();
+        if (tps <= config.getTpsThreshold()) {
+            T data = queue.poll(100, TimeUnit.MILLISECONDS);

Review comment:
       In most case tps is below 100000, we enable this when tps is greater than 100000 to protect rocketmq.
   
   in our tests, 600 queue, 150000+ tps, end-to-end latency is 18ms; 18 queue, 150000+ tps, end-to-end latency is 2ms.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] codecov-commenter edited a comment on pull request #3659: [ISSUE #3585] [Part K] move execution of notifyMessageArriving() from ReputMessageService thread to PullRequestHoldService thread

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3659:
URL: https://github.com/apache/rocketmq/pull/3659#issuecomment-995700050


   # [Codecov](https://codecov.io/gh/apache/rocketmq/pull/3659?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#3659](https://codecov.io/gh/apache/rocketmq/pull/3659?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9d4b981) into [develop](https://codecov.io/gh/apache/rocketmq/commit/ecb061ae425f76ad28a597293b98f374ceba14ec?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ecb061a) will **decrease** coverage by `2.35%`.
   > The diff coverage is `46.95%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/rocketmq/pull/3659/graphs/tree.svg?width=650&height=150&src=pr&token=4w0sxP1wZv&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/rocketmq/pull/3659?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@              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](https://codecov.io/gh/apache/rocketmq/pull/3659?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...a/org/apache/rocketmq/store/StoreStatsService.java](https://codecov.io/gh/apache/rocketmq/pull/3659/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL1N0b3JlU3RhdHNTZXJ2aWNlLmphdmE=) | `29.96% <ø> (-0.20%)` | :arrow_down: |
   | [...tmq/broker/longpolling/PullRequestHoldService.java](https://codecov.io/gh/apache/rocketmq/pull/3659/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvbG9uZ3BvbGxpbmcvUHVsbFJlcXVlc3RIb2xkU2VydmljZS5qYXZh) | `17.19% <17.77%> (-3.29%)` | :arrow_down: |
   | [.../java/org/apache/rocketmq/common/BrokerConfig.java](https://codecov.io/gh/apache/rocketmq/pull/3659/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vQnJva2VyQ29uZmlnLmphdmE=) | `32.55% <25.00%> (-0.38%)` | :arrow_down: |
   | [...e/rocketmq/broker/longpolling/PullNotifyQueue.java](https://codecov.io/gh/apache/rocketmq/pull/3659/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvbG9uZ3BvbGxpbmcvUHVsbE5vdGlmeVF1ZXVlLmphdmE=) | `98.27% <98.27%> (ø)` | |
   | [.../rocketmq/broker/filter/ConsumerFilterManager.java](https://codecov.io/gh/apache/rocketmq/pull/3659/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvZmlsdGVyL0NvbnN1bWVyRmlsdGVyTWFuYWdlci5qYXZh) | `72.19% <0.00%> (-0.90%)` | :arrow_down: |
   | [...he/rocketmq/client/impl/consumer/ProcessQueue.java](https://codecov.io/gh/apache/rocketmq/pull/3659/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9jb25zdW1lci9Qcm9jZXNzUXVldWUuamF2YQ==) | `60.55% <0.00%> (-0.85%)` | :arrow_down: |
   | [...a/org/apache/rocketmq/client/impl/MQAdminImpl.java](https://codecov.io/gh/apache/rocketmq/pull/3659/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9NUUFkbWluSW1wbC5qYXZh) | `5.09% <0.00%> (-0.03%)` | :arrow_down: |
   | [...ain/java/org/apache/rocketmq/store/MappedFile.java](https://codecov.io/gh/apache/rocketmq/pull/3659/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL01hcHBlZEZpbGUuamF2YQ==) | `50.18% <0.00%> (-0.01%)` | :arrow_down: |
   | ... and [88 more](https://codecov.io/gh/apache/rocketmq/pull/3659/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/rocketmq/pull/3659?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/rocketmq/pull/3659?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [ecb061a...9d4b981](https://codecov.io/gh/apache/rocketmq/pull/3659?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] dongeforever commented on a change in pull request #3659: [ISSUE #3585] [Part K] move execution of notifyMessageArriving() from ReputMessageService thread to PullRequestHoldService thread

Posted by GitBox <gi...@apache.org>.
dongeforever commented on a change in pull request #3659:
URL: https://github.com/apache/rocketmq/pull/3659#discussion_r788380029



##########
File path: broker/src/main/java/org/apache/rocketmq/broker/longpolling/PullNotifyQueue.java
##########
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.rocketmq.broker.longpolling;
+
+import java.util.LinkedList;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.TimeUnit;
+
+class PullNotifyQueue<T> {
+    private PullNotifyQueueConfig config;
+
+    private final LinkedBlockingQueue<T> queue;
+
+    private int activeConsumeQueueCount = 1;
+    private long activeConsumeQueueEpochTime = System.nanoTime();
+    // only change in unit test
+    private long activeConsumeQueueRefreshTime = 1_000_000_000;
+
+    private long lastBatchDrainTime;
+
+    public PullNotifyQueue(PullNotifyQueueConfig config) {
+        queue = new LinkedBlockingQueue<>(500_000);
+        this.config = config;
+    }
+
+    public void put(T data) throws InterruptedException {
+        queue.put(data);
+    }
+
+    public LinkedList<T> drain() throws InterruptedException {
+        LinkedList<T> result = new LinkedList<>();
+        long tps = config.getTps();
+        if (tps <= config.getTpsThreshold()) {
+            T data = queue.poll(100, TimeUnit.MILLISECONDS);

Review comment:
       Why use the store put Tps to control the long polling?
   

##########
File path: broker/src/main/java/org/apache/rocketmq/broker/longpolling/PullNotifyQueue.java
##########
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.rocketmq.broker.longpolling;
+
+import java.util.LinkedList;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.TimeUnit;
+
+class PullNotifyQueue<T> {
+    private PullNotifyQueueConfig config;
+
+    private final LinkedBlockingQueue<T> queue;
+
+    private int activeConsumeQueueCount = 1;
+    private long activeConsumeQueueEpochTime = System.nanoTime();
+    // only change in unit test
+    private long activeConsumeQueueRefreshTime = 1_000_000_000;
+
+    private long lastBatchDrainTime;
+
+    public PullNotifyQueue(PullNotifyQueueConfig config) {
+        queue = new LinkedBlockingQueue<>(500_000);
+        this.config = config;
+    }
+
+    public void put(T data) throws InterruptedException {
+        queue.put(data);
+    }
+
+    public LinkedList<T> drain() throws InterruptedException {
+        LinkedList<T> result = new LinkedList<>();
+        long tps = config.getTps();
+        if (tps <= config.getTpsThreshold()) {
+            T data = queue.poll(100, TimeUnit.MILLISECONDS);
+            if (data != null) {
+                result.add(data);
+            }
+        } else {

Review comment:
       The expectation here is :
   * if the queue has data, poll it as quickly as possible
   * if the queue doesn't have data, just return the already polled data.
   
   The code maybe be simplified as:
   int pollWaitTimeMs = 0;
   while(true) {
       T data = queue.poll(pollWaitTimeMs, TimeUnit.MILLISECONDS);
       if (data == null) {
         pollWaitTimeMs = 100;
      } else { 
        pollWaitTimeMs = 0;
        result.add(data);
      }
      if(result.size() > batchMax) {
         break;
      }
      if(result.size() >0 && pollWaitTimeMs > 0) {
        break;
      }
      if(System.nanoTime() - start > maxWaitTimeNonas) {
       break;
    }
   }
   
   The tps control seems unnecessary.
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] areyouok commented on pull request #3659: [ISSUE #3585] [Part K] move execution of notifyMessageArriving() from ReputMessageService thread to PullRequestHoldService thread

Posted by GitBox <gi...@apache.org>.
areyouok commented on pull request #3659:
URL: https://github.com/apache/rocketmq/pull/3659#issuecomment-998511009


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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] coveralls commented on pull request #3659: [ISSUE #3585] [Part K] move execution of notifyMessageArriving() from ReputMessageService thread to PullRequestHoldService thread

Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #3659:
URL: https://github.com/apache/rocketmq/pull/3659#issuecomment-995699943


   
   [![Coverage Status](https://coveralls.io/builds/45057229/badge)](https://coveralls.io/builds/45057229)
   
   Coverage increased (+0.05%) to 56.199% when pulling **9885f99b0a5cb36ee7bdc1993fd2902fdb9903dd on areyouok:492_PartK** into **ecb061ae425f76ad28a597293b98f374ceba14ec on apache:develop**.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] areyouok commented on pull request #3659: [ISSUE #3585] [Part K] move execution of notifyMessageArriving() from ReputMessageService thread to PullRequestHoldService thread

Posted by GitBox <gi...@apache.org>.
areyouok commented on pull request #3659:
URL: https://github.com/apache/rocketmq/pull/3659#issuecomment-1003238999


   finished


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] vongosling commented on pull request #3659: [ISSUE #3585] [Part K] move execution of notifyMessageArriving() from ReputMessageService thread to PullRequestHoldService thread

Posted by GitBox <gi...@apache.org>.
vongosling commented on pull request #3659:
URL: https://github.com/apache/rocketmq/pull/3659#issuecomment-1085323041


   Any update?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] areyouok edited a comment on pull request #3659: [ISSUE #3585] [Part K] move execution of notifyMessageArriving() from ReputMessageService thread to PullRequestHoldService thread

Posted by GitBox <gi...@apache.org>.
areyouok edited a comment on pull request #3659:
URL: https://github.com/apache/rocketmq/pull/3659#issuecomment-995914655


   Flame chart before this commit (only of the ReputMessageService thread):
   
   <img width="1370" alt="thread" src="https://user-images.githubusercontent.com/3192556/146398596-1a990120-2da4-462f-b6df-85614119d0e8.png">
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] areyouok commented on pull request #3659: [ISSUE #3585] [Part K] move execution of notifyMessageArriving() from ReputMessageService thread to PullRequestHoldService thread

Posted by GitBox <gi...@apache.org>.
areyouok commented on pull request #3659:
URL: https://github.com/apache/rocketmq/pull/3659#issuecomment-995914655


   Flame chart before this commit:
   
   <img width="1370" alt="thread" src="https://user-images.githubusercontent.com/3192556/146398596-1a990120-2da4-462f-b6df-85614119d0e8.png">
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] ni-ze commented on a change in pull request #3659: [ISSUE #3585] [Part K] move execution of notifyMessageArriving() from ReputMessageService thread to PullRequestHoldService thread

Posted by GitBox <gi...@apache.org>.
ni-ze commented on a change in pull request #3659:
URL: https://github.com/apache/rocketmq/pull/3659#discussion_r772796555



##########
File path: broker/src/main/java/org/apache/rocketmq/broker/longpolling/PullRequestHoldService.java
##########
@@ -109,11 +109,48 @@ private void checkHoldRequest() {
         }
     }
 
+    @Override
+    protected void onWaitEnd() {
+        final int batch = 1000;
+        for (int i = 0; i < batch; i++) {
+            Runnable runnable = notifyList.poll();
+            if (runnable != null) {
+                runnable.run();

Review comment:
       if runnable.run() throw throwable,  the checkHoldRequest method in the below will not invoked.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] coveralls edited a comment on pull request #3659: [ISSUE #3585] [Part K] move execution of notifyMessageArriving() from ReputMessageService thread to PullRequestHoldService thread

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3659:
URL: https://github.com/apache/rocketmq/pull/3659#issuecomment-995699943


   
   [![Coverage Status](https://coveralls.io/builds/45275012/badge)](https://coveralls.io/builds/45275012)
   
   Coverage decreased (-2.9%) to 53.286% when pulling **9d4b9814e76570f94824265ffd3da27db343f045 on areyouok:492_PartK** into **ecb061ae425f76ad28a597293b98f374ceba14ec on apache:develop**.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] coveralls edited a comment on pull request #3659: [ISSUE #3585] [Part K] move execution of notifyMessageArriving() from ReputMessageService thread to PullRequestHoldService thread

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3659:
URL: https://github.com/apache/rocketmq/pull/3659#issuecomment-995699943


   
   [![Coverage Status](https://coveralls.io/builds/45226456/badge)](https://coveralls.io/builds/45226456)
   
   Coverage increased (+0.07%) to 56.223% when pulling **e9be666c384e5d1cf7801a5787302d814047f506 on areyouok:492_PartK** into **ecb061ae425f76ad28a597293b98f374ceba14ec on apache:develop**.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] caigy commented on pull request #3659: [ISSUE #3585] [Part K] move execution of notifyMessageArriving() from ReputMessageService thread to PullRequestHoldService thread

Posted by GitBox <gi...@apache.org>.
caigy commented on pull request #3659:
URL: https://github.com/apache/rocketmq/pull/3659#issuecomment-998418595


   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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] areyouok commented on a change in pull request #3659: [ISSUE #3585] [Part K] move execution of notifyMessageArriving() from ReputMessageService thread to PullRequestHoldService thread

Posted by GitBox <gi...@apache.org>.
areyouok commented on a change in pull request #3659:
URL: https://github.com/apache/rocketmq/pull/3659#discussion_r788719612



##########
File path: broker/src/main/java/org/apache/rocketmq/broker/longpolling/PullNotifyQueue.java
##########
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.rocketmq.broker.longpolling;
+
+import java.util.LinkedList;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.TimeUnit;
+
+class PullNotifyQueue<T> {
+    private PullNotifyQueueConfig config;
+
+    private final LinkedBlockingQueue<T> queue;
+
+    private int activeConsumeQueueCount = 1;
+    private long activeConsumeQueueEpochTime = System.nanoTime();
+    // only change in unit test
+    private long activeConsumeQueueRefreshTime = 1_000_000_000;
+
+    private long lastBatchDrainTime;
+
+    public PullNotifyQueue(PullNotifyQueueConfig config) {
+        queue = new LinkedBlockingQueue<>(500_000);
+        this.config = config;
+    }
+
+    public void put(T data) throws InterruptedException {
+        queue.put(data);
+    }
+
+    public LinkedList<T> drain() throws InterruptedException {
+        LinkedList<T> result = new LinkedList<>();
+        long tps = config.getTps();
+        if (tps <= config.getTpsThreshold()) {
+            T data = queue.poll(100, TimeUnit.MILLISECONDS);

Review comment:
       we can modify the threshold to a greater number, such as 150000.
   
   In case tps greater than 150000 and a lot of queue is writing, I think the latency is not so important.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] areyouok commented on a change in pull request #3659: [ISSUE #3585] [Part K] move execution of notifyMessageArriving() from ReputMessageService thread to PullRequestHoldService thread

Posted by GitBox <gi...@apache.org>.
areyouok commented on a change in pull request #3659:
URL: https://github.com/apache/rocketmq/pull/3659#discussion_r788586985



##########
File path: broker/src/main/java/org/apache/rocketmq/broker/longpolling/PullNotifyQueue.java
##########
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.rocketmq.broker.longpolling;
+
+import java.util.LinkedList;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.TimeUnit;
+
+class PullNotifyQueue<T> {
+    private PullNotifyQueueConfig config;
+
+    private final LinkedBlockingQueue<T> queue;
+
+    private int activeConsumeQueueCount = 1;
+    private long activeConsumeQueueEpochTime = System.nanoTime();
+    // only change in unit test
+    private long activeConsumeQueueRefreshTime = 1_000_000_000;
+
+    private long lastBatchDrainTime;
+
+    public PullNotifyQueue(PullNotifyQueueConfig config) {
+        queue = new LinkedBlockingQueue<>(500_000);
+        this.config = config;
+    }
+
+    public void put(T data) throws InterruptedException {
+        queue.put(data);
+    }
+
+    public LinkedList<T> drain() throws InterruptedException {
+        LinkedList<T> result = new LinkedList<>();
+        long tps = config.getTps();
+        if (tps <= config.getTpsThreshold()) {
+            T data = queue.poll(100, TimeUnit.MILLISECONDS);

Review comment:
       to improve throughput and decrease cpu cost when tps is greater than threshold.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] dongeforever commented on a change in pull request #3659: [ISSUE #3585] [Part K] move execution of notifyMessageArriving() from ReputMessageService thread to PullRequestHoldService thread

Posted by GitBox <gi...@apache.org>.
dongeforever commented on a change in pull request #3659:
URL: https://github.com/apache/rocketmq/pull/3659#discussion_r788709066



##########
File path: broker/src/main/java/org/apache/rocketmq/broker/longpolling/PullNotifyQueue.java
##########
@@ -0,0 +1,147 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.rocketmq.broker.longpolling;
+
+import java.util.LinkedList;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.TimeUnit;
+
+class PullNotifyQueue<T> {
+    private PullNotifyQueueConfig config;
+
+    private final LinkedBlockingQueue<T> queue;
+
+    private int activeConsumeQueueCount = 1;
+    private long activeConsumeQueueEpochTime = System.nanoTime();
+    // only change in unit test
+    private long activeConsumeQueueRefreshTime = 1_000_000_000;
+
+    private long lastBatchDrainTime;
+
+    public PullNotifyQueue(PullNotifyQueueConfig config) {
+        queue = new LinkedBlockingQueue<>(500_000);
+        this.config = config;
+    }
+
+    public void put(T data) throws InterruptedException {
+        queue.put(data);
+    }
+
+    public LinkedList<T> drain() throws InterruptedException {
+        LinkedList<T> result = new LinkedList<>();
+        long tps = config.getTps();
+        if (tps <= config.getTpsThreshold()) {
+            T data = queue.poll(100, TimeUnit.MILLISECONDS);

Review comment:
       The end-to-end latency is more important in most cases for rocketmq. How about adjusting the default tps threshold to Integer.MaxValue? 
   if someone wants to promote throughput, it could be adjusted.
   
   BTW,  The cpu cost is a problem. The tag filter and property filter are unnecessary in the notification process,  It will be done twice for the pull process will do that work too.
   The notification should be lightweight as far as possible.
   




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] codecov-commenter edited a comment on pull request #3659: [ISSUE #3585] [Part K] move execution of notifyMessageArriving() from ReputMessageService thread to PullRequestHoldService thread

Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3659:
URL: https://github.com/apache/rocketmq/pull/3659#issuecomment-995700050


   # [Codecov](https://codecov.io/gh/apache/rocketmq/pull/3659?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
   > Merging [#3659](https://codecov.io/gh/apache/rocketmq/pull/3659?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e9be666) into [develop](https://codecov.io/gh/apache/rocketmq/commit/ecb061ae425f76ad28a597293b98f374ceba14ec?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ecb061a) will **increase** coverage by `0.04%`.
   > The diff coverage is `41.17%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/rocketmq/pull/3659/graphs/tree.svg?width=650&height=150&src=pr&token=4w0sxP1wZv&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/rocketmq/pull/3659?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   
   ```diff
   @@              Coverage Diff              @@
   ##             develop    #3659      +/-   ##
   =============================================
   + Coverage      49.69%   49.74%   +0.04%     
   - Complexity      4725     4736      +11     
   =============================================
     Files            555      555              
     Lines          36798    36892      +94     
     Branches        4853     4866      +13     
   =============================================
   + Hits           18286    18351      +65     
   - Misses         16214    16238      +24     
   - Partials        2298     2303       +5     
   ```
   
   
   | [Impacted Files](https://codecov.io/gh/apache/rocketmq/pull/3659?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...tmq/broker/longpolling/PullRequestHoldService.java](https://codecov.io/gh/apache/rocketmq/pull/3659/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvbG9uZ3BvbGxpbmcvUHVsbFJlcXVlc3RIb2xkU2VydmljZS5qYXZh) | `28.26% <41.17%> (+7.77%)` | :arrow_up: |
   | [...apache/rocketmq/remoting/netty/ResponseFuture.java](https://codecov.io/gh/apache/rocketmq/pull/3659/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cmVtb3Rpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3JlbW90aW5nL25ldHR5L1Jlc3BvbnNlRnV0dXJlLmphdmE=) | `85.00% <0.00%> (-5.00%)` | :arrow_down: |
   | [...rocketmq/remoting/netty/NettyRemotingAbstract.java](https://codecov.io/gh/apache/rocketmq/pull/3659/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cmVtb3Rpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3JlbW90aW5nL25ldHR5L05ldHR5UmVtb3RpbmdBYnN0cmFjdC5qYXZh) | `46.88% <0.00%> (-4.03%)` | :arrow_down: |
   | [.../rocketmq/broker/filter/ConsumerFilterManager.java](https://codecov.io/gh/apache/rocketmq/pull/3659/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvZmlsdGVyL0NvbnN1bWVyRmlsdGVyTWFuYWdlci5qYXZh) | `72.19% <0.00%> (-0.90%)` | :arrow_down: |
   | [...he/rocketmq/client/impl/consumer/ProcessQueue.java](https://codecov.io/gh/apache/rocketmq/pull/3659/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9jb25zdW1lci9Qcm9jZXNzUXVldWUuamF2YQ==) | `61.00% <0.00%> (-0.39%)` | :arrow_down: |
   | [...a/org/apache/rocketmq/store/StoreStatsService.java](https://codecov.io/gh/apache/rocketmq/pull/3659/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL1N0b3JlU3RhdHNTZXJ2aWNlLmphdmE=) | `29.96% <0.00%> (-0.20%)` | :arrow_down: |
   | [...a/org/apache/rocketmq/client/impl/MQAdminImpl.java](https://codecov.io/gh/apache/rocketmq/pull/3659/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9NUUFkbWluSW1wbC5qYXZh) | `5.09% <0.00%> (-0.03%)` | :arrow_down: |
   | [...n/java/org/apache/rocketmq/store/ha/HAService.java](https://codecov.io/gh/apache/rocketmq/pull/3659/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3RvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3N0b3JlL2hhL0hBU2VydmljZS5qYXZh) | `71.28% <0.00%> (ø)` | |
   | [...ava/org/apache/rocketmq/filter/util/BitsArray.java](https://codecov.io/gh/apache/rocketmq/pull/3659/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-ZmlsdGVyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9maWx0ZXIvdXRpbC9CaXRzQXJyYXkuamF2YQ==) | `59.82% <0.00%> (ø)` | |
   | [...che/rocketmq/remoting/netty/NettyClientConfig.java](https://codecov.io/gh/apache/rocketmq/pull/3659/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-cmVtb3Rpbmcvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3JvY2tldG1xL3JlbW90aW5nL25ldHR5L05ldHR5Q2xpZW50Q29uZmlnLmphdmE=) | `50.00% <0.00%> (ø)` | |
   | ... and [13 more](https://codecov.io/gh/apache/rocketmq/pull/3659/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/rocketmq/pull/3659?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/rocketmq/pull/3659?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [ecb061a...e9be666](https://codecov.io/gh/apache/rocketmq/pull/3659?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] areyouok commented on pull request #3659: [ISSUE #3585] [Part K] move execution of notifyMessageArriving() from ReputMessageService thread to PullRequestHoldService thread

Posted by GitBox <gi...@apache.org>.
areyouok commented on pull request #3659:
URL: https://github.com/apache/rocketmq/pull/3659#issuecomment-998512271


   I found the current commit may increase consume lantency in performance test (abount 45ms).
   
   I'm working on it.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] dongeforever commented on pull request #3659: [ISSUE #3585] [Part K] move execution of notifyMessageArriving() from ReputMessageService thread to PullRequestHoldService thread

Posted by GitBox <gi...@apache.org>.
dongeforever commented on pull request #3659:
URL: https://github.com/apache/rocketmq/pull/3659#issuecomment-1017154624


   is there any impact on the "end to end" latency?
   
   Maybe a performance test report is needed.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [rocketmq] coveralls edited a comment on pull request #3659: [ISSUE #3585] [Part K] move execution of notifyMessageArriving() from ReputMessageService thread to PullRequestHoldService thread

Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3659:
URL: https://github.com/apache/rocketmq/pull/3659#issuecomment-995699943


   
   [![Coverage Status](https://coveralls.io/builds/46399891/badge)](https://coveralls.io/builds/46399891)
   
   Coverage decreased (-0.09%) to 51.183% when pulling **aa69a9371ca2d50b0c773abc0fe4d9d8745ce59e on areyouok:492_PartK** into **ca92d367fda6030adde4ce87ee09b335047857ae on apache:develop**.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@rocketmq.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org