peterxcli commented on code in PR #10023:
URL: https://github.com/apache/ozone/pull/10023#discussion_r3036582654
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyCreateRequest.java:
##########
@@ -497,20 +498,34 @@ protected void validateAtomicRewrite(OmKeyInfo dbKeyInfo,
KeyArgs keyArgs)
}
}
- if (keyArgs.hasExpectedETag()) {
- String expectedETag = keyArgs.getExpectedETag();
- if (dbKeyInfo == null) {
- throw new OMException("Key not found for If-Match",
- OMException.ResultCodes.KEY_NOT_FOUND);
- }
- if (!dbKeyInfo.hasEtag()) {
- throw new OMException("Key does not have an ETag",
- OMException.ResultCodes.ETAG_NOT_AVAILABLE);
- }
- if (!dbKeyInfo.isEtagEquals(expectedETag)) {
- throw new OMException("ETag mismatch",
- OMException.ResultCodes.ETAG_MISMATCH);
- }
+ }
Review Comment:
> My current worry is that if a single client sends a PutObject but retries
it again (e.g., due to some network restriction, etc.), the retry might fail.
We can try to look into this when checking our AWS S3 spec compatibility.
Thanks @ivandika3, this prompted me to review the [conditional put requests
in the AWS
spec](https://docs.aws.amazon.com/AmazonS3/latest/API/API_PutObject.html),
which mentions that both `If-Match` and `If-None-Match` requests can throw two
types of exceptions: `PreconditionFailed` (412) and
`ConditionalRequestConflict` (409). Applying this to our case,
`PreconditionFailed` should be thrown if the validation is violated when
running `createKeyRequest`, whereas `ConditionalRequestConflict` should be
thrown during `commitKeyRequest`.
I've raised a Jira ticket for this. BTW, the PR is also ready:
-
[https://issues.apache.org/jira/browse/HDDS-14968](https://issues.apache.org/jira/browse/HDDS-14968)
-
[https://github.com/apache/ozone/pull/10043](https://github.com/apache/ozone/pull/10043)
> My current worry is that if a single client sends a PutObject but retries
it again (e.g., due to some network restriction, etc.), the retry might fail.
I think upon a client retry, the execution goes through the `createKey` ->
`commitKey` flow again. This stores the latest `updateID` of the key in the
`openKeyTable` as `expectedDataGeneration`. Then, when it comes to the atomic
rewrite key commit, the stored `expectedDataGeneration` matches the current
`updateID` of the key in the `keyTable`.
So in conclusion, concurrent duplicate conditional requests might result in
one of them failing, but if the duplicate requests are dependent on each other
(i.e., causal), then it won't be a problem.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
