-
Notifications
You must be signed in to change notification settings - Fork 389
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
Refactor chainbuilder to gracefully handle shutdown signals #4391
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThis pull request introduces graceful shutdown handling in the Changes
Sequence Diagram(s)sequenceDiagram
participant OS as OS Signal
participant Main as Run Function
participant SignalHandler as Signal Goroutine
participant Cleanup as Cleanup Function
OS->>SignalHandler: Send os.Interrupt/SIGTERM
SignalHandler->>Main: Cancel context
Main->>Cleanup: Invoke deferred cleanup
Cleanup->>Main: Close channels & databases, log errors
Main->>OS: Return nil (graceful shutdown)
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
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.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tools/chainbuilder/main.go (1)
364-364
: Minor formatting issue.There appears to be a whitespace formatting issue on this line according to gofumpt.
- +🧰 Tools
🪛 golangci-lint (1.62.2)
364-364: File is not
gofumpt
-ed(gofumpt)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
tools/chainbuilder/main.go
(5 hunks)
🧰 Additional context used
🪛 golangci-lint (1.62.2)
tools/chainbuilder/main.go
364-364: File is not gofumpt
-ed
(gofumpt)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
🔇 Additional comments (7)
tools/chainbuilder/main.go (7)
7-9
: Added necessary imports for signal handling.The addition of
os/signal
andsyscall
packages will enable capturing OS interrupt signals for graceful shutdown.
129-144
: Well-implemented signal handling setup.This addition creates a proper signal handling mechanism that will:
- Listen for both SIGINT (Ctrl+C) and SIGTERM signals
- Cancel the context to propagate shutdown requests throughout the system
- Print a user-friendly message when a termination signal is received
This implementation follows Go best practices for handling OS signals.
319-320
: Added cleanup tracking flag.The boolean flag to track cleanup state will help prevent double-cleanup situations, which is a good defensive programming practice.
322-363
: Well-structured resource cleanup function.The cleanup function is well-designed with:
- Check to prevent duplicate cleanup operations
- Proper channel closing
- Error collection from goroutines
- Safe database closing with error logging
This ensures all resources are properly released regardless of how the tool is terminated.
365-366
: Ensuring cleanup with defer.Using
defer cleanup()
guarantees that cleanup will run before the function returns, even in case of panics. This is an excellent practice for resource management.
389-392
: Improved context cancellation handling.The code now properly responds to context cancellation by printing a helpful message and allowing the deferred cleanup function to handle resource cleanup.
494-495
: Simplified error return.The function now always returns nil, relying on the cleanup function to handle errors. This is consistent with the PR objective of graceful shutdown.
Currently, the chainbuilder tool only shuts down properly if it is allowed to finish its work completely. This is problematic when generating large chains, as users might want to stop the process after some time (e.g., a week) and use what has already been generated, rather than waiting for the entire process to complete.
This PR implements graceful shutdown handling for the chainbuilder tool by:
With these changes, users can now safely interrupt the chainbuilder process with Ctrl+C at any point, and the tool will gracefully shut down, preserving the chain generated up to that point.
Closes #4243