-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
bpo-30064: Fix slow asyncio sock test #20868
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
Conversation
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.
LGTM, thanks for the PR @fantix. With both the usage of SO_SNDBUF to reduce the send buffer size and the changes to start with a higher send size of 8192, then reducing by half each time (log2n), this seems to completely address the timeout issue. Even with running 200 parallel jobs for ~1000 test iterations, I did not receive any failures. With the previous version, I was able to immediately create a failure using 100 parallel jobs in the first batch of tests.
Since this patch is fairly straightforward and should be included as soon as possible to avoid further intermittent buildbot failures, I'll proceed with merging it. That being said, I would greatly appreciate a post-commit review from @1st1 and/or @asvetlov whenever they can find the time to do so.
Thanks @fantix for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9. |
Using a log2n way to fill a much smaller buffer, and receiving in a cleaner way with EOF. The failing test was reproducible using the following command thanks to @aeros : ```bash ./python -m test test_asyncio.test_sock_lowlevel --match test_sock_client_racing -j100 -F -v ``` According to test results, we may still need to bump the timeout: https://github.com/python/cpython/blob/5aad027db9618f22f6fa2274e05dd50f928d2ed7/Lib/test/test_asyncio/test_sock_lowlevel.pyGH-L256-L257 (cherry picked from commit 8f04a84) Co-authored-by: Fantix King <[email protected]>
GH-20869 is a backport of this pull request to the 3.9 branch. |
Using a log2n way to fill a much smaller buffer, and receiving in a cleaner way with EOF. The failing test was reproducible using the following command thanks to @aeros : ```bash ./python -m test test_asyncio.test_sock_lowlevel --match test_sock_client_racing -j100 -F -v ``` According to test results, we may still need to bump the timeout: https://github.com/python/cpython/blob/5aad027db9618f22f6fa2274e05dd50f928d2ed7/Lib/test/test_asyncio/test_sock_lowlevel.pyGH-L256-L257 (cherry picked from commit 8f04a84) Co-authored-by: Fantix King <[email protected]>
Many thanks to Kyle on this :gratitude: |
Using a log2n way to fill a much smaller buffer, and receiving in a cleaner way with EOF. The failing test was reproducible using the following command thanks to @aeros : ```bash ./python -m test test_asyncio.test_sock_lowlevel --match test_sock_client_racing -j100 -F -v ``` According to test results, we may still need to bump the timeout: https://github.com/python/cpython/blob/5aad027db9618f22f6fa2274e05dd50f928d2ed7/Lib/test/test_asyncio/test_sock_lowlevel.py#L256-L257
Using a log2n way to fill a much smaller buffer, and receiving in a cleaner way with EOF.
The failing test was reproducible using the following command thanks to @aeros :
./python -m test test_asyncio.test_sock_lowlevel --match test_sock_client_racing -j100 -F -v
According to test results, we may still need to bump the timeout:
cpython/Lib/test/test_asyncio/test_sock_lowlevel.py
Lines 256 to 257 in 5aad027
https://bugs.python.org/issue30064
Automerge-Triggered-By: @aeros