You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "peterxcli (via GitHub)" <gi...@apache.org> on 2026/05/04 15:07:04 UTC
[PR] HDDS-15167 Concurrent Conflict Detection for Conditional MPU Complete [ozone]
peterxcli opened a new pull request, #10183:
URL: https://github.com/apache/ozone/pull/10183
## What changes were proposed in this pull request?
If two or more independent clients send the exact same Complete Multipart Upload request at the same time, only one client should receive a successful response and the others should get a 409. That way, they know they must re-read the object, re-upload the parts if necessary, and then complete the MPU again using the latest ETag they observed.
ref: [docs.aws.amazon.com/AmazonS3/latest/API/API_CompleteMultipartUpload.html](https://docs.aws.amazon.com/AmazonS3/latest/API/API_CompleteMultipartUpload.html)
[If-Match](https://docs.aws.amazon.com/AmazonS3/latest/API/API_CompleteMultipartUpload.html#API_CompleteMultipartUpload_RequestSyntax)
> Uploads the object only if the ETag (entity tag) value provided during the WRITE operation matches the ETag of the object in S3. If the ETag values do not match, the operation returns a 412 Precondition Failed error.
If a conflicting operation occurs during the upload S3 returns a 409 ConditionalRequestConflict response. On a 409 failure you should fetch the object's ETag, re-initiate the multipart upload with CreateMultipartUpload, and re-upload each part.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-15167
## How was this patch tested?
UT/IT
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15167. Concurrent Conflict Detection for Conditional MPU Complete [ozone]
Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli commented on PR #10183:
URL: https://github.com/apache/ozone/pull/10183#issuecomment-4377179824
And I think we should apply this concept to conditional put, too.
- CreateKey.preExecute -> **admission** preparation
- CreateKey.validateAndUpdateCache -> serialized **admission**
- CommitKey.preExecute -> commit-request **admission** preparation / best-effort precondition check
- CommitKey.validateAndUpdateCache -> serialized **commit** revalidation
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15167. Broaden commit conflict detection for conditional writes [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #10183:
URL: https://github.com/apache/ozone/pull/10183#discussion_r4236619004
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java:
##########
@@ -1439,24 +1489,28 @@ protected void validateIfMatchETag(KeyArgs keyArgs, OmKeyInfo dbKeyInfo)
* Validates If-Match ETag condition and converts it to expectedDataGeneration.
* <p>
* This method checks if the existing key's ETag matches the expected ETag.
- * If it matches, the ETag condition is converted to a generation-based condition
- * for atomic commit validation in two-phase operations (CreateKey → CommitKey).
+ * If it matches, the ETag condition is converted to a generation-based
+ * condition for commit-time atomic rewrite validation.
*
* @param keyArgs the key arguments containing expected ETag
* @param dbKeyInfo the existing key info from the database
+ * @param auditMap the audit map to update with the resolved generation
* @return updated KeyArgs with expectedDataGeneration set (if ETag matched)
* @throws OMException if validation fails
*/
- protected KeyArgs validateAndRewriteIfMatchAsExpectedGeneration(
- KeyArgs keyArgs, OmKeyInfo dbKeyInfo) throws OMException {
+ private KeyArgs validateAndResolveIfMatchAsExpectedGeneration(
Review Comment:
Can we rename this function name to validateEtagAndSetExpectedGeneration?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15167. Broaden commit conflict detection for conditional writes [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #10183:
URL: https://github.com/apache/ozone/pull/10183#issuecomment-5040407095
This PR has been marked as stale due to 21 days of inactivity. Please comment or remove the stale label to keep it open. Otherwise, it will be automatically closed in 7 days.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15167. Pre-detect commit conflict for conditional writes in OM request's preExecute [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on PR #10183:
URL: https://github.com/apache/ozone/pull/10183#issuecomment-6094629296
@peterxcli , thanks for rebase the patch. I have left some inline comments.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15167. Broaden commit conflict detection for conditional writes [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on PR #10183:
URL: https://github.com/apache/ozone/pull/10183#issuecomment-4786331866
> And I think we should apply this concept to conditional put, too.
>
> - CreateKey.preExecute -> admission preparation
> - CreateKey.validateAndUpdateCache -> serialized admission
> - CommitKey.preExecute -> commit-request admission preparation / best-effort precondition check
> - CommitKey.validateAndUpdateCache -> serialized commit revalidation
@peterxcli Thanks for the concept, I agree. We can do check on the preExecute to reduce the number of transactions, but we should also do a check on validateAndUpdateCache for correctness.
Let me review this soon.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15167. Pre-detect commit conflict for conditional writes in OM request's preExecute [ozone]
Posted by "github-actions[bot] (via GitHub) via issues" <is...@ozone.apache.org>.
github-actions[bot] commented on PR #10183:
URL: https://github.com/apache/ozone/pull/10183#issuecomment-6550934556
This PR has been marked as stale due to 21 days of inactivity. Please comment or remove the stale label to keep it open. Otherwise, it will be automatically closed in 7 days.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15167. Concurrent Conflict Detection for Conditional MPU Complete [ozone]
Posted by "chungen0126 (via GitHub)" <gi...@apache.org>.
chungen0126 commented on code in PR #10183:
URL: https://github.com/apache/ozone/pull/10183#discussion_r3186222852
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadCompleteRequest.java:
##########
@@ -409,6 +404,40 @@ protected OMClientResponse getOmClientResponse(String multipartKey,
getBucketLayout(), omBucketInfo, bucketId);
}
+ private KeyArgs resolveConditionalWrite(
+ OMMetadataManager omMetadataManager, KeyArgs keyArgs)
+ throws IOException {
+ if (!keyArgs.hasExpectedETag()
+ && !keyArgs.hasExpectedDataGeneration()) {
+ return keyArgs;
+ }
+
+ String multipartKey = omMetadataManager.getMultipartKey(
+ keyArgs.getVolumeName(),
+ keyArgs.getBucketName(), keyArgs.getKeyName(),
+ keyArgs.getMultipartUploadID());
+ if (omMetadataManager.getMultipartInfoTable().get(multipartKey) == null) {
+ throw new OMException(
+ failureMessage(keyArgs.getVolumeName(), keyArgs.getBucketName(),
+ keyArgs.getKeyName()),
+ OMException.ResultCodes.NO_SUCH_MULTIPART_UPLOAD_ERROR);
+ }
+
+ OmKeyInfo existingKeyInfo = getExistingKeyInfo(
+ omMetadataManager, keyArgs);
Review Comment:
Calling rocksDB inside `preExecute` is not a big problem. We also check ACLs inside `preExecute`, it also calls rocksDB.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15167. Concurrent Conflict Detection for Conditional MPU Complete [ozone]
Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli commented on code in PR #10183:
URL: https://github.com/apache/ozone/pull/10183#discussion_r3186398479
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadCompleteRequest.java:
##########
@@ -409,6 +404,40 @@ protected OMClientResponse getOmClientResponse(String multipartKey,
getBucketLayout(), omBucketInfo, bucketId);
}
+ private KeyArgs resolveConditionalWrite(
+ OMMetadataManager omMetadataManager, KeyArgs keyArgs)
+ throws IOException {
+ if (!keyArgs.hasExpectedETag()
+ && !keyArgs.hasExpectedDataGeneration()) {
+ return keyArgs;
+ }
+
+ String multipartKey = omMetadataManager.getMultipartKey(
+ keyArgs.getVolumeName(),
+ keyArgs.getBucketName(), keyArgs.getKeyName(),
+ keyArgs.getMultipartUploadID());
+ if (omMetadataManager.getMultipartInfoTable().get(multipartKey) == null) {
+ throw new OMException(
+ failureMessage(keyArgs.getVolumeName(), keyArgs.getBucketName(),
+ keyArgs.getKeyName()),
+ OMException.ResultCodes.NO_SUCH_MULTIPART_UPLOAD_ERROR);
+ }
+
+ OmKeyInfo existingKeyInfo = getExistingKeyInfo(
+ omMetadataManager, keyArgs);
Review Comment:
oh, great.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15167. Pre-detect commit conflict for conditional writes in OM request's preExecute [ozone]
Posted by "github-actions[bot] (via GitHub) via issues" <is...@ozone.apache.org>.
github-actions[bot] commented on PR #10183:
URL: https://github.com/apache/ozone/pull/10183#issuecomment-6729052767
Thank you for your contribution. This PR is being closed due to inactivity. Please contact a maintainer if you would like to reopen it.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15167. Concurrent Conflict Detection for Conditional MPU Complete [ozone]
Posted by "chungen0126 (via GitHub)" <gi...@apache.org>.
chungen0126 commented on code in PR #10183:
URL: https://github.com/apache/ozone/pull/10183#discussion_r3183819413
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java:
##########
@@ -1308,46 +1308,153 @@ protected void validateEncryptionKeyInfo(OmBucketInfo bucketInfo, KeyArgs keyArg
}
/**
- * Validates atomic rewrite conditions for conditional writes.
- * <p>
- * For If-None-Match: * (expectedDataGeneration = EXPECTED_GEN_CREATE_IF_NOT_EXISTS),
- * the key must NOT exist.
- * <p>
- * For If-Match with a specific generation, the key must exist with matching updateID.
+ * Resolves conditional write state at request admission time.
*
- * @param dbKeyInfo the existing key info from the database (null if key doesn't exist)
- * @param keyArgs the key arguments containing expected generation
- * @throws OMException if validation fails
+ * This validates expected generations as admission-time preconditions and
+ * rewrites matching ETags into expected generations for commit-time checks.
*/
- protected void validateAtomicRewrite(OmKeyInfo dbKeyInfo, KeyArgs keyArgs)
- throws OMException {
+ protected KeyArgs resolveConditionalWriteAtAdmission(OmKeyInfo dbKeyInfo,
+ KeyArgs keyArgs, Map<String, String> auditMap) throws OMException {
if (keyArgs.hasExpectedDataGeneration()) {
- long expectedGen = keyArgs.getExpectedDataGeneration();
- // If expectedGen is EXPECTED_GEN_CREATE_IF_NOT_EXISTS, it means the key MUST NOT exist (If-None-Match)
- if (expectedGen == OzoneConsts.EXPECTED_GEN_CREATE_IF_NOT_EXISTS) {
- if (dbKeyInfo != null) {
- throw new OMException("Key already exists",
- OMException.ResultCodes.KEY_ALREADY_EXISTS);
- }
- } else {
- // If a key does not exist, or if it exists but the updateID do not match, then fail this request.
- if (dbKeyInfo == null) {
- throw new OMException("Key not found during expected rewrite",
- OMException.ResultCodes.KEY_NOT_FOUND);
- }
- if (dbKeyInfo.getUpdateID() != expectedGen) {
- throw new OMException("Generation mismatch during expected rewrite",
- OMException.ResultCodes.KEY_NOT_FOUND);
- }
+ validateAtomicRewriteAtAdmission(dbKeyInfo, keyArgs, auditMap);
+ return keyArgs;
+ }
+
+ KeyArgs resolvedKeyArgs = validateAndRewriteIfMatchAsExpectedGeneration(
+ keyArgs, dbKeyInfo);
+ if (resolvedKeyArgs.hasExpectedDataGeneration()) {
+ addRewriteGenerationToAuditMap(
+ resolvedKeyArgs.getExpectedDataGeneration(), auditMap);
+ }
+ return resolvedKeyArgs;
+ }
+
+ /**
+ * Validates the condition at request admission time.
+ *
+ * This returns precondition-style errors because the client condition is
+ * already false before the request is admitted for a serialized commit.
+ */
+ protected void validateAtomicRewriteAtAdmission(OmKeyInfo dbKeyInfo,
+ KeyArgs keyArgs, Map<String, String> auditMap) throws OMException {
+ if (!keyArgs.hasExpectedDataGeneration()) {
+ return;
+ }
+ validateExpectedDataGeneration(dbKeyInfo,
+ keyArgs.getExpectedDataGeneration(), auditMap,
+ AtomicRewritePhase.ADMISSION);
+ }
+
+ /**
+ * Validates an already admitted condition at serialized commit time.
+ *
+ * Mismatches here mean the admission snapshot became stale, so callers should
+ * surface these as atomic write conflicts.
+ */
+ protected void validateAtomicRewriteAtCommit(OmKeyInfo existing,
+ OmKeyInfo toCommit, Map<String, String> auditMap) throws OMException {
+ validateAtomicRewriteAtCommit(existing,
+ toCommit.getExpectedDataGeneration(), auditMap);
+ }
+
+ /**
+ * Validates an already admitted condition at serialized commit time.
+ *
+ * This form is for callers that must check the admitted generation before
+ * building the final key info to commit.
+ */
+ protected void validateAtomicRewriteAtCommit(OmKeyInfo existing,
+ Long expectedGen, Map<String, String> auditMap) throws OMException {
+ if (expectedGen == null) {
+ return;
+ }
+ validateExpectedDataGeneration(existing, expectedGen, auditMap,
+ AtomicRewritePhase.COMMIT);
+ }
+
+ private void validateExpectedDataGeneration(OmKeyInfo existing,
Review Comment:
I feel like splitting the validation logic into these three methods might be a bit of over-engineering. Could we simplify it?
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java:
##########
@@ -1308,46 +1308,153 @@ protected void validateEncryptionKeyInfo(OmBucketInfo bucketInfo, KeyArgs keyArg
}
/**
- * Validates atomic rewrite conditions for conditional writes.
- * <p>
- * For If-None-Match: * (expectedDataGeneration = EXPECTED_GEN_CREATE_IF_NOT_EXISTS),
- * the key must NOT exist.
- * <p>
- * For If-Match with a specific generation, the key must exist with matching updateID.
+ * Resolves conditional write state at request admission time.
*
- * @param dbKeyInfo the existing key info from the database (null if key doesn't exist)
- * @param keyArgs the key arguments containing expected generation
- * @throws OMException if validation fails
+ * This validates expected generations as admission-time preconditions and
+ * rewrites matching ETags into expected generations for commit-time checks.
*/
- protected void validateAtomicRewrite(OmKeyInfo dbKeyInfo, KeyArgs keyArgs)
- throws OMException {
+ protected KeyArgs resolveConditionalWriteAtAdmission(OmKeyInfo dbKeyInfo,
+ KeyArgs keyArgs, Map<String, String> auditMap) throws OMException {
if (keyArgs.hasExpectedDataGeneration()) {
- long expectedGen = keyArgs.getExpectedDataGeneration();
- // If expectedGen is EXPECTED_GEN_CREATE_IF_NOT_EXISTS, it means the key MUST NOT exist (If-None-Match)
- if (expectedGen == OzoneConsts.EXPECTED_GEN_CREATE_IF_NOT_EXISTS) {
- if (dbKeyInfo != null) {
- throw new OMException("Key already exists",
- OMException.ResultCodes.KEY_ALREADY_EXISTS);
- }
- } else {
- // If a key does not exist, or if it exists but the updateID do not match, then fail this request.
- if (dbKeyInfo == null) {
- throw new OMException("Key not found during expected rewrite",
- OMException.ResultCodes.KEY_NOT_FOUND);
- }
- if (dbKeyInfo.getUpdateID() != expectedGen) {
- throw new OMException("Generation mismatch during expected rewrite",
- OMException.ResultCodes.KEY_NOT_FOUND);
- }
+ validateAtomicRewriteAtAdmission(dbKeyInfo, keyArgs, auditMap);
+ return keyArgs;
+ }
+
+ KeyArgs resolvedKeyArgs = validateAndRewriteIfMatchAsExpectedGeneration(
+ keyArgs, dbKeyInfo);
+ if (resolvedKeyArgs.hasExpectedDataGeneration()) {
+ addRewriteGenerationToAuditMap(
+ resolvedKeyArgs.getExpectedDataGeneration(), auditMap);
+ }
Review Comment:
A few quick suggestions to streamline the logic:
1. Could we make `validateAndRewriteIfMatchAsExpectedGeneration` private if it's not used elsewhere?
2. The `keyArgs.hasExpectedDataGeneration()` check inside `validateAndRewriteIfMatchAsExpectedGeneration` is redundant, as the caller already filters it out.
3. Consider moving `addRewriteGenerationToAuditMap` into `validateAndRewriteIfMatchAsExpectedGeneration`. This will make the logic much cleaner.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15167. Broaden commit conflict detection for conditional writes [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #10183:
URL: https://github.com/apache/ozone/pull/10183#discussion_r4236662060
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java:
##########
@@ -1369,46 +1369,96 @@ protected void validateEncryptionKeyInfo(OmBucketInfo bucketInfo, KeyArgs keyArg
}
/**
- * Validates atomic rewrite conditions for conditional writes.
- * <p>
- * For If-None-Match: * (expectedDataGeneration = EXPECTED_GEN_CREATE_IF_NOT_EXISTS),
- * the key must NOT exist.
- * <p>
- * For If-Match with a specific generation, the key must exist with matching updateID.
+ * Resolves conditional write state at request admission time.
*
- * @param dbKeyInfo the existing key info from the database (null if key doesn't exist)
- * @param keyArgs the key arguments containing expected generation
- * @throws OMException if validation fails
+ * This validates expected generations as admission-time preconditions and
+ * rewrites matching ETags into expected generations for commit-time checks.
*/
- protected void validateAtomicRewrite(OmKeyInfo dbKeyInfo, KeyArgs keyArgs)
- throws OMException {
+ protected KeyArgs resolveConditionalWriteAtAdmission(OmKeyInfo dbKeyInfo,
Review Comment:
This resolveConditionalWriteAtAdmission is used in both pre-execute and validateAndUpdateCache, so AtAdmission seem not accurate.
BTW, OMKeyCreateRequest.preExecute doesn't call this resolveConditionalWriteAtAdmission, is it on purpose?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15167. Broaden commit conflict detection for conditional writes [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #10183:
URL: https://github.com/apache/ozone/pull/10183#discussion_r4236641145
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java:
##########
@@ -1369,46 +1369,96 @@ protected void validateEncryptionKeyInfo(OmBucketInfo bucketInfo, KeyArgs keyArg
}
/**
- * Validates atomic rewrite conditions for conditional writes.
- * <p>
- * For If-None-Match: * (expectedDataGeneration = EXPECTED_GEN_CREATE_IF_NOT_EXISTS),
- * the key must NOT exist.
- * <p>
- * For If-Match with a specific generation, the key must exist with matching updateID.
+ * Resolves conditional write state at request admission time.
*
- * @param dbKeyInfo the existing key info from the database (null if key doesn't exist)
- * @param keyArgs the key arguments containing expected generation
- * @throws OMException if validation fails
+ * This validates expected generations as admission-time preconditions and
+ * rewrites matching ETags into expected generations for commit-time checks.
*/
- protected void validateAtomicRewrite(OmKeyInfo dbKeyInfo, KeyArgs keyArgs)
- throws OMException {
+ protected KeyArgs resolveConditionalWriteAtAdmission(OmKeyInfo dbKeyInfo,
+ KeyArgs keyArgs, Map<String, String> auditMap) throws OMException {
if (keyArgs.hasExpectedDataGeneration()) {
- long expectedGen = keyArgs.getExpectedDataGeneration();
- // If expectedGen is EXPECTED_GEN_CREATE_IF_NOT_EXISTS, it means the key MUST NOT exist (If-None-Match)
- if (expectedGen == OzoneConsts.EXPECTED_GEN_CREATE_IF_ABSENT) {
- if (dbKeyInfo != null) {
- throw new OMException("Key already exists",
- OMException.ResultCodes.KEY_ALREADY_EXISTS);
- }
- } else {
- // If a key does not exist, or if it exists but the updateID do not match, then fail this request.
- if (dbKeyInfo == null) {
- throw new OMException("Key not found during expected rewrite",
- OMException.ResultCodes.KEY_NOT_FOUND);
- }
- if (dbKeyInfo.getUpdateID() != expectedGen) {
- throw new OMException("Generation mismatch during expected rewrite",
- OMException.ResultCodes.KEY_NOT_FOUND);
- }
+ validateAtomicRewriteAtAdmission(dbKeyInfo, keyArgs.getExpectedDataGeneration(), auditMap);
+ return keyArgs;
+ }
+
+ return validateAndResolveIfMatchAsExpectedGeneration(keyArgs, dbKeyInfo, auditMap);
+ }
+
+ /**
+ * Validates the condition at request admission time.
+ *
+ * This form is for callers that resolve the admitted generation from
+ * persisted open-key state rather than from request key arguments.
+ */
+ protected void validateAtomicRewriteAtAdmission(OmKeyInfo dbKeyInfo,
Review Comment:
Can we rename this function to validateExpectedGenAtAdmission, and next function to validateExpectedGenAtCommit, since they are used not only in the atomic rewrite case, but also create-if-not-exists case?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15167. Pre-detect commit conflict for conditional writes in OM request's preExecute [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #10183:
URL: https://github.com/apache/ozone/pull/10183#discussion_r4236692316
##########
hadoop-hdds/docs/content/design/s3-conditional-requests.md:
##########
@@ -105,8 +105,8 @@ temporary part state.
| | | |
|---|---|---|
|**Header**|**Meaning**|**Failure result**|
-|`If-None-Match: "*"`|Complete the MPU only if no committed object currently exists at the destination key.|`412 Precondition Failed` if the destination key already exists.|
-|`If-Match: "<etag>"`|Complete the MPU only if the current committed object exists and its ETag matches.|`412 Precondition Failed` if the object is missing or the ETag does not match.|
+|`If-None-Match: "*"`|Complete the MPU only if no committed object currently exists at the destination key.|`412 Precondition Failed` if the destination key already exists when the request is admitted; `409 ConditionalRequestConflict` if a conflicting write wins before the admitted complete request commits.|
Review Comment:
Does "### AWS S3 Conditional Write Specification" need be updated too?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15167. Broaden commit conflict detection for conditional writes [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #10183:
URL: https://github.com/apache/ozone/pull/10183#issuecomment-5111216374
Thank you for your contribution. This PR is being closed due to inactivity. Please contact a maintainer if you would like to reopen it.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15167. Concurrent Conflict Detection for Conditional MPU Complete [ozone]
Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli commented on PR #10183:
URL: https://github.com/apache/ozone/pull/10183#issuecomment-4372734056
cc @YutaLin
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15167. Broaden commit conflict detection for conditional writes [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #10183:
URL: https://github.com/apache/ozone/pull/10183#discussion_r4236557127
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java:
##########
@@ -1369,46 +1369,96 @@ protected void validateEncryptionKeyInfo(OmBucketInfo bucketInfo, KeyArgs keyArg
}
/**
- * Validates atomic rewrite conditions for conditional writes.
- * <p>
- * For If-None-Match: * (expectedDataGeneration = EXPECTED_GEN_CREATE_IF_NOT_EXISTS),
- * the key must NOT exist.
- * <p>
- * For If-Match with a specific generation, the key must exist with matching updateID.
+ * Resolves conditional write state at request admission time.
*
- * @param dbKeyInfo the existing key info from the database (null if key doesn't exist)
- * @param keyArgs the key arguments containing expected generation
- * @throws OMException if validation fails
+ * This validates expected generations as admission-time preconditions and
+ * rewrites matching ETags into expected generations for commit-time checks.
*/
- protected void validateAtomicRewrite(OmKeyInfo dbKeyInfo, KeyArgs keyArgs)
- throws OMException {
+ protected KeyArgs resolveConditionalWriteAtAdmission(OmKeyInfo dbKeyInfo,
+ KeyArgs keyArgs, Map<String, String> auditMap) throws OMException {
if (keyArgs.hasExpectedDataGeneration()) {
- long expectedGen = keyArgs.getExpectedDataGeneration();
- // If expectedGen is EXPECTED_GEN_CREATE_IF_NOT_EXISTS, it means the key MUST NOT exist (If-None-Match)
- if (expectedGen == OzoneConsts.EXPECTED_GEN_CREATE_IF_ABSENT) {
- if (dbKeyInfo != null) {
- throw new OMException("Key already exists",
- OMException.ResultCodes.KEY_ALREADY_EXISTS);
- }
- } else {
- // If a key does not exist, or if it exists but the updateID do not match, then fail this request.
- if (dbKeyInfo == null) {
- throw new OMException("Key not found during expected rewrite",
- OMException.ResultCodes.KEY_NOT_FOUND);
- }
- if (dbKeyInfo.getUpdateID() != expectedGen) {
- throw new OMException("Generation mismatch during expected rewrite",
- OMException.ResultCodes.KEY_NOT_FOUND);
- }
+ validateAtomicRewriteAtAdmission(dbKeyInfo, keyArgs.getExpectedDataGeneration(), auditMap);
+ return keyArgs;
+ }
+
+ return validateAndResolveIfMatchAsExpectedGeneration(keyArgs, dbKeyInfo, auditMap);
+ }
+
+ /**
+ * Validates the condition at request admission time.
+ *
+ * This form is for callers that resolve the admitted generation from
+ * persisted open-key state rather than from request key arguments.
+ */
+ protected void validateAtomicRewriteAtAdmission(OmKeyInfo dbKeyInfo,
+ Long expectedGen, Map<String, String> auditMap) throws OMException {
+ if (expectedGen == null) {
+ return;
+ }
+ addRewriteGenerationToAuditMap(expectedGen, auditMap);
+
+ if (expectedGen == OzoneConsts.EXPECTED_GEN_CREATE_IF_ABSENT) {
+ if (dbKeyInfo != null) {
+ throw new OMException("Key already exists", OMException.ResultCodes.KEY_ALREADY_EXISTS);
+ }
+ return;
+ }
+
+ if (dbKeyInfo == null) {
+ throw new OMException("Key not found during expected rewrite", OMException.ResultCodes.KEY_NOT_FOUND);
+ }
+ if (expectedGen != dbKeyInfo.getUpdateID()) {
+ throw new OMException("Generation mismatch during expected rewrite", OMException.ResultCodes.KEY_NOT_FOUND);
+ }
+ }
+
+ /**
+ * Validates an already admitted condition at serialized commit time.
+ *
+ * This form is for callers that must check the admitted generation before
+ * building the final key info to commit.
+ */
+ protected void validateAtomicRewriteAtCommit(OmKeyInfo existing,
+ Long expectedGen, Map<String, String> auditMap) throws OMException {
+ if (expectedGen == null) {
+ return;
+ }
+ addRewriteGenerationToAuditMap(expectedGen, auditMap);
+
+ if (expectedGen == OzoneConsts.EXPECTED_GEN_CREATE_IF_ABSENT) {
+ if (existing != null) {
+ throw new OMException("Atomic create-if-not-exists conflicted with "
+ + "an existing key",
+ OMException.ResultCodes.ATOMIC_WRITE_CONFLICT);
}
+ return;
+ }
+
+ if (existing == null) {
+ throw new OMException("Atomic rewrite conflicted because the key no "
+ + "longer exists",
+ OMException.ResultCodes.ATOMIC_WRITE_CONFLICT);
Review Comment:
Shall we return OMException.ResultCodes.KEY_NOT_FOUND instead of ATOMIC_WRITE_CONFLICT here?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15167. Broaden commit conflict detection for conditional writes [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #10183:
URL: https://github.com/apache/ozone/pull/10183#discussion_r4236545056
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java:
##########
@@ -1369,46 +1369,96 @@ protected void validateEncryptionKeyInfo(OmBucketInfo bucketInfo, KeyArgs keyArg
}
/**
- * Validates atomic rewrite conditions for conditional writes.
- * <p>
- * For If-None-Match: * (expectedDataGeneration = EXPECTED_GEN_CREATE_IF_NOT_EXISTS),
- * the key must NOT exist.
- * <p>
- * For If-Match with a specific generation, the key must exist with matching updateID.
+ * Resolves conditional write state at request admission time.
*
- * @param dbKeyInfo the existing key info from the database (null if key doesn't exist)
- * @param keyArgs the key arguments containing expected generation
- * @throws OMException if validation fails
+ * This validates expected generations as admission-time preconditions and
+ * rewrites matching ETags into expected generations for commit-time checks.
*/
- protected void validateAtomicRewrite(OmKeyInfo dbKeyInfo, KeyArgs keyArgs)
- throws OMException {
+ protected KeyArgs resolveConditionalWriteAtAdmission(OmKeyInfo dbKeyInfo,
+ KeyArgs keyArgs, Map<String, String> auditMap) throws OMException {
if (keyArgs.hasExpectedDataGeneration()) {
- long expectedGen = keyArgs.getExpectedDataGeneration();
- // If expectedGen is EXPECTED_GEN_CREATE_IF_NOT_EXISTS, it means the key MUST NOT exist (If-None-Match)
- if (expectedGen == OzoneConsts.EXPECTED_GEN_CREATE_IF_ABSENT) {
- if (dbKeyInfo != null) {
- throw new OMException("Key already exists",
- OMException.ResultCodes.KEY_ALREADY_EXISTS);
- }
- } else {
- // If a key does not exist, or if it exists but the updateID do not match, then fail this request.
- if (dbKeyInfo == null) {
- throw new OMException("Key not found during expected rewrite",
- OMException.ResultCodes.KEY_NOT_FOUND);
- }
- if (dbKeyInfo.getUpdateID() != expectedGen) {
- throw new OMException("Generation mismatch during expected rewrite",
- OMException.ResultCodes.KEY_NOT_FOUND);
- }
+ validateAtomicRewriteAtAdmission(dbKeyInfo, keyArgs.getExpectedDataGeneration(), auditMap);
+ return keyArgs;
+ }
+
+ return validateAndResolveIfMatchAsExpectedGeneration(keyArgs, dbKeyInfo, auditMap);
+ }
+
+ /**
+ * Validates the condition at request admission time.
+ *
+ * This form is for callers that resolve the admitted generation from
+ * persisted open-key state rather than from request key arguments.
+ */
+ protected void validateAtomicRewriteAtAdmission(OmKeyInfo dbKeyInfo,
+ Long expectedGen, Map<String, String> auditMap) throws OMException {
+ if (expectedGen == null) {
+ return;
+ }
+ addRewriteGenerationToAuditMap(expectedGen, auditMap);
+
+ if (expectedGen == OzoneConsts.EXPECTED_GEN_CREATE_IF_ABSENT) {
+ if (dbKeyInfo != null) {
+ throw new OMException("Key already exists", OMException.ResultCodes.KEY_ALREADY_EXISTS);
+ }
+ return;
+ }
+
+ if (dbKeyInfo == null) {
+ throw new OMException("Key not found during expected rewrite", OMException.ResultCodes.KEY_NOT_FOUND);
+ }
+ if (expectedGen != dbKeyInfo.getUpdateID()) {
+ throw new OMException("Generation mismatch during expected rewrite", OMException.ResultCodes.KEY_NOT_FOUND);
Review Comment:
Based on doc change, this should return ATOMIC_WRITE_CONFLICT instead of KEY_NOT_FOUND?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15167. Pre-detect commit conflict for conditional writes in OM request's preExecute [ozone]
Posted by "github-actions[bot] (via GitHub) via issues" <is...@ozone.apache.org>.
github-actions[bot] closed pull request #10183: HDDS-15167. Pre-detect commit conflict for conditional writes in OM request's preExecute
URL: https://github.com/apache/ozone/pull/10183
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15167. Broaden commit conflict detection for conditional writes [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #10183: HDDS-15167. Broaden commit conflict detection for conditional writes
URL: https://github.com/apache/ozone/pull/10183
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15167. Concurrent Conflict Detection for Conditional MPU Complete [ozone]
Posted by "peterxcli (via GitHub)" <gi...@apache.org>.
peterxcli commented on code in PR #10183:
URL: https://github.com/apache/ozone/pull/10183#discussion_r3186148091
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/s3/multipart/S3MultipartUploadCompleteRequest.java:
##########
@@ -409,6 +404,40 @@ protected OMClientResponse getOmClientResponse(String multipartKey,
getBucketLayout(), omBucketInfo, bucketId);
}
+ private KeyArgs resolveConditionalWrite(
+ OMMetadataManager omMetadataManager, KeyArgs keyArgs)
+ throws IOException {
+ if (!keyArgs.hasExpectedETag()
+ && !keyArgs.hasExpectedDataGeneration()) {
+ return keyArgs;
+ }
+
+ String multipartKey = omMetadataManager.getMultipartKey(
+ keyArgs.getVolumeName(),
+ keyArgs.getBucketName(), keyArgs.getKeyName(),
+ keyArgs.getMultipartUploadID());
+ if (omMetadataManager.getMultipartInfoTable().get(multipartKey) == null) {
+ throw new OMException(
+ failureMessage(keyArgs.getVolumeName(), keyArgs.getBucketName(),
+ keyArgs.getKeyName()),
+ OMException.ResultCodes.NO_SUCH_MULTIPART_UPLOAD_ERROR);
+ }
+
+ OmKeyInfo existingKeyInfo = getExistingKeyInfo(
+ omMetadataManager, keyArgs);
Review Comment:
I am thinking if calling rocksdb inside preExecute is a good idea
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15167. Broaden commit conflict detection for conditional writes [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #10183:
URL: https://github.com/apache/ozone/pull/10183#discussion_r4236632717
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java:
##########
@@ -1439,24 +1489,28 @@ protected void validateIfMatchETag(KeyArgs keyArgs, OmKeyInfo dbKeyInfo)
* Validates If-Match ETag condition and converts it to expectedDataGeneration.
* <p>
* This method checks if the existing key's ETag matches the expected ETag.
- * If it matches, the ETag condition is converted to a generation-based condition
- * for atomic commit validation in two-phase operations (CreateKey → CommitKey).
+ * If it matches, the ETag condition is converted to a generation-based
+ * condition for commit-time atomic rewrite validation.
*
* @param keyArgs the key arguments containing expected ETag
* @param dbKeyInfo the existing key info from the database
+ * @param auditMap the audit map to update with the resolved generation
* @return updated KeyArgs with expectedDataGeneration set (if ETag matched)
* @throws OMException if validation fails
*/
- protected KeyArgs validateAndRewriteIfMatchAsExpectedGeneration(
- KeyArgs keyArgs, OmKeyInfo dbKeyInfo) throws OMException {
+ private KeyArgs validateAndResolveIfMatchAsExpectedGeneration(
+ KeyArgs keyArgs, OmKeyInfo dbKeyInfo, Map<String, String> auditMap) throws OMException {
validateIfMatchETag(keyArgs, dbKeyInfo);
- if (!keyArgs.hasExpectedETag() || keyArgs.hasExpectedDataGeneration()) {
+ if (!keyArgs.hasExpectedETag()) {
return keyArgs;
}
+ long expectedGen = dbKeyInfo.getUpdateID();
+ addRewriteGenerationToAuditMap(expectedGen, auditMap);
+
return keyArgs.toBuilder()
- .setExpectedDataGeneration(dbKeyInfo.getUpdateID())
+ .setExpectedDataGeneration(expectedGen)
Review Comment:
BTW, it looks like verify the UpdateID in Ozone is stricter than AWS S3 in Etag case, since any metadata update will increase the UpdateID, while the content can still remains the same.
So if another client update the object's any metadata between the createKey and commitKey, then commitKey will fail in this case.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15167. Broaden commit conflict detection for conditional writes [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #10183:
URL: https://github.com/apache/ozone/pull/10183#discussion_r4236382229
##########
hadoop-hdds/docs/content/design/s3-conditional-requests.md:
##########
@@ -118,6 +118,9 @@ temporary part state.
object for `If-None-Match: *`.
- The MPU parts remain associated with the multipart upload until the
complete request succeeds or the upload is aborted.
+- After a `409 ConditionalRequestConflict`, the client must fetch the
Review Comment:
- Does this apply to 409 only, or both 412 and 409 failure code?
- Does this apply to `If-Match: "<etag>"` only or both `If-Match: "<etag>"` and `If-None-Match`?
- Does it always require "create a new multipart upload, re-upload the parts" steps or just "refetch the latest object ETag, and complete the new uploading using the latest condition" is required?
Can you make it more clear in the doc?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15167. Broaden commit conflict detection for conditional writes [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #10183:
URL: https://github.com/apache/ozone/pull/10183#issuecomment-4597616719
This PR has been marked as stale due to 21 days of inactivity. Please comment or remove the stale label to keep it open. Otherwise, it will be automatically closed in 7 days.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15167. Broaden commit conflict detection for conditional writes [ozone]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #10183:
URL: https://github.com/apache/ozone/pull/10183#issuecomment-4784740113
This PR has been marked as stale due to 21 days of inactivity. Please comment or remove the stale label to keep it open. Otherwise, it will be automatically closed in 7 days.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15167. Broaden commit conflict detection for conditional writes [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #10183:
URL: https://github.com/apache/ozone/pull/10183#discussion_r4236620520
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMKeyRequest.java:
##########
@@ -1439,24 +1489,28 @@ protected void validateIfMatchETag(KeyArgs keyArgs, OmKeyInfo dbKeyInfo)
* Validates If-Match ETag condition and converts it to expectedDataGeneration.
* <p>
* This method checks if the existing key's ETag matches the expected ETag.
- * If it matches, the ETag condition is converted to a generation-based condition
- * for atomic commit validation in two-phase operations (CreateKey → CommitKey).
+ * If it matches, the ETag condition is converted to a generation-based
+ * condition for commit-time atomic rewrite validation.
*
* @param keyArgs the key arguments containing expected ETag
* @param dbKeyInfo the existing key info from the database
+ * @param auditMap the audit map to update with the resolved generation
* @return updated KeyArgs with expectedDataGeneration set (if ETag matched)
* @throws OMException if validation fails
*/
- protected KeyArgs validateAndRewriteIfMatchAsExpectedGeneration(
- KeyArgs keyArgs, OmKeyInfo dbKeyInfo) throws OMException {
+ private KeyArgs validateAndResolveIfMatchAsExpectedGeneration(
+ KeyArgs keyArgs, OmKeyInfo dbKeyInfo, Map<String, String> auditMap) throws OMException {
validateIfMatchETag(keyArgs, dbKeyInfo);
- if (!keyArgs.hasExpectedETag() || keyArgs.hasExpectedDataGeneration()) {
+ if (!keyArgs.hasExpectedETag()) {
Review Comment:
We can move this if statement ahead of validateIfMatchETag.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15167. Broaden commit conflict detection for conditional writes [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on code in PR #10183:
URL: https://github.com/apache/ozone/pull/10183#discussion_r4236382229
##########
hadoop-hdds/docs/content/design/s3-conditional-requests.md:
##########
@@ -118,6 +118,9 @@ temporary part state.
object for `If-None-Match: *`.
- The MPU parts remain associated with the multipart upload until the
complete request succeeds or the upload is aborted.
+- After a `409 ConditionalRequestConflict`, the client must fetch the
Review Comment:
- Does this apply to 409 only, or both 412 and 409 failure code?
- Does this apply to `If-Match: "<etag>"` only or both `If-Match: "<etag>"` and `If-None-Match`?
- Does it always require "create a new multipart upload, re-upload the parts" steps or just "refetch the latest object ETag, and complete the new uploading using the latest condition" is required?
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org
Re: [PR] HDDS-15167. Broaden commit conflict detection for conditional writes [ozone]
Posted by "ChenSammi (via GitHub)" <gi...@apache.org>.
ChenSammi commented on PR #10183:
URL: https://github.com/apache/ozone/pull/10183#issuecomment-5807340633
@peterxcli , could you rebase the patch? I can help with the review.
--
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: issues-unsubscribe@ozone.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@ozone.apache.org
For additional commands, e-mail: issues-help@ozone.apache.org