[RayJob] Support ActiveDeadlineSeconds #1933
Conversation
|
|
||
| // TODO (kevin85421): For light-weight mode, calculate the number of failed retries and transition |
There was a problem hiding this comment.
We may consider implementing backoffLimit if ActiveDeadlineSeconds proves to be insufficient.
|
cc @andrewsykim would you mind reviewing this PR? |
| @@ -114,7 +115,6 @@ type RayJobStatus struct { | |||
| RayClusterStatus RayClusterStatus `json:"rayClusterStatus,omitempty"` | |||
| // observedGeneration is the most recent generation observed for this RayJob. It corresponds to the | |||
| // RayJob's generation, which is updated on mutation by the API Server. | |||
| // +optional | |||
There was a problem hiding this comment.
What's the reason for this change?
There was a problem hiding this comment.
I just found that it is useless. There is no change in the CRD YAML after I remove the +optional.
| @@ -142,6 +142,11 @@ func (r *RayJobReconciler) Reconcile(ctx context.Context, request ctrl.Request) | |||
| if shouldUpdate := r.updateStatusToSuspendingIfNeeded(ctx, rayJobInstance); shouldUpdate { | |||
| break | |||
| } | |||
| if pastActiveDeadline(rayJobInstance) { | |||
| r.Log.Info("The RayJob has passed the activeDeadlineSeconds. Transition the status to `Complete`.", "StartTime", rayJobInstance.Status.StartTime, "ActiveDeadlineSeconds", *rayJobInstance.Spec.ActiveDeadlineSeconds) | |||
| rayJobInstance.Status.JobDeploymentStatus = rayv1.JobDeploymentStatusComplete | |||
There was a problem hiding this comment.
Do you think exceeding deadline seconds warrants a new status DeadlineExceeded?
There was a problem hiding this comment.
If not -- if user sees status Complete, are the ray-operator logs the only way to know if a job successfully completed or timed out?
There was a problem hiding this comment.
I will improve the observability of Complete before KubeRay v1.1.0 release. Last week, I chatted with the Flyte community, and they require sufficient observability to distinguish between "Succeeded" and "Failed" statuses, including the reasons for each (e.g. backoffLimit, app-level bug, DeadlineExceeded).
There was a problem hiding this comment.
Adding a Reason field sgtm 👍
There was a problem hiding this comment.
@Yicheng-Lu-llll will take this issue.
Why are these changes needed?
StartTimerefers to the time when the Ray job starts running on the RayCluster. This PR redefinesStartTimeto the time whenJobDeploymentStatustransitioned fromNewtoInitializing.ActiveDeadlineSeconds: Similar toActiveDeadlineSecondsin K8s Job https://kubernetes.io/docs/concepts/workloads/controllers/job/.ActiveDeadlineSecondsif the RayJob is inNew,Complete,Suspending, andSuspended.ActiveDeadlineSecondsif the RayJob is in theInitializingorRunningstate, which may occupy some computing resources.ActiveDeadlineSeconds, KubeRay will transition the status toComplete.Related issue number
Checks