-
Notifications
You must be signed in to change notification settings - Fork 241
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
Fix complete MPU with missing parts #5691
Fix complete MPU with missing parts #5691
Conversation
Hello killiang,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
6845b72
to
a30b1c6
Compare
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
|
In case of a complete MPU error after the storage of the object MD, the abort must delete object MDs and object Data, some part may have been deleted already in the MPU metadata, hence we need to verify in the object MD for the parts it's linked to, to avoid creating orphans Issue: CLDSRV-570
a30b1c6
to
cb95908
Compare
558e17b
to
bc34e74
Compare
4b578f2
to
0b46258
Compare
…on and MD orphans Remove duplicated import in tests Issue: CLDSRV-570
046a124
to
a934fe1
Compare
@@ -98,6 +126,13 @@ function abortMultipartUpload(authInfo, bucketName, objectKey, uploadId, log, | |||
} | |||
// flatten the array | |||
locations = [].concat(...locations); | |||
|
|||
if (objectMD?.location) { | |||
const objectLocationLeft = objectMD.location.filter(loc => |
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.
nit: "left" looks like the directoin, maybe best to rename to something more intuitive, maybe remainingObjectLocations
more importantly, this can be quite CPU intensive for large objects (n^2
complexity, with n
up to 10000): need to make this more efficient...
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.
Maybe a more optimized approach for this would be to use a map ?
if (objectMD?.location) {
const locationMap = new Map(locations.map(loc => [loc.key, true]));
const objectLocationLeft = objectMD.location.filter(loc => !locationMap.has(loc.key));
locations = locations.concat(objectLocationLeft);
}
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.
since we are building a list of locations, maybe we could directly use a map
instead (and pass it to async.eachLimit directly, to process each "pair"?)
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.
Pushed a more optimized version here
e9ae9c2
FOr locations being a Set directly, I can do it, but as we'll need to map over it anyway for getting keys, we'll end up needing an array (as map
function does not exist for Sets)
24477ba
to
c7c7ba1
Compare
c7c7ba1
to
d13a6f3
Compare
/approve |
I have successfully merged the changeset of this pull request
The following branches have NOT changed:
Please check the status of the associated issue CLDSRV-570. Goodbye killiang. The following options are set: approve |
In case of a complete MPU error after the storage of the object MD, the abort must delete object MDs and object Data, some part may have been deleted already in the MPU metadata, hence we need to verify in the object MD for the parts it's linked to, to avoid creating orphans
Issue: CLDSRV-570