Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Support persistent connections (Connection: keep-alive) #405

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

Merged
WyriHaximus merged 2 commits into reactphp:master from clue-labs:keepalive
Apr 10, 2021

Conversation

@clue
Copy link
Member

@clue clue commented Apr 10, 2021

This changeset finally adds support for persistent connections (Connection: keep-alive) 🎉

This is a pure feature addition that should show a noticeable performance improvement when using persistent connections (which is the default pretty much everywhere). Together with #404, this improves benchmarking performance from ~11000 requests per second to ~22000 requests per second on a single CPU core.

$ docker run -it --rm --net=host jordi/ab -n100000 -c80 -k http://localhost:8080/
...
Requests per second: 22327.67 [#/sec] (mean)
...

This has limited support for pipelining (which is rarely used in practice), I'll look into this in a follow-up PR because this requires a considerable refactoring first. If you do not wish to use persistent connections, you can always use a middleware like this:

$server = new Server(
 $loop,
 function (ServerRequestInterface $request, callable $next) {
 // disable keep-alive (not usually recommended)
 return $next($request)->withHeader('Connection', 'close');
 },
 function (ServerRequestInterface $request) {
 return new Response(
 200,
 []
 'Hello world!'
 );
 }
);

Resolves #39

dbalabka and ging-dev reacted with thumbs up emoji acasademont reacted with hooray emoji acasademont reacted with heart emoji
@WyriHaximus WyriHaximus merged commit c919cb7 into reactphp:master Apr 10, 2021
@clue clue deleted the keepalive branch April 10, 2021 21:55
Copy link

What a great achievement. Congratulations and thanks for your work.
clue and ging-dev reacted with heart emoji clue reacted with rocket emoji

Copy link

Thank you @clue ! How about keep-alive timeouts? If the client just disappears wouldn't the server socket be kept alive forever, potentially causing socket starvation? This nginx blog post summarizes the pros and cons of using keep alive quite well.

https://www.nginx.com/blog/http-keepalives-and-web-performance/

and

http://nginx.org/en/docs/http/ngx_http_core_module.html#keepalive_requests

Should this time-out be configurable somehow? Maybe we should discuss this in a separate issue?
alexpts reacted with thumbs up emoji

Copy link
Member

@acasademont Not sure if @clue already has a branch with this. But essentially exactly that is why we added the loop as a constructor argument for the server, so we can implement a timeout once this landed. It's also more about closing idle connections after X seconds.

Copy link

Yep! It's basically a way to "garbage-collect" idle browser connections. I'd like to run some tests to see how it behaves, especially where reactphp sits behind CloudFront or GCLB, which support Keep Alive connections to backends.

Copy link

Seems like GCLB will close the connection after 10 minutes of idling. I will deploy now this new release and see how it goes :D

https://cloud.google.com/load-balancing/docs/https#timeouts_and_retries

Copy link

There seems to be a problem with php-pm not being able to fully work with keepalive connections. Opened php-pm/php-pm#538 for reference, I will keep digging

WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Aug 22, 2021 
This new middleware introduces a timeout of closing inactive
connections between connections after a configured amount of seconds.
This builds on top of reactphp#405 and partially on reactphp#422
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Aug 22, 2021 
This new middleware introduces a timeout of closing inactive
connections between connections after a configured amount of seconds.
This builds on top of reactphp#405 and partially on reactphp#422
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Aug 30, 2021 
This new middleware introduces a timeout of closing inactive
connections between connections after a configured amount of seconds.
This builds on top of reactphp#405 and partially on reactphp#422
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Sep 1, 2021 
This builds on top of reactphp#405 and further builds out reactphp#423 by also
close connections with inactive requests.
Copy link
Member Author

clue commented Oct 7, 2021

Thank you @clue ! How about keep-alive timeouts? If the client just disappears wouldn't the server socket be kept alive forever, potentially causing socket starvation? This nginx blog post summarizes the pros and cons of using keep alive quite well.

https://www.nginx.com/blog/http-keepalives-and-web-performance/

and

http://nginx.org/en/docs/http/ngx_http_core_module.html#keepalive_requests

Should this time-out be configurable somehow? Maybe we should discuss this in a separate issue?

@acasademont Thanks for the feedback! As a starting point, this PR indeed intentionally doesn't implement a keep-alive timeout. This is done to mimic initial connections which don't implement a timeout either. The documentation also suggests running this behind a reverse proxy server, so this server would usually take care of this timeout handling anyway.

That said, I agree that the next step would be to implement proper timeouts as briefly discussed in #194 and as started by @WyriHaximus in #425. Let's continue this discussion there? 👍
WyriHaximus reacted with heart emoji

WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Nov 9, 2021 
This new middleware introduces a timeout of closing inactive
connections between connections after a configured amount of seconds.
This builds on top of reactphp#405 and partially on reactphp#422
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Nov 9, 2021 
This builds on top of reactphp#405 and further builds out reactphp#423 by also
close connections with inactive requests.
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Nov 9, 2021 
This new middleware introduces a timeout of closing inactive
connections between connections after a configured amount of seconds.
This builds on top of reactphp#405 and partially on reactphp#422
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Nov 10, 2021 
This builds on top of reactphp#405 and further builds out reactphp#423 by also
close connections with inactive requests.
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Aug 27, 2022 
This new middleware introduces a timeout of closing inactive
connections between connections after a configured amount of seconds.
This builds on top of reactphp#405 and partially on reactphp#422
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Aug 27, 2022 
This builds on top of reactphp#405 and further builds out reactphp#423 by also
close connections with inactive requests.
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Sep 14, 2022 
This builds on top of reactphp#405 and further builds out reactphp#423 by also
close connections with inactive requests.
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Sep 14, 2022 
This builds on top of reactphp#405 and further builds out reactphp#423 by also
close connections with inactive requests.
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Sep 14, 2022 
This builds on top of reactphp#405 and further builds out reactphp#423 by also
close connections with inactive requests.
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Sep 14, 2022 
This new middleware introduces a timeout of closing inactive
connections between connections after a configured amount of seconds.
This builds on top of reactphp#405 and partially on reactphp#422
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Sep 14, 2022 
This builds on top of reactphp#405 and further builds out reactphp#423 by also
close connections with inactive requests.
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Sep 14, 2022 
This builds on top of reactphp#405 and further builds out reactphp#423 by also
close connections with inactive requests.
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Sep 14, 2022 
This builds on top of reactphp#405 and further builds out reactphp#423 by also
close connections with inactive requests.
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Sep 15, 2022 
This new middleware introduces a timeout of closing inactive
connections between requests after a configured amount of seconds.
This builds on top of reactphp#405 and partially on reactphp#422
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Feb 19, 2024 
This changeset resolves a small memory leak that causes roughly 1KB per connection tops. Which isn't a big issue but will make memory fluctuate more. The changeset doesn't introduce any performance degradation.
Resolves: reactphp#514
Builds on top of: reactphp#405, reactphp#467, and many others
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Feb 19, 2024 
This changeset resolves a small memory leak that causes roughly 1KB per connection tops. Which isn't a big issue but will make memory fluctuate more. The changeset doesn't introduce any performance degradation.
Resolves: reactphp#514
Builds on top of: reactphp#405, reactphp#467, and many others
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Feb 19, 2024 
This changeset resolves a small memory leak that causes roughly 1KB per connection tops. Which isn't a big issue but will make memory fluctuate more. The changeset doesn't introduce any performance degradation.
Resolves: reactphp#514
Builds on top of: reactphp#405, reactphp#467, and many others
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Mar 8, 2024 
This changeset resolves a small memory leak that causes roughly 1KB per connection tops. Which isn't a big issue but will make memory fluctuate more. The changeset doesn't introduce any performance degradation.
Resolves: reactphp#514
Builds on top of: reactphp#405, reactphp#467, and many others
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Mar 25, 2024 
This new middleware introduces a timeout of closing inactive
connections between requests after a configured amount of seconds.
This builds on top of reactphp#405 and partially on reactphp#422
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Mar 25, 2024 
This new middleware introduces a timeout of closing inactive
connections between requests after a configured amount of seconds.
This builds on top of reactphp#405 and partially on reactphp#422
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Mar 25, 2024 
This new middleware introduces a timeout of closing inactive
connections between requests after a configured amount of seconds.
This builds on top of reactphp#405 and partially on reactphp#422
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Mar 26, 2024 
This new middleware introduces a timeout of closing inactive
connections between requests after a configured amount of seconds.
This builds on top of reactphp#405 and partially on reactphp#422
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Mar 26, 2024 
This new middleware introduces a timeout of closing inactive
connections between requests after a configured amount of seconds.
This builds on top of reactphp#405 and partially on reactphp#422
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Mar 26, 2024 
This new middleware introduces a timeout of closing inactive
connections between requests after a configured amount of seconds.
This builds on top of reactphp#405 and partially on reactphp#422
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Mar 26, 2024 
This new middleware introduces a timeout of closing inactive
connections between requests after a configured amount of seconds.
This builds on top of reactphp#405 and partially on reactphp#422
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Mar 26, 2024 
This new middleware introduces a timeout of closing inactive
connections between requests after a configured amount of seconds.
This builds on top of reactphp#405 and partially on reactphp#422
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Mar 30, 2024 
This new middleware introduces a timeout of closing inactive
connections between requests after a configured amount of seconds.
This builds on top of reactphp#405 and partially on reactphp#422
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Mar 30, 2024 
This new middleware introduces a timeout of closing inactive
connections between requests after a configured amount of seconds.
This builds on top of reactphp#405 and partially on reactphp#422
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Mar 30, 2024 
This new middleware introduces a timeout of closing inactive
connections between requests after a configured amount of seconds.
This builds on top of reactphp#405 and partially on reactphp#422
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Mar 30, 2024 
This new middleware introduces a timeout of closing inactive
connections between requests after a configured amount of seconds.
This builds on top of reactphp#405 and partially on reactphp#422
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Apr 6, 2024 
This new middleware introduces a timeout of closing inactive
connections between requests after a configured amount of seconds.
This builds on top of reactphp#405 and partially on reactphp#422
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Apr 10, 2024 
This new middleware introduces a timeout of closing inactive
connections between requests after a configured amount of seconds.
This builds on top of reactphp#405 and partially on reactphp#422
WyriHaximus added a commit to WyriHaximus-labs/http that referenced this pull request Apr 10, 2024 
This new middleware introduces a timeout of closing inactive
connections between requests after a configured amount of seconds.
This builds on top of reactphp#405 and partially on reactphp#422
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@WyriHaximus WyriHaximus WyriHaximus approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

v1.3.0

Development

Successfully merging this pull request may close these issues.

Support persistent connections (aka HTTP/1.1 Keep-Alive)

AltStyle によって変換されたページ (->オリジナル) /