[WIP] Fix exception when Continue-as-New and Termination happens at the same time#901
[WIP] Fix exception when Continue-as-New and Termination happens at the same time#901davidmrdavid wants to merge 6 commits intomainfrom
Conversation
| { | ||
| ExecutionCompletedEvent executionCompletedEvent; | ||
| continuedAsNew = (completeOrchestratorAction.OrchestrationStatus == OrchestrationStatus.ContinuedAsNew); | ||
| continuedAsNew = (completeOrchestratorAction.OrchestrationStatus == OrchestrationStatus.ContinuedAsNew) && (runtimeState.OrchestrationStatus != OrchestrationStatus.Terminated); |
There was a problem hiding this comment.
Instead of making a change here, I think it would be better to make a change further upstream in OrchestrationRuntimeState.SetMarkerEvents. Instead of always throwing if we see that ExecutionCompletedEvent == null, we could add an additional check that allows transitioning from a ContinuedAsNew state to the Terminated state. I feel like this is a reasonable thing to allow since termination should be considered forceful.
I'd be fine going even further than this too. I don't see a strong reason to throw an exception if multiple completion events are found as long as we can make the expected outcome predictable. Ideally we'd simply log a warning if we encounter multiple completion events. I feel like failing with an exception is only useful when we don't have a reasonable way to procede.
|
Closing stale PR. Please reopen if still relevant. |
4 participants
A feature branch, for testing, containing the community contribution here: #888
This is a fix contributed by @fuifuiyong.
Creating this branch for creating a private package with which to test this, and to perform edits as necessary.