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

Do not accept new requests on shutdown #1685

Merged
merged 5 commits into from
Feb 20, 2019
Merged

Do not accept new requests on shutdown #1685

merged 5 commits into from
Feb 20, 2019

Conversation

mainameiz
Copy link
Contributor

@mainameiz mainameiz commented Dec 8, 2018

Related to #1186

Problem:

When puma process receives TERM signal it tries to finish all current requests and terminate. But... it also accepts "new requests" and these requests are hanged. When the processing of all current requests (which was made before shutdown signal is received) are finished, all "new requests" are closed with error Connection reset by peer.
This error prevents load balancers (like nginx) to forward these requests to other web-server, because this error does not guarantee that the request was not processed, but this guarantee is required to forward not idempotent HTTP request (like POST).

Seems like puma should refuse new requests by closing a socket that accepts new connections.

Read #1186 (comment) for implementation details.

This solution is simple and fast. But probably there is a better solution.

@mainameiz
Copy link
Contributor Author

@schneems could you take a look?

@schneems
Copy link
Contributor

schneems commented Jun 10, 2019

Just now seeing this, looks like this handles the case for threaded only servers, I ended up having to do something extremely similar for multi-process workers. In #1808

nateberkopec added a commit that referenced this pull request Sep 2, 2019
…uests-on-shutdown"

This reverts commit b6022f9, reversing
changes made to cee9a8e.
nateberkopec added a commit that referenced this pull request Sep 3, 2019
…uests-on-shutdown"

This reverts commit b6022f9, reversing
changes made to cee9a8e.
@nateberkopec nateberkopec mentioned this pull request Sep 3, 2019
1 task
@mainameiz mainameiz deleted the do-not-accept-new-requests-on-shutdown branch March 28, 2021 18:06
@dentarg dentarg mentioned this pull request Apr 3, 2021
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