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/13 12:05:51 UTC
[GitHub] [rocketmq] panzhi33 opened a new pull request #3639: [ISSUE #3624]fix DefaultMQPushConsumerTest.testPullMessage_ExceptionO…
panzhi33 opened a new pull request #3639:
URL: https://github.com/apache/rocketmq/pull/3639
…ccursWhenComputePullFromWhere throws RejectedExecution
**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] panzhi33 commented on a change in pull request #3639: [ISSUE #3624]fix DefaultMQPushConsumerTest.testPullMessage_ExceptionO…
Posted by GitBox <gi...@apache.org>.
panzhi33 commented on a change in pull request #3639:
URL: https://github.com/apache/rocketmq/pull/3639#discussion_r768289937
##########
File path: client/src/main/java/org/apache/rocketmq/client/producer/RequestFutureHolder.java
##########
@@ -74,14 +69,39 @@ public void scanExpiredRequest() {
}
}
- private RequestFutureHolder() {
+ public synchronized void startScheduledTask() {
+ if (this.producerNum.incrementAndGet() == 1) {
+ this.getScheduledExecutorService().scheduleAtFixedRate(new Runnable() {
+ @Override
+ public void run() {
+ try {
+ RequestFutureHolder.getInstance().scanExpiredRequest();
+ } catch (Throwable e) {
+ log.error("scan RequestFutureTable exception", e);
+ }
+ }
+ }, 1000 * 3, 1000, TimeUnit.MILLISECONDS);
+ }
+ }
+
+ public synchronized void shutdown() {
+ if (this.producerNum.decrementAndGet() == 0) {
+ this.getScheduledExecutorService().shutdown();
Review comment:
ok
--
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 #3639: [ISSUE #3624]fix DefaultMQPushConsumerTest.testPullMessage_ExceptionO…
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3639:
URL: https://github.com/apache/rocketmq/pull/3639#issuecomment-992445983
[](https://coveralls.io/builds/44956845)
Coverage increased (+0.03%) to 56.264% when pulling **71a298767a23a15a09882f0593c4e538e23dbf7d on panzhi33:fix-3624** into **4a8263b4609cd16b5e7c194528bd875711e01852 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 #3639: [ISSUE #3624]fix DefaultMQPushConsumerTest.testPullMessage_ExceptionO…
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3639:
URL: https://github.com/apache/rocketmq/pull/3639#issuecomment-992445983
[](https://coveralls.io/builds/45001069)
Coverage decreased (-0.06%) to 56.172% when pulling **9716632c43ca3fc66e47c21d35724b67abda4446 on panzhi33:fix-3624** into **4a8263b4609cd16b5e7c194528bd875711e01852 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 #3639: [ISSUE #3624]fix DefaultMQPushConsumerTest.testPullMessage_ExceptionO…
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3639:
URL: https://github.com/apache/rocketmq/pull/3639#issuecomment-992445983
[](https://coveralls.io/builds/44955130)
Coverage decreased (-0.03%) to 56.202% when pulling **48022493a5e8155b00f73dd7cbf8cb5871b255fc on panzhi33:fix-3624** into **4a8263b4609cd16b5e7c194528bd875711e01852 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 merged pull request #3639: [ISSUE #3624]fix DefaultMQPushConsumerTest.testPullMessage_ExceptionO…
Posted by GitBox <gi...@apache.org>.
areyouok merged pull request #3639:
URL: https://github.com/apache/rocketmq/pull/3639
--
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 #3639: [ISSUE #3624]fix DefaultMQPushConsumerTest.testPullMessage_ExceptionO…
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #3639:
URL: https://github.com/apache/rocketmq/pull/3639#issuecomment-992472013
# [Codecov](https://codecov.io/gh/apache/rocketmq/pull/3639?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 [#3639](https://codecov.io/gh/apache/rocketmq/pull/3639?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4802249) into [develop](https://codecov.io/gh/apache/rocketmq/commit/50d45f23a186051d177990fd00a89d152c5c4dae?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (50d45f2) will **increase** coverage by `0.77%`.
> The diff coverage is `56.66%`.
[](https://codecov.io/gh/apache/rocketmq/pull/3639?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 #3639 +/- ##
=============================================
+ Coverage 48.86% 49.63% +0.77%
- Complexity 4659 4715 +56
=============================================
Files 555 555
Lines 36726 36772 +46
Branches 4840 4846 +6
=============================================
+ Hits 17946 18253 +307
+ Misses 16532 16234 -298
- Partials 2248 2285 +37
```
| [Impacted Files](https://codecov.io/gh/apache/rocketmq/pull/3639?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...cketmq/broker/filter/MessageEvaluationContext.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvZmlsdGVyL01lc3NhZ2VFdmFsdWF0aW9uQ29udGV4dC5qYXZh) | `30.76% <0.00%> (ø)` | |
| [...tmq/broker/longpolling/PullRequestHoldService.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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.48% <0.00%> (ø)` | |
| [...ocketmq/client/consumer/DefaultMQPushConsumer.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvY29uc3VtZXIvRGVmYXVsdE1RUHVzaENvbnN1bWVyLmphdmE=) | `55.94% <0.00%> (ø)` | |
| [...mq/client/consumer/store/LocalFileOffsetStore.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvY29uc3VtZXIvc3RvcmUvTG9jYWxGaWxlT2Zmc2V0U3RvcmUuamF2YQ==) | `56.31% <0.00%> (-0.56%)` | :arrow_down: |
| [.../rocketmq/client/impl/ClientRemotingProcessor.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9DbGllbnRSZW1vdGluZ1Byb2Nlc3Nvci5qYXZh) | `2.91% <0.00%> (ø)` | |
| [...java/org/apache/rocketmq/common/Configuration.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vQ29uZmlndXJhdGlvbi5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...org/apache/rocketmq/common/message/MessageExt.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vbWVzc2FnZS9NZXNzYWdlRXh0LmphdmE=) | `66.66% <0.00%> (-1.57%)` | :arrow_down: |
| [...rocketmq/remoting/netty/NettyRemotingAbstract.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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.22%)` | :arrow_down: |
| [...n/java/org/apache/rocketmq/test/util/FileUtil.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-dGVzdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvdGVzdC91dGlsL0ZpbGVVdGlsLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...in/java/org/apache/rocketmq/test/util/MQAdmin.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-dGVzdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvdGVzdC91dGlsL01RQWRtaW4uamF2YQ==) | `44.44% <0.00%> (+5.55%)` | :arrow_up: |
| ... and [46 more](https://codecov.io/gh/apache/rocketmq/pull/3639/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/3639?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/3639?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 [1635856...4802249](https://codecov.io/gh/apache/rocketmq/pull/3639?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] panzhi33 commented on a change in pull request #3639: [ISSUE #3624]fix DefaultMQPushConsumerTest.testPullMessage_ExceptionO…
Posted by GitBox <gi...@apache.org>.
panzhi33 commented on a change in pull request #3639:
URL: https://github.com/apache/rocketmq/pull/3639#discussion_r768385111
##########
File path: client/src/main/java/org/apache/rocketmq/client/producer/RequestFutureHolder.java
##########
@@ -37,19 +38,13 @@
private static final RequestFutureHolder INSTANCE = new RequestFutureHolder();
private ConcurrentHashMap<String, RequestResponseFuture> requestFutureTable = new ConcurrentHashMap<String, RequestResponseFuture>();
private final AtomicInteger producerNum = new AtomicInteger(0);
Review comment:
like this?
if(set.size > 0 && state != running) {
startScheduledTask
}
if(set.size<=0 && state != shutdown){
shutdown
}
--
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] panzhi33 commented on a change in pull request #3639: [ISSUE #3624]fix DefaultMQPushConsumerTest.testPullMessage_ExceptionO…
Posted by GitBox <gi...@apache.org>.
panzhi33 commented on a change in pull request #3639:
URL: https://github.com/apache/rocketmq/pull/3639#discussion_r769266575
##########
File path: client/src/main/java/org/apache/rocketmq/client/producer/RequestFutureHolder.java
##########
@@ -74,14 +69,40 @@ public void scanExpiredRequest() {
}
}
- private RequestFutureHolder() {
+ public synchronized void startScheduledTask() {
+ if (this.producerNum.incrementAndGet() == 1) {
+ this.getScheduledExecutorService().scheduleAtFixedRate(new Runnable() {
+ @Override
+ public void run() {
+ try {
+ RequestFutureHolder.getInstance().scanExpiredRequest();
+ } catch (Throwable e) {
+ log.error("scan RequestFutureTable exception", e);
+ }
+ }
+ }, 1000 * 3, 1000, TimeUnit.MILLISECONDS);
+ }
+ }
+
+ public synchronized void shutdown() {
+ if (this.producerNum.decrementAndGet() == 0) {
+ this.scheduledExecutorService.shutdown();
Review comment:
yes，it would be better
--
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] panzhi33 commented on a change in pull request #3639: [ISSUE #3624]fix DefaultMQPushConsumerTest.testPullMessage_ExceptionO…
Posted by GitBox <gi...@apache.org>.
panzhi33 commented on a change in pull request #3639:
URL: https://github.com/apache/rocketmq/pull/3639#discussion_r768371294
##########
File path: client/src/main/java/org/apache/rocketmq/client/producer/RequestFutureHolder.java
##########
@@ -37,19 +38,13 @@
private static final RequestFutureHolder INSTANCE = new RequestFutureHolder();
private ConcurrentHashMap<String, RequestResponseFuture> requestFutureTable = new ConcurrentHashMap<String, RequestResponseFuture>();
private final AtomicInteger producerNum = new AtomicInteger(0);
Review comment:
Don't understand what you mean
--
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 #3639: [ISSUE #3624]fix DefaultMQPushConsumerTest.testPullMessage_ExceptionO…
Posted by GitBox <gi...@apache.org>.
areyouok commented on a change in pull request #3639:
URL: https://github.com/apache/rocketmq/pull/3639#discussion_r767890360
##########
File path: client/src/main/java/org/apache/rocketmq/client/producer/RequestFutureHolder.java
##########
@@ -74,14 +69,39 @@ public void scanExpiredRequest() {
}
}
- private RequestFutureHolder() {
+ public synchronized void startScheduledTask() {
+ if (this.producerNum.incrementAndGet() == 1) {
+ this.getScheduledExecutorService().scheduleAtFixedRate(new Runnable() {
+ @Override
+ public void run() {
+ try {
+ RequestFutureHolder.getInstance().scanExpiredRequest();
+ } catch (Throwable e) {
+ log.error("scan RequestFutureTable exception", e);
+ }
+ }
+ }, 1000 * 3, 1000, TimeUnit.MILLISECONDS);
+ }
+ }
+
+ public synchronized void shutdown() {
+ if (this.producerNum.decrementAndGet() == 0) {
+ this.getScheduledExecutorService().shutdown();
Review comment:
set the scheduledExecutorService to null?
--
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 #3639: [ISSUE #3624]fix DefaultMQPushConsumerTest.testPullMessage_ExceptionO…
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3639:
URL: https://github.com/apache/rocketmq/pull/3639#issuecomment-992445983
[](https://coveralls.io/builds/45018221)
Coverage increased (+0.002%) to 56.239% when pulling **e334a7110bb9ec09d2042e40d4bbb8f4b7e9b60c on panzhi33:fix-3624** into **4a8263b4609cd16b5e7c194528bd875711e01852 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 a change in pull request #3639: [ISSUE #3624]fix DefaultMQPushConsumerTest.testPullMessage_ExceptionO…
Posted by GitBox <gi...@apache.org>.
areyouok commented on a change in pull request #3639:
URL: https://github.com/apache/rocketmq/pull/3639#discussion_r768311983
##########
File path: client/src/main/java/org/apache/rocketmq/client/producer/RequestFutureHolder.java
##########
@@ -37,19 +38,13 @@
private static final RequestFutureHolder INSTANCE = new RequestFutureHolder();
private ConcurrentHashMap<String, RequestResponseFuture> requestFutureTable = new ConcurrentHashMap<String, RequestResponseFuture>();
private final AtomicInteger producerNum = new AtomicInteger(0);
Review comment:
there is no need use AtomicInteger.
I suggest change to HashSet(DefaultMQProducerImpl) and startScheduledTask/shutdown support re-entry.
eg: called DefaultMQProducerImpl.shutdown got exception, and the use may call shutdown again.
--
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 #3639: [ISSUE #3624]fix DefaultMQPushConsumerTest.testPullMessage_ExceptionO…
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3639:
URL: https://github.com/apache/rocketmq/pull/3639#issuecomment-992445983
[](https://coveralls.io/builds/44980459)
Coverage decreased (-0.1%) to 56.109% when pulling **0bc17639aa8fa43aaf8d46b2e5bcbd79df733a70 on panzhi33:fix-3624** into **4a8263b4609cd16b5e7c194528bd875711e01852 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 #3639: [ISSUE #3624]fix DefaultMQPushConsumerTest.testPullMessage_ExceptionO…
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3639:
URL: https://github.com/apache/rocketmq/pull/3639#issuecomment-992445983
[](https://coveralls.io/builds/44994472)
Coverage decreased (-0.05%) to 56.182% when pulling **c38faf40906b6a5b217c7327ffe62f64ba73948b on panzhi33:fix-3624** into **4a8263b4609cd16b5e7c194528bd875711e01852 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 a change in pull request #3639: [ISSUE #3624]fix DefaultMQPushConsumerTest.testPullMessage_ExceptionO…
Posted by GitBox <gi...@apache.org>.
areyouok commented on a change in pull request #3639:
URL: https://github.com/apache/rocketmq/pull/3639#discussion_r768311983
##########
File path: client/src/main/java/org/apache/rocketmq/client/producer/RequestFutureHolder.java
##########
@@ -37,19 +38,13 @@
private static final RequestFutureHolder INSTANCE = new RequestFutureHolder();
private ConcurrentHashMap<String, RequestResponseFuture> requestFutureTable = new ConcurrentHashMap<String, RequestResponseFuture>();
private final AtomicInteger producerNum = new AtomicInteger(0);
Review comment:
there is no need use AtomicInteger.
I suggest change to HashSet<DefaultMQProducerImpl> and startScheduledTask/shutdown support re-entry.
eg: called DefaultMQProducerImpl.shutdown got exception, and the use may call shutdown again.
##########
File path: client/src/main/java/org/apache/rocketmq/client/producer/RequestFutureHolder.java
##########
@@ -74,14 +69,40 @@ public void scanExpiredRequest() {
}
}
- private RequestFutureHolder() {
+ public synchronized void startScheduledTask() {
+ if (this.producerNum.incrementAndGet() == 1) {
+ this.getScheduledExecutorService().scheduleAtFixedRate(new Runnable() {
+ @Override
+ public void run() {
+ try {
+ RequestFutureHolder.getInstance().scanExpiredRequest();
+ } catch (Throwable e) {
+ log.error("scan RequestFutureTable exception", e);
+ }
+ }
+ }, 1000 * 3, 1000, TimeUnit.MILLISECONDS);
+ }
+ }
+
+ public synchronized void shutdown() {
+ if (this.producerNum.decrementAndGet() == 0) {
+ this.scheduledExecutorService.shutdown();
+ this.scheduledExecutorService = null;
+ }
}
- public AtomicInteger getProducerNum() {
- return producerNum;
+ private RequestFutureHolder() {
}
- public ScheduledExecutorService getScheduledExecutorService() {
+ private ScheduledExecutorService getScheduledExecutorService() {
+ if (null == scheduledExecutorService || scheduledExecutorService.isShutdown()) {
Review comment:
no need to check isShutdown
##########
File path: client/src/main/java/org/apache/rocketmq/client/producer/RequestFutureHolder.java
##########
@@ -74,14 +69,40 @@ public void scanExpiredRequest() {
}
}
- private RequestFutureHolder() {
+ public synchronized void startScheduledTask() {
+ if (this.producerNum.incrementAndGet() == 1) {
+ this.getScheduledExecutorService().scheduleAtFixedRate(new Runnable() {
+ @Override
+ public void run() {
+ try {
+ RequestFutureHolder.getInstance().scanExpiredRequest();
+ } catch (Throwable e) {
+ log.error("scan RequestFutureTable exception", e);
+ }
+ }
+ }, 1000 * 3, 1000, TimeUnit.MILLISECONDS);
+ }
+ }
+
+ public synchronized void shutdown() {
+ if (this.producerNum.decrementAndGet() == 0) {
+ this.scheduledExecutorService.shutdown();
Review comment:
check null
--
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 #3639: [ISSUE #3624]fix DefaultMQPushConsumerTest.testPullMessage_ExceptionO…
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3639:
URL: https://github.com/apache/rocketmq/pull/3639#issuecomment-992472013
# [Codecov](https://codecov.io/gh/apache/rocketmq/pull/3639?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 [#3639](https://codecov.io/gh/apache/rocketmq/pull/3639?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (0bc1763) into [develop](https://codecov.io/gh/apache/rocketmq/commit/50d45f23a186051d177990fd00a89d152c5c4dae?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (50d45f2) will **increase** coverage by `0.76%`.
> The diff coverage is `56.19%`.
[](https://codecov.io/gh/apache/rocketmq/pull/3639?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 #3639 +/- ##
=============================================
+ Coverage 48.86% 49.62% +0.76%
- Complexity 4659 4713 +54
=============================================
Files 555 555
Lines 36726 36773 +47
Branches 4840 4846 +6
=============================================
+ Hits 17946 18250 +304
+ Misses 16532 16232 -300
- Partials 2248 2291 +43
```
| [Impacted Files](https://codecov.io/gh/apache/rocketmq/pull/3639?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...cketmq/broker/filter/MessageEvaluationContext.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvZmlsdGVyL01lc3NhZ2VFdmFsdWF0aW9uQ29udGV4dC5qYXZh) | `30.76% <0.00%> (ø)` | |
| [...tmq/broker/longpolling/PullRequestHoldService.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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.48% <0.00%> (ø)` | |
| [...ocketmq/client/consumer/DefaultMQPushConsumer.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvY29uc3VtZXIvRGVmYXVsdE1RUHVzaENvbnN1bWVyLmphdmE=) | `55.94% <0.00%> (ø)` | |
| [...mq/client/consumer/store/LocalFileOffsetStore.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvY29uc3VtZXIvc3RvcmUvTG9jYWxGaWxlT2Zmc2V0U3RvcmUuamF2YQ==) | `56.31% <0.00%> (-0.56%)` | :arrow_down: |
| [.../rocketmq/client/impl/ClientRemotingProcessor.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9DbGllbnRSZW1vdGluZ1Byb2Nlc3Nvci5qYXZh) | `2.91% <0.00%> (ø)` | |
| [...java/org/apache/rocketmq/common/Configuration.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vQ29uZmlndXJhdGlvbi5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...org/apache/rocketmq/common/message/MessageExt.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vbWVzc2FnZS9NZXNzYWdlRXh0LmphdmE=) | `66.66% <0.00%> (-1.57%)` | :arrow_down: |
| [...rocketmq/remoting/netty/NettyRemotingAbstract.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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.22%)` | :arrow_down: |
| [...n/java/org/apache/rocketmq/test/util/FileUtil.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-dGVzdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvdGVzdC91dGlsL0ZpbGVVdGlsLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...in/java/org/apache/rocketmq/test/util/MQAdmin.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-dGVzdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvdGVzdC91dGlsL01RQWRtaW4uamF2YQ==) | `38.88% <0.00%> (ø)` | |
| ... and [46 more](https://codecov.io/gh/apache/rocketmq/pull/3639/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/3639?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/3639?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 [1635856...0bc1763](https://codecov.io/gh/apache/rocketmq/pull/3639?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] coveralls commented on pull request #3639: [ISSUE #3624]fix DefaultMQPushConsumerTest.testPullMessage_ExceptionO…
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #3639:
URL: https://github.com/apache/rocketmq/pull/3639#issuecomment-992445983
[](https://coveralls.io/builds/44954410)
Coverage increased (+0.03%) to 56.263% when pulling **46791bf94733ac31ccc3f7b5e818d757b2bbd79a on panzhi33:fix-3624** into **4a8263b4609cd16b5e7c194528bd875711e01852 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] codecov-commenter edited a comment on pull request #3639: [ISSUE #3624]fix DefaultMQPushConsumerTest.testPullMessage_ExceptionO…
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3639:
URL: https://github.com/apache/rocketmq/pull/3639#issuecomment-992472013
# [Codecov](https://codecov.io/gh/apache/rocketmq/pull/3639?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 [#3639](https://codecov.io/gh/apache/rocketmq/pull/3639?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e334a71) into [develop](https://codecov.io/gh/apache/rocketmq/commit/50d45f23a186051d177990fd00a89d152c5c4dae?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (50d45f2) will **increase** coverage by `0.93%`.
> The diff coverage is `55.83%`.
[](https://codecov.io/gh/apache/rocketmq/pull/3639?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 #3639 +/- ##
=============================================
+ Coverage 48.86% 49.80% +0.93%
- Complexity 4659 4739 +80
=============================================
Files 555 555
Lines 36726 36772 +46
Branches 4840 4848 +8
=============================================
+ Hits 17946 18314 +368
+ Misses 16532 16173 -359
- Partials 2248 2285 +37
```
| [Impacted Files](https://codecov.io/gh/apache/rocketmq/pull/3639?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...cketmq/broker/filter/MessageEvaluationContext.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvZmlsdGVyL01lc3NhZ2VFdmFsdWF0aW9uQ29udGV4dC5qYXZh) | `30.76% <0.00%> (ø)` | |
| [...tmq/broker/longpolling/PullRequestHoldService.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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.48% <0.00%> (ø)` | |
| [...ocketmq/client/consumer/DefaultMQPushConsumer.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvY29uc3VtZXIvRGVmYXVsdE1RUHVzaENvbnN1bWVyLmphdmE=) | `56.43% <0.00%> (+0.49%)` | :arrow_up: |
| [...mq/client/consumer/store/LocalFileOffsetStore.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvY29uc3VtZXIvc3RvcmUvTG9jYWxGaWxlT2Zmc2V0U3RvcmUuamF2YQ==) | `56.31% <0.00%> (-0.56%)` | :arrow_down: |
| [.../rocketmq/client/impl/ClientRemotingProcessor.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9DbGllbnRSZW1vdGluZ1Byb2Nlc3Nvci5qYXZh) | `2.91% <0.00%> (ø)` | |
| [...java/org/apache/rocketmq/common/Configuration.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vQ29uZmlndXJhdGlvbi5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...org/apache/rocketmq/common/message/MessageExt.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vbWVzc2FnZS9NZXNzYWdlRXh0LmphdmE=) | `66.66% <0.00%> (-1.57%)` | :arrow_down: |
| [...rocketmq/remoting/netty/NettyRemotingAbstract.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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) | `50.91% <0.00%> (-0.19%)` | :arrow_down: |
| [...n/java/org/apache/rocketmq/test/util/FileUtil.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-dGVzdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvdGVzdC91dGlsL0ZpbGVVdGlsLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...in/java/org/apache/rocketmq/test/util/MQAdmin.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-dGVzdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvdGVzdC91dGlsL01RQWRtaW4uamF2YQ==) | `44.44% <0.00%> (+5.55%)` | :arrow_up: |
| ... and [45 more](https://codecov.io/gh/apache/rocketmq/pull/3639/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/3639?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/3639?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 [1635856...e334a71](https://codecov.io/gh/apache/rocketmq/pull/3639?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] panzhi33 commented on a change in pull request #3639: [ISSUE #3624]fix DefaultMQPushConsumerTest.testPullMessage_ExceptionO…
Posted by GitBox <gi...@apache.org>.
panzhi33 commented on a change in pull request #3639:
URL: https://github.com/apache/rocketmq/pull/3639#discussion_r768254475
##########
File path: client/src/main/java/org/apache/rocketmq/client/producer/RequestFutureHolder.java
##########
@@ -74,14 +69,39 @@ public void scanExpiredRequest() {
}
}
- private RequestFutureHolder() {
+ public synchronized void startScheduledTask() {
+ if (this.producerNum.incrementAndGet() == 1) {
+ this.getScheduledExecutorService().scheduleAtFixedRate(new Runnable() {
+ @Override
+ public void run() {
+ try {
+ RequestFutureHolder.getInstance().scanExpiredRequest();
+ } catch (Throwable e) {
+ log.error("scan RequestFutureTable exception", e);
+ }
+ }
+ }, 1000 * 3, 1000, TimeUnit.MILLISECONDS);
+ }
+ }
+
+ public synchronized void shutdown() {
+ if (this.producerNum.decrementAndGet() == 0) {
+ this.getScheduledExecutorService().shutdown();
Review comment:
ok
--
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 #3639: [ISSUE #3624]fix DefaultMQPushConsumerTest.testPullMessage_ExceptionO…
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3639:
URL: https://github.com/apache/rocketmq/pull/3639#issuecomment-992472013
# [Codecov](https://codecov.io/gh/apache/rocketmq/pull/3639?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 [#3639](https://codecov.io/gh/apache/rocketmq/pull/3639?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (71a2987) into [develop](https://codecov.io/gh/apache/rocketmq/commit/50d45f23a186051d177990fd00a89d152c5c4dae?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (50d45f2) will **increase** coverage by `0.86%`.
> The diff coverage is `56.66%`.
[](https://codecov.io/gh/apache/rocketmq/pull/3639?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 #3639 +/- ##
=============================================
+ Coverage 48.86% 49.73% +0.86%
- Complexity 4659 4729 +70
=============================================
Files 555 555
Lines 36726 36772 +46
Branches 4840 4846 +6
=============================================
+ Hits 17946 18287 +341
+ Misses 16532 16200 -332
- Partials 2248 2285 +37
```
| [Impacted Files](https://codecov.io/gh/apache/rocketmq/pull/3639?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...cketmq/broker/filter/MessageEvaluationContext.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvZmlsdGVyL01lc3NhZ2VFdmFsdWF0aW9uQ29udGV4dC5qYXZh) | `30.76% <0.00%> (ø)` | |
| [...tmq/broker/longpolling/PullRequestHoldService.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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.48% <0.00%> (ø)` | |
| [...ocketmq/client/consumer/DefaultMQPushConsumer.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvY29uc3VtZXIvRGVmYXVsdE1RUHVzaENvbnN1bWVyLmphdmE=) | `56.43% <0.00%> (+0.49%)` | :arrow_up: |
| [...mq/client/consumer/store/LocalFileOffsetStore.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvY29uc3VtZXIvc3RvcmUvTG9jYWxGaWxlT2Zmc2V0U3RvcmUuamF2YQ==) | `56.31% <0.00%> (-0.56%)` | :arrow_down: |
| [.../rocketmq/client/impl/ClientRemotingProcessor.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9DbGllbnRSZW1vdGluZ1Byb2Nlc3Nvci5qYXZh) | `2.91% <0.00%> (ø)` | |
| [...java/org/apache/rocketmq/common/Configuration.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vQ29uZmlndXJhdGlvbi5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...org/apache/rocketmq/common/message/MessageExt.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vbWVzc2FnZS9NZXNzYWdlRXh0LmphdmE=) | `66.66% <0.00%> (-1.57%)` | :arrow_down: |
| [...rocketmq/remoting/netty/NettyRemotingAbstract.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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.22%)` | :arrow_down: |
| [...n/java/org/apache/rocketmq/test/util/FileUtil.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-dGVzdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvdGVzdC91dGlsL0ZpbGVVdGlsLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...in/java/org/apache/rocketmq/test/util/MQAdmin.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-dGVzdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvdGVzdC91dGlsL01RQWRtaW4uamF2YQ==) | `38.88% <0.00%> (ø)` | |
| ... and [51 more](https://codecov.io/gh/apache/rocketmq/pull/3639/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/3639?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/3639?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 [1635856...71a2987](https://codecov.io/gh/apache/rocketmq/pull/3639?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 #3639: [ISSUE #3624]fix DefaultMQPushConsumerTest.testPullMessage_ExceptionO…
Posted by GitBox <gi...@apache.org>.
areyouok commented on a change in pull request #3639:
URL: https://github.com/apache/rocketmq/pull/3639#discussion_r769190299
##########
File path: client/src/main/java/org/apache/rocketmq/client/producer/RequestFutureHolder.java
##########
@@ -74,14 +73,42 @@ public void scanExpiredRequest() {
}
}
- private RequestFutureHolder() {
+ public synchronized void startScheduledTask(DefaultMQProducerImpl producer) {
+ this.producerSet.add(producer);
+ if (this.producerSet.size() >= 1 && this.serviceState != ServiceState.RUNNING) {
+ this.serviceState = ServiceState.START_FAILED;
+
+ this.getScheduledExecutorService().scheduleAtFixedRate(new Runnable() {
+ @Override
+ public void run() {
+ try {
+ RequestFutureHolder.getInstance().scanExpiredRequest();
+ } catch (Throwable e) {
+ log.error("scan RequestFutureTable exception", e);
+ }
+ }
+ }, 1000 * 3, 1000, TimeUnit.MILLISECONDS);
+
+ this.serviceState = ServiceState.RUNNING;
+ }
}
- public AtomicInteger getProducerNum() {
- return producerNum;
+ public synchronized void shutdown(DefaultMQProducerImpl producer) {
+ this.producerSet.remove(producer);
+ if (this.producerSet.size() <= 0 && null != this.scheduledExecutorService && this.serviceState != ServiceState.SHUTDOWN_ALREADY) {
+ this.scheduledExecutorService.shutdown();
+ this.scheduledExecutorService = null;
+ this.serviceState = ServiceState.SHUTDOWN_ALREADY;
+ }
}
- public ScheduledExecutorService getScheduledExecutorService() {
+ private RequestFutureHolder() {
+ }
+
+ private ScheduledExecutorService getScheduledExecutorService() {
Review comment:
this method should inline
##########
File path: client/src/main/java/org/apache/rocketmq/client/producer/RequestFutureHolder.java
##########
@@ -74,14 +73,42 @@ public void scanExpiredRequest() {
}
}
- private RequestFutureHolder() {
+ public synchronized void startScheduledTask(DefaultMQProducerImpl producer) {
+ this.producerSet.add(producer);
+ if (this.producerSet.size() >= 1 && this.serviceState != ServiceState.RUNNING) {
Review comment:
no need check producerSet.size()
##########
File path: client/src/main/java/org/apache/rocketmq/client/producer/RequestFutureHolder.java
##########
@@ -17,39 +17,38 @@
package org.apache.rocketmq.client.producer;
+import java.util.HashSet;
import java.util.Iterator;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.Executors;
import java.util.concurrent.ScheduledExecutorService;
-import java.util.concurrent.ThreadFactory;
-import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.TimeUnit;
import org.apache.rocketmq.client.common.ClientErrorCode;
import org.apache.rocketmq.client.exception.RequestTimeoutException;
+import org.apache.rocketmq.client.impl.producer.DefaultMQProducerImpl;
import org.apache.rocketmq.client.log.ClientLogger;
+import org.apache.rocketmq.common.ServiceState;
+import org.apache.rocketmq.common.ThreadFactoryImpl;
import org.apache.rocketmq.logging.InternalLogger;
public class RequestFutureHolder {
private static InternalLogger log = ClientLogger.getLog();
private static final RequestFutureHolder INSTANCE = new RequestFutureHolder();
private ConcurrentHashMap<String, RequestResponseFuture> requestFutureTable = new ConcurrentHashMap<String, RequestResponseFuture>();
- private final AtomicInteger producerNum = new AtomicInteger(0);
- private final ScheduledExecutorService scheduledExecutorService = Executors
- .newSingleThreadScheduledExecutor(new ThreadFactory() {
- @Override
- public Thread newThread(Runnable r) {
- return new Thread(r, "RequestHouseKeepingService");
- }
- });
+ private final Set<DefaultMQProducerImpl> producerSet = new HashSet<>();
+ private ScheduledExecutorService scheduledExecutorService = null;
+ private ServiceState serviceState = ServiceState.CREATE_JUST;
Review comment:
this serviceState is duplicate. we can use scheduledExecutorService==null to check
##########
File path: client/src/main/java/org/apache/rocketmq/client/producer/RequestFutureHolder.java
##########
@@ -74,14 +69,40 @@ public void scanExpiredRequest() {
}
}
- private RequestFutureHolder() {
+ public synchronized void startScheduledTask() {
+ if (this.producerNum.incrementAndGet() == 1) {
+ this.getScheduledExecutorService().scheduleAtFixedRate(new Runnable() {
+ @Override
+ public void run() {
+ try {
+ RequestFutureHolder.getInstance().scanExpiredRequest();
+ } catch (Throwable e) {
+ log.error("scan RequestFutureTable exception", e);
+ }
+ }
+ }, 1000 * 3, 1000, TimeUnit.MILLISECONDS);
+ }
+ }
+
+ public synchronized void shutdown() {
+ if (this.producerNum.decrementAndGet() == 0) {
+ this.scheduledExecutorService.shutdown();
Review comment:
may thutdown() throws exception?
the below code maybe better?
```
ScheduledExecutorService ses = this.scheduledExecutorService;
this.scheduledExecutorService = null;
ses.shutdown();
```
--
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 #3639: [ISSUE #3624]fix DefaultMQPushConsumerTest.testPullMessage_ExceptionO…
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3639:
URL: https://github.com/apache/rocketmq/pull/3639#issuecomment-992472013
# [Codecov](https://codecov.io/gh/apache/rocketmq/pull/3639?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 [#3639](https://codecov.io/gh/apache/rocketmq/pull/3639?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (c38faf4) into [develop](https://codecov.io/gh/apache/rocketmq/commit/50d45f23a186051d177990fd00a89d152c5c4dae?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (50d45f2) will **increase** coverage by `0.77%`.
> The diff coverage is `61.48%`.
[](https://codecov.io/gh/apache/rocketmq/pull/3639?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 #3639 +/- ##
=============================================
+ Coverage 48.86% 49.63% +0.77%
- Complexity 4659 4716 +57
=============================================
Files 555 555
Lines 36726 36779 +53
Branches 4840 4848 +8
=============================================
+ Hits 17946 18257 +311
+ Misses 16532 16230 -302
- Partials 2248 2292 +44
```
| [Impacted Files](https://codecov.io/gh/apache/rocketmq/pull/3639?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...cketmq/broker/filter/MessageEvaluationContext.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvZmlsdGVyL01lc3NhZ2VFdmFsdWF0aW9uQ29udGV4dC5qYXZh) | `30.76% <0.00%> (ø)` | |
| [...tmq/broker/longpolling/PullRequestHoldService.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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.48% <0.00%> (ø)` | |
| [...ocketmq/client/consumer/DefaultMQPushConsumer.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvY29uc3VtZXIvRGVmYXVsdE1RUHVzaENvbnN1bWVyLmphdmE=) | `55.94% <0.00%> (ø)` | |
| [...mq/client/consumer/store/LocalFileOffsetStore.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvY29uc3VtZXIvc3RvcmUvTG9jYWxGaWxlT2Zmc2V0U3RvcmUuamF2YQ==) | `56.31% <0.00%> (-0.56%)` | :arrow_down: |
| [.../rocketmq/client/impl/ClientRemotingProcessor.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9DbGllbnRSZW1vdGluZ1Byb2Nlc3Nvci5qYXZh) | `2.91% <0.00%> (ø)` | |
| [...java/org/apache/rocketmq/common/Configuration.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vQ29uZmlndXJhdGlvbi5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...org/apache/rocketmq/common/message/MessageExt.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vbWVzc2FnZS9NZXNzYWdlRXh0LmphdmE=) | `66.66% <0.00%> (-1.57%)` | :arrow_down: |
| [...rocketmq/remoting/netty/NettyRemotingAbstract.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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) | `50.91% <0.00%> (-0.19%)` | :arrow_down: |
| [...n/java/org/apache/rocketmq/test/util/FileUtil.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-dGVzdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvdGVzdC91dGlsL0ZpbGVVdGlsLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...in/java/org/apache/rocketmq/test/util/MQAdmin.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-dGVzdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvdGVzdC91dGlsL01RQWRtaW4uamF2YQ==) | `38.88% <0.00%> (ø)` | |
| ... and [42 more](https://codecov.io/gh/apache/rocketmq/pull/3639/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/3639?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/3639?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 [1635856...c38faf4](https://codecov.io/gh/apache/rocketmq/pull/3639?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 #3639: [ISSUE #3624]fix DefaultMQPushConsumerTest.testPullMessage_ExceptionO…
Posted by GitBox <gi...@apache.org>.
areyouok commented on a change in pull request #3639:
URL: https://github.com/apache/rocketmq/pull/3639#discussion_r768311983
##########
File path: client/src/main/java/org/apache/rocketmq/client/producer/RequestFutureHolder.java
##########
@@ -37,19 +38,13 @@
private static final RequestFutureHolder INSTANCE = new RequestFutureHolder();
private ConcurrentHashMap<String, RequestResponseFuture> requestFutureTable = new ConcurrentHashMap<String, RequestResponseFuture>();
private final AtomicInteger producerNum = new AtomicInteger(0);
Review comment:
there is no need use AtomicInteger.
I suggest change to HashSet《DefaultMQProducerImpl》 and startScheduledTask/shutdown support re-entry.
eg: called DefaultMQProducerImpl.shutdown got exception, and the user may call shutdown again.
--
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] panzhi33 commented on a change in pull request #3639: [ISSUE #3624]fix DefaultMQPushConsumerTest.testPullMessage_ExceptionO…
Posted by GitBox <gi...@apache.org>.
panzhi33 commented on a change in pull request #3639:
URL: https://github.com/apache/rocketmq/pull/3639#discussion_r768371294
##########
File path: client/src/main/java/org/apache/rocketmq/client/producer/RequestFutureHolder.java
##########
@@ -37,19 +38,13 @@
private static final RequestFutureHolder INSTANCE = new RequestFutureHolder();
private ConcurrentHashMap<String, RequestResponseFuture> requestFutureTable = new ConcurrentHashMap<String, RequestResponseFuture>();
private final AtomicInteger producerNum = new AtomicInteger(0);
Review comment:
Don't understand what you mean
--
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 #3639: [ISSUE #3624]fix DefaultMQPushConsumerTest.testPullMessage_ExceptionO…
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #3639:
URL: https://github.com/apache/rocketmq/pull/3639#issuecomment-992472013
# [Codecov](https://codecov.io/gh/apache/rocketmq/pull/3639?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 [#3639](https://codecov.io/gh/apache/rocketmq/pull/3639?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (5782bcc) into [develop](https://codecov.io/gh/apache/rocketmq/commit/50d45f23a186051d177990fd00a89d152c5c4dae?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (50d45f2) will **increase** coverage by `0.89%`.
> The diff coverage is `56.19%`.
[](https://codecov.io/gh/apache/rocketmq/pull/3639?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 #3639 +/- ##
=============================================
+ Coverage 48.86% 49.75% +0.89%
- Complexity 4659 4732 +73
=============================================
Files 555 555
Lines 36726 36773 +47
Branches 4840 4846 +6
=============================================
+ Hits 17946 18298 +352
+ Misses 16532 16193 -339
- Partials 2248 2282 +34
```
| [Impacted Files](https://codecov.io/gh/apache/rocketmq/pull/3639?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...cketmq/broker/filter/MessageEvaluationContext.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-YnJva2VyL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9icm9rZXIvZmlsdGVyL01lc3NhZ2VFdmFsdWF0aW9uQ29udGV4dC5qYXZh) | `30.76% <0.00%> (ø)` | |
| [...tmq/broker/longpolling/PullRequestHoldService.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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.48% <0.00%> (ø)` | |
| [...ocketmq/client/consumer/DefaultMQPushConsumer.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvY29uc3VtZXIvRGVmYXVsdE1RUHVzaENvbnN1bWVyLmphdmE=) | `56.43% <0.00%> (+0.49%)` | :arrow_up: |
| [...mq/client/consumer/store/LocalFileOffsetStore.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvY29uc3VtZXIvc3RvcmUvTG9jYWxGaWxlT2Zmc2V0U3RvcmUuamF2YQ==) | `56.31% <0.00%> (-0.56%)` | :arrow_down: |
| [.../rocketmq/client/impl/ClientRemotingProcessor.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-Y2xpZW50L3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jbGllbnQvaW1wbC9DbGllbnRSZW1vdGluZ1Byb2Nlc3Nvci5qYXZh) | `2.91% <0.00%> (ø)` | |
| [...java/org/apache/rocketmq/common/Configuration.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vQ29uZmlndXJhdGlvbi5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...org/apache/rocketmq/common/message/MessageExt.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-Y29tbW9uL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS9yb2NrZXRtcS9jb21tb24vbWVzc2FnZS9NZXNzYWdlRXh0LmphdmE=) | `66.66% <0.00%> (-1.57%)` | :arrow_down: |
| [...rocketmq/remoting/netty/NettyRemotingAbstract.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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) | `50.91% <0.00%> (-0.19%)` | :arrow_down: |
| [...n/java/org/apache/rocketmq/test/util/FileUtil.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-dGVzdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvdGVzdC91dGlsL0ZpbGVVdGlsLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...in/java/org/apache/rocketmq/test/util/MQAdmin.java](https://codecov.io/gh/apache/rocketmq/pull/3639/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-dGVzdC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcm9ja2V0bXEvdGVzdC91dGlsL01RQWRtaW4uamF2YQ==) | `44.44% <0.00%> (+5.55%)` | :arrow_up: |
| ... and [43 more](https://codecov.io/gh/apache/rocketmq/pull/3639/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/3639?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/3639?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 [1635856...5782bcc](https://codecov.io/gh/apache/rocketmq/pull/3639?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 #3639: [ISSUE #3624]fix DefaultMQPushConsumerTest.testPullMessage_ExceptionO…
Posted by GitBox <gi...@apache.org>.
areyouok commented on a change in pull request #3639:
URL: https://github.com/apache/rocketmq/pull/3639#discussion_r768259450
##########
File path: client/src/main/java/org/apache/rocketmq/client/producer/RequestFutureHolder.java
##########
@@ -74,14 +69,39 @@ public void scanExpiredRequest() {
}
}
- private RequestFutureHolder() {
+ public synchronized void startScheduledTask() {
+ if (this.producerNum.incrementAndGet() == 1) {
+ this.getScheduledExecutorService().scheduleAtFixedRate(new Runnable() {
+ @Override
+ public void run() {
+ try {
+ RequestFutureHolder.getInstance().scanExpiredRequest();
+ } catch (Throwable e) {
+ log.error("scan RequestFutureTable exception", e);
+ }
+ }
+ }, 1000 * 3, 1000, TimeUnit.MILLISECONDS);
+ }
+ }
+
+ public synchronized void shutdown() {
+ if (this.producerNum.decrementAndGet() == 0) {
+ this.getScheduledExecutorService().shutdown();
Review comment:
1. shutdown
2. set to null
--
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 #3639: [ISSUE #3624]fix DefaultMQPushConsumerTest.testPullMessage_ExceptionO…
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #3639:
URL: https://github.com/apache/rocketmq/pull/3639#issuecomment-992445983
[](https://coveralls.io/builds/44978561)
Coverage decreased (-0.1%) to 56.138% when pulling **5782bcca66bc244305534624b8db5903f4d83e3f on panzhi33:fix-3624** into **4a8263b4609cd16b5e7c194528bd875711e01852 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] panzhi33 commented on a change in pull request #3639: [ISSUE #3624]fix DefaultMQPushConsumerTest.testPullMessage_ExceptionO…
Posted by GitBox <gi...@apache.org>.
panzhi33 commented on a change in pull request #3639:
URL: https://github.com/apache/rocketmq/pull/3639#discussion_r768385111
##########
File path: client/src/main/java/org/apache/rocketmq/client/producer/RequestFutureHolder.java
##########
@@ -37,19 +38,13 @@
private static final RequestFutureHolder INSTANCE = new RequestFutureHolder();
private ConcurrentHashMap<String, RequestResponseFuture> requestFutureTable = new ConcurrentHashMap<String, RequestResponseFuture>();
private final AtomicInteger producerNum = new AtomicInteger(0);
Review comment:
like this
if(set.size > 0 && state != running) {
startScheduledTask
}
if(set.size<=0 && state != shutdown){
shutdown
}
--
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