Skip to content
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

test_ready_remove_worker broken on Python 3.11 #8054

Open
hendrikmakait opened this issue Jul 31, 2023 · 6 comments
Open

test_ready_remove_worker broken on Python 3.11 #8054

hendrikmakait opened this issue Jul 31, 2023 · 6 comments
Labels
bug Something is broken tests Unit tests and/or continuous integration

Comments

@hendrikmakait
Copy link
Member

It looks #7698 broke test_ready_remove_worker on Python 3.11:

test_ready_remove_worker is broken on Python 3.11

This test fails with RuntimeError: ('Unclosed Comms', [<TCP (closed) ConnectionPool local=tcp://127.0.0.1:62390 remote=tcp://127.0.0.1:62381>, <TCP (closed) ConnectionPool local=tcp://127.0.0.1:62391 remote=tcp://127.0.0.1:62381>]).
(https://github.com/dask/distributed/actions/runs/5690294964/job/15423369445#step:19:1681)

cc @graingert

@hendrikmakait hendrikmakait added bug Something is broken tests Unit tests and/or continuous integration and removed needs triage labels Jul 31, 2023
@crusaderky
Copy link
Collaborator

Ah you beat me two it.
There are two more tests that started failing at the same time, test_closed_worker_during_shuffle and test_restarting_during_transfer_raises_killed_worker.
XREF #8055

@crusaderky
Copy link
Collaborator

crusaderky commented Jul 31, 2023

Timing and topic point to #7698. If a resolution is not trivial, maybe we should revert it for the time being?

@hendrikmakait
Copy link
Member Author

Regarding the P2P errors, I've described the problem over here: #8011

@hendrikmakait
Copy link
Member Author

I'll dig deeper into test_ready_remove_worker but if it's hard to fix, I agree that we should revert #7698 and solve its issues before merging it back into main. cc @fjetter

@fjetter
Copy link
Member

fjetter commented Jul 31, 2023

I don't want to revert #7698 just because of a couple of leaked connections. This PR has a significant real world impact and I'd rather temporarily disable the connection leak detection that reverting this PR

@hendrikmakait
Copy link
Member Author

I don't want to revert #7698 just because of a couple of leaked connections. This PR has a significant real world impact and I'd rather temporarily disable the connection leak detection that reverting this PR

Works for me, after an initial investigation the leak doesn't seem to be trivial to fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is broken tests Unit tests and/or continuous integration
Projects
None yet
Development

No branches or pull requests

3 participants