samples-go icon indicating copy to clipboard operation
samples-go copied to clipboard

feat: add cancel-in-progress sample

Open StarpTech opened this issue 3 years ago • 6 comments
trafficstars

What was changed

This example demonstrates how to implement a workflow that ensures that only one run of the child workflow is executed at a time. Subsequent runs will cancel the running child workflow and re-run it with the latest sent options through SignalWithStartWorkflow.

Those semantics are useful, especially when implementing a CI pipeline. New commits during the workflow execution should short-circuit the child workflow and only build the most recent commit.

Why?

At WunderGraph, we use temporal to build our CI and CD platform. The solution has been implemented in collaboration with @mfateev I promised him to make a PR :smiley:

Checklist

  1. Closes

  2. How was this tested:

Manually, through the sample with a local temporal cluster.

  1. Any docs updates needed?

I'm not familiar with the template boundaries @@@SNIPSTART in the samples. Please tell me the instructions.

StarpTech avatar Oct 23 '22 21:10 StarpTech

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Oct 23 '22 21:10 CLAassistant

Thanks for the contribution!

To confirm, this example just starts a child workflow when the parent workflow starts and then waits for a signal to cancel the child workflow and start a new one? If I send 3 signals you start 3 children without waiting for any of the children to complete?

Some notes:

  • This could easily start dozens of child workflows and you'd never know if one errored. As a general purpose example, you may want to ensure only one child runs at a time.
  • You can completely miss child workflow failure if signal comes in at same time which is not safe for a general purpose example.
  • You're checking the parent context to see whether the child is cancelled.
  • For a general sample, it may not make sense to drain the signal channel. If I send 2 signals as a caller, it may be strange that one was ignored.

Since this is essentially restart-child-on-signal, not sure it deserves an entire sample. But if so I think it could be simplified and clarified a bit. You might also want a test or two. For instance, make a test where you replicate the condition where the "Child execution cancelled." log appears.

cretz avatar Oct 24 '22 12:10 cretz

Hi, @cretz it would help a lot if you could point to the relevant places for your argumentation.

This could easily start dozens of child workflows and you'd never know if one errored. As a general purpose example, you may want to ensure only one child runs at a time.

How so? A child workflow error will cancel the loop and return the error to the parent workflow

You're checking the parent context to see whether the child is canceled.

I don't think so. The cancellable context is created here https://github.com/temporalio/samples-go/pull/225/files#diff-e7190ffa6a984a8e3be3db5ad97758314062293387073e586d442ddd9d664b81R29

For a general sample, it may not make sense to drain the signal channel. If I send 2 signals as a caller, it may be strange that one was ignored.

That's the whole idea of this sample. I'm only interested in the last signal.

Since this is essentially restart-child-on-signal, not sure it deserves an entire sample.

Where can I find this example?

StarpTech avatar Oct 24 '22 14:10 StarpTech

To confirm, this example just starts a child workflow when the parent workflow starts and then waits for a signal to cancel the child workflow and start a new one? If I send 3 signals you start 3 children without waiting for any of the children to complete?

No, I start a workflow with a signal. When another signal is sent during the execution of the child workflow the workflow is canceled and a new one is started with the options from the last available signal.

StarpTech avatar Oct 24 '22 14:10 StarpTech

:+1: Will do full review...

cretz avatar Oct 24 '22 15:10 cretz

@cretz great review. I will address this soon.

StarpTech avatar Oct 24 '22 16:10 StarpTech