From a5df0b576462732f5a34abe60dce93f7f0c6a52f Mon Sep 17 00:00:00 2001 From: Kerkesni Date: Wed, 30 Nov 2022 16:54:09 +0100 Subject: [PATCH 1/4] BB-335 ignore objects with a pending replication status when expiring object (cherry picked from commit 2c647db90f7ee4c082abd5282a31ada17028d436) --- .../tasks/LifecycleDeleteObjectTask.js | 42 ++++++++++++++++++- .../LifecycleDeleteObjectTask.spec.js | 23 ++++++++++ 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/extensions/lifecycle/tasks/LifecycleDeleteObjectTask.js b/extensions/lifecycle/tasks/LifecycleDeleteObjectTask.js index 04e0abcc28..8f45ec6787 100644 --- a/extensions/lifecycle/tasks/LifecycleDeleteObjectTask.js +++ b/extensions/lifecycle/tasks/LifecycleDeleteObjectTask.js @@ -6,6 +6,7 @@ const BackbeatTask = require('../../../lib/tasks/BackbeatTask'); const { attachReqUids } = require('../../../lib/clients/utils'); class ObjectLockedError extends Error {} +class PendingReplicationError extends Error {} class LifecycleDeleteObjectTask extends BackbeatTask { /** @@ -18,9 +19,14 @@ class LifecycleDeleteObjectTask extends BackbeatTask { const procState = proc.getStateVars(); super(); Object.assign(this, procState); + this.objectMD = null; } _getMetadata(entry, log, done) { + // only retreiving object metadata once + if (this.objectMD) { + return done(null, this.objectMD); + } const { owner: canonicalId, accountId } = entry.getAttribute('target'); const backbeatClient = this.getBackbeatClient(canonicalId, accountId); if (!backbeatClient) { @@ -56,7 +62,8 @@ class LifecycleDeleteObjectTask extends BackbeatTask { errors.InternalError. customizeDescription('error parsing metadata blob')); } - return done(null, res.result); + this.objectMD = res.result; + return done(null, this.objectMD); }); } @@ -164,6 +171,33 @@ class LifecycleDeleteObjectTask extends BackbeatTask { }); } + /** + * Throws error if object has a 'PENDING' replication status + * @param {ActionQueueEntry} entry entry object + * @param {Logger} log logger instance + * @param {Function} done callback + * @returns {undefined} + */ + _checkReplicationStatus(entry, log, done) { + const actionType = entry.getActionType(); + // skip check if entry is an incomplete MPU + // as we only replicate complete objects + if (actionType === 'deleteMPU') { + return done(); + } + return this._getMetadata(entry, log, (err, objMD) => { + if (err) { + return done(err); + } + const replicationStatus = objMD.getReplicationStatus(); + if (['PENDING', 'PROCESSING', 'FAILED'].includes(replicationStatus)) { + const error = new PendingReplicationError('object has a pending replication status'); + return done(error); + } + return done(); + }); + } + /** * Execute the action specified in action entry to delete an object * @@ -183,6 +217,7 @@ class LifecycleDeleteObjectTask extends BackbeatTask { return async.series([ next => this._checkDate(entry, log, next), next => this._checkObjectLockState(entry, log, next), + next => this._checkReplicationStatus(entry, log, next), next => this._executeDelete(entry, log, next), ], err => { if (err && err instanceof ObjectLockedError) { @@ -190,6 +225,11 @@ class LifecycleDeleteObjectTask extends BackbeatTask { entry.getLogInfo()); return done(); } + if (err && err instanceof PendingReplicationError) { + log.debug('Object has pending replication status, skipping', + entry.getLogInfo()); + return done(); + } if (err && err.statusCode === 404) { log.debug('Unable to find object to delete, skipping', entry.getLogInfo()); diff --git a/tests/unit/lifecycle/LifecycleDeleteObjectTask.spec.js b/tests/unit/lifecycle/LifecycleDeleteObjectTask.spec.js index ecf81ae5cf..ebd5805dfd 100644 --- a/tests/unit/lifecycle/LifecycleDeleteObjectTask.spec.js +++ b/tests/unit/lifecycle/LifecycleDeleteObjectTask.spec.js @@ -98,6 +98,29 @@ describe('LifecycleDeleteObjectTask', () => { }); }); + [ + 'PENDING', + 'PROCESSING', + 'FAILED', + ].forEach(status => { + it(`should skip replicating object : ${status}`, done => { + objMd.setReplicationStatus('PENDING'); + const entry = ActionQueueEntry.create('deleteObject') + .setAttribute('target.owner', 'testowner') + .setAttribute('target.bucket', 'testbucket') + .setAttribute('target.accountId', 'testid') + .setAttribute('target.key', 'testkey') + .setAttribute('target.version', 'testversion') + .setAttribute('details.lastModified', '2022-05-13T17:51:31.261Z'); + s3Client.setResponse(null, {}); + task.processActionEntry(entry, err => { + assert.strictEqual(s3Client.calls.deleteObject, 0); + assert.ifError(err); + done(); + }); + }); + }); + it('should expire current version of locked object with legal hold', done => { objMd.setLegalHold(true); From 38707c50535ec1c4e0a84bf61c2be64183c868e7 Mon Sep 17 00:00:00 2001 From: Nicolas Humbert Date: Wed, 11 Oct 2023 12:32:07 +0200 Subject: [PATCH 2/4] BB-450 Replication status lifecycle check must support current version --- lib/clients/backbeat-2017-07-01.api.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/clients/backbeat-2017-07-01.api.json b/lib/clients/backbeat-2017-07-01.api.json index 42bb0610f7..cc5fd2bf9e 100644 --- a/lib/clients/backbeat-2017-07-01.api.json +++ b/lib/clients/backbeat-2017-07-01.api.json @@ -611,8 +611,7 @@ "type": "structure", "required": [ "Bucket", - "Key", - "VersionId" + "Key" ], "members": { "Bucket": { From a35704e862b25a85e5834bc9d2b9de525a5798ad Mon Sep 17 00:00:00 2001 From: Nicolas Humbert Date: Wed, 11 Oct 2023 12:34:32 +0200 Subject: [PATCH 3/4] BB-450 fix BucketFileLogReader --- lib/queuePopulator/BucketFileLogReader.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/queuePopulator/BucketFileLogReader.js b/lib/queuePopulator/BucketFileLogReader.js index 3d7b0d0879..dcf5b7dad8 100644 --- a/lib/queuePopulator/BucketFileLogReader.js +++ b/lib/queuePopulator/BucketFileLogReader.js @@ -1,5 +1,5 @@ const arsenal = require('arsenal'); -const MetadataFileClient = arsenal.storage.metadata.MetadataFileClient; +const MetadataFileClient = arsenal.storage.metadata.file.MetadataFileClient; const LogReader = require('./LogReader'); From 2165fd1a87d0f5971832cd0a5fd29398bb767794 Mon Sep 17 00:00:00 2001 From: Nicolas Humbert Date: Wed, 11 Oct 2023 15:25:42 +0200 Subject: [PATCH 4/4] BB-450 fix and add tests --- .../tasks/LifecycleDeleteObjectTask.js | 2 +- .../LifecycleDeleteObjectTask.spec.js | 32 ++++++++++++++++--- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/extensions/lifecycle/tasks/LifecycleDeleteObjectTask.js b/extensions/lifecycle/tasks/LifecycleDeleteObjectTask.js index 8f45ec6787..ee14ef55a4 100644 --- a/extensions/lifecycle/tasks/LifecycleDeleteObjectTask.js +++ b/extensions/lifecycle/tasks/LifecycleDeleteObjectTask.js @@ -190,7 +190,7 @@ class LifecycleDeleteObjectTask extends BackbeatTask { return done(err); } const replicationStatus = objMD.getReplicationStatus(); - if (['PENDING', 'PROCESSING', 'FAILED'].includes(replicationStatus)) { + if (replicationStatus && replicationStatus !== 'COMPLETED') { const error = new PendingReplicationError('object has a pending replication status'); return done(error); } diff --git a/tests/unit/lifecycle/LifecycleDeleteObjectTask.spec.js b/tests/unit/lifecycle/LifecycleDeleteObjectTask.spec.js index ebd5805dfd..6a9c1788a2 100644 --- a/tests/unit/lifecycle/LifecycleDeleteObjectTask.spec.js +++ b/tests/unit/lifecycle/LifecycleDeleteObjectTask.spec.js @@ -99,12 +99,34 @@ describe('LifecycleDeleteObjectTask', () => { }); [ - 'PENDING', - 'PROCESSING', - 'FAILED', + 'COMPLETED', + undefined, ].forEach(status => { - it(`should skip replicating object : ${status}`, done => { - objMd.setReplicationStatus('PENDING'); + it(`should delete replicating object with status: ${status}`, done => { + objMd.setReplicationStatus(status); + const entry = ActionQueueEntry.create('deleteObject') + .setAttribute('target.owner', 'testowner') + .setAttribute('target.bucket', 'testbucket') + .setAttribute('target.accountId', 'testid') + .setAttribute('target.key', 'testkey') + .setAttribute('target.version', 'testversion') + .setAttribute('details.lastModified', '2022-05-13T17:51:31.261Z'); + s3Client.setResponse(null, {}); + task.processActionEntry(entry, err => { + assert.strictEqual(s3Client.calls.deleteObject, 1); + assert.ifError(err); + done(); + }); + }); + }); + + [ + 'PENDING', + 'PROCESSING', + 'FAILED', + ].forEach(status => { + it(`should skip replicating object with status: ${status}`, done => { + objMd.setReplicationStatus(status); const entry = ActionQueueEntry.create('deleteObject') .setAttribute('target.owner', 'testowner') .setAttribute('target.bucket', 'testbucket')