-
Notifications
You must be signed in to change notification settings - Fork 321
Don't fail orchestrations on missing blobs from previous executions #1189
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
Merged
Merged
Changes from 9 commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
4c099e0
Log a warning on failure to download or decompress a blob
AnatoliB 045da67
Stop loading history on missing blobs
AnatoliB 57be86b
Extract TryDecompressLargeEntityPropertiesAsync method
AnatoliB 59fb463
Add comments to TryDecompressLargeEntityPropertiesAsync
AnatoliB d9391fb
Rename a parameter for consistency
AnatoliB 50e2c24
Fixed comment
AnatoliB 20cfbed
Add a warning to TryDecompressLargeEntityPropertiesAsync
AnatoliB 722a883
Restore an accidentally removed comment
AnatoliB 3dcd4e5
Add a comment in GetHistoryEventsAsync
AnatoliB 9a74e40
Skip history loading if sentinel execution ID doesn't match
AnatoliB 32851e1
Revert missing blob handling
AnatoliB 9ae4b1c
Allow expectedExecutionId == null
AnatoliB 8d640a9
Move sentinel initialization
AnatoliB 1ed474e
Add a comment
AnatoliB 440dce1
Fix comment
AnatoliB 8c24eea
Prefer sentinel ExecutionId over the first history entry ExecutionId
AnatoliB File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit confused about how/why we'd find outdated history at this point. Shouldn't we have filtered out all the outdated history events via the check on line 173 (where we check the
ExecutionIdvalue)?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly, this is the source of our troubles. Line 173 doesn't perform proper filtering because
executionIdcontains an old execution ID, which happens because we passed an oldexpectedExecutionIdtoGetHistoryEntitiesResponseInfoAsync, soresults.Entities[0]also belongs to the outdated history, and line 173 filters out everything unrelated to the old execution. Eventually, if we don't encounter missing blobs, we hit this line:durabletask/src/DurableTask.AzureStorage/AzureStorageOrchestrationService.cs
Line 1072 in 3bc255c