bpo-41682: fixed flaky test test_sendfile_close_peer_in_the_middle_of_receiving #30845

kumaraditya303 · 2022-01-24T08:28:06Z

https://bugs.python.org/issue41682

kumaraditya303 · 2022-01-24T08:55:51Z

...And we finally have green CI 🎉

kumaraditya303 · 2022-01-24T11:46:19Z

cc @gvanrossum @vstinner @pablogsal

erlend-aasland

Assuming Guido's reasoning is correct, this is IMO the correct fix. LGTM!

(But note that this test has been flakey for two years; maybe there was another reason for the failures before?)

erlend-aasland

...And we finally have green CI 🎉

Actually, it has been green since Nikita's #30801 😉 Please sync with main and remove the Windows guard added by #30801. If this PR fixes the test on Windows, we no longer need the guard.

kumaraditya303 · 2022-01-24T15:57:34Z

Assuming Guido's reasoning is correct, this is IMO the correct fix. LGTM!

(But note that this test has been flakey for two years; maybe there was another reason for the failures before?)

I was already trying to find the size of DATA based on some stack overflow answers before Guido provided his reasoning as I once tried making DATA very high and it worked so I knew it was about fine tuning the DATA length.

kumaraditya303 · 2022-01-24T16:02:02Z

...And we finally have green CI 🎉

Actually, it has been green since Nikita's #30801 😉 Please sync with main and remove the Windows guard added by #30801. If this PR fixes the test on Windows, we no longer need the guard.

@erlend-aasland
I already removed the skip for windows, no?

erlend-aasland · 2022-01-24T16:03:24Z

I already removed the skip for windows, no?

Great, I see it. Thanks.

Lib/test/test_asyncio/test_sendfile.py

Fidget-Spinner · 2022-01-24T16:16:01Z

I'm gonna test with buildbots. For now, please hold off on committing anything (well you can, but it'll make the buildbots status harder to track ;).

If this works, we need to give you an award for saving thousands of contributors from this pain.

bedevere-bot · 2022-01-24T16:16:09Z

🤖 New build scheduled with the buildbot fleet by @Fidget-Spinner for commit 369390f 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

erlend-aasland · 2022-01-24T16:22:55Z

If this works, we need to give you an award for saving thousands of contributors from this pain.

We could consider giving this patch a NEWS entry, given the amount of frustration and gray hair this test has caused 😆

gvanrossum · 2022-01-24T16:44:26Z

I'll leave this in Erlend's capable hands.

erlend-aasland · 2022-01-24T21:02:19Z

Thanks for you input, @asvetlov!

I would make the PR title and message more accurate before merging; maintaining a clear and self-documenting git history really helps when issues pop up again (I suspect this might do in a Windows release or five). Here's a quick 'n dirty suggestion, but feel free to word it differently:

bpo-NNNNN: Double the size of test_asyncio.test_sendfile data buffer

Some Windows versions use a large internal transmit buffer, which can
make tests that rely on fragmented transmits flakey (for example
test_sendfile_close_peer_in_the_middle_of_receiving).

We now use a buffer slightly larger than 256 KiB.

Other than that, I have no further remarks. (The failed buildbot is not related to this PR.)

Here's your CI Janitor Award: 🏆🥇 Thanks for keeping the bots green! Thanks, Guido and Kumar, for the reasoning and the patch.

(Andrew, Guido, or Ken can merge; I don't have the commit bit.)

erlend-aasland · 2022-01-24T21:04:37Z

(The bot that edit our PR comments in order to create bpo links can be very, very irritating at times...)

miss-islington · 2022-01-24T21:04:50Z

Thanks @kumaraditya303 for the PR, and @pablogsal for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9, 3.10.
🐍🍒⛏🤖

pablogsal · 2022-01-24T21:04:57Z

Merged ;)

bedevere-bot · 2022-01-24T21:05:14Z

GH-30860 is a backport of this pull request to the 3.10 branch.

bedevere-bot · 2022-01-24T21:05:18Z

GH-30861 is a backport of this pull request to the 3.9 branch.

(cherry picked from commit 1c705fd) Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>

bedevere-bot · 2022-01-24T21:23:37Z

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot ARM64 macOS 3.x has failed when building commit 1c705fd.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/725/builds/837) and take a look at the build logs.
Check if the failure is related to this commit (1c705fd) or if it is a false positive.
If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/725/builds/837

Summary of the results of the build (if available):

== Tests result: ENV CHANGED ==

411 tests OK.

10 slowest tests:

test_concurrent_futures: 3 min 49 sec
test_multiprocessing_spawn: 2 min 33 sec
test_multiprocessing_forkserver: 1 min 51 sec
test_unparse: 1 min 24 sec
test_asyncio: 1 min 18 sec
test_tokenize: 1 min 13 sec
test_logging: 1 min
test_lib2to3: 54.5 sec
test_capi: 52.4 sec
test_ssl: 47.5 sec

1 test altered the execution environment:
test_ftplib

17 tests skipped:
test_dbm_gnu test_devpoll test_epoll test_gdb test_ioctl
test_msilib test_multiprocessing_fork test_ossaudiodev test_spwd
test_startfile test_tix test_tk test_ttk_guionly test_winconsoleio
test_winreg test_winsound test_zipfile64

Total duration: 9 min 17 sec

Click to see traceback logs

Traceback (most recent call last):
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/asyncore.py", line 90, in read
    obj.handle_read_event()
    ^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/test/test_ftplib.py", line 384, in handle_read_event
    self._do_ssl_handshake()
    ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/test/test_ftplib.py", line 345, in _do_ssl_handshake
    self.socket.do_handshake()
    ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/ssl.py", line 1345, in do_handshake
    self._sslobj.do_handshake()
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^
ssl.SSLZeroReturnError: TLS/SSL connection has been closed (EOF) (_ssl.c:998)


Traceback (most recent call last):
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/multiprocessing/resource_tracker.py", line 209, in main
    cache[rtype].remove(name)
    ^^^^^^^^^^^^^^^^^^^^^^^^^
KeyError: '/psm_9d7d4089'


Traceback (most recent call last):
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/multiprocessing/resource_tracker.py", line 209, in main
    cache[rtype].remove(name)
    ^^^^^^^^^^^^^^^^^^^^^^^^^
KeyError: '/psm_774f2276'


Traceback (most recent call last):
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/threading.py", line 1031, in _bootstrap_inner
    self.run()
    ^^^^^^^^^^
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/test/test_ftplib.py", line 298, in run
    asyncore.loop(timeout=0.1, count=1)
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/asyncore.py", line 214, in loop
    poll_fun(timeout, map)
    ^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/asyncore.py", line 157, in poll
    read(obj)
    ^^^^^^^^^
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/asyncore.py", line 94, in read
    obj.handle_error()
    ^^^^^^^^^^^^^^^^^^
  File "/Users/buildbot/buildarea/3.x.pablogsal-macos-m1.macos-with-brew/build/Lib/test/test_ftplib.py", line 421, in handle_error
    raise Exception
    ^^^^^^^^^^^^^^^
Exception
k

vstinner · 2022-01-25T17:38:59Z

@pablogsal: Merged ;)

Be careful next time you merge a PR, the merged commit title is just "fixed flaky test" which doesn't mention test_asyncio or the bpo number, so no comment was added to https://bugs.python.org/issue41682

That's the annoying part with GH PR: it's possible to change the title and description in GitHub, and then get I completly different title and description when you merge it...

…_receiving (GH-30845) (#30860) (cherry picked from commit 1c705fd) Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>

…_receiving (GH-30845) (#30861) (cherry picked from commit 1c705fd) Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>

pablogsal · 2022-01-25T20:36:47Z

@pablogsal: Merged ;)

Be careful next time you merge a PR, the merged commit title is just "fixed flaky test" which doesn't mention test_asyncio or the bpo number, so no comment was added to https://bugs.python.org/issue41682

That's the annoying part with GH PR: it's possible to change the title and description in GitHub, and then get I completly different title and description when you merge it...

I also used the GitHub phone app to merge it which o think made things even more confusing as I didn't see that the commit title would be different :(

vstinner · 2022-01-25T23:08:29Z

I agree, the UI on a phone is "not great" :-( Especially for a merge operation.

…_receiving (pythonGH-30845) (python#30861) (cherry picked from commit 1c705fd) Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com> Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>

fixed flaky test

369390f

the-knights-who-say-ni added the CLA signed label Jan 24, 2022

bedevere-bot added tests Tests in the Lib/test dir awaiting review labels Jan 24, 2022

sobolevn added the skip news label Jan 24, 2022

kumaraditya303 marked this pull request as ready for review January 24, 2022 08:56

kumaraditya303 requested review from 1st1 and asvetlov as code owners January 24, 2022 08:56

erlend-aasland approved these changes Jan 24, 2022

View reviewed changes

bedevere-bot added awaiting core review and removed awaiting review labels Jan 24, 2022

erlend-aasland requested changes Jan 24, 2022

View reviewed changes

erlend-aasland added needs backport to 3.9 only security fixes needs backport to 3.10 only security fixes labels Jan 24, 2022

erlend-aasland approved these changes Jan 24, 2022

View reviewed changes

erlend-aasland reviewed Jan 24, 2022

View reviewed changes

Lib/test/test_asyncio/test_sendfile.py Show resolved Hide resolved

Fidget-Spinner added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jan 24, 2022

bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jan 24, 2022

asvetlov approved these changes Jan 24, 2022

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting core review labels Jan 24, 2022

pablogsal merged commit 1c705fd into python:main Jan 24, 2022

bedevere-bot removed the awaiting merge label Jan 24, 2022

bedevere-bot removed the needs backport to 3.10 only security fixes label Jan 24, 2022

bedevere-bot removed the needs backport to 3.9 only security fixes label Jan 24, 2022

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 24, 2022

fixed flaky test (pythonGH-30845)

309ec06

(cherry picked from commit 1c705fd) Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jan 24, 2022

fixed flaky test (pythonGH-30845)

dff8351

(cherry picked from commit 1c705fd) Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>

kumaraditya303 deleted the flaky branch January 25, 2022 12:34

aeros mentioned this pull request Oct 3, 2023

[Windows] : Proactor test_sendfile_close_peer_in_the_middle_of_receiving failure #85848

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bpo-41682: fixed flaky test test_sendfile_close_peer_in_the_middle_of_receiving #30845

bpo-41682: fixed flaky test test_sendfile_close_peer_in_the_middle_of_receiving #30845

kumaraditya303 commented Jan 24, 2022 •

edited by bedevere-bot

Loading

kumaraditya303 commented Jan 24, 2022

kumaraditya303 commented Jan 24, 2022

erlend-aasland left a comment

erlend-aasland left a comment •

edited

Loading

kumaraditya303 commented Jan 24, 2022

kumaraditya303 commented Jan 24, 2022

erlend-aasland commented Jan 24, 2022

Fidget-Spinner commented Jan 24, 2022

bedevere-bot commented Jan 24, 2022

erlend-aasland commented Jan 24, 2022

gvanrossum commented Jan 24, 2022

erlend-aasland commented Jan 24, 2022 •

edited

Loading

erlend-aasland commented Jan 24, 2022

miss-islington commented Jan 24, 2022

pablogsal commented Jan 24, 2022

bedevere-bot commented Jan 24, 2022

bedevere-bot commented Jan 24, 2022

bedevere-bot commented Jan 24, 2022

vstinner commented Jan 25, 2022

pablogsal commented Jan 25, 2022

vstinner commented Jan 25, 2022

bpo-41682: fixed flaky test test_sendfile_close_peer_in_the_middle_of_receiving #30845

bpo-41682: fixed flaky test test_sendfile_close_peer_in_the_middle_of_receiving #30845

Conversation

kumaraditya303 commented Jan 24, 2022 • edited by bedevere-bot Loading

kumaraditya303 commented Jan 24, 2022

kumaraditya303 commented Jan 24, 2022

erlend-aasland left a comment

Choose a reason for hiding this comment

erlend-aasland left a comment • edited Loading

Choose a reason for hiding this comment

kumaraditya303 commented Jan 24, 2022

kumaraditya303 commented Jan 24, 2022

erlend-aasland commented Jan 24, 2022

Fidget-Spinner commented Jan 24, 2022

bedevere-bot commented Jan 24, 2022

erlend-aasland commented Jan 24, 2022

gvanrossum commented Jan 24, 2022

erlend-aasland commented Jan 24, 2022 • edited Loading

erlend-aasland commented Jan 24, 2022

miss-islington commented Jan 24, 2022

pablogsal commented Jan 24, 2022

bedevere-bot commented Jan 24, 2022

bedevere-bot commented Jan 24, 2022

bedevere-bot commented Jan 24, 2022

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

vstinner commented Jan 25, 2022

pablogsal commented Jan 25, 2022

vstinner commented Jan 25, 2022

kumaraditya303 commented Jan 24, 2022 •

edited by bedevere-bot

Loading

erlend-aasland left a comment •

edited

Loading

erlend-aasland commented Jan 24, 2022 •

edited

Loading