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 merge] Exp on pr277 and exercise the CI #287

Closed
wants to merge 10 commits into from

Conversation

morucci
Copy link

@morucci morucci commented May 29, 2020

Experiment for pr277


This change is Reviewable

the-allanc and others added 7 commits April 14, 2020 10:35
…y#263.

The previous behaviour was that when we exceed our threshold of permitted
keep-alive connections, we would evict the least recently used connection
by forcibly shutting down the socket. This would cause problems with clients
which wouldn't be expecting socket errors.

Now, if we have a connection that we would usually keep-alive, but we have
already reached our limit of allowed keep-alive connections, then we close
the connection gracefully by sending a "Connection: Close" header for
HTTP/1.1 or omitting the "Connection" header for HTTP/1.0.

This has the downside of having cheroot holding on to connections which are
less recently used, rather than the most recent ones - but it does ensure that
we make the decision about whether to keep or drop a connection at the time we
are writing the headers, which allows the client to have a better expectation
about the state of the socket once the response has been read.

However, we still forcibly close sockets for idle keep-alive connections once
the server timeout has been exceeded.
@morucci morucci force-pushed the exp-pr277 branch 3 times, most recently from 9c3654a to d54ccce Compare May 29, 2020 18:05
@webknjaz webknjaz marked this pull request as draft May 31, 2020 23:47
@morucci morucci force-pushed the exp-pr277 branch 2 times, most recently from dbbee24 to 7029460 Compare June 4, 2020 07:27
@webknjaz
Copy link
Member

I've merged #277 with a few hacks in tests.

@webknjaz webknjaz closed this Jul 13, 2020
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