[RayJob] Fail fast on SidecarMode submitter failure during Initializing#4946
Open
tmrtmrtmrtmr wants to merge 2 commits into
Open
[RayJob] Fail fast on SidecarMode submitter failure during Initializing#4946tmrtmrtmrtmr wants to merge 2 commits into
tmrtmrtmrtmr wants to merge 2 commits into
Conversation
tmrtmrtmrtmr
requested review from
MortalHappiness,
andrewsykim,
kevin85421 and
rueian
as code owners
June 23, 2026 19:41
In SidecarMode the `ray-job-submitter` runs as a container inside the head Pod, with the head Pod's RestartPolicy forced to Never. If that submitter exits non-zero before the RayCluster reaches the `Ready` state (for example, the job fails while workers are still being scheduled), the terminated submitter container keeps the head Pod NotReady, so the RayCluster never reaches `Ready`. The RayJob `Initializing -> Running` transition is gated on `RayCluster.Status.State == Ready`, and the only submitter-failure detector (`checkSubmitterAndUpdateStatusIfNeeded`) runs exclusively in the `Running` state. As a result the RayJob is wedged in `Initializing` forever: it never transitions to `Failed`, `shutdownAfterJobFinishes` never runs, and the cluster is never torn down while the autoscaler keeps retrying workers. Detect a terminal submitter failure during the `Initializing` state (for SidecarMode) and fail fast. The shared, feature-gate-aware detection logic is extracted into `checkSidecarSubmitterFailedAndUpdateStatus` so the `Initializing` and `Running` states stay consistent (including the `SidecarSubmitterRestart` feature gate). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Timur Vankov <timur.vankov@silvermont.team>
tmrtmrtmrtmr
force-pushed
the
fix/sidecar-initializing-submitter-failfast
branch
from
June 23, 2026 19:47
514b835 to
9aa0f23
Compare
…ng Initializing The Initializing fail-fast set the failure reason from `rayJob.Status.JobStatus` (`AppFailed` when the job ran and failed, `SubmissionFailed` otherwise), but the job status is only polled in the `Running` state, so during `Initializing` it is always empty and the reason was always `SubmissionFailed`. That is wrong for a genuine application failure that fails while the cluster is still initializing (e.g. the e2e "Failing RayJob without cluster shutdown" case, which asserts `AppFailed`). When the submitter sidecar has failed during `Initializing`, best-effort poll the Ray job status from the dashboard before deciding the reason. The dashboard URL is resolved via the head Service (already up in SidecarMode) when `Status.DashboardURL` is not yet set. The poll is a no-op on any error (no dashboard client, empty JobId, URL resolution failure, dashboard unreachable, job not found), so the reason falls back to `SubmissionFailed` when the job status cannot be determined. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Timur Vankov <timur.vankov@silvermont.team>
Contributor
|
cc @machichima |
Collaborator
I am drafting the design for #4637, which focus on the case where head pod failed, which is a bit different from this PR |
machichima
reviewed
Jun 26, 2026
| rayJob.Status.JobStatus = jobInfo.JobStatus | ||
| } | ||
|
|
||
| func (r *RayJobReconciler) checkSidecarSubmitterFailedAndUpdateStatus(ctx context.Context, rayJob *rayv1.RayJob, headPod *corev1.Pod) bool { |
Collaborator
There was a problem hiding this comment.
There's some duplicate code between this function and checkSidecarSubmitterFailed. Could we have checkSidecarSubmitterFailed do the detection and return the container status, then pass that status into this function so it only focuses on updating the status? And maybe rename them to something like detectSidecarSubmitterFailure and updateStatusForSidecarSubmitterFailure.
Then we can drop "It uses the same detection logic as checkSidecarSubmitterFailedAndUpdateStatus so the two stay in sync." comment
There was a problem hiding this comment.
#4981
I finish it
@machichima
Closed
1 task
Open
1 task
Contributor
|
Hi @tmrtmrtmrtmr, do you mind addressing the review comment when you get the time? |
Author
yes |
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
5 participants
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.
Why are these changes needed?
In
SidecarMode, theray-job-submitterruns as a container inside the head Pod with the head Pod'sRestartPolicyset toNever. If the submitter exits non-zero before the RayCluster reachesReady(e.g. the job fails while workers are still being scheduled), the terminated submitter container keeps the head PodReady=False, soRayCluster.Status.Statenever becomesReady.The RayJob
Initializing -> Runningtransition is gated onRayCluster.Status.State == Ready, and the only submitter-failure detector (checkSubmitterAndUpdateStatusIfNeeded) runs only in theRunningstate. As a result the RayJob is wedged inInitializingforever: it never transitions toFailed,shutdownAfterJobFinishesnever runs, and the autoscaler keeps retrying workers — even though the submitter failure is already observable.This PR detects a terminal submitter failure during
Initializing(SidecarMode) and fails fast. The feature-gate-aware detection (SidecarSubmitterRestart) is extracted intocheckSidecarSubmitterFailedAndUpdateStatusand shared by theInitializingandRunningstates; theInitializingcall skips when the head Pod does not exist yet.Related issue number
Closes #4945
Checks
Testing:
go build ./controllers/ray/...,go vet ./controllers/ray/,gofumpt -l— cleanTestCheckSidecarSubmitterFailedAndUpdateStatus: submitterTerminatedexit != 0 →Failed/SubmissionFailed; same withJobStatusalreadyFailed→AppFailed; exit 0 → no-op; stillRunning→ no-op;SidecarSubmitterRestartenabled → routes to the restart-count checkgo test ./controllers/ray/ -run 'TestCheckSidecarSubmitterFailedAndUpdateStatus|TestCheckIsRestartCountExceeded'— pass