-
-
Notifications
You must be signed in to change notification settings - Fork 142
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 LimitConcurrentRequestsMiddleware to limit how many next handlers can be executed concurrently #272
Conversation
$pending--; | ||
$that->processQueue(); | ||
|
||
throw $error; |
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.
This seems to cause build errors on PHP versions below 7: https://travis-ci.org/reactphp/http/jobs/313201152#L517
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.
Indeed, updated to explicitly reject()
in case the rejection value is not an Exception
: https://github.com/reactphp/http/pull/272/files#diff-ab60bdad04437904e8a3c3e4c8eff52fR137
$once = $this->expectCallableOnce(); | ||
$deferred = new Deferred(function () use ($once) { | ||
$once(); | ||
throw new RuntimeException('Cancelled'); |
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.
Travis is crashing on this: https://travis-ci.org/reactphp/http/jobs/313374574#L512
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.
Duh, updated invalid reference just now
Rebased to resolve merge conflict with #264 |
README.md
Outdated
])); | ||
``` | ||
|
||
More sophisticated examples include limiting the total number of of requests |
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.
of of
src/Io/PauseBufferStream.php
Outdated
* | ||
* This class is used to buffer all events that happen on a given stream while | ||
* it is paused. This allows you to pause a stream and no longer watch for any | ||
* of its events. Once the stream is resumed, all buffered event will be |
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.
all buffered events
* ])); | ||
* ``` | ||
* | ||
* More sophisticated examples include limiting the total number of of requests |
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.
of of
Thanks for spotting, updated |
🚨 Merge conflicts 🚨 |
Rebased to resolve merge conflict now that #274 is in |
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.
👍
The new ~~~
LimitHandlersMiddleware
(name subject to discussion / change)~~~LimitConcurrentRequestsMiddleware
allows consumers to limit how many next handlers can be executed concurrently. This can be useful in a streaming context, where the number of concurrent buffers have to be limited. Or if can be used to avoid concurrent execution of handlers entirely. See also README changes for more details.Accordingly, this functionality is also a requirement for implementing a buffering server (via #259). This allows you to limit this so that for example up to 100 requests with a maximum of 2 MiB each can be buffered and may take up to 200 MiB concurrently.
In a follow-up PR, we will eventually implement similar logic to limit total memory consumption instead. This will allow a higher concurrency under the reasonable assumption that many requests are way below a given limit (see #255).
Supersedes / closes #218, thanks @WyriHaximus!
Refs #194
Refs #259