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

Tally active requests in pool to close unsuited, idle connections #259

Merged
merged 5 commits into from
Mar 7, 2020

Conversation

trowski
Copy link
Member

@trowski trowski commented Mar 4, 2020

Fixes #256.

@trowski
Copy link
Member Author

trowski commented Mar 5, 2020

@nicolas-grekas This PR is a more robust solution, as @kelunik pointed out that the same situation could happen with multiple HTTP/2 requests, followed by an HTTP/1.x request (and certainly in other scenarios). This PR now closes connections in the pool if they are idle but unable to fulfill a new request.

This PR also relaxes reusing an HTTP/1.x keep-alive connection without an explicit timeout (which was the issue that caused the timeouts you saw) for a POST or PUT request as long as it is within the given connection grace period (2 seconds by default). Prior, if the connection did not have an explicit timeout, it would not be reused.

if (!$this->isAdditionalConnectionAllowed($uri)
&& ($this->activeRequestCounts[$connectionId] ?? 0) === 0
) {
// No additional connections allowed, but this connection is idle and unsuited for this request.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of the comment, we could refactor the second condition of the if into an $this->isIdle($connection) method.

@kelunik kelunik merged commit 7ab9ee0 into master Mar 7, 2020
@kelunik kelunik deleted the alt-fix-256 branch March 7, 2020 18:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Timeout when using ConnectionLimitingPool
2 participants