Copilot commented on code in PR #897:
URL:
https://github.com/apache/incubator-seata-go/pull/897#discussion_r2377431880
##########
pkg/saga/statemachine/statelang/state_instance.go:
##########
@@ -319,7 +319,10 @@ func (s *StateInstanceImpl) SetIgnoreStatus(ignoreStatus
bool) {
}
func (s *StateInstanceImpl) IsForCompensation() bool {
- return s.stateIdCompensatedFor == ""
+ // A state instance is considered a compensation execution if it points
+ // back to an original forward state via stateIdCompensatedFor.
+ // When this field is non-empty, this instance is a compensation.
+ return s.stateIdCompensatedFor != ""
Review Comment:
The logic appears inverted based on the comment. A state instance should be
considered 'for compensation' when it IS compensating another state
(stateIdCompensatedFor != ''), but the comment suggests the opposite semantics.
##########
pkg/saga/statemachine/engine/config/default_statemachine_config.go:
##########
@@ -641,13 +696,24 @@ func NewDefaultStateMachineConfig(opts ...Option)
(*DefaultStateMachineConfig, e
c.stateMachineRepository = repository.GetStateMachineRepositoryImpl()
c.stateLogRepository = repository.NewStateLogRepositoryImpl()
+
repository.GetStateMachineRepositoryImpl().SetDefaultTenantId(c.defaultTenantId)
c.syncProcessCtrlEventPublisher =
process_ctrl.NewProcessCtrlEventPublisher(c.syncEventBus)
c.asyncProcessCtrlEventPublisher =
process_ctrl.NewProcessCtrlEventPublisher(c.asyncEventBus)
for _, opt := range opts {
opt(c)
}
+
repository.GetStateMachineRepositoryImpl().SetDefaultTenantId(c.defaultTenantId)
+
+ if _, statErr := os.Stat("config.yaml"); statErr == nil {
+ if err := c.LoadConfig("config.yaml"); err == nil {
+
repository.GetStateMachineRepositoryImpl().SetDefaultTenantId(c.defaultTenantId)
+ log.Printf("Successfully loaded config from
config.yaml")
+ } else {
+ log.Printf("Failed to load config file (using
default/env values): %v", err)
Review Comment:
[nitpick] Hard-coded config file name 'config.yaml' makes configuration
inflexible. Consider making this configurable or checking multiple standard
config file names.
```suggestion
// Try to load the first available config file from a list of standard
names
configFiles := []string{"config.yaml", "config.yml",
"state-machine.yaml", "state-machine.yml"}
var loadedConfigFile string
for _, fname := range configFiles {
if _, statErr := os.Stat(fname); statErr == nil {
if err := c.LoadConfig(fname); err == nil {
repository.GetStateMachineRepositoryImpl().SetDefaultTenantId(c.defaultTenantId)
log.Printf("Successfully loaded config from
%s", fname)
loadedConfigFile = fname
} else {
log.Printf("Failed to load config file %s
(using default/env values): %v", fname, err)
}
break
```
##########
pkg/saga/statemachine/store/db/statelog.go:
##########
@@ -949,9 +1118,9 @@ func execStateInstanceStatementForUpdate(obj
statelang.StateInstance, stmt *sql.
serializedError,
obj.Status(),
obj.SerializedOutputParams(),
- obj.EndTime(),
+ time.Now(),
obj.ID(),
- obj.MachineInstanceID(),
+ obj.StateMachineInstance().ID(),
Review Comment:
Hard-coded time.Now() for updated time instead of using obj.UpdatedTime()
may cause inconsistent timestamps and break optimistic locking logic.
##########
pkg/saga/statemachine/store/db/statelog.go:
##########
@@ -949,9 +1118,9 @@ func execStateInstanceStatementForUpdate(obj
statelang.StateInstance, stmt *sql.
serializedError,
obj.Status(),
obj.SerializedOutputParams(),
- obj.EndTime(),
+ time.Now(),
Review Comment:
Hard-coded time.Now() for updated time instead of using obj.UpdatedTime()
may cause inconsistent timestamps and break optimistic locking logic.
```suggestion
obj.UpdatedTime(),
```
--
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]
