-
Notifications
You must be signed in to change notification settings - Fork 47
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
Use server GracefulStop instead of force stop #219
Conversation
stopSignalHandler <- true | ||
log.Flush() | ||
return | ||
} | ||
s.Stop() // Graceful stop |
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.
Why not stop signalHandler and flush log?
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.
s.Stop() is a graceful stop which is only reserved for cert rotation. Since we are not trying to exit process, just stop the server, we will not need to return or flush logs.
Cherry-pick PR to 202405: #282 |
Why I did it
It is better for server to gracefully stop after all pending requests are finished while blocking incoming requests instead of force closing all requests.
How I did it
Use GracefulStop API instead of Stop
How to verify it
UT, will add sonic-mgmt testcase
Verified via manual test that if during server is serving request, if certs are modified, server will not force close request, but will in fact finish the request and then server will be closed and that no new request is allowed. Will create a sonic-mgmt testcase for this.
Which release branch to backport (provide reason below if selected)
Description for the changelog
Link to config_db schema for YANG module changes
A picture of a cute animal (not mandatory but encouraged)