Keep draining remote stdout and stderr in SSHCluster - #9343
Open
m0g3r wants to merge 1 commit into
Open
Conversation
Worker.start and Scheduler.start read the remote process' stderr only until it announces its address, and never read stdout at all. Once that startup handshake is over nothing consumes the SSH channel any more, so the receive window fills up and the remote process blocks forever the next time it writes a log line. Forward both streams to the logger for the lifetime of the process instead, and cancel the forwarding tasks when the process is closed. Closes dask#9033 Co-Authored-By: Claude <noreply@anthropic.com>
m0g3r
marked this pull request as ready for review
August 11, 2026 21:45
Contributor
Unit Test ResultsSee test report for an extended history of previous test failures. This is useful for diagnosing flaky tests. 40 files + 1 40 suites +1 14h 31m 26s ⏱️ + 3m 45s For more details on these failures, see this check. Results for commit c15c483. ± Comparison against base commit 40fcd99. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SSHClustercan deadlock after running for a while because the scheduler stops reading the remote process’sstderronce startup is complete. Eventually the pipe fills up, which blocks the remote process the next time it tries to write a log message.This fix keeps draining both output streams for the entire lifetime of the process. I also added a regression test that hangs without the fix.
Closes #9033
Behaviour note
Remote log lines are now forwarded for the whole life of the cluster at
logger.info, matching what the existing startup loop already does. That surfaces remote logs that were previously lost after startup, but it is a visible change in log volume.Tests
The new test drives
ssh.Scheduleragainst an in-processasyncsshserver, so it exercises real SSH flow control without needing a reachable sshd. It writes past the 2 MiB default channel window after the startup banner and asserts the remote still finishes.FAILED ... TimeoutErrorafter 30s1 passed in 0.72sRun locally on macOS:
pytest distributed/deploy/tests/test_ssh.py::test_remote_process_not_blocked_by_unread_output --leaks=fds,processes,threads— 1 passed, no leakspytest distributed/deploy/tests/test_spec_cluster.py distributed/deploy/tests/test_cluster.py distributed/deploy/tests/test_subprocess.py— 33 passed, 3 skippedruff check/ruff format --checkon both changed files — cleanmypy --warn-unused-configs distributed/deploy/ssh.py— no new errors versus an unmodified checkouttest_ssh.pyisxfailon macOS, so the tests needing a real sshd could not run locally. CI covered them: onubuntu-24.04-arm py312 test-ci not ci1the new test passed along withtest_basic,test_n_workers,test_keywords,test_config_inherited_by_subprocess,test_list_of_connect_optionsandtest_remote_python.The test carries
@pytest.mark.leaking("fds")becauseasyncssh.create_serverleaks 2 fds on its own, confirmed separately with a server-only test containing no code from this repository.Unrelated CI failure
The
pre-commitjob fails onmypyindistributed/shuffle/tests/test_merge.pyandtest_shuffle.py, which this PR does not touch. It fails identically on #9341 from 2026-08-05; the mypy hook installsgit+https://github.com/dask/daskunpinned, so upstream drift made twotype: ignorecomments redundant. I have left those files alone to keep this PR in scope.Disclosure: this change was developed with AI assistance.