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

AzureBlobStorage Etag bug? #6908

Closed
OracPrime opened this issue Jan 22, 2021 · 2 comments
Closed

AzureBlobStorage Etag bug? #6908

OracPrime opened this issue Jan 22, 2021 · 2 comments
Labels
stale Issues with no activity for the past 6 months

Comments

@OracPrime
Copy link
Contributor

#3516 is related to this.

In the AzureBlobStorage, there is some inconsitency with Etags. In the WriteStateAsync, the BlobRequestConditions are set to this
new BlobRequestConditions { IfMatch = grainState.ETag != null ? new ETag(grainState.ETag) : (ETag?)null },
which I believe is correct (if Etag on what we're writing is null, don't have to match, overwrite anyway).

However in ClearStateAsync the code is this:
blob.DeleteIfExistsAsync(DeleteSnapshotsOption.None, conditions: new BlobRequestConditions { IfMatch = new ETag(grainState.ETag) })
which fails with a BlobRequestConditions not met exception. I believe the condition should match that in Write.

If @ReubenBond concurs, happy to submit a PR (which would also correct a copy and paste error in some of the logging)

@ReubenBond
Copy link
Member

ReubenBond commented Jan 22, 2021

Enough people have been tripped up by this that I think some change needs to happen.

What you're suggesting sounds good to me.

The code for WriteStateAsync does not look correct, though. I believe that if ETag is null, we should set IfNoneMatch instead of IfMatch, like so: new BlobRequestConditions { IfNoneMatch = new ETag(Constants.Wildcard) } } - that will prevent overwrites and maintain consistency in the case where two writers race to create a new blob. With the existing condition, both writers could "succeed", but the later one would clobber the earlier one's write without observing it (i.e, without acknowledging that it's been observed by specifying its etag), so it would be a lost write.

The Azure Storage SDK source code uses that same condition when overwrite is false in this method: https://github.com/Azure/azure-sdk-for-net/blob/e2fad94b95fa2a27a73af758e4e2c39b724c5949/sdk/storage/Azure.Storage.Blobs/src/BlockBlobClient.cs#L2235

richorama added a commit to richorama/orleans that referenced this issue Aug 6, 2021
richorama added a commit to richorama/orleans that referenced this issue Aug 6, 2021
dotnet#6908 adjust ETag options on Clear/Write
@ReubenBond
Copy link
Member

Fixed by #7187

@ghost ghost locked as resolved and limited conversation to collaborators Oct 3, 2021
@ghost ghost added the stale Issues with no activity for the past 6 months label Dec 7, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
stale Issues with no activity for the past 6 months
Projects
None yet
Development

No branches or pull requests

2 participants