Add Sagas to workflows #744
Conversation
sunny-b
requested review from
mfateev,
Sushisource,
alexshtin,
Spikhalskiy,
bergundy,
cretz and
mmcshane
as code owners
|
|
|
Hrmm, I wonder if this is best at https://github.com/temporalio/samples-go/. We often keep even utilities there that are not directly needed by the core of the project (encryption converters, DSL-based workflows, Prometheus metrics, JWT-based auth, etc). |
|
@cretz If the consensus is that it's better in the samples repo, I can move it there. I mainly added it to the |
|
|
||
| // since the cancelFunc is non-blocking, we use a canceled flag and lock | ||
| // to ensure compensation steps aren't run after the cancelFunc is called. | ||
| canceled bool |
There was a problem hiding this comment.
I ran into a strange edge case in one of my tests where some compensation steps were executed even after the Saga context was canceled. The activity would return a canceled error but for some reason the activity was still executed. I'm not sure if that's expected or not. It also wasn't consistent because sometimes the activity wouldn't execute but sometimes it did.
It didn't seem like a great user experience so I added this flag to prevent Compensate from running after Cancel is called.
internal/internal_saga.go
Outdated
| ctx Context | ||
| options *SagaOptions | ||
| compensationStack []*compensationOp | ||
| once sync.Once |
There was a problem hiding this comment.
We cannot use Go synchronization primitives in workflows.
internal/internal_saga.go
Outdated
| // since the cancelFunc is non-blocking, we use a canceled flag and lock | ||
| // to ensure compensation steps aren't run after the cancelFunc is called. | ||
| canceled bool | ||
| lock sync.RWMutex |
There was a problem hiding this comment.
Same here, Go sync primitives cannot be used (and is never required anyways because all workflow execution is single-threaded)
|
Thanks! Yeah, there are some regrets about the Java one being part of the SDK since it's so simple. Still discussing internally, but we might want to move this to samples-go. Cleanup actions are a pretty basic concept (though many may not recognize the name "saga") and of course supported in Go with defer and disconnected context and what not. And of course multi-activity execution (sequential and parallel) and error management on them are not unique to cleanup. I'll try to update soon. |
|
After some internal discussion, it was determined that the Java Saga implementation and this one don't add much over someone just looping themselves. But we may add more full-featured "Saga" support in the future to SDKs, but we need consensus on exactly what to do for all SDKs (see temporalio/features#50). Closing for now. |
What was changed
This PR adds Sagas to the sdk. This allows Go-SDK users to create compensating transactions. In essence, it allows users to easily specify rollback steps in their workflows that can be executed in the case that the workflow fails.
Why?
Sagas already exist in the Java-SDK and this PR is would make the Go-SDK have better feature parity with the Java-SDK. Additionally, there are third-party libraries that exist for this already so there is desire for this to be a native feature.
Checklist
This doesn't close a specific issue. However, I asked about it in the Temporal Slack and it seemed like there was interest to create this.
How was this tested:
I created a pretty thorough test suite with a test workflow that tests various edge cases (if compensation fails, if saga is cancelled, if the workflow is cancelled, parallel execution, etc)
I didn't add anything to the README, though there is a Saga example in the Java Examples repo and it might be good to add an example to the Go examples repo as well.