fix: gracefully suspend bee node on interrupt signal #3671
Conversation
vladopajic
requested review from
a team,
istae and
notanatol
and removed request for
a team
cmd/bee/cmd/start.go
Outdated
| }, | ||
| stop: func() { | ||
| // Shutdown | ||
| // Whenever program is being stoppend we need to close |
|
The interrupt signal still does not work when batch synchronization is in progress. Also when a node is waiting to be funded. |
vladopajic
changed the title
cmd/bee/cmd/start.go
Outdated
| select { | ||
| case <-sysInterruptChannel: | ||
| logger.Debug("received interrupt signal") | ||
| case <-time.After(shutdownTimeout): |
There was a problem hiding this comment.
This is not correct. Adding a hard-coded timeout is not the right thing to do. There could be valid cases when the node takes more than 5 secs to stop.
As the comment said, the correct way to handle this is to check for a second interrupt signal to force stop if user does not have the patience. I am not sure if this can be done using context cancellation.
There was a problem hiding this comment.
Nice work. This looks good. Can you manually test the following scenarios, as we dont have proper unit tests in this package.
- Ctrl-C waiting for the chequebook deployment
- Ctrl-C while postage sync is running
- Ideally we should test if Ctrl-C also works during bootstrapping. But right now we have not yet updated the postage snapshot feed. When we do, we should test this.
- Randomly try to do Ctrl-C during different times. Also trying this after node warmup period (5mins) is done.
|
@aloknerurkar I have manually testes all those scenarios. However there could be some issue if some logic is not responding to context cancelation signal. This would be hard to catch on this level; instead it should be covered/tested in respective package. |
Checklist
Description
Closes: #3660
This change is