Skip to content

Commit

Permalink
Merge branch 'feature/ARSN-235-update-object-before-deleting-it' into…
Browse files Browse the repository at this point in the history 
… q/7.10
  • Loading branch information
@bert-e
bert-e committed Nov 14, 2022
2 parents 0f9da6a + 9a97572 commit 7c1bd45
Show file tree
Hide file tree
Showing 6 changed files with 293 additions and 145 deletions.
262 changes: 180 additions & 82 deletions lib/storage/metadata/mongoclient/MongoClientInterface.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const constants = require('../../../constants');
const { reshapeExceptionError } = require('../../../errorUtils');
const errors = require('../../../errors').default;
const BucketInfo = require('../../../models/BucketInfo').default;
const ObjectMD = require('../../../models/ObjectMD').default;
const jsutil = require('../../../jsutil');

const MongoClient = require('mongodb').MongoClient;
Expand Down Expand Up @@ -69,13 +70,6 @@ function inc(str) {
String.fromCharCode(str.charCodeAt(str.length - 1) + 1)) : str;
}

function generatePHDVersion(versionId) {
return {
isPHD: true,
versionId,
};
}

/**
* @constructor
*
Expand Down Expand Up @@ -934,6 +928,11 @@ class MongoClientInterface {
}
c.findOne({
_id: key,
// filtering out objects flagged for deletion
$or: [
{ 'value.deleted': { $exists: false } },
{ 'value.deleted': { $eq: false } },
],
}, {}, (err, doc) => {
if (err) {
log.error('findOne: error getting object',
Expand Down Expand Up @@ -994,6 +993,11 @@ class MongoClientInterface {
}
c.find({
_id: filter,
// filtering out objects flagged for deletion
$or: [
{ 'value.deleted': { $exists: false } },
{ 'value.deleted': { $eq: false } },
],
}, {}).
sort({
_id: 1,
Expand Down Expand Up @@ -1107,7 +1111,7 @@ class MongoClientInterface {
this.getLatestVersion(c, objName, vFormat, log, (err, version) => {
if (err && !err.is.NoSuchKey) {
log.error('getLatestVersion: error getting latest version',
{ error: err.message });
{ error: err.message, bucket: bucketName, key: objName });
return cb(err);
}
if ((err && err.is.NoSuchKey) || (version.isDeleteMarker && vFormat === BUCKET_VERSIONS.v1)) {
Expand All @@ -1117,15 +1121,21 @@ class MongoClientInterface {
// atomic condition on the PHD flag and the mst
// version:
// eslint-disable-next-line
c.findOneAndDelete({
'_id': masterKey,
const filter = {
'value.isPHD': true,
'value.versionId': mst.versionId,
}, {}, err => {
};
this.internalDeleteObject(c, bucketName, masterKey, filter, log, err => {
if (err) {
// the PHD master might get updated when a PUT is performed
// before the repair is done, we don't want to return an error
// in this case
if (err.is.NoSuchKey) {
return cb(null);
}
log.error(
'findOneAndDelete: error finding and deleting',
{ error: err.message });
'deleteOrRepairPHD: error deleting object',
{ error: err.message, bucket: bucketName, key: objName });
return cb(errors.InternalError);
}
// do not test result.ok === 1 because
Expand Down Expand Up @@ -1162,33 +1172,47 @@ class MongoClientInterface {
const masterKey = formatMasterKey(objName, params.vFormat);
const versionKey = formatVersionKey(objName, params.versionId, params.vFormat);
const _vid = generateVersionId(this.replicationGroupId);
const mst = generatePHDVersion(_vid);
c.bulkWrite([{
updateOne: {
filter: {
async.series([
next => c.updateOne(
{
// Can't filter out objects with deletiong flag
// as it will try and recreate an object with the same _id
// instead we reset the flag to false, the data might be
// inconsistent with the current state of the object but
// this is not an issue as the object is in a temporary
// placeholder (PHD) state
_id: masterKey,
},
update: {
$set: { _id: masterKey, value: mst },
},
upsert: true,
},
}, {
deleteOne: {
filter: {
_id: versionKey,
{
$set: {
'_id': masterKey,
'value.isPHD': true,
'value.versionId': _vid,
'value.deleted': false,
},
},
},
}], {
ordered: true,
}, err => {
{ upsert: true },
next,
),
// delete version
next => this.internalDeleteObject(c, bucketName, versionKey, {}, log,
err => {
// we don't return an error in case we don't find
// a version as we expect this case when dealing with
// a versioning suspended object.
if (err && err.is.NoSuchKey) {
return next(null);
}
return next(err);
}),
], err => {
if (err) {
log.error(
'deleteObjectVerMaster: error deleting object',
{ error: err.message });
'deleteObjectVerMaster: error deleting the object',
{ error: err.message, bucket: bucketName, key: objName });
return cb(errors.InternalError);
}
return this.deleteOrRepairPHD(c, bucketName, objName, mst, params.vFormat, log, cb);
return this.deleteOrRepairPHD(c, bucketName, objName, { versionId: _vid }, params.vFormat, log, cb);
});
}

Expand All @@ -1207,19 +1231,17 @@ class MongoClientInterface {
*/
deleteObjectVerNotMaster(c, bucketName, objName, params, log, cb) {
const versionKey = formatVersionKey(objName, params.versionId, params.vFormat);
c.findOneAndDelete({
_id: versionKey,
}, {}, (err, result) => {
this.internalDeleteObject(c, bucketName, versionKey, {}, log, err => {
if (err) {
if (err.is.NoSuchKey) {
log.error(
'deleteObjectVerNotMaster: unable to find target object to delete',
{ error: err.message, bucket: bucketName, key: objName });
return cb(errors.NoSuchKey);
}
log.error(
'findOneAndDelete: error when version is not master',
{ error: err.message });
return cb(errors.InternalError);
}
if (result.ok !== 1) {
log.error(
'findOneAndDelete: failed when version is not master',
{ error: err.message });
'deleteObjectVerNotMaster: error deleting object with no version',
{ error: err.message, bucket: bucketName, key: objName });
return cb(errors.InternalError);
}
return cb(null);
Expand Down Expand Up @@ -1248,10 +1270,14 @@ class MongoClientInterface {
// find the master version
c.findOne({
_id: masterKey,
$or: [
{ 'value.deleted': { $exists: false } },
{ 'value.deleted': { $eq: false } },
],
}, {}, (err, mst) => {
if (err) {
log.error('deleteObjectVer: error deleting versioned object',
{ error: err.message });
{ error: err.message, bucket: bucketName, key: objName });
return cb(errors.InternalError);
}
return next(null, mst);
Expand Down Expand Up @@ -1295,22 +1321,102 @@ class MongoClientInterface {
*/
deleteObjectNoVer(c, bucketName, objName, params, log, cb) {
const masterKey = formatMasterKey(objName, params.vFormat);
c.findOneAndDelete({
_id: masterKey,
}, {}, (err, result) => {
this.internalDeleteObject(c, bucketName, masterKey, {}, log, err => {
if (err) {
// Should not return an error when no object is found
if (err.is.NoSuchKey) {
return cb(null);
}
log.error(
'deleteObjectNoVer: error deleting object with no version',
{ error: err.message });
{ error: err.message, bucket: bucketName, key: objName });
return cb(errors.InternalError);
}
if (result.ok !== 1) {
log.error(
'deleteObjectNoVer: failed deleting object with no version',
{ error: err.message });
return cb(null);
});
}

/**
* Flags the object before deleting it, this is done
* to keep object metadata in the oplog, as oplog delete
* events don't contain any object metadata
* @param {Object} collection MongoDB collection
* @param {string} bucketName bucket name
* @param {string} key Key of the object to delete
* @param {object} filter additional query filters
* @param {Logger}log logger instance
* @param {Function} cb callback containing error
* and BulkWriteResult
* @return {undefined}
*/
internalDeleteObject(collection, bucketName, key, filter, log, cb) {
// filter used when finding and updating object
const findFilter = Object.assign({
_id: key,
$or: [
{ 'value.deleted': { $exists: false } },
{ 'value.deleted': { $eq: false } },
],
}, filter);
// filter used when deleting object
const updateDeleteFilter = Object.assign({
'_id': key,
'value.deleted': true,
}, filter);
async.waterfall([
// Adding delete flag when getting the object
// to avoid having race conditions.
next => collection.findOneAndUpdate(findFilter, {
$set: {
'_id': key,
'value.deleted': true,
},
}, {
upsert: false,
}, (err, doc) => {
if (err) {
log.error('internalDeleteObject: error getting object',
{ bucket: bucketName, object: key, error: err.message });
return next(errors.InternalError);
}
if (!doc.value) {
log.error('internalDeleteObject: unable to find target object to delete',
{ bucket: bucketName, object: key });
return next(errors.NoSuchKey);
}
const obj = doc.value;
const objMetadata = new ObjectMD(obj.value);
objMetadata.setOriginOp('s3:ObjectRemoved:Delete');
objMetadata.setDeleted(true);
return next(null, objMetadata.getValue());
}),
// We update the full object to get the whole object metadata
// in the oplog update event
(objMetadata, next) => collection.bulkWrite([
{
updateOne: {
filter: updateDeleteFilter,
update: {
$set: { _id: key, value: objMetadata },
},
upsert: false,
},
}, {
deleteOne: {
filter: updateDeleteFilter,
},
},
], { ordered: true }, () => next(null)),
], (err, res) => {
if (err) {
if (err.is.NoSuchKey) {
return cb(err);
}
log.error('internalDeleteObject: error deleting object',
{ bucket: bucketName, object: key, error: err.message });
return cb(errors.InternalError);
}
return cb(null);
return cb(null, res);
});
}

Expand Down Expand Up @@ -2129,7 +2235,7 @@ class MongoClientInterface {
return cb(err);
}
const masterKey = formatMasterKey(objName, vFormat);
const filter = { _id: masterKey };
const filter = {};
try {
MongoUtils.translateConditions(0, 'value', filter,
params.conditions);
Expand All @@ -2139,33 +2245,25 @@ class MongoClientInterface {
});
return cb(errors.InternalError);
}
return c.findOneAndDelete(filter, (err, res) => {
if (err) {
log.error('error occurred when attempting to delete object', {
method,
error: err.message,
});
return cb(errors.InternalError);
}
if (res.ok !== 1) {
log.error('failed to delete object', {
method,
error: err.message,
});
return cb(errors.InternalError);
}
/*
* unable to find an object that matches the conditions
*/
if (!res.value) {
log.debug('unable to find target object to delete', {
method,
filter,
});
return cb(errors.NoSuchKey);
}
return cb();
});
return this.internalDeleteObject(c, bucketName, masterKey, filter, log,
err => {
if (err) {
// unable to find an object that matches the conditions
if (err.is.NoSuchKey) {
log.error('unable to find target object to delete', {
method,
filter,
});
return cb(errors.NoSuchKey);
}
log.error('error occurred when attempting to delete object', {
method,
error: err.message,
});
return cb(errors.InternalError);
}
return cb();
});
});
}

Expand Down
Loading

0 comments on commit 7c1bd45

Please sign in to comment.