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.

Sign up for GitHub

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

Jump to bottom

[BUG] - Node should cleanly exit on SIGTERM #1697

Closed
disassembler opened this issue Aug 14, 2020 · 6 comments · Fixed by #3641
Closed

[BUG] - Node should cleanly exit on SIGTERM #1697

disassembler opened this issue Aug 14, 2020 · 6 comments · Fixed by #3641
Assignees
@dcoutts @Jimbo4350 @intricate
Labels
cli revision is used to group all the Node CLI tickets enhancement New feature or request minorfix Something that needs to be fixed (e.g. a hard-coded constant), but that isnt a bug

Comments

@disassembler
Copy link
Contributor

Internal/Exernal
Internal if an IOHK staff member.

Summary
The node only cleanly exits on SIGINT (ctrl-c) and not SIGTERM. This makes the default systemd signal handler stop the node in a way it takes longer to restart. We work around this by changing the signal it gets in the systemd service, but this should probably be fixed in the node.

Steps to reproduce
Steps to reproduce the behavior:

  1. Go to '...'
  2. Click on '....'
  3. Scroll down to '....'
  4. See error

Expected behavior
A clear and concise description of what you expected to happen.

System info (please complete the following information):

  • OS: [e.g. Ubuntu]
  • Version [e.g. 20.04]
  • Node version

Screenshots and attachments

  • If applicable, add screenshots, config files and/or logs to help explain the problem.

Additional context
Add any other context about the problem here.
@disassembler disassembler added the bug Something isn't working label Aug 14, 2020
@rvl
Copy link
Contributor

rvl commented Aug 18, 2020

The way we handle this in cardano-wallet is to install a signal handler which translates SIGTERM to SIGINT.

https://github.com/input-output-hk/cardano-wallet/blob/master/lib/launcher/src/Cardano/Startup/POSIX.hs

@kevinhammond kevinhammond added the minorfix Something that needs to be fixed (e.g. a hard-coded constant), but that isnt a bug label Aug 25, 2020
@kevinhammond kevinhammond added the cli revision is used to group all the Node CLI tickets label Aug 25, 2020
@vix-io vix-io added enhancement New feature or request and removed bug Something isn't working labels Sep 23, 2020
@disassembler
Copy link
Contributor Author

I'm not sure why this was changed to an enhancement @vix-io. This is absolutely a bug...

@reqlez reqlez mentioned this issue Dec 28, 2021
Closed
swinful added a commit to swinful/cnode_stakepool that referenced this issue Dec 30, 2021
@swinful
/opt/etc/rc.d/cnode_relay:
02ec28c 
  - Fixes #3 via recommendations from [reqlez]. Using SIGINT improves node
  start times.
    + IntersectMBO/cardano-node#1697
@reqlez
Copy link

reqlez commented Jan 11, 2022

I'm not sure why this was changed to an enhancement @vix-io. This is absolutely a bug...

I don't see any activity for @vix-io in Github... does he still work with IOG?

@vix-io
Copy link
Contributor

vix-io commented Jan 12, 2022

Hi @disassembler , not my decision - this was changed based on internal discussions with the node team.

@DavidEichmann
Copy link
Contributor

I suspect this is related to this GHC issue: https://gitlab.haskell.org/ghc/ghc/-/issues/10169

DavidEichmann added a commit that referenced this issue Feb 23, 2022
@DavidEichmann
RTS workaround converting SIGTERM into SIGINT
f25613b 
This is important for graceful shutdown, cleaning up resources in
`bracket` statements.

Fixes #1697
@DavidEichmann DavidEichmann mentioned this issue Feb 23, 2022
Merged
DavidEichmann added a commit that referenced this issue Feb 24, 2022
@DavidEichmann
RTS workaround converting SIGTERM into SIGINT
fcba44c 
This is important for graceful shutdown, cleaning up resources in
`bracket` statements.

Fixes #1697
@DavidEichmann
Copy link
Contributor

DavidEichmann commented Mar 1, 2022 •
edited
Loading

PR: #3641
GHC MR: https://gitlab.haskell.org/ghc/ghc/-/merge_requests/7681

DavidEichmann added a commit that referenced this issue Mar 2, 2022
@DavidEichmann
RTS workaround converting SIGTERM into killing the main thread
08cae7b 
This is important for graceful shutdown, cleaning up resources in
`bracket` statements.

Fixes #1697
iohk-bors bot added a commit that referenced this issue Mar 3, 2022
@iohk-bors @DavidEichmann @jbgi
Merge #3641
79bf77f 
3641: RTS workaround converting SIGTERM into SIGINT r=DavidEichmann a=DavidEichmann

This is important for graceful shutdown, cleaning up resources in
`bracket` statements.

Fixes #1697

Co-authored-by: David Eichmann <EichmannD@gmail.com>
Co-authored-by: Jean-Baptiste Giraudeau <jean-baptiste.giraudeau@iohk.io>
@iohk-bors iohk-bors bot closed this as completed in ce26b8e Mar 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dcoutts dcoutts

@Jimbo4350 Jimbo4350

@intricate intricate

Labels
cli revision is used to group all the Node CLI tickets enhancement New feature or request minorfix Something that needs to be fixed (e.g. a hard-coded constant), but that isnt a bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

RTS workaround converting SIGTERM into SIGINT IntersectMBO/cardano-node
9 participants
@disassembler @rvl @dcoutts @DavidEichmann @reqlez @Jimbo4350 @kevinhammond @intricate @vix-io