Skip to content

Commit 8cd19f1

Browse files
Share the transition attempt metadata helper
The x-amz-meta-scal-s3-transition-attempt key was open-coded in six places across lifecycle, gc and replication, each with a slightly different way of reading or clearing it - one of which would throw on user metadata that does not parse. Move it behind a small helper. Issue: BB-814
1 parent e69f2cc commit 8cd19f1

7 files changed

Lines changed: 73 additions & 58 deletions

File tree

extensions/gc/tasks/GarbageCollectorTask.js

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ const { ObjectMD } = require('arsenal').models;
55
const BackbeatTask = require('../../../lib/tasks/BackbeatTask');
66
const { BatchDeleteCommand } = require('@scality/cloudserverclient');
77
const { GarbageCollectorMetrics } = require('../GarbageCollectorMetrics');
8+
const { clearTransitionAttempt } = require('../../../lib/util/transitionAttempt');
89
/** @typedef { import('../GarbageCollector.js') } GarbageCollector */
910

1011
class GarbageCollectorTask extends BackbeatTask {
@@ -290,10 +291,8 @@ class GarbageCollectorTask extends BackbeatTask {
290291
.setDataStoreName(newLocation)
291292
.setAmzStorageClass(newLocation)
292293
.setOriginOp('s3:LifecycleTransition')
293-
.setTransitionInProgress(false)
294-
.setUserMetadata({
295-
'x-amz-meta-scal-s3-transition-attempt': undefined,
296-
});
294+
.setTransitionInProgress(false);
295+
clearTransitionAttempt(objMD);
297296
this._putMetadata(entry, objMD, log, err => {
298297
GarbageCollectorMetrics.onS3Request(log, 'putMetadata', 'archive', err);
299298
if (!err) {

extensions/lifecycle/tasks/LifecycleColdStatusArchiveTask.js

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ const ObjectMDArchive = require('arsenal').models.ObjectMDArchive;
44
const ActionQueueEntry = require('../../../lib/models/ActionQueueEntry');
55
const LifecycleUpdateTransitionTask = require('./LifecycleUpdateTransitionTask');
66
const { LifecycleMetrics } = require('../LifecycleMetrics');
7+
const { clearTransitionAttempt } = require('../../../lib/util/transitionAttempt');
78

89
class SkipMdUpdateError extends Error {}
910

@@ -115,10 +116,8 @@ class LifecycleColdStatusArchiveTask extends LifecycleUpdateTransitionTask {
115116
objectMD.setDataStoreName(coldLocation)
116117
.setAmzStorageClass(coldLocation)
117118
.setTransitionInProgress(false)
118-
.setOriginOp('s3:LifecycleTransition')
119-
.setUserMetadata({
120-
'x-amz-meta-scal-s3-transition-attempt': undefined,
121-
});
119+
.setOriginOp('s3:LifecycleTransition');
120+
clearTransitionAttempt(objectMD);
122121
}
123122

124123
this._putMetadata(entry, objectMD, log, err => {

extensions/lifecycle/tasks/LifecycleResetTransitionInProgressTask.js

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
'use strict';
22

33
const { LifecycleRequeueTask } = require('./LifecycleRequeueTask');
4+
const { setTransitionAttempt } = require('../../../lib/util/transitionAttempt');
45

56
class LifecycleResetTransitionInProgressTask extends LifecycleRequeueTask {
67
/**
@@ -19,9 +20,7 @@ class LifecycleResetTransitionInProgressTask extends LifecycleRequeueTask {
1920
}
2021
md.setOriginOp('s3:LifecycleTransition:Retry');
2122
md.setTransitionInProgress(false);
22-
md.setUserMetadata({
23-
'x-amz-meta-scal-s3-transition-attempt': try_,
24-
});
23+
setTransitionAttempt(md, try_);
2524
return true;
2625
}
2726

extensions/lifecycle/tasks/LifecycleTask.js

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ const ReplicationAPI = require('../../replication/ReplicationAPI');
2424
const { LifecycleMetrics, LIFECYCLE_MARKER_METRICS_LOCATION } = require('../LifecycleMetrics');
2525
const locationsConfig = require('../../../conf/locationConfig.json') || {};
2626
const { rulesSupportTransition } = require('../util/rules');
27+
const { getTransitionAttempt } = require('../../../lib/util/transitionAttempt');
2728
const { stampTraceHeaders } = require('arsenal/build/lib/tracing').kafka;
2829
const { decode } = versioning.VersionID;
2930

@@ -1176,15 +1177,7 @@ class LifecycleTask extends BackbeatTask {
11761177
}
11771178

11781179
_getTransitionActionEntry(params, objectMD, log, cb) {
1179-
let attempt;
1180-
const umd = objectMD.getUserMetadata();
1181-
if (umd) {
1182-
const parsed = JSON.parse(umd);
1183-
const rawAttempt = parsed['x-amz-meta-scal-s3-transition-attempt'];
1184-
if (rawAttempt) {
1185-
attempt = Number.parseInt(rawAttempt, 10);
1186-
}
1187-
}
1180+
const attempt = getTransitionAttempt(objectMD.getUserMetadata());
11881181

11891182
const entry = ReplicationAPI.createCopyLocationAction({
11901183
bucketName: params.bucket,

extensions/lifecycle/tasks/LifecycleUpdateTransitionTask.js

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@ const BackbeatTask = require('../../../lib/tasks/BackbeatTask');
66
const ActionQueueEntry = require('../../../lib/models/ActionQueueEntry');
77
const ObjectMD = require('arsenal').models.ObjectMD;
88
const { LifecycleMetrics, getCopyLocationMetricsType } = require('../LifecycleMetrics');
9+
const {
10+
getTransitionAttempt,
11+
setTransitionAttempt,
12+
clearTransitionAttempt,
13+
} = require('../../../lib/util/transitionAttempt');
914
/** @typedef { import('../objectProcessor/LifecycleObjectProcessor.js') } LifecycleObjectProcessor */
1015

1116
class LifecycleUpdateTransitionTask extends BackbeatTask {
@@ -71,10 +76,8 @@ class LifecycleUpdateTransitionTask extends BackbeatTask {
7176
.setDataStoreName(newLocationName)
7277
.setAmzStorageClass(newLocationName)
7378
.setOriginOp('s3:LifecycleTransition')
74-
.setUserMetadata({
75-
'x-amz-meta-scal-s3-transition-attempt': undefined,
76-
})
7779
.setTransitionInProgress(false);
80+
clearTransitionAttempt(objMD);
7881
}
7982

8083
_putMetadata(entry, objMD, log, done) {
@@ -232,20 +235,10 @@ class LifecycleUpdateTransitionTask extends BackbeatTask {
232235
next(err, objMD);
233236
}),
234237
(objMD, next) => {
235-
const userMDStr = objMD.getUserMetadata() || '{}';
236-
const userMD = JSON.parse(userMDStr);
238+
const tryCount = (getTransitionAttempt(objMD.getUserMetadata()) || 0) + 1;
237239

238-
let tryCount = userMD['x-amz-meta-scal-s3-transition-attempt'];
239-
if (tryCount === undefined) {
240-
tryCount = 1;
241-
} else {
242-
tryCount = parseInt(tryCount, 10) + 1;
243-
}
244-
245-
objMD.setTransitionInProgress(false)
246-
.setUserMetadata({
247-
'x-amz-meta-scal-s3-transition-attempt': tryCount,
248-
});
240+
objMD.setTransitionInProgress(false);
241+
setTransitionAttempt(objMD, tryCount);
249242

250243
return this._putMetadata(entry, objMD, log, next);
251244
},

extensions/replication/ReplicationQueuePopulator.js

Lines changed: 2 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@ const { LifecycleMetrics, LOCALIZATION_TYPE } = require('../lifecycle/LifecycleM
1010
const config = require('../../lib/Config');
1111
const locationsConfig = require('../../conf/locationConfig.json') || {};
1212
const safeJsonParse = require('../../lib/util/safeJsonParse');
13+
const { getTransitionAttempt } = require('../../lib/util/transitionAttempt');
1314
const { traceHeadersFromEntry } = require('arsenal/build/lib/tracing').kafka;
1415

15-
const TRANSITION_ATTEMPT_MD = 'x-amz-meta-scal-s3-transition-attempt';
1616
const { transitionTasksTopic } = config.extensions.lifecycle;
1717

1818
// Where clean room objects are localized when their metadata does not name a
@@ -202,7 +202,7 @@ class ReplicationQueuePopulator extends QueuePopulatorExtension {
202202
contentLength,
203203
resultsTopic: transitionTasksTopic,
204204
transitionTime: transitionTime.toISOString(),
205-
attempt: this._getTransitionAttempt(queueEntry),
205+
attempt: getTransitionAttempt(queueEntry.getUserMetadata()),
206206
});
207207
// 'transition' is what the lifecycle transition processor dispatches
208208
// on to pick up the copyLocation result.
@@ -269,26 +269,6 @@ class ReplicationQueuePopulator extends QueuePopulatorExtension {
269269
return defaultLocalLocation;
270270
}
271271

272-
/**
273-
* Number of times the data mover already tried to copy this object. The
274-
* transition processor bumps the counter on failure, which produces a new
275-
* oplog entry and re-triggers the copy.
276-
* @param {ObjectQueueEntry} queueEntry - parsed entry
277-
* @return {Number|undefined} attempt count, if any
278-
*/
279-
_getTransitionAttempt(queueEntry) {
280-
const umd = queueEntry.getUserMetadata();
281-
if (!umd) {
282-
return undefined;
283-
}
284-
const { error, result } = safeJsonParse(umd);
285-
if (error) {
286-
return undefined;
287-
}
288-
const attempt = Number.parseInt(result[TRANSITION_ATTEMPT_MD], 10);
289-
return Number.isInteger(attempt) ? attempt : undefined;
290-
}
291-
292272
/**
293273
* Filter if the entry is considered a valid master key entry.
294274
* There is a case where a single null entry looks like a master key and

lib/util/transitionAttempt.js

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
const safeJsonParse = require('./safeJsonParse');
2+
3+
// How many times the transition of an object was attempted. Kept in the
4+
// object user metadata so that it survives across processes: the transition
5+
// processor bumps it on failure, and it is cleared once the object made it to
6+
// its new location.
7+
const TRANSITION_ATTEMPT_MD = 'x-amz-meta-scal-s3-transition-attempt';
8+
9+
/**
10+
* Read the transition attempt count from raw object user metadata.
11+
* @param {String|undefined} userMetadata - serialized user metadata, as
12+
* returned by ObjectMD.getUserMetadata()
13+
* @return {Number|undefined} attempt count, or undefined if the object was
14+
* never transitioned, or the metadata cannot be read
15+
*/
16+
function getTransitionAttempt(userMetadata) {
17+
if (!userMetadata) {
18+
return undefined;
19+
}
20+
const { error, result } = safeJsonParse(userMetadata);
21+
if (error) {
22+
return undefined;
23+
}
24+
const attempt = Number.parseInt(result[TRANSITION_ATTEMPT_MD], 10);
25+
return Number.isInteger(attempt) ? attempt : undefined;
26+
}
27+
28+
/**
29+
* Set the transition attempt count on an object.
30+
* @param {ObjectMD} objMD - object metadata to update in place
31+
* @param {Number} attempt - attempt count
32+
* @return {ObjectMD} the updated object metadata
33+
*/
34+
function setTransitionAttempt(objMD, attempt) {
35+
return objMD.setUserMetadata({ [TRANSITION_ATTEMPT_MD]: attempt });
36+
}
37+
38+
/**
39+
* Forget the transition attempt count, once the transition succeeded.
40+
* @param {ObjectMD} objMD - object metadata to update in place
41+
* @return {ObjectMD} the updated object metadata
42+
*/
43+
function clearTransitionAttempt(objMD) {
44+
return setTransitionAttempt(objMD, undefined);
45+
}
46+
47+
module.exports = {
48+
TRANSITION_ATTEMPT_MD,
49+
getTransitionAttempt,
50+
setTransitionAttempt,
51+
clearTransitionAttempt,
52+
};

0 commit comments

Comments
 (0)