Fixing Azure Storage History/Instance Table Inconsistency #1253
Conversation
…ory-table-inconsistency
cgillum
left a comment
cgillum
left a comment
There was a problem hiding this comment.
I like the test you added! A couple comments:
| ["ExecutionId"] = executionId, | ||
| ["LastUpdatedTime"] = runtimeState.Events.Last().Timestamp, | ||
| ["RuntimeStatus"] = runtimeState.OrchestrationStatus.ToString(), | ||
| ["CompletedTime"] = runtimeState.Events.Last().Timestamp // do we want to do this as a rough proxy or DateTime.UtcNow? |
There was a problem hiding this comment.
How about runtimeState.CompletedTime?
One slightly quirky thing about this method is that there's an implicit assumption that the orchestration is complete, but technically somebody could call this for a non-completed orchestration. It might be worth either a) adding some comments indicating that this should only be called for completed orchestrations (maybe even having an explicit check for this) or b) making the implementation defensive so that it works regardless of the orchestration state. In that case, we'd have to check to confirm that the orchestration is complete before attempting to set the "CompletedTime" field.
There was a problem hiding this comment.
Good point, I went with option a) (comments + method name change + method does nothing if the orchestration is not in a terminal state)
…ory-table-inconsistency
sophiatev
deleted the
stevosyan/fixing-azure-storage-instance-history-table-inconsistency
branch
If a process ends right after the history table is updated but before the instance table is updated in a call to
AzureStorageOrchestrationService.CompleteTaskOrchestrationWorkItemAsyncfor a terminal orchestration, then storage is left in an inconsistent state. The instance table shows the orchestration as running while the history table shows the orchestration completed with anExecutionCompletedEventat the end. For non-terminal orchestrations, this is not a problem because the call to complete the next work item will reconcile these two tables. However, in the current implementation, the call toAzureStorageOrchestrationService.LockNextTaskOrchestrationWorkItemAsyncwill recognize that the orchestration is in a terminal state based on its history and simply discard the work item, meaning the instance table is never updated. To fix this, we add a check in the case that a work item is received for an orchestration in a terminal state to confirm that the instance table has been correctly updated. If not, it is updated, and any orphaned blobs deleted.This PR also adds an integration test for this scenario.
Fixes #1252