go-spacemesh
go-spacemesh copied to clipboard
Listen for \q to gracefully shutdown the node
Motivation
Issue https://github.com/spacemeshos/go-spacemesh/issues/5321
Listening for \q will allow smapp to gracefully shutdown the node.
Codecov Report
Attention: 18 lines
in your changes are missing coverage. Please review.
Comparison is base (
3675992
) 0.0% compared to head (234a33b
) 77.6%. Report is 42 commits behind head on develop.
Files | Patch % | Lines |
---|---|---|
node/node.go | 0.0% | 18 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## develop #5389 +/- ##
==========================================
+ Coverage 0 77.6% +77.6%
==========================================
Files 0 266 +266
Lines 0 30906 +30906
==========================================
+ Hits 0 24003 +24003
- Misses 0 5389 +5389
- Partials 0 1514 +1514
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
- Works as expected on Windows.
- And works the same on MacOS except for one thing not related to these changes.
- I didn't check it on Linux, but I believe it should work as well :)
Inconsistent behavior when Node is compacting the database and receives \q
:
- On MacOS: it does not produce any log messages anymore (like "app cleanup starting..."), and just closes after a while. It works the same on previous versions of go-spacemesh using SIGTERM signal.
- On Windows: it produces log messages and closes after a while
Except for this inconsistent behavior — LGTM.
So I'd be glad to have these changes if nobody is against the approach with stdin
:)
I don't think listening to stdin is necessary to make a graceful shutdown on windows work. Instead we should try adding syscall.SIGINT
to the argument list of signal.NotifyContext
in node.go
this should capture multiple signals on windows that are used to trigger an application to exit.
See also this change for it: https://go-review.googlesource.com/c/go/+/187739 it was added in Go 1.14. Relevant part in the release notes: https://go.dev/doc/go1.14#ossignalpkgossignal
EDIT: my bad - the problem is not the receiving program in windows its the sender. A assume GeneralConsoleCtrlEvent
cannot be used? https://learn.microsoft.com/en-us/windows/console/generateconsolectrlevent?redirectedfrom=MSDN
bors merge
The merge failed because reading from os.Stdin
immediately returns io.EOF
when the node runs inside docker - shutting down the app shortly after starting.
I feel like this can be closed, reading from os.Stdin
won't work when the node is run inside a container (there is no stdin in that environment) and using a hook for syscall.SIGINT
won't work on windows.
I believe a proper solution would be to add a handler to the private grpc endpoint that can be called to shut down the node gracefully. It would then call stop
/cancel
on the top level context, that the grpc service needs to be passed in which will gracefully shut down the node on all platforms as expected.
I will close this PR for now, but we should re-visit fixing #5321 at some point in the future.