Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CTRL-C doesn't work during node startup #2584

Closed
mrBliss opened this issue Sep 4, 2020 · 4 comments · Fixed by #3452
Closed

CTRL-C doesn't work during node startup #2584

mrBliss opened this issue Sep 4, 2020 · 4 comments · Fixed by #3452
Assignees
Labels
consensus issues related to ouroboros-consensus

Comments

@mrBliss
Copy link
Contributor

mrBliss commented Sep 4, 2020

I noticed that when starting cardano-node, a single CTRL-C is not enough to stop the node while the ChainDB is still being opened. Opening the ChainDB can take a while, as block validity could be checked, blocks have to be replayed to bring the ledger snapshots up to date (or even from genesis!), and initial chain selection happens. A second CTRL-C does stop the node.

The cause of this is that we're opening the ChainDB using ResourceRegistry.allocate: https://github.com/input-output-hk/ouroboros-network/blob/d81d370008727ac9811bbc4fc70ea0cbcde7cd28/ouroboros-consensus/src/Ouroboros/Consensus/Node.hs#L189-L193
and ResourceRegistry.allocate masks asynchronous exceptions while running the opening action: https://github.com/input-output-hk/ouroboros-network/blob/d81d370008727ac9811bbc4fc70ea0cbcde7cd28/ouroboros-consensus/src/Ouroboros/Consensus/Util/ResourceRegistry.hs#L822-L823

@mrBliss mrBliss added consensus issues related to ouroboros-consensus priority low labels Sep 4, 2020
@kderme
Copy link
Contributor

kderme commented Sep 7, 2020

I think the first ^C restores the default interrupt handler, so the second always kills the application https://gitlab.haskell.org/ghc/ghc/-/blob/master/libraries/base/GHC/TopHandler.hs#L114

However windows work differently. I've seen programs in the past that require 2 ^C to stop on linux but are unresponsive on Windows

@mrBliss
Copy link
Contributor Author

mrBliss commented Sep 7, 2020

I think the first ^C restores the default interrupt handler, so the second always kills the application https://gitlab.haskell.org/ghc/ghc/-/blob/master/libraries/base/GHC/TopHandler.hs#L114

Indeed.

However windows work differently. I've seen programs in the past that require 2 ^C to stop on linux but are unresponsive on Windows

I haven't tested this particular instance of it on Windows, so I don't know.

@kderme kderme self-assigned this Sep 8, 2020
@kderme
Copy link
Contributor

kderme commented Sep 9, 2020

Indeed mask_ makes the first ^C do nothing (the db-analyser uses withDB instead of allocate, which also uses mask).
However, I think the point of ResourceRegistry is to use it in a more fine grained way, when allocating specific resources (like a handle) and not opening a db which can take long. We already use allocate for specific resources (and if we're not doing it for some of them, we should fix it). With that, I think allocate can be replaced by a simple onException when we open the chain db. ChainDB.Impl.closeDB won't run in case of an async exception, but important resources will be released.

It is possible that some resources need to be released in a specific order (which is currently ensured by ChainDB.Impl.closeDB). So even if an exception comes at any point, we may want to somehow enforce that order.

Another note is that closeDB seems to do other operations, for example, change the TVar of the VolatileDB to indicate DbClosed. I think, in case of async exceptions, we shouldn't care about those, since the program will soon exit.

@mrBliss
Copy link
Contributor Author

mrBliss commented Sep 9, 2020

It is possible that some resources need to be released in a specific order (which is currently ensured by ChainDB.Impl.closeDB). So even if an exception comes at any point, we may want to somehow enforce that order.

This is exactly the reason why we use the ResourceRegistry to open/close the ChainDB. Previously (before #1488), we used withDB, but this resulted in the wrong order of closing things and we were running into ClosedDBErrors. I remember realising that when using a ResourceRegistry, it should be all or nothing, either you use it for everything and get the right order, or you don't it use it. A mix of bracket and ResourceRegistry gives you the wrong order.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consensus issues related to ouroboros-consensus
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants