Fixing race condition in LeaserCheckpointer where it can fail with a ContainerAlreadyExists error #253
Conversation
This file contains 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
… the container being created. We can handle it easily if we just allow ContainerAlreadyExists to be considered successful.
richardpark-msft
force-pushed
the
eh-storage-409-bug
branch
from
7777ba7 to
7c9b9c7
Compare
|
/azp run Azure.azure-event-hubs-go |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
/azp run Azure.azure-event-hubs-go |
|
Azure Pipelines successfully started running 2 pipeline(s). |
jhendrixMSFT
approved these changes
Apr 25, 2022
… list. There's a bit in the API documentation that makes it seem like we coudl get an empty page _with_ a 'next' marker. This could mean we'd have to call _back_ intot he paging API in a loop. Rather than doing that I'll just grab the container info directly and check for 404.
|
/azp run Azure.azure-event-hubs-go |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
/azp run Azure.azure-event-hubs-go |
|
Azure Pipelines successfully started running 2 pipeline(s). |
catalinaperalta
approved these changes
Apr 26, 2022
|
/azp run Azure.azure-event-hubs-go |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
/azp run Azure.azure-event-hubs-go |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
/azp run Azure.azure-event-hubs-go |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
/azp run Azure.azure-event-hubs-go |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
/azp run Azure.azure-event-hubs-go |
|
Azure Pipelines successfully started running 2 pipeline(s). |
…rlying cancel function in a goroutine but we don't have to.
|
/azp run Azure.azure-event-hubs-go |
|
Azure Pipelines successfully started running 2 pipeline(s). |
|
btw, the live test run with the data race fix is here: I'm not sure why they're not being associated with the PR itself. |
jhendrixMSFT
approved these changes
Apr 28, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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's a slight race condition between checking the store exists and the container being created.
We can handle it easily if we just allow ContainerAlreadyExists to be considered successful. Also, since the storage tests were failing (unrelated to my change) in race detection I also fixed that as well.
Fixes #252
Fixes #225