Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
mfateev opened this issue Jun 19, 2019 · 5 comments
Labels

Comments

@mfateev
Copy link
Contributor

mfateev commented Jun 19, 2019

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
Copy link

jefflill commented Jun 21, 2019

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
Copy link
Contributor Author

mfateev commented Jun 21, 2019

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
Copy link

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
Copy link

BTW: We're also located in Seattle.

@mfateev
Copy link
Contributor Author

mfateev commented Jun 21, 2019

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.

@mfateev mfateev added the bug label Jun 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants