[core][test] Bound two unbounded waits that turn a test failure into a CI timeout

[jhasm]

Why are these changes needed?

//python/ray/tests:test_generators and //python/ray/dag:tests/experimental/test_compiled_graphs are the two worst offenders in the RocksDB GCS premerge job. Both are reported as TIMEOUT/FLAKY at the target level, consuming the full Bazel budget on every attempt, even though the underlying failure is a single test case.

Comparing premerge #70949, which ran the redis and rocksdb jobs on the same commit, the suite totals are effectively identical (29,427s redis vs 29,644s rocksdb, +0.7%), so there is no general backend latency tax. The damage is concentrated:

target	redis	rocksdb	ratio
//python/ray/dag:.../test_compiled_graphs	1193.6s	3600.2s	3.02×	FLAKY
//python/ray/tests:test_generators	416.7s	915.1s	2.20×	TIMEOUT
//python/ray/tests:test_multi_node_3	162.1s	207.9s	1.28×	passing

Everything else is ≤1.5×. And it is not a timing-margin problem: the retry of the same rocksdb shard passed test_generators in 343.7s, i.e. faster than redis. The distribution is bimodal, which points at a wedge rather than a slowdown.

Reading the timeout dumps, both targets wedge on an unbounded wait that has nothing to do with the assertion under test:

1. Node._kill_process_type waits with timeout=None whenever the caller passes wait=True, which Cluster.remove_node always does. SIGKILL cannot reap a process parked in uninterruptible sleep, and a process blocked in the fsync that the RocksDB GCS issues on every write is exactly that. So ray_start_cluster teardown blocks forever. In the failing test_generators attempt, pytest-timeout fired at 180s and teardown then absorbed the remaining ~700s until Bazel killed the target at 900s — twice, because of --flaky_test_attempts=2. One test-case failure cost 30 minutes of CI and was reported as TIMEOUT instead of a clean FAILED-then-retry.

2. run_string_as_driver / run_string_as_driver_stdout_stderr call proc.communicate() with no timeout, so a driver that hangs during shutdown blocks the test forever. That is precisely what test_compiled_graphs::test_async_shutdown does, and it is the point where that target's timeout dump lands.

Neither wait is load-bearing: nothing depends on waiting forever, only on waiting long enough.

What this changes

• Node._kill_process_type: bound the post-SIGKILL wait at 30s even when wait=True, and log the pid and process type when it expires. Reaping is normally instantaneous, so this is inert in the healthy case; when it does expire, Cluster.remove_node's existing any_processes_alive() assertion now reports a real error in seconds instead of hanging.
• run_string_as_driver / run_string_as_driver_stdout_stderr: add a timeout parameter defaulting to 300s. On expiry, kill the driver, log whatever it produced, and re-raise TimeoutExpired. 300s is well above the 180s pytest-timeout that already governs almost every caller, so no existing blocking driver should be affected. Pass timeout=None to restore the old behaviour.

This makes the failures bounded and attributable. It deliberately does not attempt to fix the underlying test-case flake, which is still under investigation and has not been reproduced outside CI — 48/48 local runs of the four test_dynamic_generator_reconstruction_nondeterministic variants passed under both backends, with rocksdb showing no slowdown (median 68.9s vs 70.2s for the in-memory GCS).

Related issue number

Follow-up to #64702 (REP-64). Not a duplicate — I checked open PRs and none touch these two waits.

Checks

• I've signed off every commit (DCO).
• I've made sure the tests are passing.

Tested locally:

• test_dynamic_generator_reconstruction_nondeterministic[None-False] and [None-True] under TEST_GCS_ROCKSDB=1: 2 passed in 136.9s
• test_output.py -k test_disable_driver_logs_breakpoint: 1 passed
• direct exercise of all three run_string_as_driver* paths, including the new timeout path (kills the driver and raises TimeoutExpired)
• pre-commit run clean on all three changed files

AI assistance (GitHub Copilot CLI) was used for the CI log analysis and to draft these changes; every line was reviewed by me.

[gemini-code-assist]

Code Review

This pull request introduces default timeouts when running driver scripts as separate processes and when reaping killed Ray processes, preventing unbounded waits that can cause CI test suites to hang. The reviewer identified a potential issue where calling proc.communicate() without a timeout after proc.kill() could still block indefinitely if the process is in an uninterruptible sleep state (D state), and suggested adding a timeout to these fallback calls.

[jhasm]

CI status on 580a1ce676

buildkite/microcheck #51110 is red on //python/ray/air:test_experiment_restore. That failure is pre-existing and unrelated to this PR:

1. Neither new code path executed. grep over the full job log finds zero occurrences of "Driver did not exit within" and zero of "uninterruptible syscall", so neither the new driver timeout nor the new reap timeout fired.
2. The test does not use the changed helpers. test_experiment_restore.py drives its subprocess with its own subprocess.Popen + _kill_process_if_needed, not run_string_as_driver*, and does not use Cluster/Node._kill_process_type.
3. It reproduces on master. Running RAY_TRAIN_V2_ENABLED=0 pytest python/ray/air/tests/test_experiment_restore.py locally:
   • on this branch: 2 failed in 360.29s
   • with node.py, _common/test_utils.py and _private/test_utils.py reverted to upstream/master: 2 failed in 360.30s — identical failure, same SplitCoordinator Fatal Python error: Aborted.

The observed failure modes are the harness's own 180s pytest timeout inside time.sleep(timeout_s) and assert 0.9375 == 1.0, i.e. training did not reach the expected progress. The test has a history of timing sensitivity (see #53387, "[tune] relax test_experiment_restore timeout").

The two core: python tests entries (test_object_manager_fault_tolerance, test_scheduling) were reported FLAKY, i.e. they passed on retry.

AI assistance was used for this investigation.

[jhasm]

Rebased onto 70373c54d6 (current master) and re-ran CI as build #51236. No code changes in the rebase — the diff is identical.

Final result of the previous run (#51110): core: python tests completed green. Three targets were reported FLAKY (test_object_manager_fault_tolerance, test_scheduling, test_placement_group_5) and passed on retry, and the log contains zero occurrences of either new log line, so neither the new driver timeout nor the new reap timeout ever fired.

The only hard failure was //python/ray/air:test_experiment_restore, which reproduces on master with these three files reverted (see the analysis above). Master has moved 8 commits since, none of them touching python/ray/air, python/ray/train/v2, or the Ray Data execution path, so that one is expected to stay red independently of this PR.

[cursor]

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

^{Reviewed by Cursor Bugbot for commit 97aca2d. Configure here.}

[jhasm]

CI triage for premerge #71139

Two finished failures, neither caused by this PR.

1. //python/ray/tests:test_tpu — TIMEOUT. Pre-existing under-sized target.

test_tpu.py is listed in the size = "medium" py_test_module_list in python/ray/tests/BUILD.bazel:33, which gives it a 300s Bazel budget. It does not fit in that budget:

Build	PR	Result
#71122	unrelated PR	test_tpu PASSED in 266.1s
#71139	this PR	TIMEOUT (attempt 1 at ~303s, 618s total over 2 attempts)

At 266s of a 300s budget on an unrelated PR, the target is at ~89% of budget on a good day, so any runner noise tips it over. The 618s figure is just --flaky_test_attempts=2 × ~300s, not a single 618s hang.

Evidence it is not this PR:

• Neither new code path executed. grep over the full job log returns zero occurrences of "did not exit within" and zero of "uninterruptible syscall", so the new reap bound never fired.
• The test does not touch the changed helpers. test_tpu.py has no reference to run_string_as_driver*, Cluster, remove_node, or _kill_process_type; it uses the ray_start_cluster fixture and hangs inside the test body (test_dispatch_integration_multi_slice), not in teardown.
• This PR can only shorten waits, never introduce one. Every change here replaces an unbounded wait with a bounded one.
• Not reproducible locally on this branch. pytest python/ray/tests/test_tpu.py -k dispatch_integration — 3 consecutive runs, 5 passed each, 337.6s / 336.7s / 338.6s. Note that those 5 integration tests alone already exceed the 300s budget for the whole 182-test file.

I have deliberately not re-sized the target here: test_tpu.py shares a py_test_module_list with ~40 other files, so splitting it out is a separate change that belongs with the TPU/core owners rather than in this PR.

2. //python/ray/air:test_experiment_restore — same pre-existing failure triaged earlier. It reproduces on master with these three files reverted (2 failed in 360.29s on the branch vs 2 failed in 360.30s on master).

AI assistance was used for this investigation.