Eager activity execution should be limited to 3 per workflow task completion by default

[bergundy]

In #946 we set MaxConcurrentActivityExecutionSize default to 3, instead we should have set the max number of eager activities requested per workflow task completion to 3.

I wish we hadn't exposed MaxConcurrentActivityExecutionSize and DisableEagerActivities, next time when we add new options we should mark them experimental, but in this case I think it might be okay to remove these, risking a breaking change.

In other SDKs, we've allowed disabling eager activity execution per activity invocation via activity options and have not yet exposed a way to configure the number of eager activity requests, it's hardcoded to 3.
I would like us to expose a way to configure the number of eager activity requests eventually but would rather wait until we know more about the implications of turning this on before exposing so we can document how to tune that parameter.
Since eager activities are still disabled by default server side that should be enough for now.

[cretz]

In #946 we set MaxConcurrentActivityExecutionSize default to 3, instead we should have set the max number of eager activities requested per workflow task completion to 3.

It is MaxConcurrentEagerActivityExecutionSize and I still it is best IMO. The whole point of having this throttle was to not overload the worker or unexpected consequences of doing thousands of these things. If you made this per workflow, you're only stopping the rare case of ExecuteActivity called 3+ times before either is waited. That's hardly ever. In the meantime, I'd start 1000 workflows that each start an activity, and then they would all be eager. It would make the option useless.

We have MaxConcurrent settings for activities, local activities, sessions, pollers, tasks, and now eager activities. These are consistent and across the worker. It'd be inconsistent and almost worthless to do per workflow or per workflow task.

I wish we hadn't exposed MaxConcurrentActivityExecutionSize and DisableEagerActivities

Why not? These are perfectly valid settings and are inline with other settings this SDK uses. They both have worker-level value. I'd like to see these options in every SDK. Everyone should be able to set max concurrent and disable eager activities at the worker level. Remember however that 0 in Go is "unset" which means "default" and we don't want to default to disabled. That's why there are two settings.

next time when we add new options we should mark them experimental

We don't usually do this with new option additions, e.g. https://typescript.temporal.io/api/interfaces/common.ActivityOptions#alloweagerdispatch. Are you saying all SDKs should have marked them experimental, or are you saying Go uniquely did something wrong here?

but in this case I think it might be okay to remove these, risking a breaking change.

Concur, these are unused and such a breaking change to these options is fine. Can you clarify exactly what they should change to?

I would like us to expose a way to configure the number of eager activity requests eventually but would rather wait until we know more about the implications of turning this on before exposing so we can document how to tune that parameter.

I remember being asked for this explicitly when I developed this feature, but maybe I wasn't supposed to make it user overridable? It would be a deviation from the norm with all the other configuration options we create on new features like this. I think Go was the first SDK to offer a user-facing API related to eager activities so there wasn't a model to follow that I recall. Why does how we tune the default affect whether the user-overridable option exists? We can remove and hardcode if we must.

I could quite easily argue that eager activity should never be a per-activity option. A workflow author does not need to concern themselves with such a thing.

[cretz]

After discussion, we just need to remove the "3" default from max concurrent eager (so leave that as unlimited) and hardcode a new setting of max 3 per workflow task.