-
Notifications
You must be signed in to change notification settings - Fork 99
Conversation
PR Summary
-
Introduction of Shutdown Signal Handlers
A new function namedregisterShutdownSignalHandlers()has been added. Its purpose is to manage shutdown signals, these are alerts that inform the system when it needs to stop the program. This is crucial to maintain the program's integrity and prevent abrupt termination that might result in data loss or corruption. -
Graceful Shutdown Implementation
We've added a function calledshutDownGracefully(signal: string). This is used to note down the received signals and execute any cleanup operations required. Cleanup operations refer to the tasks completed by software when it receives a command to stop, to ensure all data and processes are correctly closed down. -
Placeholder for Future Cleanup Tasks
Thecleanup()function added is currently a placeholder. This means that while it does not currently perform any operations, it is meant to contain cleanup tasks in the future. Cleanup tasks are actions to be taken prior to shutting down a system to ensure no ongoing processes are interrupted, and no data is lost. -
Integration of Shutdown Handlers
The startup method of the application (Booster.start()) will now call theregisterShutdownSignalHandlers()function. This ensures that the shutdown handlers are active from the moment the project starts running, providing a safety net for proper system termination.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@BelgianNoise Booster currently does not have a dedicated cleanup upon shutdown, but I think it would be beneficial to implement one. It should probably:
- Stop accepting incoming GraphQL connections/requests and close WebSocket connections.
- Close connections to DB and read model stores (this would have to be provider-specific).
- Close any other connections, such as to Event Hubs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much for your feedback, we have done some more internal debugging and found that it was an issue in our provider that was causing issues with the child process shenanigans & nyc.
That being said, this could still be a nice addition to the core package imo. However I won't be able to focus on it right now as I would need to find time outside working hours to make these changes and get more familiar with the code :)
I am stil eager to go deeper into the code and add the things you mentioned above, but it won't be in teh short term.
I will revert my PR back to a draft and I am leaving it up to you if you would rather see it closed for now.
(Feel free to hijack this PR if desired)
Edit: I do not have access to revert back to draft :)
(Oops, commented with diff account earlier)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@BelgianNoise I think we can go ahead with this addition and eventually implement the cleanup method in future PRs :)
Description
I was having trouble generating code coverage for my tests with
nyc.Took me some time but I found out that
nycrequires the process it spawn to gracefully exit.I was not completely sure what that meant, so I had to do some digging.
This PR adds handling of signals to gracefully shut down the process.
Changes
Checks
Additional information
I added a
cleanup()function that is currently empty, question is: does Booster have dedicated functions to close down the server, clean data, close connections ?Didn't add docs on the functions as I find them self explanatory.
This fixes the issue for me locally, but I am not entirely sure if this is the way to go. (approach + location in Booster code itself)
Does this require any documentation to be updated ?