Repository navigation
Small fixes around the use of TCP MSS in http.client #67491
Description
| BPO | 23302 |
|---|---|
| Nosy | @pitrou, @vadmium, @demianbrecht |
| Files |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = None
closed_at = <Date 2015-01-23.16:03:23.257>
created_at = <Date 2015-01-22.17:00:35.436>
labels = ['type-bug', 'library']
title = 'Small fixes around the use of TCP MSS in http.client'
updated_at = <Date 2015-01-23.16:03:23.254>
user = 'https://github.com/demianbrecht'bugs.python.org fields:
activity = <Date 2015-01-23.16:03:23.254>
actor = 'python-dev'
assignee = 'none'
closed = True
closed_date = <Date 2015-01-23.16:03:23.257>
closer = 'python-dev'
components = ['Library (Lib)']
creation = <Date 2015-01-22.17:00:35.436>
creator = 'demian.brecht'
dependencies = []
files = ['37822', '37826']
hgrepos = []
issue_num = 23302
keywords = ['patch']
message_count = 5.0
messages = ['234500', '234524', '234542', '234545', '234561']
nosy_count = 5.0
nosy_names = ['pitrou', 'neologix', 'python-dev', 'martin.panter', 'demian.brecht']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue23302'
versions = ['Python 3.5']
Activity
demianbrecht commentedon Jan 22, 2015
There are a couple of small issues with the determination of whether or not a request can fit in a single TCP/IP packet in http.client.
I suggest two changes be made to fix this:
This is still a best guess based on minimal TCP/IP header sizes, but if options or extension headers are used, the packet may still be split at the lower levels. This is fine because what we're trying to avoid is multiple send()s where the payload is less than the MSS.
neologix commentedon Jan 22, 2015
Or we should acknowledge that this is overkill, and take the same approach as all major web browser: disable the Nagle algorithm.
For a protocol like http which is transaction oriented it's probably the best thing to do.
demianbrecht commentedon Jan 23, 2015
I'm not opposed to that either. The only downside really (at least as far as I'm aware) is the potential substantial influx of packets should an iterable comprised of small chunks of data be passed in as the body, although I would consider that quite a strange edge case for HTTP requests.
I'll put together another patch that disables nagle by default and see if anyone has a contrary opinion.
demianbrecht commentedon Jan 23, 2015
I've attached a new patch disabling Nagle by default, but doing so in connect() as to allow users to override it if they really want to. I've also removed the use of mss in HTTPConnection. This is a backwards incompatible change in two ways:
That said, I do agree that this is a simpler, more portable choice to solve the problem.
python-dev commentedon Jan 23, 2015
New changeset 42971b769c0b by Benjamin Peterson in branch 'default':
http.client: disable Nagle's algorithm (closes bpo-23302)
https://hg.python.org/cpython/rev/42971b769c0b