Skip to content

Keep draining remote stdout and stderr in SSHCluster - #9343

Open
m0g3r wants to merge 1 commit into
dask:mainfrom
m0g3r:ssh-drain-remote-output
Open

Keep draining remote stdout and stderr in SSHCluster#9343
m0g3r wants to merge 1 commit into
dask:mainfrom
m0g3r:ssh-drain-remote-output

Conversation

@m0g3r

@m0g3r m0g3r commented Aug 11, 2026

Copy link
Copy Markdown

SSHCluster can deadlock after running for a while because the scheduler stops reading the remote process’s stderr once 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.Scheduler against an in-process asyncssh server, 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.

  • Without the fix: FAILED ... TimeoutError after 30s
  • With the fix: 1 passed in 0.72s

Run 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 leaks
  • pytest distributed/deploy/tests/test_spec_cluster.py distributed/deploy/tests/test_cluster.py distributed/deploy/tests/test_subprocess.py — 33 passed, 3 skipped
  • ruff check / ruff format --check on both changed files — clean
  • mypy --warn-unused-configs distributed/deploy/ssh.py — no new errors versus an unmodified checkout

test_ssh.py is xfail on macOS, so the tests needing a real sshd could not run locally. CI covered them: on ubuntu-24.04-arm py312 test-ci not ci1 the new test passed along with test_basic, test_n_workers, test_keywords, test_config_inherited_by_subprocess, test_list_of_connect_options and test_remote_python.

The test carries @pytest.mark.leaking("fds") because asyncssh.create_server leaks 2 fds on its own, confirmed separately with a server-only test containing no code from this repository.

Unrelated CI failure

The pre-commit job fails on mypy in distributed/shuffle/tests/test_merge.py and test_shuffle.py, which this PR does not touch. It fails identically on #9341 from 2026-08-05; the mypy hook installs git+https://github.com/dask/dask unpinned, so upstream drift made two type: ignore comments redundant. I have left those files alone to keep this PR in scope.


Disclosure: this change was developed with AI assistance.

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
m0g3r marked this pull request as ready for review August 11, 2026 21:45
@m0g3r m0g3r changed the title [unsupervised AI] Keep draining remote stdout and stderr in SSHCluster Keep draining remote stdout and stderr in SSHCluster Aug 11, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Unit Test Results

See 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
 4 160 tests +    1   3 979 ✅  -     1    178 💤 ±  0  3 ❌ +2 
80 958 runs  +2 730  76 712 ✅ +2 581  4 243 💤 +147  3 ❌ +2 

For more details on these failures, see this check.

Results for commit c15c483. ± Comparison against base commit 40fcd99.

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.

Scheduler deadlock when using SSHCluster due to stderr blocking

1 participant