-
Notifications
You must be signed in to change notification settings - Fork 248
AbortMPU cleanup - revisited #5854
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
base: development/8.8
Are you sure you want to change the base?
Conversation
Hello williamlardier,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Incorrect fix versionThe
Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:
Please check the |
- CompleteMPU was and is still, at the time of this commit, wrongly promoting s3 objects when completing a MPU even if the parts are not valid. - The AbortMPU cleanup logic was added as an effort to cleanup new occurences of this problem, and still handle old ones not yet aborted. - The logic currently is unable to clean up if the current master version has a different upload id: in case a new version was pushed, we end up never cleaning the ghost. - This commit introduces a dynamic listing of the versionIDs of the object. We use the delimiter versions algorithm and the versionID separator to list only the current object versions, with pagination support, and then we detect the one to cleanup. Issue: CLDSRV-669
5cf7573
to
a814106
Compare
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files
... and 2 files with indirect coverage changes @@ Coverage Diff @@
## development/8.8 #5854 +/- ##
===================================================
+ Coverage 65.08% 65.25% +0.16%
===================================================
Files 188 188
Lines 11948 11983 +35
===================================================
+ Hits 7776 7819 +43
+ Misses 4172 4164 -8
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
const listParams = { | ||
listingType: 'DelimiterVersions', | ||
// To only list the specific key, we need to add the versionId separator | ||
prefix: `${objectKey}${versioning.VersioningConstants.VersionId.Separator}`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will need further tests to ensure this works file - we do not officially have a way to list all versions of just one object
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test done, it's working well. Example:
{
"CommonPrefixes": [],
"Versions": [
{
"key": "bug-test-bda30555",
"value": {
"Size": 1048576,
"ETag": "3eaf2c9512f76a55da265ad51ac21393-1",
"VersionId": "98248542529027999999RG00001",
"IsDeleteMarker": false,
"LastModified": "2025-07-02T11:57:50.972Z",
"Owner": {
"DisplayName": "acc",
"ID": "5874c85ff66d0ede3bd6e8fe9610f0640d1700a4607f8b7e414503fbd2cb03de"
},
"StorageClass": "STANDARD"
}
},
{
"key": "bug-test-bda30555",
"value": {
"Size": 1048576,
"ETag": "3eaf2c9512f76a55da265ad51ac21393-1",
"VersionId": "98248542540139999999RG00001",
"IsDeleteMarker": false,
"LastModified": "2025-07-02T11:57:39.857Z",
"Owner": {
"DisplayName": "acc",
"ID": "5874c85ff66d0ede3bd6e8fe9610f0640d1700a4607f8b7e414503fbd2cb03de"
},
"StorageClass": "STANDARD"
}
}
],
"IsTruncated": false
}
(while the same key with suffix exists)
a814106
to
a71a04f
Compare
a71a04f
to
f70c06a
Compare
Incorrect fix versionThe
Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:
Please check the |
Relates to https://scality.atlassian.net/wiki/spaces/OS/pages/3258220958/Retriable+CompleteMultipartUpload
See ticket & commits for context
Issue: CLDSRV-669
Following this PR, we will fix the behavior of the CompleteMPU API to avoid promoting wrongly s3 objects if not all parts are valid. This might not be feasible because we want to be unified with S3C, that is not 100% standard with MPU management, as we can abort after a complete officially.