Skip to content

refactor: halt at app failure #2268

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

Merged
julienrbrt merged 9 commits into from
May 20, 2025
Merged

refactor: halt at app failure #2268

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
5e77da5
refactor: halt at app failure
julienrbrt May 15, 2025
d4d6fc3
updates
julienrbrt May 15, 2025
1f75ad7
lint + update
julienrbrt May 15, 2025
341a37b
clone nicely
julienrbrt May 16, 2025
c6a785f
feedback
julienrbrt May 19, 2025
900fb06
Merge branch 'main' into julien/halt
julienrbrt May 19, 2025
e5dcf6b
fix merge
julienrbrt May 19, 2025
3db6aca
updates
julienrbrt May 20, 2025
5cdd35f
Merge branch 'main' into julien/halt
julienrbrt May 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
feedback
  • Loading branch information
@julienrbrt
julienrbrt committed May 19, 2025
commit c6a785f5302213df2372368edc2b6b7f43574bdb
16 changes: 11 additions & 5 deletions node/full.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,10 @@ func (n *FullNode) startInstrumentationServer() (*http.Server, *http.Server) {

// Run implements the Service interface.
// It starts all subservices and manages the node's lifecycle.
func (n *FullNode) Run(ctx context.Context) error {
func (n *FullNode) Run(parentCtx context.Context) error {
ctx, cancelNode := context.WithCancel(parentCtx)
defer cancelNode() // safety net

// begin prometheus metrics gathering if it is enabled
if n.nodeConfig.Instrumentation != nil &&
(n.nodeConfig.Instrumentation.IsPrometheusEnabled() || n.nodeConfig.Instrumentation.IsPprofEnabled()) {
Expand Down Expand Up @@ -395,6 +398,8 @@ func (n *FullNode) Run(ctx context.Context) error {
return fmt.Errorf("error while starting data sync service: %w", err)
}

// only the first error is propagated
// any error is an issue, so blocking is not a problem
errCh := make(chan error, 1)
Comment thread
julienrbrt marked this conversation as resolved.

if n.nodeConfig.Node.Aggregator {
Expand All @@ -418,14 +423,14 @@ func (n *FullNode) Run(ctx context.Context) error {
case err := <-errCh:
if err != nil {
n.Logger.Error("unrecoverable error in one of the go routines...", "error", err)
goto Cleanup
cancelNode() // propagate shutdown to all child goroutines

@alpe alpe May 20, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

}
case <-ctx.Done():
// Block until context is canceled
case <-parentCtx.Done():
// Block until parent context is canceled
n.Logger.Info("context canceled, stopping node")
cancelNode() // propagate shutdown to all child goroutines

@alpe alpe May 20, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fyi: good to do some housekeeping but when the parent is cancelled, all childs are, too.

}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

Cleanup:
// Perform cleanup
n.Logger.Info("halting full node...")
n.Logger.Info("shutting down full node sub services...")
Expand Down Expand Up @@ -517,6 +522,7 @@ Cleanup:
if ctx.Err() != nil {
return ctx.Err()
}

return multiErr // Return shutdown errors if context was okay
}

Expand Down