Open silvestrpredko opened 2 weeks ago
silvestrpredko
commented
2 weeks ago 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.
swanandx
commented
2 weeks ago 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 :)
coveralls
commented
2 weeks ago | 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 | 0.0% | --> |
| 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 |
silvestrpredko
commented
2 weeks ago 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.
coveralls
commented
2 weeks ago | 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 | 0.0% | --> |
| 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 |
silvestrpredko
commented
2 weeks ago 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
coveralls
commented
2 weeks ago | 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 | 0.0% | --> |
| 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 |
Type of change
New feature (non-breaking change which adds functionality)
Checklist:
cargo fmtCHANGELOG.mdif it's relevant to the users of the library. If it's not relevant mention why.