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 2020/06/10 07:30:31 UTC
[GitHub] [rocketmq] Jaskey opened a new pull request #2084: support graceful shutdown push consumer
Jaskey opened a new pull request #2084:
URL: https://github.com/apache/rocketmq/pull/2084
## What is the purpose of the change
For now, RocketMQ does not support a graceful shutdown for push consumer, which means that when a user shutdown a push consumer, the threadpool will shutdown but rocketmq client will not wait for the existing task to be completed, which may cause
1. Error is found when shutdown. Since the application since application may shutdown some external source like datasource when application shutdowns, so the existing task may fail.
2. Message duplication. Even though the task can be completed, since rocketmq will update consumer offset after the shutdown method returns, so the latest consumer offset should be updated after the remaining tasks are completed
## Brief changelog
Add a parameter awaitTerminationMillisWhenShutdown(default is 0), to determine how long should the rocketmq client to await for the consumer threadpool to terminate before update consumer offset
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] Jaskey commented on a change in pull request #2084: [ISSUE#2085]support graceful shutdown push consumer
Posted by GitBox <gi...@apache.org>.
Jaskey commented on a change in pull request #2084:
URL: https://github.com/apache/rocketmq/pull/2084#discussion_r437960321
##########
File path: client/src/main/java/org/apache/rocketmq/client/impl/consumer/ConsumeMessageConcurrentlyService.java
##########
@@ -92,9 +93,9 @@ public void run() {
}, this.defaultMQPushConsumer.getConsumeTimeout(), this.defaultMQPushConsumer.getConsumeTimeout(), TimeUnit.MINUTES);
}
- public void shutdown() {
Review comment:
fine!
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] coveralls edited a comment on pull request #2084: [ISSUE#2085]support graceful shutdown push consumer
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #2084:
URL: https://github.com/apache/rocketmq/pull/2084#issuecomment-641825003
[](https://coveralls.io/builds/31360738)
Coverage decreased (-0.04%) to 51.16% when pulling **38f983ec27d9913e79b014a974b5f8acd8472944 on Jaskey:dev/graceful-shutdown** into **583f665b82f7756666d3282b08023ac9128a6117 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] codecov-commenter edited a comment on pull request #2084: [ISSUE#2085]support graceful shutdown push consumer
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #2084:
URL: https://github.com/apache/rocketmq/pull/2084#issuecomment-641825093
# [Codecov](https://codecov.io/gh/apache/rocketmq/pull/2084?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`develop@d66243c`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/rocketmq/pull/2084?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## develop #2084 +/- ##
==========================================
Coverage ? 45.45%
Complexity ? 4243
==========================================
Files ? 545
Lines ? 35849
Branches ? 4762
==========================================
Hits ? 16296
Misses ? 17498
Partials ? 2055
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/rocketmq/pull/2084?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/rocketmq/pull/2084?src=pr&el=footer). Last update [d66243c...38f983e](https://codecov.io/gh/apache/rocketmq/pull/2084?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] Jaskey commented on pull request #2084: [ISSUE#2085]support graceful shutdown push consumer
Posted by GitBox <gi...@apache.org>.
Jaskey commented on pull request #2084:
URL: https://github.com/apache/rocketmq/pull/2084#issuecomment-642509704
@dongeforever @duhenglucky one more discussion. Graceful shutdown should be a basic feature, what do you think of the default await millis? Should we give a non-zero default value rather than 0
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] Jaskey commented on pull request #2084: [ISSUE#2085]support graceful shutdown push consumer
Posted by GitBox <gi...@apache.org>.
Jaskey commented on pull request #2084:
URL: https://github.com/apache/rocketmq/pull/2084#issuecomment-642508464
@dongeforever @RongtongJin would you please help to review this pr ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] codecov-commenter commented on pull request #2084: [[ISSUE#2085]support graceful shutdown push consumer
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on pull request #2084:
URL: https://github.com/apache/rocketmq/pull/2084#issuecomment-641825093
# [Codecov](https://codecov.io/gh/apache/rocketmq/pull/2084?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`develop@d66243c`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/rocketmq/pull/2084?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## develop #2084 +/- ##
==========================================
Coverage ? 45.43%
Complexity ? 4241
==========================================
Files ? 545
Lines ? 35847
Branches ? 4762
==========================================
Hits ? 16287
Misses ? 17508
Partials ? 2052
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/rocketmq/pull/2084?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/rocketmq/pull/2084?src=pr&el=footer). Last update [d66243c...c77e123](https://codecov.io/gh/apache/rocketmq/pull/2084?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] ShannonDing merged pull request #2084: [ISSUE#2085]support graceful shutdown push consumer
Posted by GitBox <gi...@apache.org>.
ShannonDing merged pull request #2084:
URL: https://github.com/apache/rocketmq/pull/2084
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] duhenglucky commented on a change in pull request #2084: [[ISSUE#2085]support graceful shutdown push consumer
Posted by GitBox <gi...@apache.org>.
duhenglucky commented on a change in pull request #2084:
URL: https://github.com/apache/rocketmq/pull/2084#discussion_r437955362
##########
File path: client/src/main/java/org/apache/rocketmq/client/impl/consumer/ConsumeMessageConcurrentlyService.java
##########
@@ -92,9 +93,9 @@ public void run() {
}, this.defaultMQPushConsumer.getConsumeTimeout(), this.defaultMQPushConsumer.getConsumeTimeout(), TimeUnit.MINUTES);
}
- public void shutdown() {
Review comment:
It would be better to keep this method forward compatible.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] Jaskey commented on pull request #2084: [ISSUE#2085]support graceful shutdown push consumer
Posted by GitBox <gi...@apache.org>.
Jaskey commented on pull request #2084:
URL: https://github.com/apache/rocketmq/pull/2084#issuecomment-674762760
@ShannonDing which version will be this pr included ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] codecov-commenter edited a comment on pull request #2084: [ISSUE#2085]support graceful shutdown push consumer
Posted by GitBox <gi...@apache.org>.
codecov-commenter edited a comment on pull request #2084:
URL: https://github.com/apache/rocketmq/pull/2084#issuecomment-641825093
# [Codecov](https://codecov.io/gh/apache/rocketmq/pull/2084?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`develop@d66243c`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/rocketmq/pull/2084?src=pr&el=tree)
```diff
@@ Coverage Diff @@
## develop #2084 +/- ##
==========================================
Coverage ? 45.44%
Complexity ? 4243
==========================================
Files ? 545
Lines ? 35849
Branches ? 4762
==========================================
Hits ? 16290
Misses ? 17503
Partials ? 2056
```
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/rocketmq/pull/2084?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/rocketmq/pull/2084?src=pr&el=footer). Last update [d66243c...38f983e](https://codecov.io/gh/apache/rocketmq/pull/2084?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] coveralls edited a comment on pull request #2084: [ISSUE#2085]support graceful shutdown push consumer
Posted by GitBox <gi...@apache.org>.
coveralls edited a comment on pull request #2084:
URL: https://github.com/apache/rocketmq/pull/2084#issuecomment-641825003
[](https://coveralls.io/builds/31359354)
Coverage increased (+0.003%) to 51.204% when pulling **d09624529a4556e3c19f81932b56c91ac7a16f62 on Jaskey:dev/graceful-shutdown** into **583f665b82f7756666d3282b08023ac9128a6117 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [rocketmq] coveralls commented on pull request #2084: [[ISSUE#2085]support graceful shutdown push consumer
Posted by GitBox <gi...@apache.org>.
coveralls commented on pull request #2084:
URL: https://github.com/apache/rocketmq/pull/2084#issuecomment-641825003
[](https://coveralls.io/builds/31357789)
Coverage decreased (-0.04%) to 51.16% when pulling **c77e123106b98fa798606db3485e1dd2c62f0cfe on Jaskey:dev/graceful-shutdown** into **583f665b82f7756666d3282b08023ac9128a6117 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org