Skip to content
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.

Sign up for GitHub

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

Merged
merged 9 commits into from
Nov 18, 2024
+289 −14
Merged

Fix complete MPU with missing parts #5691

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
cb95908
In case of an abort, delete data and MD of object
KillianG Nov 13, 2024
dda2bce
Remove object MD before deleting data
KillianG Nov 14, 2024
d78a395
Add test for abort multipart for the fix of the issue
KillianG Nov 14, 2024
f7c4f97
Adding logs to ease future debugging
KillianG Nov 14, 2024
de934fa
Fix delete object MD with version ID
KillianG Nov 15, 2024
bc34e74
fix space before brackets
KillianG Nov 15, 2024
a934fe1
Change function order in abortMultipartUpload to avoid code duplicati…
KillianG Nov 18, 2024
e9ae9c2
Optimize listing of keys for data deletion
KillianG Nov 18, 2024
d13a6f3
Even better optimize
KillianG Nov 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 57 additions & 12 deletions lib/api/apiUtils/object/abortMultipartUpload.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ const locationConstraintCheck = require('../object/locationConstraintCheck');
const { standardMetadataValidateBucketAndObj } =
require('../../../metadata/metadataUtils');
const services = require('../../../services');
const metadata = require('../../../metadata/wrapper');

function abortMultipartUpload(authInfo, bucketName, objectKey, uploadId, log,
callback, request) {
Expand All @@ -17,6 +18,8 @@ function abortMultipartUpload(authInfo, bucketName, objectKey, uploadId, log,
preciseRequestType: request.apiMethods || 'multipartDelete',
request,
};

log.debug('processing request', { method: 'abortMultipartUpload' });
// For validating the request at the destinationBucket level
// params are the same as validating at the MPU level
// but the requestType is the more general 'objectDelete'
Expand All @@ -27,8 +30,9 @@ function abortMultipartUpload(authInfo, bucketName, objectKey, uploadId, log,
async.waterfall([
function checkDestBucketVal(next) {
standardMetadataValidateBucketAndObj(metadataValParams, authzIdentityResult, log,
(err, destinationBucket) => {
(err, destinationBucket, objectMD) => {
if (err) {
log.error('error validating request', { error: err });
return next(err, destinationBucket);
}
if (destinationBucket.policies) {
Expand All @@ -41,20 +45,21 @@ function abortMultipartUpload(authInfo, bucketName, objectKey, uploadId, log,
metadataValMPUparams.requestType =
'bucketPolicyGoAhead';
}
return next(null, destinationBucket);
return next(null, destinationBucket, objectMD);
});
},
function checkMPUval(destBucket, next) {
function checkMPUval(destBucket, objectMD, next) {
metadataValParams.log = log;
services.metadataValidateMultipart(metadataValParams,
(err, mpuBucket, mpuOverviewObj) => {
if (err) {
log.error('error validating multipart', { error: err });
return next(err, destBucket);
}
return next(err, mpuBucket, mpuOverviewObj, destBucket);
return next(err, mpuBucket, mpuOverviewObj, destBucket, objectMD);
});
},
function abortExternalMpu(mpuBucket, mpuOverviewObj, destBucket,
function abortExternalMpu(mpuBucket, mpuOverviewObj, destBucket, objectMD,
next) {
const location = mpuOverviewObj.controllingLocationConstraint;
const originalIdentityAuthzResults = request.actionImplicitDenies;
Expand All @@ -66,35 +71,37 @@ function abortMultipartUpload(authInfo, bucketName, objectKey, uploadId, log,
// eslint-disable-next-line no-param-reassign
request.actionImplicitDenies = originalIdentityAuthzResults;
if (err) {
log.error('error aborting MPU', { error: err });
return next(err, destBucket);
}
// for Azure and GCP we do not need to delete data
// for all other backends, skipDataDelete will be set to false
return next(null, mpuBucket, destBucket, skipDataDelete);
return next(null, mpuBucket, destBucket, objectMD, skipDataDelete);
});
},
function getPartLocations(mpuBucket, destBucket, skipDataDelete,
function getPartLocations(mpuBucket, destBucket, objectMD, skipDataDelete,
next) {
services.getMPUparts(mpuBucket.getName(), uploadId, log,
(err, result) => {
if (err) {
log.error('error getting parts', { error: err });
return next(err, destBucket);
}
const storedParts = result.Contents;
return next(null, mpuBucket, storedParts, destBucket,
return next(null, mpuBucket, storedParts, destBucket, objectMD,
skipDataDelete);
});
},
function deleteData(mpuBucket, storedParts, destBucket,
function deleteData(mpuBucket, storedParts, destBucket, objectMD,
skipDataDelete, next) {
if (skipDataDelete) {
return next(null, mpuBucket, storedParts, destBucket);
return next(null, mpuBucket, storedParts, destBucket, objectMD);
}
// The locations were sent to metadata as an array
// under partLocations. Pull the partLocations.
let locations = storedParts.map(item => item.value.partLocations);
if (locations.length === 0) {
return next(null, mpuBucket, storedParts, destBucket);
return next(null, mpuBucket, storedParts, destBucket, objectMD);
}
// flatten the array
locations = [].concat(...locations);
Expand All @@ -105,9 +112,47 @@ function abortMultipartUpload(authInfo, bucketName, objectKey, uploadId, log,
}
cb();
});
}, () => next(null, mpuBucket, storedParts, destBucket, objectMD));
},
function deleteObjectMetadata(mpuBucket, storedParts, destBucket, objectMD, next) {
if (!objectMD || metadataValMPUparams.uploadId !== objectMD.uploadId) {
francoisferrand marked this conversation as resolved.
Show resolved Hide resolved
return next(null, mpuBucket, storedParts, destBucket, objectMD);
}
// In case there has been an error during cleanup after a complete MPU
// (e.g. failure to delete MPU MD in shadow bucket),
// we need to ensure that the MPU metadata is deleted.
log.info('Object has existing metadata, deleting them', {
KillianG marked this conversation as resolved.
Show resolved Hide resolved
method: 'abortMultipartUpload',
bucketName,
objectKey,
KillianG marked this conversation as resolved.
Show resolved Hide resolved
});
return metadata.deleteObjectMD(bucketName, objectKey, undefined, log, err => {
KillianG marked this conversation as resolved.
Show resolved Hide resolved
if (err) {
log.error('error deleting object metadata', { error: err });
}
return next(err, mpuBucket, storedParts, destBucket, objectMD);
});
},
function deleteObjectData(mpuBucket, storedParts, destBucket, objectMD, next) {
KillianG marked this conversation as resolved.
Show resolved Hide resolved
if (!objectMD?.location) {
return next(null, mpuBucket, storedParts, destBucket);
}
// Filtering parts that has already been delete by the previous step
let partLocations = storedParts.map(item => item.value.partLocations);
partLocations = [].concat(...partLocations);
partLocations = new Set(partLocations.map(loc => loc.key));
KillianG marked this conversation as resolved.
Show resolved Hide resolved
const objectLocationLeft = objectMD.location.filter(loc => !partLocations.has(loc.key));

return async.eachLimit(objectLocationLeft, 5, (loc, cb) => {
data.delete(loc, log, err => {
if (err) {
log.fatal('delete object data failed', { err });
}
cb();
KillianG marked this conversation as resolved.
Show resolved Hide resolved
});
}, () => next(null, mpuBucket, storedParts, destBucket));
},
function deleteMetadata(mpuBucket, storedParts, destBucket, next) {
function deleteShadowObjectMetadata(mpuBucket, storedParts, destBucket, next) {
let splitter = constants.splitter;
// BACKWARD: Remove to remove the old splitter
if (mpuBucket.getMdBucketModelVersion() < 2) {
Expand Down
11 changes: 11 additions & 0 deletions lib/api/completeMultipartUpload.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ function completeMultipartUpload(authInfo, request, log, callback) {
return services.metadataValidateMultipart(metadataValParams,
(err, mpuBucket, mpuOverview, storedMetadata) => {
if (err) {
log.error('error validating request', { error: err });
return next(err, destBucket);
}
return next(null, destBucket, objMD, mpuBucket,
Expand All @@ -172,6 +173,7 @@ function completeMultipartUpload(authInfo, request, log, callback) {
if (request.post) {
return parseXml(request.post, (err, jsonList) => {
if (err) {
log.error('error parsing XML', { error: err });
return next(err, destBucket);
}
return next(null, destBucket, objMD, mpuBucket,
Expand All @@ -190,6 +192,12 @@ function completeMultipartUpload(authInfo, request, log, callback) {
storedMetadata,
}, log, err => {
if (err) {
log.error('error marking MPU object for completion', {
bucketName: mpuBucket.getName(),
objectKey,
uploadId,
error: err,
});
return next(err);
}
return next(null, destBucket, objMD, mpuBucket,
Expand All @@ -201,6 +209,7 @@ function completeMultipartUpload(authInfo, request, log, callback) {
return services.getMPUparts(mpuBucket.getName(), uploadId, log,
(err, result) => {
if (err) {
log.error('error getting parts', { error: err });
return next(err, destBucket);
}
const storedParts = result.Contents;
Expand All @@ -222,6 +231,7 @@ function completeMultipartUpload(authInfo, request, log, callback) {
// eslint-disable-next-line no-param-reassign
request.actionImplicitDenies = originalIdentityImpDenies;
if (err) {
log.error('error completing MPU externally', { error: err });
return next(err, destBucket);
}
// if mpu not handled externally, completeObjData will be null
Expand Down Expand Up @@ -452,6 +462,7 @@ function completeMultipartUpload(authInfo, request, log, callback) {
dataLocations, pseudoCipherBundle, metaStoreParams,
(err, res) => {
if (err) {
log.error('error storing object metadata', { error: err });
return next(err, destinationBucket);
}

Expand Down
Loading
Loading