[Nexus-HSM-CHASM] Migrate cancellation state transitions #9092
Conversation
| var transitionCancellationScheduled = chasm.NewTransition( | ||
| []nexusoperationpb.CancellationStatus{nexusoperationpb.CANCELLATION_STATUS_UNSPECIFIED}, | ||
| nexusoperationpb.CANCELLATION_STATUS_SCHEDULED, | ||
| func(c *Cancellation, ctx chasm.MutableContext, event EventCancellationScheduled) error { | ||
| return serviceerror.NewUnimplemented("unimplemented") | ||
| // Record when cancellation was requested | ||
| c.RequestedTime = timestamppb.New(event.Time) | ||
|
|
||
| // Emit a cancellation task | ||
| ctx.AddTask(c, chasm.TaskAttributes{}, &nexusoperationpb.CancellationTask{ | ||
| Attempt: c.Attempt, | ||
| }) | ||
|
|
||
| return nil | ||
| }, | ||
| ) | ||
|
|
||
| // EventCancellationRescheduled is triggered when cancellation is meant to be rescheduled after backing off from a | ||
| // previous attempt. | ||
| type EventCancellationRescheduled struct { | ||
| } | ||
|
|
||
| var transitionCancellationRescheduled = chasm.NewTransition( | ||
| []nexusoperationpb.CancellationStatus{nexusoperationpb.CANCELLATION_STATUS_BACKING_OFF}, | ||
| nexusoperationpb.CANCELLATION_STATUS_SCHEDULED, | ||
| func(c *Cancellation, ctx chasm.MutableContext, event EventCancellationRescheduled) error { | ||
| return serviceerror.NewUnimplemented("unimplemented") | ||
| // Clear the next attempt schedule time | ||
| c.NextAttemptScheduleTime = nil | ||
|
|
||
| // Emit a cancellation task for the retry | ||
| ctx.AddTask(c, chasm.TaskAttributes{}, &nexusoperationpb.CancellationTask{ | ||
| Attempt: c.Attempt, | ||
| }) | ||
|
|
||
| return nil |
There was a problem hiding this comment.
These 2 transitions are identical. Can we merge them into one transition definition? (since a transition takes multiple source states)? Would that be idiomatic chasm code?
gow
force-pushed
the
nexus/hsm-to-chasm-migration
branch
from
6443639 to
90e445e
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| c.LastAttemptFailure = event.Failure | ||
|
|
||
| // Terminal state - no tasks to emit | ||
| return nil |
There was a problem hiding this comment.
Missing RequestedTime when transitioning from UNSPECIFIED to FAILED
Medium Severity
The transitionCancellationFailed transition can occur from CANCELLATION_STATUS_UNSPECIFIED directly to CANCELLATION_STATUS_FAILED, but it never sets RequestedTime. In contrast, transitionCancellationScheduled (which also transitions from UNSPECIFIED) does set RequestedTime. According to the proto definition, RequestedTime represents "The time when cancellation was requested." When a cancellation immediately fails from UNSPECIFIED state, this field will remain nil, creating an inconsistency with the expected data model.
What changed?
Migrated the cancellation operation state transitions.
Depends on #9082
Why?
Migrating Nexus from HSM to CHASM
How did you test it?
Note
Implements the cancellation state machine in CHASM and enriches cancellation state data.
CANCELLATION_STATUS_*: schedule, reschedule from backoff, retryable failure -> backoff (with computed delay), non-retryable failure -> failed, and success -> succeededCancellationTaskandCancellationBackoffTaskwith appropriateScheduledTimeandAttempthandlingrequested_time, incrementsattempt, setslast_attempt_complete_time, storeslast_attempt_failure, and managesnext_attempt_schedule_timeproto/v1/operation.protoand generatedoperation.pb.goto include newCancellationStatefields (requested_time,attempt,last_attempt_complete_time,last_attempt_failure,next_attempt_schedule_time,requested_event_id)Written by Cursor Bugbot for commit 1a8c936. This will update automatically on new commits. Configure here.