Skip to content

Bump microVersionId and add isReplica handling #6178

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

Open
maeldonn wants to merge 5 commits into
base: development/9.4
Choose a base branch
from
Open

Bump microVersionId and add isReplica handling #6178

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
0ea9851
Reformat with prettier
maeldonn Jun 3, 2026
36b204a
Bump arsenal dependency
maeldonn May 28, 2026
585f37b
Use isReplica field for x-amz-replication-status response header
maeldonn May 28, 2026
a04afdb
Bump microVersionId on object metadata writes
maeldonn May 28, 2026
2893d63
Clear isReplica on direct user writes overwriting a replica
maeldonn May 28, 2026
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
27 changes: 27 additions & 0 deletions lib/api/apiUtils/object/bumpMicroVersionId.js
View file
Open in desktop

@SylvainSenechal SylvainSenechal Jun 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit : I think putting it in versioning.js instead of creating a new file could have been enough 🤔

@maeldonn maeldonn Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather keep it as its own file: versioning.js is already 585 lines and handles versionId semantics, whereas microVersionId is a separate revision marker (used by cascaded CRR loop detection). Six call sites import it, so a focused helper file felt clearer than growing versioning.js. Happy to move it if you feel strongly though.

Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
const { versioning } = require('arsenal');
const { config } = require('../../../Config');

/**
* Bump objectMD.microVersionId. microVersionId is a generic
* metadata-revision marker, not a CRR-specific field, but cascaded CRR
* is its only consumer today - so we gate on replicationInfo to avoid
* inflating storage on objects that wouldn't use it. The gate can be
* widened later if another consumer needs it on every object.
* Pass `force = true` to bump unconditionally.
*
* @param {object} objectMD - object MD POJO or `md.getValue()`
* @param {boolean} [force] - bump even without replicationInfo
* @return {undefined}
*/
function bumpMicroVersionId(objectMD, force) {
if (!force && !objectMD?.replicationInfo) {
return;
}

const { instanceId, replicationGroupId } = config;

// eslint-disable-next-line no-param-reassign
objectMD.microVersionId = versioning.VersionID.generateVersionId(instanceId, replicationGroupId);
Comment thread
francoisferrand marked this conversation as resolved.
}

module.exports = bumpMicroVersionId;
1 change: 1 addition & 0 deletions lib/api/apiUtils/object/getReplicationInfo.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ function getReplicationInfo(s3config, objKey, bucketMD, isMD, objSize, operation
content,
role: ReplicationConfiguration.resolveSourceRole(config.role),
isNFS: bucketMD.isNFS(),
isReplica: false,

@SylvainSenechal SylvainSenechal Jun 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mixed feeling on this : I say let's keep it, it makes sense as we return status pending, but also weird because we have objectMd available in the function, so we should return objectMd.isReplica 🤔

@maeldonn maeldonn Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intentional, same as hardcoding status: 'PENDING': this builds a fresh outgoing replicationInfo for a user write, which by definition isn't a replica. The hardcoded false is precisely what flips the previous isReplica: true to false when merged onto objectMD.replicationInfo at the call sites. Reading objectMD.isReplica would defeat that.

@SylvainSenechal SylvainSenechal Jun 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true, other fields are also hardcoded

@francoisferrand francoisferrand Jun 19, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: since we need to keep "backwards" compatibility (i.e. support the missing isReplica field), can we / should we skip this field when set to false, to spare a few bytes in mongo?

};
}

Expand Down
171 changes: 88 additions & 83 deletions lib/api/apiUtils/object/versioning.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@
const versionIdUtils = versioning.VersionID;
// Use Arsenal function to generate a version ID used internally by metadata
// for null versions that are created before bucket versioning is configured
const nonVersionedObjId =
versionIdUtils.getInfVid(config.replicationGroupId);
const nonVersionedObjId = versionIdUtils.getInfVid(config.replicationGroupId);

/** decodeVID - decode the version id
* @param {string} versionId - version ID
Expand Down Expand Up @@ -101,25 +100,24 @@
nullVersionMD.originOp = 's3:StoreNullVersion';
metadata.putObjectMD(bucketName, objKey, nullVersionMD, { versionId }, log, err => {
if (err) {
log.debug('error from metadata storing null version as new version',
{ error: err });
log.debug('error from metadata storing null version as new version', { error: err });
}

cb(err);
});
}

/** check existence and get location of null version data for deletion
* @param {string} bucketName - name of bucket
* @param {string} objKey - name of object key
* @param {object} options - metadata options for getting object MD
* @param {string} options.versionId - version to get from metadata
* @param {object} mst - info about the master version
* @param {string} mst.versionId - the master version's version id
* @param {RequestLogger} log - logger instanceof
* @param {function} cb - callback
* @return {undefined} - and call callback with (err, dataToDelete)
*/
* @param {string} bucketName - name of bucket
* @param {string} objKey - name of object key
* @param {object} options - metadata options for getting object MD
* @param {string} options.versionId - version to get from metadata
* @param {object} mst - info about the master version
* @param {string} mst.versionId - the master version's version id
* @param {RequestLogger} log - logger instanceof
* @param {function} cb - callback
* @return {undefined} - and call callback with (err, dataToDelete)
*/
function _prepareNullVersionDeletion(bucketName, objKey, options, mst, log, cb) {
const nullOptions = {};
if (!options.deleteData) {
Expand All @@ -135,38 +133,40 @@
// PUT via this option
nullOptions.deleteNullKey = true;
}
return metadata.getObjectMD(bucketName, objKey, options, log,
(err, versionMD) => {
if (err) {
// the null key may not exist, hence it's a normal
// situation to have a NoSuchKey error, in which case
// there is nothing to delete
if (err.is.NoSuchKey) {
log.debug('null version does not exist', {
method: '_prepareNullVersionDeletion',
});
} else {
log.warn('could not get null version metadata', {
error: err,
method: '_prepareNullVersionDeletion',
});
}
return cb(err);
}
if (versionMD.location) {
const dataToDelete = Array.isArray(versionMD.location) ?
versionMD.location : [versionMD.location];
nullOptions.dataToDelete = dataToDelete;
return metadata.getObjectMD(bucketName, objKey, options, log, (err, versionMD) => {
if (err) {
// the null key may not exist, hence it's a normal
// situation to have a NoSuchKey error, in which case
// there is nothing to delete
if (err.is.NoSuchKey) {
log.debug('null version does not exist', {
method: '_prepareNullVersionDeletion',
});
} else {
log.warn('could not get null version metadata', {
error: err,
method: '_prepareNullVersionDeletion',
});
}
return cb(null, nullOptions);
});
return cb(err);
}
if (versionMD.location) {
const dataToDelete = Array.isArray(versionMD.location) ? versionMD.location : [versionMD.location];
nullOptions.dataToDelete = dataToDelete;
}
return cb(null, nullOptions);
});
}

function _deleteNullVersionMD(bucketName, objKey, options, log, cb) {
return metadata.deleteObjectMD(bucketName, objKey, options, log, err => {
if (err) {
log.warn('metadata error deleting null versioned key',
{ bucketName, objKey, error: err, method: '_deleteNullVersionMD' });
log.warn('metadata error deleting null versioned key', {
bucketName,
objKey,
error: err,
method: '_deleteNullVersionMD',
});
}
return cb(err);
});
Expand All @@ -193,7 +193,7 @@
version key, if needed
*/
function processVersioningState(mst, vstat, nullVersionCompatMode) {
const versioningSuspended = (vstat === 'Suspended');
const versioningSuspended = vstat === 'Suspended';
const masterIsNull = mst.exists && (mst.isNull || !mst.versionId);

if (versioningSuspended) {
Expand Down Expand Up @@ -244,7 +244,7 @@
if (masterIsNull) {
// if master is a null version or a non-versioned key,
// copy it to a new null key
const nullVersionId = (mst.isNull && mst.versionId) ? mst.versionId : nonVersionedObjId;
const nullVersionId = mst.isNull && mst.versionId ? mst.versionId : nonVersionedObjId;
if (nullVersionCompatMode) {
options.extraMD = {
nullVersionId,
Expand Down Expand Up @@ -311,8 +311,7 @@
};

if (objMD.location) {
mst.objLocation = Array.isArray(objMD.location) ?
objMD.location : [objMD.location];
mst.objLocation = Array.isArray(objMD.location) ? objMD.location : [objMD.location];
}

return mst;
Expand All @@ -332,61 +331,67 @@
* options.versioning - (true/undefined) metadata instruction to create new ver
* options.isNull - (true/undefined) whether new version is null or not
*/
function versioningPreprocessing(bucketName, bucketMD, objectKey, objMD,
log, callback) {
function versioningPreprocessing(bucketName, bucketMD, objectKey, objMD, log, callback) {
const mst = getMasterState(objMD);
const vCfg = bucketMD.getVersioningConfiguration();

if (!vCfg) {
const options = { dataToDelete: mst.objLocation };
return process.nextTick(callback, null, options);
}

const { options, nullVersionId, delOptions } =
processVersioningState(mst, vCfg.Status, config.nullVersionCompatMode);

return async.series([
function storeNullVersionMD(next) {
if (!nullVersionId) {
return process.nextTick(next);
}
const { options, nullVersionId, delOptions } = processVersioningState(
mst,
vCfg.Status,
config.nullVersionCompatMode,
);

return async.series(
[
function storeNullVersionMD(next) {
if (!nullVersionId) {
return process.nextTick(next);
}

options.nullVersionId = nullVersionId;
return _storeNullVersionMD(bucketName, objectKey, nullVersionId, objMD, log, next);
},
function prepareNullVersionDeletion(next) {
if (!delOptions) {
return process.nextTick(next);
}
return _prepareNullVersionDeletion(
bucketName, objectKey, delOptions, mst, log,
(err, nullOptions) => {
options.nullVersionId = nullVersionId;
return _storeNullVersionMD(bucketName, objectKey, nullVersionId, objMD, log, next);
},
function prepareNullVersionDeletion(next) {
Comment thread
maeldonn marked this conversation as resolved.
Dismissed
if (!delOptions) {
return process.nextTick(next);
}
return _prepareNullVersionDeletion(bucketName, objectKey, delOptions, mst, log, (err, nullOptions) => {
if (err) {
return next(err);
}
Object.assign(options, nullOptions);
return next();
});
},
function deleteNullVersionMD(next) {
if (delOptions &&
delOptions.versionId &&
delOptions.versionId !== 'null') {
// backward-compat: delete old null versioned key
return _deleteNullVersionMD(
bucketName, objectKey, { versionId: delOptions.versionId, overheadField }, log, next);
},
function deleteNullVersionMD(next) {
Comment thread
maeldonn marked this conversation as resolved.
Dismissed
if (delOptions && delOptions.versionId && delOptions.versionId !== 'null') {
// backward-compat: delete old null versioned key
return _deleteNullVersionMD(
bucketName,
objectKey,
{ versionId: delOptions.versionId, overheadField },
log,
next,
);
}
return process.nextTick(next);
},
],
Comment thread
maeldonn marked this conversation as resolved.
Dismissed
err => {
// it's possible there was a prior request that deleted the
// null version, so proceed with putting a new version
if (err && err.is.NoSuchKey) {
return callback(null, options);
}
return process.nextTick(next);
return callback(err, options);
},
], err => {
// it's possible there was a prior request that deleted the
// null version, so proceed with putting a new version
if (err && err.is.NoSuchKey) {
return callback(null, options);
}
return callback(err, options);
});
);
}

Check notice

Code scanning / CodeQL

Callback-style function (async migration) Note

This function uses a callback parameter ('callback'). Refactor to async/await.

/** Return options to pass to Metadata layer for version-specific
* operations with the given requested version ID
Expand Down Expand Up @@ -545,7 +550,7 @@
restoreRequestedAt: objMD.archive?.restoreRequestedAt,
restoreRequestedDays: objMD.archive?.restoreRequestedDays,
restoreCompletedAt: new Date(now),
restoreWillExpireAt: new Date(now + (days * scaledMsPerDay)),
restoreWillExpireAt: new Date(now + days * scaledMsPerDay),
};

/* eslint-enable no-param-reassign */
Expand Down
Loading
Loading