uber-go / cadence-client

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

MaxConcurrentActivityExecutionSize is not honored when default value for TaskListActivitiesPerSecond is used. #1310

Open petergardfjall opened 4 months ago

petergardfjall commented 4 months ago

Describe the bug We recently discovered that Cadence does not honor MaxConcurrentActivityExecutionSize when the default value for TaskListActivitiesPerSecond is used. Notably it seemed that whenever we started a workflow, the worker would start about twice the amount of activities that we had configured. Not good, since we are trying to protect a downstream resource from being oversubscribed.

After some debugging I think I found the offending code in internal_worker.go#L1025 and the creation of a "locally dispatched activity worker" (originally introduced in https://github.com/uber-go/cadence-client/pull/1029).

There is a workaround in setting TaskListActivitiesPerSecond to anything but the default, since that prevents the creation of the "extra" activity worker. Still this is surprising, incorrect and undocumented behavior.

To Reproduce Is the issue reproducible?

Steps to reproduce the behavior:

Expected behavior I expected the first activity to start and execute until completion before the second activity was allowed to start (given the worker setting: MaxConcurrentActivityExecutionSize: 1).