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

Keep orchestrator actions sorted, to avoid out-of-order events in OrchestrationRuntimeState #680

Merged
merged 1 commit into from
Mar 11, 2022

Conversation

sebastianburckhardt
Copy link
Collaborator

When debugging a failing test in Connor's integration tests Azure/azure-functions-durable-extension#1979 today I noticed an oddity: on completed OrchestrationWorkItems, any events sent by the orchestration appeared in reverse order in the runtime state. I tracked this down to TaskOrchestrationDispatcher, which returns the orchestrator actions as follows:

public IEnumerable<OrchestratorAction> OrchestratorActions => this.orchestratorActionsMap.Values;

but this is actually nondeterministic (and happened to be in reverse order for me) because the map is just a plain dictionary:

this.orchestratorActionsMap = new Dictionary<int, OrchestratorAction>();

I am a bit surprised that this has not been noticed before and has not caused more issues. The particular test that detected this was an entity test, which was immune to this problem while I used the AzureStorage backend (thanks to the MessageSorter) but as soon as I used Netherite the problem became apparent.

The fix is trivial, use a SortedDictionary instead.

@cgillum
Copy link
Member

cgillum commented Feb 17, 2022

I'm not sure I understand the issue. I wouldn't expect the list of actions in a single execution step to require any particular ordering. Is this issue specific to entities?

@sebastianburckhardt
Copy link
Collaborator Author

It is definitely an issue for entities (in this case, on orchestrator sending signals to an entity), because our programming model guarantees in-order delivery of signals.

Whether or not it is an issue for orchestrators when no entities are involved is not super clear to me. It may not cause malfunctions we are aware of, but it is certainly surprising (e.g. two executions of a deterministic orchestrator on identical input could produce a different OrchestrationRuntimeState) and I would prefer that we keep things nicely ordered and deterministic. Especially since it doesn't cost anything.

@sebastianburckhardt sebastianburckhardt merged commit 21f47bd into main Mar 11, 2022
@sebastianburckhardt sebastianburckhardt deleted the sebastian/pr-fix-sent-event-order branch March 11, 2022 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants