Only set SDKPriorityUpdateHandling if workflow update is being used

[Quinn-With-Two-Ns]

What was changed

Only set SDKPriorityUpdateHandling if workflow update is being used. Previously SDKPriorityUpdateHandling was set at the start of every workflow, this made it difficult to roll back the Go SDK to a previous version that did not understand SDKPriorityUpdateHandling without workflows getting stuck . Now rollback will only be blocked if a user was using workflow update.

Using workflow update in a workflow execution means:

• Registered an update handler
• Received any update requests

I also changed the type of an unknown SDK flag error to be treated as non deterministic to align with the behaviour of a missing workflow version call since SDK flags are basically internal SDK version calls.

Why was the flag originally set in all workflow executions

• So all new workflow executions would always use the new correct update behaviour.
• Because that is what we believed we needed to do when we added SDKFlagProtocolMessageCommand flag to handle the ordering of update messages and the workflow task completed event (where the SDK reads the flags) since this is another update related flag the same logic applies. On review I couldn't find a good reason why we needed to always have set the flag. I think the actual issue was how the SDK was reading the messages, which I have fixed.

Other options

We could not add a flag at all. This would be the simplest option, but would break all current users of update in the Go SDK. Since there are currently users activity using and paying for workflow update this option was not chosen.

Release Notes

We will need to call out this lack of roll back support if a user is using workflow update in the release notes

[cretz]

This makes sense to me.

On review I couldn't find a good reason why we needed to always have set the flag

Yeah if there is no code that explicitly takes another path when the flag is not set, then I think we're good, because we couldn't just remove pre-update code. I too would hope there is no behavior difference from a workflow started on a newer SDK and older SDK if it didn't do anything with updates.

[Sushisource]

I think this is reasonable. The approach Core takes is sort of the opposite.

All flags that Core knows about are written in the first non-replay WFT. But, also, no explicit checks are made for not-understood flags. If there is some unknown flag I just don't do anything, with the expectation that if it would cause some problem, we're going to run into it anyway.

In this case I think that results in the same behavior as what you now have. But, I'm wondering if it makes sense for us to be consistent across SDKs with whether or not we throw an error here. I don't know that it has to be.

I chose not to make unknown flags an error specifically because you might have a newer SDK do some processing and have it actually be totally fine to roll back because you didn't really use the flag, which you do by using the flag lazily. I found that the places where the flags got used became too intrusive into the code to reasonably make sense to be lazy in every spot, and that you'd end up with some other nondeterminism error anyways if you actually took new and incompatible behavior.

[Quinn-With-Two-Ns]

In the original PR to add SDK flags to the proto we said an unknown flags should error I believe.

Choosing to not error is also dangerous since if a user does roll back then rolls forward again you are in a very weird state where a flag was being used, but then stopped being used, but the SDK doesn't know it stopped being used. I agree we should standardize, i'll open a feature issue for this.

feature issue: temporalio/features#427

[Sushisource]

In the original PR to add SDK flags to the proto we said an unknown flags should error I believe.

Yeah I do pretty clearly say that right in the comment. Now I've got to go figure out what the reason was that I moved away from that in Core, because there definitely was a reason, and IIRC it had to do with the lazy usage getting out of control. Which I guess is really it - it's just that now that they're always written it'd be an automatic break to roll back.

[Quinn-With-Two-Ns]

Yeah if there is no code that explicitly takes another path when the flag is not set

Yeah there is no, no update code, that takes another path

I too would hope there is no behavior difference from a workflow started on a newer SDK and older SDK if it didn't do anything with updates.

There is one code path that is effected unfortunately. The consequence is that registering an update handler on a workflow started on a older SDK won't introduce non determinism, so almost a good thing in some ways.