kaori-seasons commented on code in PR #897:
URL:
https://github.com/apache/incubator-seata-go/pull/897#discussion_r2377467627
##########
pkg/saga/statemachine/engine/pcext/state_router_impl.go:
##########
@@ -53,15 +68,81 @@ func (t TaskStateRouter) Route(ctx context.Context,
processContext process_ctrl.
return nil, nil
}
- // The current CompensationTriggerState can mark the compensation
process is started and perform compensation
- // route processing.
- compensationTriggerState, ok :=
processContext.GetVariable(constant.VarNameCurrentCompensateTriggerState).(statelang.State)
- if ok {
+ // If current state is CompensationTrigger or compensation has been
flagged, start compensation routing
+ if state != nil && state.Type() ==
constant.StateTypeCompensationTrigger {
+ // flag into context for downstream calls (best-effort)
+ if hpc, ok :=
processContext.(process_ctrl.HierarchicalProcessContext); ok {
+
hpc.SetVariableLocally(constant.VarNameCurrentCompensateTriggerState, state)
+
hpc.SetVariableLocally(constant.VarNameFirstCompensationStateStarted, false)
+ } else {
+
processContext.SetVariable(constant.VarNameCurrentCompensateTriggerState, state)
+
processContext.SetVariable(constant.VarNameFirstCompensationStateStarted, false)
+ }
+
+ // build compensation stack from executed forward states
(latest first)
+ smInst :=
processContext.GetVariable(constant.VarNameStateMachineInst).(statelang.StateMachineInstance)
+ holder := GetCurrentCompensationHolder(ctx, processContext,
true)
+ stack := holder.StateStackNeedCompensation()
+ sm :=
processContext.GetVariable(constant.VarNameStateMachine).(statelang.StateMachine)
+ states := smInst.StateList()
+ for i := len(states) - 1; i >= 0; i-- {
+ si := states[i]
+ // exclude compensation states
+ if si.StateIDCompensatedFor() != "" {
+ continue
+ }
+ // only successful forward states are subject to
compensation
+ if si.Status() != statelang.SU {
+ continue
+ }
+ originName := GetOriginStateName(si)
+ def := sm.State(originName)
+ // ensure the definition has a compensate state
+ var task *sagaState.AbstractTaskState
+ switch s := def.(type) {
+ case *sagaState.ServiceTaskStateImpl:
+ task = s.AbstractTaskState
+ case *sagaState.ScriptTaskStateImpl:
+ task = s.AbstractTaskState
+ case *sagaState.SubStateMachineImpl:
+ if s.ServiceTaskStateImpl != nil {
+ task =
s.ServiceTaskStateImpl.AbstractTaskState
+ }
+ }
+ if task == nil || task.CompensateState() == "" {
+ continue
+ }
+ stack.Push(si)
+ }
+ // fallback: if nothing pushed, try last successful forward
state
+ if stack.Empty() {
+ for i := len(states) - 1; i >= 0; i-- {
+ si := states[i]
+ if si.StateIDCompensatedFor() != "" {
+ continue
+ }
+ if si.Status() != statelang.SU {
+ continue
+ }
+ stack.Push(si)
+ break
+ }
+ }
+
+ // mark machine compensation running
+ smInst.SetCompensationStatus(statelang.RU)
+
+ return t.compensateRoute(ctx, processContext, state)
Review Comment:
In the core method Route in process_router.go , the routing logic checks the
state type currently executed by the state machine:
- If it is a normal Task state, the StateInstruction determines the next
state to execute based on it.
- If it is a Choice state, the routing logic first selects a "next" branch
using an expression or branch condition and updates it to the StateInstruction:
```
stateInstruction.SetStateName(next)
```
This allows the state machine to execute different branches based on the
Choice condition.
Specifically, the branch selection result of the Choice state is assigned to
the corresponding field of the StateInstruction (such as StateName), thus
affecting subsequent processes.
#### Existing Issues
The compensation chain is constructed only backwards from the execution
stack. If there is no compensable state after the Choice/Failure branch, the
process simply marks it as NoCompensation, but the final state normalization is
not explicit enough.
Multi-branch, multi-stage compensation, and compensation failure lack
fine-grained state recording. Recommendations
#### Recommendations
When building a compensation chain, traverse the state list to clearly mark
the branch source and compensation result of each compensated state. We
recommend adding a new structure such as CompensationTrace[]CompensationResult,
which includes the branch, compensation status, and exception.
When a Fail End is reached without compensation, the state is normalized to
FA and the compensation status is cleared. We recommend also exposing the
compensation chain record to the instance for easy querying.
When compensating multiple branches, it is recommended that each branch
record its compensation result separately, and the final state is normalized
based on the compensation results of all branches.
Scenario Description
Assume the following SAGA state machine definition:
```
Start -> Choice
Choice:
if (A) -> Service1
else -> Service2
Service1 -> Service3
Service2 -> Service4
Service3/Service4 -> FailEnd
FailEnd: Compensation triggers
```
Data Input Example (Choice condition is false, branch to Service2)
Initial Business Parameters:
```{"userId": 111, "amount": 100, "route": "B"}```
State Machine Execution Path:
```Start → Choice (route == B, choose Service2) → Service2 → Service4 →
FailEnd```
#### Boundary Issues
- Issue 1: If the compensation chain is not constructed rigorously, FailEnd
triggers compensation, but the compensation chain incorrectly includes the
unexecuted Service1/Service3 branches.
- Issue 2: If Service4 does not define a compensation state, the
compensation chain is empty. In this case, the compensation chain is not
properly normalized, resulting in the final state machine instance remaining in
"Compensation in Progress" or "Unknown" instead of "Failure without
Compensation."
Data output without boundary processing (pseudo-JSON)
```
{
"stateMachineInstance": {
"id": "saga-123",
"status": "FA",
"compensationStatus": "RU",
"compensationChain": [
{"state": "Service1", "executed": false}, // 不该出现
{"state": "Service2", "executed": true, "compensated": false},
{"state": "Service4", "executed": true, "compensated": false}
],
"exception": "补偿链错误包含了未执行分支"
}
}
```
Issue: The compensation chain incorrectly includes unexecuted branches, and
the final state machine instance compensationStatus is not normalized (still
RU), misleading the API/Ops layer into thinking compensation is still in
progress.
Data output after boundary processing
```
{
"stateMachineInstance": {
"id": "saga-123",
"status": "FA",
"compensationStatus": "",
"compensationChain": [
{"state": "Service2", "executed": true, "compensated": false},
{"state": "Service4", "executed": true, "compensated": false}
],
"exception": null
}
}
```
Unexecuted branches (Service1) removed
The compensation chain strictly includes only states that are "actually
executed and have compensation defined."
If no compensation is actually made (Service4 has no compensation defined),
the compensationStatus field is initialized to null, the status field is
initialized to "failed" (FA), the exception field is empty, and the process is
explicitly terminated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
