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

Fix graceful shutdown if admin server is not enabled #4155

Merged

Conversation

linux019
Copy link
Contributor

If admin server is not enabled

if cfg.Admin.Enabled {
adminServer := newAdminServer(cfg, adminHandler)
go shutdownAfterSignals(adminServer, stopAdmin, done)

PBS cannot stop gracefully as it awaits on stopAdmin channel which is never triggered
wait(stopSignals, done, stopMain, stopAdmin, stopPrometheus)

oleksandr added 2 commits January 17, 2025 10:02
…-shutdown-without-admin-server
Copy link
Contributor

@guscarreon guscarreon left a comment

Choose a reason for hiding this comment

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

I love this simplification. Let me know your thoughts on my comment below

server/server.go Outdated
@@ -27,6 +27,7 @@ func Listen(cfg *config.Configuration, handler http.Handler, adminHandler http.H
stopAdmin := make(chan os.Signal)
stopMain := make(chan os.Signal)
stopPrometheus := make(chan os.Signal)
var stopChannels = []chan<- os.Signal{stopMain}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: Maybe it's just me but I find append more straightforward, can we modify to append(stopChannels, stopMain)?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, can we use the := operand instead of var?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"append" is superfluous to just initialize the slice

Copy link
Contributor

@guscarreon guscarreon left a comment

Choose a reason for hiding this comment

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

LGTM

@bsardo bsardo merged commit d4e27cb into prebid:master Feb 11, 2025
4 checks passed
@linux019 linux019 deleted the fix-graceful-shutdown-without-admin-server branch February 12, 2025 08:40
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.

None yet

3 participants