Graceful shutdown
Type of change
New feature (non-breaking change which adds functionality)
Checklist:
- [x] Formatted with
cargo fmt - [x] Make an entry to
CHANGELOG.mdif it's relevant to the users of the library. If it's not relevant mention why.
Short description:
I wanna make a graceful shutdown without a significant redesign of the system. For me watch seems like a good solution. Please don't hesitate to suggest improvements to the initial design.
i also made a poc for graceful shutdown using cancellation token, you can check it here https://github.com/bytebeamio/rumqtt/compare/main...rumqttd-shutdown !
we can compare both approaches and combine good stuff from both :)
Pull Request Test Coverage Report for Build 9501924124
Details
- 0 of 71 (0.0%) changed or added relevant lines in 4 files are covered.
- 4 unchanged lines in 2 files lost coverage.
- Overall coverage decreased (-0.1%) to 36.033%
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
|---|---|---|---|
| rumqttd/src/link/bridge.rs | 0 | 1 | 0.0% |
| rumqttd/src/link/timer.rs | 0 | 1 | 0.0% |
| rumqttd/src/link/console.rs | 0 | 4 | 0.0% |
| rumqttd/src/server/broker.rs | 0 | 65 | 0.0% |
| <!-- | Total: | 0 | 71 |
| Files with Coverage Reduction | New Missed Lines | % |
|---|---|---|
| rumqttd/src/link/timer.rs | 1 | 0.0% |
| rumqttd/src/server/broker.rs | 3 | 0.0% |
| <!-- | Total: | 4 |
| Totals | |
|---|---|
| Change from base Build 9501879560: | -0.1% |
| Covered Lines: | 5970 |
| Relevant Lines: | 16568 |
💛 - Coveralls
i also made a poc for graceful shutdown using cancellation token, you can check it here main...rumqttd-shutdown !
we can compare both approaches and combine good stuff from both :)
I like that handle returns in start. The reason why I was ignoring CancellationToken it's that tokio-utils is an optional dependency and I just don't want to change it. So it's more up to you what changes you would like to make, and what it should be used at the end. Please write below your decision.
Pull Request Test Coverage Report for Build 9521027502
Details
- 0 of 73 (0.0%) changed or added relevant lines in 5 files are covered.
- 5 unchanged lines in 3 files lost coverage.
- Overall coverage decreased (-0.09%) to 36.042%
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
|---|---|---|---|
| rumqttd/src/link/bridge.rs | 0 | 1 | 0.0% |
| rumqttd/src/link/timer.rs | 0 | 1 | 0.0% |
| rumqttd/src/link/console.rs | 0 | 4 | 0.0% |
| rumqttd/src/main.rs | 0 | 8 | 0.0% |
| rumqttd/src/server/broker.rs | 0 | 59 | 0.0% |
| <!-- | Total: | 0 | 73 |
| Files with Coverage Reduction | New Missed Lines | % |
|---|---|---|
| rumqttd/src/link/timer.rs | 1 | 0.0% |
| rumqttd/src/main.rs | 1 | 0.0% |
| rumqttd/src/server/broker.rs | 3 | 0.0% |
| <!-- | Total: | 5 |
| Totals | |
|---|---|
| Change from base Build 9501879560: | -0.09% |
| Covered Lines: | 5970 |
| Relevant Lines: | 16564 |
💛 - Coveralls
Hey @swanandx. I redesigned the approach a little bit. It seems like a good solution. Please check it out.
P.S I also added
joinAPI call for situation when you don't want to handle a graceful shutdown
Pull Request Test Coverage Report for Build 9522368042
Details
- 0 of 80 (0.0%) changed or added relevant lines in 5 files are covered.
- 5 unchanged lines in 3 files lost coverage.
- Overall coverage decreased (-0.1%) to 36.027%
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
|---|---|---|---|
| rumqttd/src/link/bridge.rs | 0 | 1 | 0.0% |
| rumqttd/src/link/timer.rs | 0 | 1 | 0.0% |
| rumqttd/src/link/console.rs | 0 | 4 | 0.0% |
| rumqttd/src/main.rs | 0 | 8 | 0.0% |
| rumqttd/src/server/broker.rs | 0 | 66 | 0.0% |
| <!-- | Total: | 0 | 80 |
| Files with Coverage Reduction | New Missed Lines | % |
|---|---|---|
| rumqttd/src/link/timer.rs | 1 | 0.0% |
| rumqttd/src/main.rs | 1 | 0.0% |
| rumqttd/src/server/broker.rs | 3 | 0.0% |
| <!-- | Total: | 5 |
| Totals | |
|---|---|
| Change from base Build 9501879560: | -0.1% |
| Covered Lines: | 5970 |
| Relevant Lines: | 16571 |
💛 - Coveralls
Hey, @swanandx do you have a chance to look at this PR?
Pull Request Test Coverage Report for Build 10043003235
Details
- 0 of 80 (0.0%) changed or added relevant lines in 5 files are covered.
- No unchanged relevant lines lost coverage.
- Overall first build on silvestr/graceful-shutdown at 36.033%
| Changes Missing Coverage | Covered Lines | Changed/Added Lines | % |
|---|---|---|---|
| rumqttd/src/link/bridge.rs | 0 | 1 | 0.0% |
| rumqttd/src/link/timer.rs | 0 | 1 | 0.0% |
| rumqttd/src/link/console.rs | 0 | 4 | 0.0% |
| rumqttd/src/main.rs | 0 | 8 | 0.0% |
| rumqttd/src/server/broker.rs | 0 | 66 | 0.0% |
| <!-- | Total: | 0 | 80 |
| Totals | |
|---|---|
| Change from base Build 9675366490: | 36.0% |
| Covered Lines: | 5970 |
| Relevant Lines: | 16568 |
💛 - Coveralls
@swanandx Just I kindly reminder. Could you please take a look?
@swanandx Just I kindly reminder. Could you please take a look?
Hey, I sincerely apologise for the delay, but I'm not having any bandwidth as of now or in near future. So this might be delayed 😕
@swanandx Any updates on this? Is there another member that can take over the review?