You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@ozone.apache.org by "SaketaChalamchala (via GitHub)" <gi...@apache.org> on 2025/10/09 13:53:36 UTC
[PR] HDDS-13776. Fail DirectoryPurge requests if previous snapshot ID validation fails. [ozone]
SaketaChalamchala opened a new pull request, #9130:
URL: https://github.com/apache/ozone/pull/9130
## What changes were proposed in this pull request?
If a snapshot is created for a bucket while DirectoryDeletingService is processing deleted directories from the same bucket then, the `DirectoryPurge` requests submitted by DirectoryDeletingService should not execute and should be processed when snapshot is deep cleaned.
## What is the link to the Apache JIRA
https://issues.apache.org/jira/browse/HDDS-13776
## How was this patch tested?
--
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-13776. Fail DirectoryPurge requests if previous snapshot ID validation fails. [ozone]
Posted by "ivandika3 (via GitHub)" <gi...@apache.org>.
ivandika3 commented on PR #9130:
URL: https://github.com/apache/ozone/pull/9130#issuecomment-3435328316
@jojochuang @SaketaChalamchala This seems to break build in master.
--
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-13776. Fail DirectoryPurge requests if previous snapshot ID validation fails. [ozone]
Posted by "smengcl (via GitHub)" <gi...@apache.org>.
smengcl commented on PR #9130:
URL: https://github.com/apache/ozone/pull/9130#issuecomment-3387237873
> Good catch @SaketaChalamchala This is caused by [HDDS-12982](https://issues.apache.org/jira/browse/HDDS-12982)
>
> So we have two options (1) work on this RP, or (2) revert [HDDS-12982](https://issues.apache.org/jira/browse/HDDS-12982)
>
> Regardless, it looks like we need a test to validate this scenario, otherwise we could have caught the bug.
Thanks @SaketaChalamchala for the fix.
+1 that we'd better add a test 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-13776. Fail DirectoryPurge requests if previous snapshot ID validation fails. [ozone]
Posted by "swamirishi (via GitHub)" <gi...@apache.org>.
swamirishi commented on PR #9130:
URL: https://github.com/apache/ozone/pull/9130#issuecomment-3438550879
@jojochuang we don't need this since HDDS-13799 is already reverts the initial change. Maybe a PR with test case would be better
--
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-13776. Fail DirectoryPurge requests if previous snapshot ID validation fails. [ozone]
Posted by "adoroszlai (via GitHub)" <gi...@apache.org>.
adoroszlai commented on PR #9130:
URL: https://github.com/apache/ozone/pull/9130#issuecomment-3435422475
Reverted due to compile error.
--
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-13776. Fail DirectoryPurge requests if previous snapshot ID validation fails. [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang merged PR #9130:
URL: https://github.com/apache/ozone/pull/9130
--
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-13776. Fail DirectoryPurge requests if previous snapshot ID validation fails. [ozone]
Posted by "jojochuang (via GitHub)" <gi...@apache.org>.
jojochuang commented on PR #9130:
URL: https://github.com/apache/ozone/pull/9130#issuecomment-3434890742
I'll go ahead and merge it. If we want to add a test we can add later.
--
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-13776. Fail DirectoryPurge requests if previous snapshot ID validation fails. [ozone]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #9130:
URL: https://github.com/apache/ozone/pull/9130#discussion_r2417133340
##########
hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/key/OMDirectoriesPurgeRequestWithFSO.java:
##########
@@ -111,8 +111,11 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager, Execut
// Validating previous snapshot can ensure the chain hasn't changed.
UUID expectedPreviousSnapshotId = purgeDirsRequest.getExpectedPreviousSnapshotID().hasUuid()
? fromProtobuf(purgeDirsRequest.getExpectedPreviousSnapshotID().getUuid()) : null;
- validatePreviousSnapshotId(fromSnapshotInfo, omMetadataManager.getSnapshotChainManager(),
- expectedPreviousSnapshotId);
+ if (!validatePreviousSnapshotId(fromSnapshotInfo, omMetadataManager.getSnapshotChainManager(),
+ expectedPreviousSnapshotId)) {
+ return new OMDirectoriesPurgeResponseWithFSO(createErrorOMResponse(omResponse,
+ new OMException("Snapshot validation failed", OMException.ResultCodes.INVALID_REQUEST)));
+ }
Review Comment:
The error message 'Snapshot validation failed' is too generic and doesn't provide enough context for debugging. Consider adding more specific information about what aspect of validation failed or why the request should be retried.
```suggestion
String actualPrevSnapshotId = (fromSnapshotInfo != null && fromSnapshotInfo.getPreviousSnapshot() != null)
? fromSnapshotInfo.getPreviousSnapshot().toString() : "null";
String errorMsg = String.format(
"Snapshot validation failed for fromSnapshot='%s': expected previousSnapshotId=%s, actual previousSnapshotId=%s",
fromSnapshot,
expectedPreviousSnapshotId != null ? expectedPreviousSnapshotId.toString() : "null",
actualPrevSnapshotId
);
return new OMDirectoriesPurgeResponseWithFSO(createErrorOMResponse(omResponse,
new OMException(errorMsg, OMException.ResultCodes.INVALID_REQUEST)));
```
--
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