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

Add ability to force close persistent connections #41578

Closed
jsumners opened this issue Jan 18, 2022 · 2 comments
Closed

Add ability to force close persistent connections #41578

jsumners opened this issue Jan 18, 2022 · 2 comments
Labels
feature request Issues that request new features to be added to Node.js. http Issues or PRs related to the http subsystem.

Comments

@jsumners
Copy link
Contributor

What is the problem this feature will solve?

As noted in https://nodejs.org/dist/latest-v16.x/docs/api/net.html#serverclosecallback, a net.Server will not fully close until all persistent (keep-alive) connections have reached the end of their timeout. In other languages/frameworks, e.g. Java, C#, or PHP, this is not a problem because applications written with them are typically hosted within a dedicated HTTP server that will deal with this on the application's behalf. However, if we look at another language in which applications are typically written to provide a direct HTTP server we can see that this problem showed up there and has been solved -- golang/go#4674 (comment) (final resolution https://go.dev/doc/go1.8#http_shutdown):

HTTP Server Graceful Shutdown
The HTTP Server now has support for graceful shutdown using the new Server.Shutdown method and abrupt shutdown using the new Server.Close method.

https://pkg.go.dev/net/http#Server.Close

What is the feature you are proposing to solve the problem?

I propose that Node.js internally track persistent connections and provide a method to forcefully close them.

What alternatives have you considered?

We have implemented a, what I would call hacky, solution for this in Fastify -- fastify/fastify#3619

The hope is that if Node.js internalizes it, it can be more performant.

@jsumners jsumners added the feature request Issues that request new features to be added to Node.js. label Jan 18, 2022
@jsumners
Copy link
Contributor Author

cc: @mcollina

@mcollina
Copy link
Member

cc @nodejs/http

@mcollina mcollina added the http Issues or PRs related to the http subsystem. label Jan 18, 2022
RafaelGSS pushed a commit that referenced this issue May 10, 2022
Fixes: #41578

PR-URL: #42812
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js. http Issues or PRs related to the http subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants