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

enhance: Error groups and handle graceful shutdowns #7

Merged
merged 2 commits into from
Oct 21, 2024

Conversation

LaurenceJJones
Copy link
Contributor

The goal of this PR is to handle system signals across the whole process rather than a push section (unless there a reason for this 😓 ):

  • Runs go routines in a error group
  • Context is shared between routines to handle when we should shutdown
  • Server is informed to graceful shutdown when we told to shutdown to allow remote connections time to disconnect

I know this is experimental and I havent had to chance to live test these changes, but this will allow the process to actually respond correctly to system signals and not cause docker / system to force kill the container if the system is wanting to shutdown.

if err := srv.Shutdown(ctx); err != nil {
log.Fatal(err) // failure/timeout shutting down the server gracefully
}
select {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This can be removed or adapted, as this is only the case when the shutdown context is not completed within 5 seconds.

@LaurenceJJones
Copy link
Contributor Author

I will invest some time tomorrow, to see if I can move the panic recovery as that is also inside a go routine so that needs to be also handled

@LaurenceJJones
Copy link
Contributor Author

I will invest some time tomorrow, to see if I can move the panic recovery as that is also inside a go routine so that needs to be also handled

Nevermind, it was defer function my bed time reading eyes saw it as a go routine 😆 so this should be handled, since its a IIF call it should be handled by the upper context.

@andrasbacsai
Copy link
Member

Thank you! I tested it and it works!

@andrasbacsai andrasbacsai merged commit 29b87c9 into coollabsio:next Oct 21, 2024
1 check passed
@LaurenceJJones LaurenceJJones deleted the context-groups branch October 22, 2024 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants