Skip to content

Revert getaddrinfo fast path for python 3.8#6978

Merged
crusaderky merged 1 commit intodask:mainfrom
fjetter:getaddrinfo_py38
Aug 31, 2022
Merged

Revert getaddrinfo fast path for python 3.8#6978
crusaderky merged 1 commit intodask:mainfrom
fjetter:getaddrinfo_py38

Conversation

@fjetter
Copy link
Member

@fjetter fjetter commented Aug 31, 2022

This is a partial revert of #6847

This change introduced a regression in distributed.tests.test_client::test_reconnect that only affects python 3.8 versions (mostly OSX but rarely also ubuntu)

I suggest to remove this optimization for 3.8

FWIW On my fork I triggered a run using the in #6847 (comment) proposed getaddrinfo performance debugging in case this reveals anything useful, see https://github.com/fjetter/distributed/actions/runs/2963091290

@github-actions
Copy link
Contributor

Unit Test Results

See test report for an extended history of previous test failures. This is useful for diagnosing flaky tests.

       15 files  ±0         15 suites  ±0   6h 25m 43s ⏱️ - 17m 14s
  3 053 tests ±0    2 969 ✔️ ±0    84 💤 ±0  0 ±0 
22 585 runs  ±0  21 604 ✔️ +1  981 💤  - 1  0 ±0 

Results for commit c19251e. ± Comparison against base commit 817ead3.

@fjetter fjetter marked this pull request as ready for review August 31, 2022 12:30
@fjetter fjetter linked an issue Aug 31, 2022 that may be closed by this pull request
@crusaderky crusaderky merged commit d72fd21 into dask:main Aug 31, 2022
@fjetter
Copy link
Member Author

fjetter commented Aug 31, 2022

FWIW running with the debugging settings yields indeed an exception

RuntimeError: Getting address info 127.0.0.1:58249, type=<SocketKind.SOCK_STREAM: 1>, flags=<AddressInfo.AI_NUMERICSERV|AI_NUMERICHOST: 4100> took 109.271ms: [(<AddressFamily.AF_INET: 2>, <SocketKind.SOCK_STREAM: 1>, 6, '', ('127.0.0.1', 58249))]

gjoseph92 pushed a commit to gjoseph92/distributed that referenced this pull request Oct 31, 2022
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.

Flaky distributed/tests/test_client.py::test_reconnect

2 participants