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

bpo-35380: Enable TCP_NODELAY for proactor event loop #10867

Merged
merged 2 commits into from Dec 3, 2018

Conversation

Projects
None yet
5 participants
@asvetlov
Copy link
Contributor

asvetlov commented Dec 3, 2018

Restore a consitensy with selector based event loops.
TCP_NODELAY is enabled starting from Python 3.6

https://bugs.python.org/issue35380

@1st1

1st1 approved these changes Dec 3, 2018

Copy link
Member

1st1 left a comment

I'm a bit cautious (i.e. I'm +0) with backporting this to 3.7. Are you sure it's 100% safe?

@@ -168,6 +168,17 @@ def _run_until_complete_cb(fut):
futures._get_loop(fut).stop()


if hasattr(socket, 'TCP_NODELAY'):

This comment has been minimized.

@1st1

1st1 Dec 3, 2018

Member

I'd add a new asyncio.sockutils module for this kind of stuff.

This comment has been minimized.

@asvetlov

asvetlov Dec 3, 2018

Contributor

sockutils.py sounds great but in the case I doubt if the PR can be backported.

This comment has been minimized.

@1st1

1st1 Dec 3, 2018

Member

Indeed, I suggest to do that in a follow up PR specifically for the master branch.

This comment has been minimized.

@asvetlov
@asvetlov

This comment has been minimized.

Copy link
Contributor

asvetlov commented Dec 3, 2018

Yes, the change is safe: Windows supports TCP_NODELAY very well for any version, at least I used it on Win 2000 and XP.

Without backport third parties like aiohttp should check for event loop type and enable the mode for proactor only, which looks very odd.

@1st1

This comment has been minimized.

Copy link
Member

1st1 commented Dec 3, 2018

Without backport third parties like aiohttp should check for event loop type and enable the mode for proactor only, which looks very odd.

SGTM

@asvetlov asvetlov merged commit 3bc0eba into python:master Dec 3, 2018

5 checks passed

Azure Pipelines PR #20181203.24 succeeded
Details
bedevere/issue-number Issue number 35380 found
Details
bedevere/news News entry found in Misc/NEWS.d
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@miss-islington

This comment has been minimized.

Copy link

miss-islington commented Dec 3, 2018

Thanks @asvetlov for the PR 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7.
🐍🍒🤖

@bedevere-bot

This comment has been minimized.

Copy link

bedevere-bot commented Dec 3, 2018

@asvetlov: Please replace # with GH- in the commit message next time. Thanks!

@asvetlov asvetlov deleted the asvetlov:tcp-nodelay-windows branch Dec 3, 2018

@bedevere-bot

This comment has been minimized.

Copy link

bedevere-bot commented Dec 3, 2018

GH-10872 is a backport of this pull request to the 3.7 branch.

miss-islington added a commit to miss-islington/cpython that referenced this pull request Dec 3, 2018

bpo-35380: Enable TCP_NODELAY for proactor event loop (pythonGH-10867)
(cherry picked from commit 3bc0eba)

Co-authored-by: Andrew Svetlov <andrew.svetlov@gmail.com>
@miss-islington

This comment has been minimized.

Copy link

miss-islington commented Dec 3, 2018

Sorry, @asvetlov, I could not cleanly backport this to 3.6 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 3bc0ebab17bf5a2c29d2214743c82034f82e6573 3.6

@bedevere-bot

This comment has been minimized.

Copy link

bedevere-bot commented Dec 3, 2018

GH-10874 is a backport of this pull request to the 3.6 branch.

asvetlov added a commit to asvetlov/cpython that referenced this pull request Dec 3, 2018

[3.6] bpo-35380: Enable TCP_NODELAY for proactor event loop (pythonGH…
…-10867).

(cherry picked from commit 3bc0eba)

Co-authored-by: Andrew Svetlov <andrew.svetlov@gmail.com>

asvetlov added a commit that referenced this pull request Dec 3, 2018

[3.7] bpo-35380: Enable TCP_NODELAY for proactor event loop (GH-10867) (
GH-10872)

* bpo-35380: Enable TCP_NODELAY for proactor event loop (GH-10867)
(cherry picked from commit 3bc0eba)

Co-authored-by: Andrew Svetlov <andrew.svetlov@gmail.com>

asvetlov added a commit that referenced this pull request Dec 5, 2018

[3.6] bpo-35380: Enable TCP_NODELAY for proactor event loop (GH-10867)…
…. (GH-10874)

* [3.6] bpo-35380: Enable TCP_NODELAY for proactor event loop (GH-10867).
(cherry picked from commit 3bc0eba)

Co-authored-by: Andrew Svetlov <andrew.svetlov@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment