Repository navigation
Conversation
99a311c to
23a6575
Compare
0fcc771 to
1cd3b74
Compare
|
@coderabbitai review |
|
|
@coderabbitai approve |
✅ Action performedComments resolved and changes approved. |
1cd3b74 to
6eb92c2
Compare
23a6575 to
b681e69
Compare
|
@coderabbitai review |
|
|
🔎 Reviewing |
There was a problem hiding this comment.
This PR makes bot shutdown wait for the auxiliary supervisor (the metrics recorder upkeep, the liq_* refreshes and the dashboard price feed) instead of only sending it the stop. Every shutdown trigger, and a signal that arrives before startup completes, now goes through one drain_session helper. That helper sends the auxiliary stop and joins a wait on it, bounded by the same 90 s drain timeout, with the existing bot, server and detached-task drains. Because the waits run at the same time, the worst-case shutdown time does not grow.
I found no defects at this head. I checked the task-supervisor wait() and Shutdown handling: wait() reads a watch value, so a second call or a supervisor that already stopped returns at once, and an error only logs a warning. An error from the main drain still propagates after the auxiliary wait. The trigger test makes the auxiliary wait the only yield point (finished bot task, empty tracker, current-thread runtime) and checks it with one now_or_never poll, so it fails if the wait is removed. The stuck-supervisor test shows the bound holds. The change touches no stored state, config or shared signer, so upgrade and rollback are not affected. The fixes from all earlier threads are present, and the remaining limit (aborted tasks and a blocking P&L replay are not awaited) is stated in the doc comment and the PR's Risks section.
claude-opus-5-5 · high · 25 min
|
@coderabbitai approve |
✅ Action performedComments resolved and changes approved. |
|
🔎 Reviewing |
There was a problem hiding this comment.
This PR makes the bot wait for the auxiliary supervisor (the liq_* refreshes and the dashboard price feed) during shutdown. Before, it only sent the stop. One new drain_session now handles every shutdown trigger and the signal-before-startup path. It sends the stop, then joins the existing bot, server and detached-task drains with a wait on the auxiliary supervisor. That wait uses the same drain timeout, so a stuck supervisor delays shutdown by that timeout at most and adds no second one. As a result, those tasks are aborted before the telemetry flush and the clean-stop report.
Overall: the change is small and correct, and the earlier threads are all fixed. The doc comments now describe only what the wait covers. every_shutdown_trigger_waits_for_the_auxiliary_supervisor makes the auxiliary wait the only yield point and checks it with a single now_or_never() poll, so it fails if the wait is removed. The paused-time test shows the timeout bounds a supervisor that never stops. One panel lane said the test runs on a multi-thread runtime and can pass without the wait. That is wrong: plain #[tokio::test] uses the current-thread runtime, so the finding was dropped. No upgrade or shared-stall risk: the change touches no persisted state, and the auxiliary wait runs beside the existing drains. The diff adds no sensitive data to this public repo.
claude-opus-5-5 · high · 52 min
b681e69 to
afed2f1
Compare
6eb92c2 to
0b4fe3a
Compare
|
@coderabbitai approve |
✅ Action performedComments resolved and changes approved. |
afed2f1 to
e4e303e
Compare
0b4fe3a to
a4239d6
Compare
|
@coderabbitai approve |
✅ Action performedComments resolved and changes approved. |
a4239d6 to
f40853a
Compare
e4e303e to
56ebbf5
Compare
|
@coderabbitai approve |
✅ Action performedComments resolved and changes approved. |
Part of RAI-3134.
On shutdown the bot now waits for the auxiliary supervisor to handle its stop, beside its other drains. Before, it sent the stop and returned at once. That supervisor runs the
liq_refreshes and the dashboard price feed, so they are now aborted before the telemetry flush and the clean-stop report run.Live effect: next release. Shutdown waits for that supervisor for up to 90 s, then goes on with a warning · Risk: low (shutdown order; a stuck supervisor can delay shutdown by up to 90 s) · Ships: next bot release · Blocks: needs #1708 below it
Risks
Proof