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

Jump to bottom

Feature: ARSN-235 update object before deleting it #1944

Merged
merged 5 commits into from
Nov 14, 2022
Merged

Feature: ARSN-235 update object before deleting it #1944

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fd63b85
feature: ARSN-235 update object before deletion
Kerkesni Jun 30, 2022
d852eef
feature: ARSN-235 ignore objects flagged for deletion when getting ob…
Kerkesni Jun 30, 2022
b61138a
feature: ARSN-235 ignore objects flagged for deletion when listing ob…
Kerkesni Jun 30, 2022
ef024dd
feature: ARSN-235 fix unit tests
Kerkesni Jun 30, 2022
9a97572
feature: ARSN-235 document oplog
Kerkesni Oct 28, 2022
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
262 changes: 180 additions & 82 deletions lib/storage/metadata/mongoclient/MongoClientInterface.js
View file
Open in desktop
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: {
Kerkesni marked this conversation as resolved.
Show resolved Hide resolved
'_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,
francoisferrand marked this conversation as resolved.
Show resolved Hide resolved
$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([
francoisferrand marked this conversation as resolved.
Show resolved Hide resolved
// Adding delete flag when getting the object
// to avoid having race conditions.
next => collection.findOneAndUpdate(findFilter, {
$set: {
'_id': key,
'value.deleted': true,
Kerkesni marked this conversation as resolved.
Show resolved Hide resolved
},
}, {
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);
Kerkesni marked this conversation as resolved.
Show resolved Hide resolved
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: {
Kerkesni marked this conversation as resolved.
Show resolved Hide resolved
francoisferrand marked this conversation as resolved.
Show resolved Hide resolved
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