Apply modern event loop algorithm with new SDK flag

[cretz]

What was changed

(description as mostly taken from a comment above the SDK flag)

Before this flag, workflow async logic did the following (in no particular order):

• Reorder jobs itself
• Schedule primary workflow method when the initialize job was seen
• Tick the event loop after each job processed
• Check all conditions at once and resolve all that are satisfied

We have since learned that all of these are wrong in one way or another. So if this flag is set on the first activation/task of a workflow, the following logic now applies respectively:

• Leave Core's job ordering as is
• Do not schedule the primary workflow method until after all jobs have been applied and it's not already present.
• Tick the event loop only once after everything applied
• Only resolve the first condition that is satisfied and re-run the entire event loop

We know this solves at least #427 where all-handlers-finished was improperly assumed true too early.

Checklist

1. Closes [Bug] Replay after UnhandledCommand can cause main workflow body to complete before signals are handled #427
2. Closes [Feature Request] Align activation job application with TS changes #375

[Sushisource]

Suggested change

	// We only run the scheduler after each job in legacy event loop logic
	// We run the scheduler after each job in legacy event loop logic

[cretz]

Meant to say "we only do X in legacy" to clarify we don't do it in non-legacy

[THardy98]

Not blocking, small comments/changes.

and just to verify my understanding:
Check all conditions at once and resolve all that are satisfied.
->
Only resolve the first condition that is satisfied and re-run the entire event loop.

we're doing this so that we can give subsequent signals (i.e. if multiple signals are sent) a chance to be handled, in the case that resolved conditions from the first signal cause the workflow to complete/terminate/finish

[cretz]

and just to verify my understanding:
Check all conditions at once and resolve all that are satisfied.
->
Only resolve the first condition that is satisfied and re-run the entire event loop.

we're doing this so that we can give subsequent signals (i.e. if multiple signals are sent) a chance to be handled, in the case that resolved conditions from the first signal cause the workflow to complete/terminate/finish

There's a few reasons why you don't want to tick the event loop after each job. I should have linked this to #375 which I am doing now, but also see temporalio/sdk-python#606 and we have had internal discussions on this. Basically waking up the event loop mid-job-apply causes conditions and other things to be confusingly applied too early. It becomes hard to reason about when a coroutine in a handler may run. This also aligns the event loop with other SDKs (in addition to fixing a few other things).

[THardy98]

There's a few reasons why you don't want to tick the event loop after each job. I should have linked this to #375 which I am doing now, but also see temporalio/sdk-python#606 and we have had internal discussions on this. Basically waking up the event loop mid-job-apply causes conditions and other things to be confusingly applied too early. It becomes hard to reason about when a coroutine in a handler may run. This also aligns the event loop with other SDKs (in addition to fixing a few other things).

Yeah I found these issues trying to gather some more context on this, very helpful. In effect, this boils down to predictable handler behavior (at least wrt to the principle bug fix). Thanks for the explanation :)

[vchirikov]

Hi @cretz , when it will be released?

[cretz]

Next .NET SDK release, but we do not have a firm date at this time, should not be too long