coinbase / temporal-ruby

Ruby SDK for Temporal
Apache License 2.0
213 stars 81 forks source link

Remove cancelation commands when underlying futures are closed #275

Closed jeffschoner closed 6 months ago

jeffschoner commented 7 months ago

Summary

This change filters out activity and timer cancellation commands before sending back the result of the workflow task to Temporal server. It does this when those commands are associated with activities or timers that have already been canceled.

Motivation

Existing code in future.rb already ensures that cancellation is a no-op on activity and timer futures that have already completed at that point in workflow execution. However, this doesn't cover all cases where an activity or timer is being canceled in the same workflow task where it is completing or failing. Consider the following scenario:

In an earlier workflow task:

In a later workflow task:

Because the future will still not be complete when the signal is received, it will be canceled and the command produced. However, later in the processing of the history window, we encounter a history event indicating that cancellation is not valid because the activity has already finished. When the RequestCancelActivityTaskCommand is sent to Temporal server, it will reject it as invalid and retry the workflow task. This will continue indefinitely, putting the workflow in a "stuck" state.

Testing

DeRauk commented 6 months ago

Thank you!