Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

KEP-2170: Add the TrainJob state transition design #2298

Merged
merged 5 commits into from
Nov 2, 2024
Merged

KEP-2170: Add the TrainJob state transition design #2298

Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
950f384
KEP-2170: Add the TrainJob state transition design
tenzen-y Oct 20, 2024
01628fc
Replace actual jobs with TrainJob
tenzen-y Oct 23, 2024
e184fea
Remove the JobSet conditions propagation and Add expanding runtime fr…
tenzen-y Oct 23, 2024
1cca202
Expand the Creation Failed reasons
tenzen-y Nov 1, 2024
b81a633
Rename Completed to Complete
tenzen-y Nov 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 83 additions & 0 deletions docs/proposals/2170-kubeflow-training-v2/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,43 @@ type TrainJob struct {
Status TrainJobStatus `json:"status,omitempty"`
}

const (
Copy link
Member

@andreyvelich andreyvelich Oct 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we define those consts as TrainJobConditionType similar to Batch/Job: https://github.com/kubernetes/api/blob/master/batch/v1/types.go#L617 ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so since our conditions typed are metav1.Condition and the Type is typed string.
The batch/v1 Job uses the typed JobConditionType since Job conditions typed are dedicated JobCondition: https://github.com/kubernetes/api/blob/2be23f6c5a7184af9846ff458a11751765ea2bde/batch/v1/types.go#L662

When we use the dedicated typed TrainJobConditionType, we need to cast the TrainJobConditionType to string everywhere. That is not ideal.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, you are right.

// TrainJobSuspended means the TrainJob is suspended.
TrainJobSuspended string = "Suspended"

// TrainJobCompleted means that the TrainJob has completed its execution.
TrainJobCompleted string = "Completed"

// TrainJobFailed means that the actual jobs have failed its execution.
TrainJobFailed string = "Failed"

// TrainJobCreated means that the actual jobs creation has succeeded.
TrainJobCreated string = "Created"
)

const (
// TrainJobSuspendedReason is the "Suspended" condition reason.
// When the TrainJob is suspended, this is added.
TrainJobSuspendedReason string = "Suspended"

// TrainJobResumedReason is the "Suspended" condition reason.
// When the TrainJob suspension is changed from True to False, this is added.
TrainJobResumedReason string = "Resumed"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tenzen-y @kannon92 Do we have that status reason in JobSet or Job ?

Copy link
Member Author

@tenzen-y tenzen-y Oct 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we have. In the batch/v1 Job level, JobResumed is used as a reason for the Suspended condition type.
In the JobSet level, ResumeJobs is used as a reason for the Suspended condition type.


// TrainJobJobsCreationSucceededReason is the "Created" condition reason.
// When the creating objects succeeded after building succeeded, this is added.
TrainJobJobsCreationSucceededReason string = "JobsCreationSucceeded"

// TrainJobJobsBuildFailedReason is the "Created" condition reason.
// When the building objects based on the TrainJob and the specified runtime failed,
// this is added.
TrainJobJobsBuildFailedReason string = "JobsBuildFailed"

// TrainJobJobsCreationFailedReason is "Created" condition reason.
// When the creating objects failed even though building succeeded, this is added.
TrainJobJobsCreationFailedReason string = "JobsCreationFailed"
Comment on lines +305 to +316
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we introduce those conditions in the 2nd iteration ?
I think, we should discuss if users really want to decouple conditions when TrainJob's object creation failed and when TrainJob fails.

Copy link
Member Author

@tenzen-y tenzen-y Oct 21, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are reasons, so we use these in the following. Additionally, if we do not distinguish JobCreationFailed and JobBuildFailed, it is hard to understand which part failed. So, these reasons must be included in the first phase.

status:
  conditions:
  - type: Created
    status: false
    reason: JobsCreationFailed
status:
  conditions:
  - type: Created
    status: false
    reason: JobsBuildFailed
status:
  conditions:
  - type: Created
    status: true
    reason: JobsCreationSucceeded

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we have validation webhook, what is the use-case you see when we can hit the JobsBuildFailed reason ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can not validate in advance if NewObject succeed since we can not know in advance what happens during the NewObject.

Only during reconciling, we can know the actual error.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, I guess one of the examples could be if NewObject calls Kubernetes API server and this call fails.
In that case, we want to transition TrainJob to JobsBuildFailed status.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, we want to transition TrainJob to JobsBuildFailed status.

Yes, that's right. In the current JobSet plugin, the building error could happen in the following:

Additionally, every plugin could return errors during Kubeflow Job Pipeline Framerowk.

)

type TrainJobSpec struct {
// Reference to the training runtime.
// The field is immutable.
Expand Down Expand Up @@ -916,6 +953,52 @@ spec:
claimName: user-123-volume
```

### State Transition

In this section, we're explaining the TrainJob state transition (`.status.conditions`).
The basic TrainJob state machine is the below.
Especially, if we specify the TrainingRuntime or ClusterTrainingRuntime as a runtime,
the TrainJob terminal condition (`Failed` or `Completed`) is decided based on the JobSet terminal state (`status.terminalState`)
instead of computing from JobSet conditions.

```mermaid
stateDiagram-v2
#CREATION
state created_choice <<choice>>
[*] --> created_choice: TrainJob is submitted.
created_choice --> Created=True: Succeeded to build and deploy Jobs.
created_choice --> Created=False: Failed to build and deploy Jobs.
Comment on lines +969 to +970
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be helpful to add small clarified message what does Failed to build Jobs mean ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Created=False --> Created=False: Wait for updated appropriate TrainJob.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean by Wait for updated appropriate TrainJob ?
I thought if build or deploy Jobs fails, we transition TrainJob to Failed condition ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TrainJob is the mutable object. So, we wait for updating TrainJob with proper fields here.

Copy link
Member

@andreyvelich andreyvelich Oct 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some API in TrainJob is immutable, like managedBy, but the Trainer API is mutable, right ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's right. We allow users to modify the TrainJob except for some fields like managedBy.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sense, in that case my question is why do we transition TrainJob in the Failed state if the underlying JobSet is Failed ? Since TrainJob image is mutable, user can override it.
Or the main motivation is that when TrainJob is in Created state, it only can transition to Failed or Succeeded ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in that case my question is why do we transition TrainJob in the Failed state if the underlying JobSet is Failed ? Since TrainJob image is mutable, user can override it.

AFAIK, both batch/v1 Job, and JobSet can not be restarted once those have reached the terminal phase (complete or failed).

Created=False --> Created=True: Succeeded to build and deploy Jobs.

#SUSPENSION
state suspended_choice <<choice>>
Created=True --> suspended_choice: Handle TrainJob suspension.
suspended_choice --> Suspended=True: TrainJob is suspended.
Suspended=True --> Suspended=True: Wait for unsuspending.
Suspended=True --> Suspended=False: TrainJob is unsuspended.
suspended_choice --> Suspended=False: TrainJob is not suspended.

#FAILURE
state terminal_choice <<choice>>
Suspended=False --> terminal_choice: Actual Jobs go to terminal phase.
terminal_choice --> Failed=True: Actual Jobs (e.g., JobSet) failed.
Failed=True --> [*]

#COMPLETION
terminal_choice --> Completed=True: Actual Jobs (e.g., JobSet) completed.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we actually call this condition Succeeded to be consistent with JobSet: https://github.com/kubernetes-sigs/jobset/blob/main/api/jobset/v1alpha2/jobset_types.go#L182-L183 ?

From my understanding Completed: True mens that TrainJob is in Failed or Succeeded state.

Copy link
Member Author

@tenzen-y tenzen-y Nov 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically, Job and JobSet succeed (.replicatedJobsStatus.succeed), and failed (.replicatedJobsStatus.failed) count are not terminal phase, and those are intermediate one. Additionally those are not guaranteed consistency when Success and Failure criteria are conflicts.

So, instead of success count, we should rely on the .status.terminalState or conditions.

Copy link
Member

@andreyvelich andreyvelich Nov 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we going to have Success state for the TrainJob conditions ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TrainJob has only Suspend, Completed, Failed, and Created conditions.

Completed=True --> [*]
```

In the above state transition, the `Created=False` will happen in the following situations and
those different situations can be identified by the condition reasons (`.status.conditions.[type="Created"].reason`).

- `JobsBuildFailed`: When the TrainJob controller failed to construct objects (resources) using the [runtime framework interfaces](../../../pkg/runtime.v2/framework/interface.go)
- `JobsCreationFailed`: When the TrainJob controller succeeded to construct objects, but it failed to deploy objects to the cluster.

Additionally, we extend the [runtime framework interfaces](../../../pkg/runtime.v2/framework/interface.go)
to allow each plugin to propagate the arbitrary conditions to the TrainJob.

## The Training Runtime API

The `TrainingRuntime` is the pre-created configurations of model training on the cluster,
Expand Down
Loading