[Dashboard] Fix potential job actor leak by ensuring retrieval of actor instance before termination.

[daiping8]

Why are these changes needed?

To close issue #55858

Problem Analysis

Let's analyze this from the code details. Let's look at the code in job_manager.py that handles job pending timeouts by the loop.

Normal process

When a job times out, JobStatus will be FAILED and the entire loop will be break. At the end of the function, ray.kill(job_supervisor, no_restart=True) will be executed.

Faulty process

When a job is in the pending state, for some reason the old node that submitted the job dies, and a new node is created . For the new job manager created after the raylet on new node (new raylet) starts, it continuously monitors the task status through _monitor_job_internal.

Just like the scenario I mentioned in #55858 (a new raylet is in the process of starting up, and the job happens to reach its timeout during this period), the loop that checks for job pending timeouts terminates during its first iteration, so the code responsible for obtaining the handle is never executed. It fails to obtain the job_supervisor handle.

So ray.kill(job_supervisor, no_restart=True) will not be executed.

Solution

• Fix potential job actor leak by ensuring retrieval of actor instance before termination.
• Add a test to validate job timeout behavior during new raylet creation.

Related issue number

Closes #55858

Checks

• I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
• I've run scripts/format.sh to lint the changes in this PR.
• I've included any doc changes needed for https://docs.ray.io/en/master/.
  • I've added any new APIs to the API Reference. For example, if I added a
    method in Tune, I've added it in doc/source/tune/api/ under the
    corresponding .rst file.
• I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
• Testing Strategy
  • Unit tests
  • Release tests
  • This PR is not tested :(

---

Note

Ensures _monitor_job_internal fetches the job supervisor actor before killing it to avoid leaks, and adds a test covering pending-job timeout during new raylet creation.

• Jobs Manager (python/ray/dashboard/modules/job/job_manager.py):
  • Ensure cleanup: before killing, fetch job_supervisor via _get_actor_for_job(job_id) if missing to avoid actor leaks on early loop exit.
• Tests (python/ray/dashboard/modules/job/tests/test_job_manager.py):
  • Add test_pending_job_timeout_during_raylet_creation to simulate new raylet creation, verify pending job times out, and confirm supervisor actor is removed.

^{Written by Cursor Bugbot for commit 5274288. This will update automatically on new commits. Configure here.}

[gemini-code-assist]

Code Review

This pull request addresses a potential job actor leak by ensuring the actor handle is retrieved before termination. The fix in job_manager.py is correct and directly solves the described issue. The accompanying test in test_job_manager.py effectively validates the fix by simulating a raylet restart and checking for proper job timeout and actor cleanup. I've provided a few suggestions to improve code conciseness in the fix and to enhance clarity in the new test.

[edoakes]

@israbbani PTAL

[jjyao]

@daiping8 Raylet will not restart. If raylet fails, the entire node is gone and a new Node will be created. Could you update the PR description to be more accurate. I think you mean GCS FT?

[daiping8]

@jjyao Thank you for your attention. I have clarified the relevant content in the descriptions of the issue and PR. The core problem is that the job_supervisor handle can be None in specific scenarios, thereby preventing resource release.

This scenarios is an extreme edge case, which makes it difficult to describe and understand. Thank you again for your patience.

[jjyao]

@daiping8 could you rebase, I just merged my fix.

[github-actions]

This pull request has been automatically marked as stale because it has not had
any activity for 14 days. It will be closed in another 14 days if no further activity occurs.
Thank you for your contributions.

You can always ask for help on our discussion forum or Ray's public slack channel.

If you'd like to keep this open, just leave any comment, and the stale label will be removed.

[edoakes]

@daiping8 @jjyao there is really no reason for us to be running the JobManager on every node. This is an artifact of a half-finished feature to load balance job submission across nodes, which is not really necessary because we can still schedule drivers across the cluster while only having the job manager on the head node.

So my preferred fix for this would actually be to simplify things by running the JobManager only on the head node and removing the _recover_running_jobs logic entirely. What do you think?

[jjyao]

So my preferred fix for this would actually be to simplify things by running the JobManager only on the head node and removing the _recover_running_jobs logic entirely. What do you think?

@edoakes yea, strictly speaking we don't officially support GCS FT for jobs, so we don't need _recover_running_jobs but in reality, many OSS users still run GCS FT for ray jobs and hence _recover_running_jobs was introduced in the first place.

[daiping8]

@jjyao Sorry for my late reply.

1. Following your suggestions, I've rewritten the test cases. Please review.

2. Should we uniformly change new raylet in the PR description to new head node for better clarity? As mentioned above, the JobManager might only run on the head node in the future. Let me know if this change is needed.

[daiping8]

@jjyao As per your guidance, all revisions have been completed. Please review again.

[jjyao]

Very sorry for the late review

[daiping8]

@jjyao A doc test unrelated to this PR has failed. I'm attempting to fix this issue in PR #59169, so please merge PR #59169 first.

[daiping8]

@jjyao All CI test have passed! Thanks.