cadence-workflow / cadence-go-client

Framework for authoring workflows and activities running on top of the Cadence orchestration engine.
https://cadenceworkflow.io
MIT License
349 stars 132 forks source link

MutableSideEffect fails if called twice with the same id during a single decision #763

Open mfateev opened 5 years ago

mfateev commented 5 years ago

To reproduce:

func Workflow(ctx workflow.Context, name string) error {
    // Get Greeting.
    ao := workflow.ActivityOptions{
        ScheduleToStartTimeout: time.Minute,
        StartToCloseTimeout:    time.Minute,
        HeartbeatTimeout:       time.Second * 20,
    }
    ctx = workflow.WithActivityOptions(ctx, ao)

    logger := workflow.GetLogger(ctx)

    f := func(ctx workflow.Context) interface{} {
        return rand.Intn(100)
    }

    e := func(a, b interface{}) bool {
        if a == b {
            return true
        }

        return false
    }

    var result int
    sideEffectValue := workflow.MutableSideEffect(ctx, "value-1", f, e)
    err := sideEffectValue.Get(&result)
    if err != nil {
        return err
    }

    logger.Debug("MutableSideEffect-1", zap.Int("Value", result))

        // Uncommenting Sleep fixes the issue
    //workflow.Sleep(ctx, time.Second)

    //************** THIS CALL FAILS **************
    sideEffectValue = workflow.MutableSideEffect(ctx, "value-1", f, e)
    err = sideEffectValue.Get(&result)
    if err != nil {
        return err
    }

    logger.Debug("MutableSideEffect-2", zap.Int("Value", result))

    logger.Info("helloworld workflow done")
    return nil
}

Failure:

1.5609699662562912e+09  error   internal/internal_task_handlers.go:747  non-deterministic-error {"build_hash": "unknown-build-hash", "hostname": "maxim-C02XD0AAJGH6", "runtime_env": "", "service_name": "cadence-samples", "zone": "", "Domain": "samples-domain", "TaskList": "cadence-samples-tasklist", "WorkerID": "6973@maxim-C02XD0AAJGH6@cadence-samples-tasklist", "WorkflowType": "code.uber.internal/devexp/cadence-samples/cmd/samples/recipes/helloworld.Workflow", "WorkflowID": "helloworld_6e42e4ef-f176-431c-b2e7-67a2b6b37229", "RunID": "546d497e-ca1e-4fd8-9651-a730b9c07a74", "error": "adding duplicate decision DecisionType: Marker, ID: MutableSideEffect_value-1, state=Created, isDone()=false, history=[Created]"}
code.uber.internal/devexp/cadence-samples/vendor/go.uber.org/cadence/internal.(*workflowExecutionContextImpl).ProcessWorkflowTask
    /Users/maxim/sqlcadence/src/code.uber.internal/devexp/cadence-samples/vendor/go.uber.org/cadence/internal/internal_task_handlers.go:747
code.uber.internal/devexp/cadence-samples/vendor/go.uber.org/cadence/internal.(*workflowTaskHandlerImpl).ProcessWorkflowTask
    /Users/maxim/sqlcadence/src/code.uber.internal/devexp/cadence-samples/vendor/go.uber.org/cadence/internal/internal_task_handlers.go:631
code.uber.internal/devexp/cadence-samples/vendor/go.uber.org/cadence/internal.(*workflowTaskPoller).processWorkflowTask
    /Users/maxim/sqlcadence/src/code.uber.internal/devexp/cadence-samples/vendor/go.uber.org/cadence/internal/internal_task_pollers.go:236
code.uber.internal/devexp/cadence-samples/vendor/go.uber.org/cadence/internal.(*workflowTaskPoller).ProcessTask
    /Users/maxim/sqlcadence/src/code.uber.internal/devexp/cadence-samples/vendor/go.uber.org/cadence/internal/internal_task_pollers.go:208
code.uber.internal/devexp/cadence-samples/vendor/go.uber.org/cadence/internal.(*baseWorker).processTask
    /Users/maxim/sqlcadence/src/code.uber.internal/devexp/cadence-samples/vendor/go.uber.org/cadence/internal/internal_worker_base.go:293
1.560969966256382e+09   warn    internal/internal_task_pollers.go:375   Failed to process decision task.    {"build_hash": "unknown-build-hash", "hostname": "maxim-C02XD0AAJGH6", "runtime_env": "", "service_name": "cadence-samples", "zone": "", "Domain": "samples-domain", "TaskList": "cadence-samples-tasklist", "WorkerID": "6973@maxim-C02XD0AAJGH6@cadence-samples-tasklist", "WorkflowType": "code.uber.internal/devexp/cadence-samples/cmd/samples/recipes/helloworld.Workflow", "WorkflowID": "helloworld_6e42e4ef-f176-431c-b2e7-67a2b6b37229", "RunID": "546d497e-ca1e-4fd8-9651-a730b9c07a74", "error": "adding duplicate decision DecisionType: Marker, ID: MutableSideEffect_value-1, state=Created, isDone()=false, history=[Created]"}
1.560969966282509e+09   error   internal/internal_task_handlers.go:747  non-deterministic-error {"build_hash": "unknown-build-hash", "hostname": "maxim-C02XD0AAJGH6", "runtime_env": "", "service_name": "cadence-samples", "zone": "", "Domain": "samples-domain", "TaskList": "cadence-samples-tasklist", "WorkerID": "6973@maxim-C02XD0AAJGH6@cadence-samples-tasklist", "WorkflowType": "code.uber.internal/devexp/cadence-samples/cmd/samples/recipes/helloworld.Workflow", "WorkflowID": "helloworld_6e42e4ef-f176-431c-b2e7-67a2b6b37229", "RunID": "546d497e-ca1e-4fd8-9651-a730b9c07a74", "error": "adding duplicate decision DecisionType: Marker, ID: MutableSideEffect_value-1, state=Created, isDone()=false, history=[Created]"}
code.uber.internal/devexp/cadence-samples/vendor/go.uber.org/cadence/internal.(*workflowExecutionContextImpl).ProcessWorkflowTask
    /Users/maxim/sqlcadence/src/code.uber.internal/devexp/cadence-samples/vendor/go.uber.org/cadence/internal/internal_task_handlers.go:747
code.uber.internal/devexp/cadence-samples/vendor/go.uber.org/cadence/internal.(*workflowTaskHandlerImpl).ProcessWorkflowTask
    /Users/maxim/sqlcadence/src/code.uber.internal/devexp/cadence-samples/vendor/go.uber.org/cadence/internal/internal_task_handlers.go:631
code.uber.internal/devexp/cadence-samples/vendor/go.uber.org/cadence/internal.(*workflowTaskPoller).processWorkflowTask
    /Users/maxim/sqlcadence/src/code.uber.internal/devexp/cadence-samples/vendor/go.uber.org/cadence/internal/internal_task_pollers.go:236
code.uber.internal/devexp/cadence-samples/vendor/go.uber.org/cadence/internal.(*workflowTaskPoller).ProcessTask
    /Users/maxim/sqlcadence/src/code.uber.internal/devexp/cadence-samples/vendor/go.uber.org/cadence/internal/internal_task_pollers.go:208
code.uber.internal/devexp/cadence-samples/vendor/go.uber.org/cadence/internal.(*baseWorker).processTask
    /Users/maxim/sqlcadence/src/code.uber.internal/devexp/cadence-samples/vendor/go.uber.org/cadence/internal/internal_worker_base.go:293
1.56096996628255e+09    warn    internal/internal_task_pollers.go:375   Failed to process decision task.    {"build_hash": "unknown-build-hash", "hostname": "maxim-C02XD0AAJGH6", "runtime_env": "", "service_name": "cadence-samples", "zone": "", "Domain": "samples-domain", "TaskList": "cadence-samples-tasklist", "WorkerID": "6973@maxim-C02XD0AAJGH6@cadence-samples-tasklist", "WorkflowType": "code.uber.internal/devexp/cadence-samples/cmd/samples/recipes/helloworld.Workflow", "WorkflowID": "helloworld_6e42e4ef-f176-431c-b2e7-67a2b6b37229", "RunID": "546d497e-ca1e-4fd8-9651-a730b9c07a74", "error": "adding duplicate decision DecisionType: Marker, ID: MutableSideEffect_value-1, state=Created, isDone()=false, history=[Created]"}
jefflill commented 5 years ago

We ran into another somewhat related issue where we were unable to set a nil mutable side effect value via the GOLANG API.

FYI: I've decided not to use the MutableSideEffect() function for the .NET Cadence Client and am using local activities combined with a local dictionary to achieve (what I believe is) the same effect. I'm also been tweaking some of the terminology to hopefully make things easier to understand for new .NET workflow developers. I'm replacing the mutable side effect phrase and am going to call these workflow variables instead and am implementing a GetVariable()/SetVariable() pattern instead.

So, this issue no longer really matters for the .NET client.

We've made some other similar changes and after we get a first cut of this all working in a week or two I'd love to sit down with you folks and go over these to see what you think (e.g. do you hate the changes?).

mfateev commented 5 years ago

The use case of MutableSide effect is to be able to poll for some external state changes from the workflow code without recording a Marker on every check. When using local activity a marker is recorded on every invocation.

Would you explain how workflow variables are intended to be used?

Do you plan to move the Cadence .NET client out of the NEON repo into a stand alone repository?

jefflill commented 5 years ago

We're developing this in the open and will be releasing it as open source under the Apache 2.0 License. Here's a link to the readme for the overall repo:

https://github.com/nforgeio/neonKUBE

And more links inside to the .NET and GOLANG source:

https://github.com/nforgeio/neonKUBE/tree/master/Lib/Neon.Cadence https://github.com/nforgeio/neonKUBE/blob/master/Go/src/github.com/cadence-proxy/README.md

We totally misunderstood the mutable side effect use case and didn't realize these could be set externally, so I'll need to rethink this (and we'll probably care about this issue again).

jefflill commented 5 years ago

BTW: We're also located in Seattle.

mfateev commented 5 years ago

I did a quick look at the code and it looks like you are defining APIs which are very different from the Java Client. We would prefer for APIs be as much consistent as possible among languages.

I can come to your office to sync on the project. Contact me directly on Slack to schedule.