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

Fix triggering replication when multiple destinations are set #331

Open
wants to merge 1 commit into
base: development/1.15
Choose a base branch
from
+298 −16
Open

Fix triggering replication when multiple destinations are set #331

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
56 changes: 40 additions & 16 deletions CRR/ReplicationStatusUpdater.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,16 +114,19 @@ class ReplicationStatusUpdater {
* Determines if an object should be updated based on its replication metadata properties.
* @private
* @param {ObjectMD} objMD - The metadata of the object.
* @param {string} site - The destination site name.
* @returns {boolean} True if the object should be updated.
*/
_objectShouldBeUpdated(objMD) {
_objectShouldBeUpdated(objMD, site) {
return this.replicationStatusToProcess.some(filter => {
if (filter === 'NEW') {
// Either site specific replication info is missing
// or are initialized with empty fields.
return (!objMD.getReplicationInfo()
|| objMD.getReplicationInfo().status === '');
|| !objMD.getReplicationSiteStatus(site));
}
return (objMD.getReplicationInfo()
&& objMD.getReplicationInfo().status === filter);
&& objMD.getReplicationSiteStatus(site) === filter);
});
}

Expand Down Expand Up @@ -172,36 +175,54 @@ class ReplicationStatusUpdater {
// codebase easier to maintain and upgrade, as opposed to having multiple branches or versions of
// the code for different schema versions.
objMD = new ObjectMD(JSON.parse(mdRes.Body));
if (!this._objectShouldBeUpdated(objMD)) {
if (!this._objectShouldBeUpdated(objMD, storageClass)) {
skip = true;
return process.nextTick(next);
}
// Initialize replication info, if missing
// This is particularly important if the object was created before
// enabling replication on the bucket.
if (!objMD.getReplicationInfo()
|| !objMD.getReplicationSiteStatus(storageClass)) {
let replicationInfo = objMD.getReplicationInfo();
if (!replicationInfo || !replicationInfo.status) {
const { Rules, Role } = repConfig;
const destination = Rules[0].Destination.Bucket;
Copy link
Contributor

@francoisferrand francoisferrand Dec 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

this may not be correct, if there is more than 1 desitination....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

crrExistingObjects doesn't currently support triggering replication for multiple sites at once. So this would work as it will initialize the replication info when triggering the first site and then append the other sites' info to the fields when the other ones are triggered.

Copy link
Contributor

Choose a reason for hiding this comment

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

even without triggering multiple sites at once : if a user requests replicatoin to the second "site", this will initialize with the 1st destination (--> trigger replication!), then we will add the second one...

When there is no replicationInfo (i.e. typically when it is empty), should we not just initialize an empty replicationInfo, and let the next block ("Update replication info with site specific info") fill the details for the requested destination?

Copy link
Contributor

@francoisferrand francoisferrand Dec 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

in cloudserver, it seems this is initialized to bucketMD.replicationConfig.destination : should we do the same?

Copy link
Contributor Author

@Kerkesni Kerkesni Dec 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

This is a weird one... So, we currently only support having a single destination bucket for all replication rules of a bucket. When creating the rules/workflows via UI it's even worse, the destination bucket becomes the name of the bucket we are replicating from.
The value of this field is not used in Zenko. When creating a location, Cloudserver initializes a client class for the respective backend (aws/azure/...) that keeps the name of the destination bucket in memory, that's the value we use when replicating and not what's in the replication rule (only the storageClass is used to know which client to use).

// set replication properties
const ops = objMD.getContentLength() === 0 ? ['METADATA']
: ['METADATA', 'DATA'];
const backends = [{
site: storageClass,
status: 'PENDING',
dataStoreVersionId: '',
}];
const replicationInfo = {
replicationInfo = {
status: 'PENDING',
backends,
content: ops,
backends: [],
destination,
storageClass,
storageClass: '',
role: Role,
storageType: this.storageType,
storageType: '',
Comment on lines +197 to +199
Copy link
Contributor

Choose a reason for hiding this comment

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

what are these 2 fields (storageClass and storageType) used for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

storageClass is what each location's replication queue processor uses to check if it should replicate an object or not.
storageType is used by Cloudserver in Backbeat routes to do some pre-checks (check if versioning is supported on the backend and that the location is valid)

Copy link
Contributor

@francoisferrand francoisferrand Dec 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

that is how these fields are used... my question is really what information do they actually store, what these fields represent (for example, the storageClass of the object is already known, and stored in .location[] and dataStoreName ; and this cannot be the storageClass of the 'remote' object, since there may be multiple destinations...)

same for destination btw, I don't understand this field initialize with a single bucket name :-/ Or maybe this is a left-over from the first replication (not supporting multi-targets)?

Copy link
Contributor

@francoisferrand francoisferrand Dec 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

the code which initializes these in cloudserver:

function _getReplicationInfo(rule, replicationConfig, content, operationType,
    objectMD, bucketMD) {
    const storageTypes = [];
    const backends = [];
    const storageClasses = _getStorageClasses(rule);
    if (!storageClasses) {
        return undefined;
    }
    storageClasses.forEach(storageClass => {
        const storageClassName =
              storageClass.endsWith(':preferred_read') ?
              storageClass.split(':')[0] : storageClass;
        const location = s3config.locationConstraints[storageClassName];
        if (location && replicationBackends[location.type]) {
            storageTypes.push(location.type);
        }
        backends.push(_getBackend(objectMD, storageClassName));
    });
    if (storageTypes.length > 0 && operationType) {
        content.push(operationType);
    }
    return {
        status: 'PENDING',
        backends,
        content,
        destination: replicationConfig.destination,
        storageClass: storageClasses.join(','),
        role: replicationConfig.role,
        storageType: storageTypes.join(','),
        isNFS: bucketMD.isNFS(),
    };
}

--> it seems these fields (along with role) are not multi-destination/rule aware?
--> these should be mostly useless for Zenko's multibackend replication (otherwise we probably have bugs), but may still be needed for CRR (and may cause bugs if multiple rules/destinations were used in that case)

Copy link
Contributor Author

@Kerkesni Kerkesni Dec 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

storageClass and storageType are multi-destination aware (although their name suggests otherwise), they contain the list of all storage classes and storage types we are replicating to.
Example:

storageClass: "aws-location,azure-blob"
storageType: "aws_s3,azure"

In Zenko, these fields are duplicates of information we already have, as backbeat/cloudserver have a list of all location information, we could just use the storage class stored in the rules to get the info we want. I think these are more of a relic from S3C that we can't really remove right now as S3C uses them.

In CRR, the role is also a list. I don't think this works in Zenko tho.
The destination field is a weird one, i explained how it works in the previous comment

};
objMD.setReplicationInfo(replicationInfo);
}
// Update replication info with site specific info
if (objMD.getReplicationSiteStatus(storageClass) === undefined) {
// When replicating to multiple destinations,
// the storageClass and storageType properties
// become comma-separated lists of the storage
// classes and types of the replication destinations.
const storageClasses = objMD.getReplicationStorageClass()
? `${objMD.getReplicationStorageClass()},${storageClass}` : storageClass;
objMD.setReplicationStorageClass(storageClasses);
if (this.storageType) {
const storageTypes = objMD.getReplicationStorageType()
? `${objMD.getReplicationStorageType()},${this.storageType}` : this.storageType;
objMD.setReplicationStorageType(storageTypes);
}
// Add site to the list of replication backends
const backends = objMD.getReplicationBackends();
backends.push({
site: storageClass,
status: 'PENDING',
dataStoreVersionId: '',
});
objMD.setReplicationBackends(backends);
}

objMD.setReplicationSiteStatus(storageClass, 'PENDING');
objMD.setReplicationStatus('PENDING');
Expand Down Expand Up @@ -273,13 +294,16 @@ class ReplicationStatusUpdater {
}),
(repConfig, next) => {
const { Rules } = repConfig;
const storageClass = Rules[0].Destination.StorageClass || this.siteName;
const storageClass = this.siteName || Rules[0].Destination.StorageClass;
if (!storageClass) {
const errMsg = 'missing SITE_NAME environment variable, must be set to'
+ ' the value of "site" property in the CRR configuration';
this.log.error(errMsg);
return next(new Error(errMsg));
}
if (!this.siteName) {
this.log.warn(`missing SITE_NAME environment variable, triggering replication to the ${storageClass} storage class`);
}
return eachLimit(versions, this.workers, (i, apply) => {
const { Key, VersionId } = i;
this._markObjectPending(bucket, Key, VersionId, storageClass, repConfig, apply);
Expand Down
258 changes: 258 additions & 0 deletions tests/unit/CRR/ReplicationStatusUpdater.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ describe('ReplicationStatusUpdater', () => {
replicationStatusToProcess: ['NEW'],
targetPrefix: 'toto',
listingLimit: 10,
siteName: 'aws-location',
}, logger);
});

Expand Down Expand Up @@ -129,6 +130,263 @@ describe('ReplicationStatusUpdater', () => {
return done();
});
});

[
{
description: 'for an object with a null replication info',
replicationInfo: null,
replicationStatusToProcess: ['NEW'],
expectedReplicationInfo: {
status: 'PENDING',
backends: [
{
site: 'aws-location',
status: 'PENDING',
dataStoreVersionId: '',
},
],
content: ['METADATA', 'DATA'],
destination: 'arn:aws:s3:::sourcebucket',
storageClass: 'aws-location',
role: 'arn:aws:iam::root:role/s3-replication-role',
storageType: 'aws_s3',
dataStoreVersionId: '',
isNFS: null,
},
}, {
description: 'for an object with empty replication info',
replicationInfo: {
status: '',
backends: [],
content: [],
destination: '',
storageClass: '',
role: '',
storageType: '',
dataStoreVersionId: '',
isNFS: null,
},
replicationStatusToProcess: ['NEW'],
expectedReplicationInfo: {
status: 'PENDING',
backends: [
{
site: 'aws-location',
status: 'PENDING',
dataStoreVersionId: '',
},
],
content: ['METADATA', 'DATA'],
destination: 'arn:aws:s3:::sourcebucket',
storageClass: 'aws-location',
role: 'arn:aws:iam::root:role/s3-replication-role',
storageType: 'aws_s3',
dataStoreVersionId: '',
isNFS: null,
},
}, {
description: 'for an object with a failed replication',
replicationInfo: {
status: 'FAILED',
backends: [
{
site: 'aws-location',
status: 'FAILED',
dataStoreVersionId: '',
},
],
content: ['METADATA', 'DATA'],
destination: 'arn:aws:s3:::sourcebucket',
storageClass: 'aws-location',
role: 'arn:aws:iam::root:role/s3-replication-role',
storageType: 'aws_s3',
dataStoreVersionId: '',
isNFS: null,
},
replicationStatusToProcess: ['FAILED'],
expectedReplicationInfo: {
status: 'PENDING',
backends: [
{
site: 'aws-location',
status: 'PENDING',
dataStoreVersionId: '',
},
],
content: ['METADATA', 'DATA'],
destination: 'arn:aws:s3:::sourcebucket',
storageClass: 'aws-location',
role: 'arn:aws:iam::root:role/s3-replication-role',
storageType: 'aws_s3',
dataStoreVersionId: '',
isNFS: null,
},
}, {
description: 'for an object with a completed replication',
replicationInfo: {
status: 'COMPLETED',
backends: [
{
site: 'aws-location',
status: 'COMPLETED',
dataStoreVersionId: '',
},
],
content: ['METADATA', 'DATA'],
destination: 'arn:aws:s3:::sourcebucket',
storageClass: 'aws-location',
role: 'arn:aws:iam::root:role/s3-replication-role',
storageType: 'aws_s3',
dataStoreVersionId: '',
isNFS: null,
},
replicationStatusToProcess: ['COMPLETED'],
expectedReplicationInfo: {
status: 'PENDING',
backends: [
{
site: 'aws-location',
status: 'PENDING',
dataStoreVersionId: '',
},
],
content: ['METADATA', 'DATA'],
destination: 'arn:aws:s3:::sourcebucket',
storageClass: 'aws-location',
role: 'arn:aws:iam::root:role/s3-replication-role',
storageType: 'aws_s3',
dataStoreVersionId: '',
isNFS: null,
},
}, {
description: 'of a single site for an object with multiple replication destinations',
replicationInfo: {
status: 'FAILED',
backends: [
{
site: 'azure-location',
status: 'COMPLETED',
dataStoreVersionId: '',
},
{
site: 'aws-location',
status: 'FAILED',
dataStoreVersionId: '',
},
],
content: ['METADATA', 'DATA'],
destination: 'arn:aws:s3:::sourcebucket',
storageClass: 'azure-location,aws-location',
role: 'arn:aws:iam::root:role/s3-replication-role',
storageType: 'azure,aws_s3',
dataStoreVersionId: '',
isNFS: null,
},
replicationStatusToProcess: ['FAILED'],
expectedReplicationInfo: {
status: 'PENDING',
backends: [
{
site: 'azure-location',
status: 'COMPLETED',
dataStoreVersionId: '',
}, {
site: 'aws-location',
status: 'PENDING',
dataStoreVersionId: '',
},
],
content: ['METADATA', 'DATA'],
destination: 'arn:aws:s3:::sourcebucket',
storageClass: 'azure-location,aws-location',
role: 'arn:aws:iam::root:role/s3-replication-role',
storageType: 'azure,aws_s3',
dataStoreVersionId: '',
isNFS: null,
},
}, {
description: 'of a single non initialized site for an object with multiple replication destinations',
replicationInfo: {
status: 'FAILED',
backends: [
{
site: 'azure-location',
status: 'COMPLETED',
dataStoreVersionId: '',
},
{
site: 'azure-location-2',
status: 'FAILED',
dataStoreVersionId: '',
},
],
content: ['METADATA', 'DATA'],
destination: 'arn:aws:s3:::sourcebucket',
storageClass: 'azure-location,azure-location-2',
role: 'arn:aws:iam::root:role/s3-replication-role',
storageType: 'azure,azure',
dataStoreVersionId: '',
isNFS: null,
},
replicationStatusToProcess: ['NEW'],
expectedReplicationInfo: {
status: 'PENDING',
backends: [
{
site: 'azure-location',
status: 'COMPLETED',
dataStoreVersionId: '',
}, {
site: 'azure-location-2',
status: 'FAILED',
dataStoreVersionId: '',
}, {
site: 'aws-location',
status: 'PENDING',
dataStoreVersionId: '',
},
],
content: ['METADATA', 'DATA'],
destination: 'arn:aws:s3:::sourcebucket',
storageClass: 'azure-location,azure-location-2,aws-location',
role: 'arn:aws:iam::root:role/s3-replication-role',
storageType: 'azure,azure,aws_s3',
dataStoreVersionId: '',
isNFS: null,
},
},
].forEach(params => {
it(`should trigger replication ${params.description}`, done => {
crr.bb.getMetadata = jest.fn((p, cb) => {
const objectMd = JSON.parse(getMetadataRes.Body);
objectMd.replicationInfo = params.replicationInfo;
cb(null, { Body: JSON.stringify(objectMd) });
});
crr.siteName = 'aws-location';
crr.storageType = 'aws_s3';
crr.replicationStatusToProcess = params.replicationStatusToProcess;
crr.run(err => {
assert.ifError(err);

expect(crr.s3.listObjectVersions).toHaveBeenCalledTimes(1);
expect(crr.s3.getBucketReplication).toHaveBeenCalledTimes(1);
expect(crr.bb.getMetadata).toHaveBeenCalledTimes(1);
expect(crr.bb.putMetadata).toHaveBeenCalledTimes(1);
expect(crr.bb.putMetadata).toHaveBeenCalledWith(
expect.objectContaining({
Body: expect.stringContaining(JSON.stringify(params.expectedReplicationInfo)),
}),
expect.any(Function),
);

assert.strictEqual(crr._nProcessed, 1);
assert.strictEqual(crr._nSkipped, 0);
assert.strictEqual(crr._nUpdated, 1);
assert.strictEqual(crr._nErrors, 0);
return done();
});
});
});
});

describe('ReplicationStatusUpdater with specifics', () => {
Expand Down
Loading