Add CRR Cascaded capabilities by SylvainSenechal · Pull Request #6179 · scality/cloudserver

SylvainSenechal · 2026-05-29T18:00:35Z

Crr cascaded design : https://github.com/scality/citadel/pull/349

Related PRs :
Arsenal : scality/Arsenal#2628
CloudserverClient : scality/cloudserverclient#24
Backbeat : scality/backbeat#2747
S3utils : scality/s3utils#395

bert-e · 2026-05-29T18:00:39Z

Hello sylvainsenechal,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Available options

name	description	privileged	authored
`/after_pull_request`	Wait for the given pull request id to be merged before continuing with the current one.
`/bypass_author_approval`	Bypass the pull request author's approval	⭐
`/bypass_build_status`	Bypass the build and test status	⭐
`/bypass_commit_size`	Bypass the check on the size of the changeset `TBA`	⭐
`/bypass_incompatible_branch`	Bypass the check on the source branch prefix	⭐
`/bypass_jira_check`	Bypass the Jira issue check	⭐
`/bypass_peer_approval`	Bypass the pull request peers' approval	⭐
`/bypass_leader_approval`	Bypass the pull request leaders' approval	⭐
`/approve`	Instruct Bert-E that the author has approved the pull request.		✍️
`/create_pull_requests`	Allow the creation of integration pull requests.
`/create_integration_branches`	Allow the creation of integration branches.
`/no_octopus`	Prevent Wall-E from doing any octopus merge and use multiple consecutive merge instead
`/unanimity`	Change review acceptance criteria from `one reviewer at least` to `all reviewers`
`/wait`	Instruct Bert-E not to run until further notice.

Available commands

name	description	privileged
`/help`	Print Bert-E's manual in the pull request.
`/status`	Print Bert-E's current status in the pull request `TBA`
`/clear`	Remove all comments from Bert-E from the history `TBA`
`/retry`	Re-start a fresh build `TBA`
`/build`	Re-start a fresh build `TBA`
`/force_reset`	Delete integration branches & pull requests, and restart merge process from the beginning.
`/reset`	Try to remove integration branches unless there are commits on them which do not appear on the source branch.

Status report is not available.

SylvainSenechal · 2026-05-29T18:03:07Z

+                bucketName: request.bucketName,
+                objectKey: request.objectKey,
+            });
+            return callback(errors.OperationAborted);


many 409 available in arsenal. we can pick this one, maybe some other ones, or create our own

claude · 2026-05-29T18:04:39Z

        "@hapi/joi": "^17.1.1",
        "@smithy/node-http-handler": "^3.0.0",
-        "arsenal": "git+https://github.com/scality/Arsenal#8.4.2",
+        "arsenal": "file:../Arsenal",


arsenal and @scality/cloudserverclient are pointing to local filesystem paths (file:../Arsenal, file:../cloudserverclient) instead of pinned git tags. This will break installs for anyone who does not have these directories locally, and CI will fail to resolve them.

Suggested change

"arsenal": "file:../Arsenal",

"arsenal": "git+https://github.com/scality/Arsenal#8.4.2",

— Claude Code

claude · 2026-05-29T18:04:46Z

    "devDependencies": {
        "@eslint/compat": "^1.2.2",
-        "@scality/cloudserverclient": "1.0.7",
+        "@scality/cloudserverclient": "file:../cloudserverclient",


Same issue: file:../cloudserverclient should be pinned to a published version or git tag.

— Claude Code

claude · 2026-05-29T18:04:55Z

+});
+
+describe('CRR cascade — putMetadata', () => {
+    it('second write with the same microVersionId returns loop-detected', async () => {


Test names using it() should start with should per project conventions.

— Claude Code

claude · 2026-05-29T18:05:03Z

+            }],
+        },
+    }));
+});


Missing after() hook to clean up the test buckets (TEST_BUCKET, TEST_BUCKET_CRR, DEST_BUCKET). Without cleanup, leftover buckets accumulate across test runs and can cause name collisions or resource leaks.

— Claude Code

bucket/keys names are randomized
I intentionnally didnt add any after to keep the test and file lighter.

Do we want to add after cleanup ?

claude · 2026-05-29T18:05:14Z

Dependency pinning: arsenal and @scality/cloudserverclient in package.json use file: paths pointing to local directories. These must be pinned to git tags or published versions before merge.
- arsenal on line 36: file:../Arsenal → should be a git tag (e.g. 8.4.2)
- @scality/cloudserverclient on line 68: file:../cloudserverclient → should be a versioned reference
Test naming: it() descriptions in tests/functional/backbeat/crrCascade.js should start with should per project conventions.
Test cleanup: No after() hook to delete the three buckets created in before(). Leftover buckets will accumulate across test runs.

Review by Claude Code

SylvainSenechal · 2026-05-29T18:09:19Z

all these worked locally, need to wait for the bumps now

SylvainSenechal · 2026-05-29T18:09:35Z

        "@hapi/joi": "^17.1.1",
        "@smithy/node-http-handler": "^3.0.0",
-        "arsenal": "git+https://github.com/scality/Arsenal#8.4.2",
+        "arsenal": "file:../Arsenal",


codecov · 2026-05-29T18:39:41Z

❌ 29 Tests Failed:

Tests completed	Failed	Passed	Skipped
9151	29	9122	0

View the top 3 failed test(s) by shortest run time

should accept limits equal to (nodes x workers)::Test put bucket rate limit validation against node and worker count should accept limits equal to (nodes x workers)

Stack Traces | 0.004s run time

ifError got unwanted exception: Invalid response body while trying to fetch http://127.0.0.1:8000/putratelimitestbucket/?rate-limit: Premature close

should accept limits greater than (nodes x workers)::Test put bucket rate limit validation against node and worker count should accept limits greater than (nodes x workers)

Stack Traces | 0.004s run time

ifError got unwanted exception: Invalid response body while trying to fetch http://127.0.0.1:8000/putratelimitestbucket/?rate-limit: Premature close

should allow zero as a valid RequestsPerSecond value::Test put bucket rate limit should allow zero as a valid RequestsPerSecond value

Stack Traces | 0.004s run time

ifError got unwanted exception: Invalid response body while trying to fetch http://127.0.0.1:8000/putratelimitestbucket/?rate-limit: Premature close

should return the rate limit config::Test get bucket rate limit should return the rate limit config

Stack Traces | 0.004s run time

ifError got unwanted exception: Invalid response body while trying to fetch http://127.0.0.1:8000/getratelimitestbucket/?rate-limit: Premature close

should update existing rate limit config::Test put bucket rate limit should update existing rate limit config

Stack Traces | 0.004s run time

ifError got unwanted exception: Invalid response body while trying to fetch http://127.0.0.1:8000/putratelimitestbucket/?rate-limit: Premature close

should set the rate limit config::Test put bucket rate limit should set the rate limit config

Stack Traces | 0.005s run time

ifError got unwanted exception: Invalid response body while trying to fetch http://127.0.0.1:8000/putratelimitestbucket/?rate-limit: Premature close

should delete the bucket rate limit config::Test delete bucket rate limit should delete the bucket rate limit config

Stack Traces | 0.008s run time

ifError got unwanted exception: Invalid response body while trying to fetch http://127.0.0.1:8000/deleteratelimitestbucket/?rate-limit: Premature close

should allow a restore if the quota is full but the objet fits with its reserved storage space::quota evaluation with scuba metrics should allow a restore if the quota is full but the objet fits with its reserved storage space

Stack Traces | 0.017s run time

Invalid response body while trying to fetch http://127.0.0.1:8000/quota-test-bucket15/?quota=true: Premature close

should allow writes after deleting data with quotas::quota evaluation with scuba metrics should allow writes after deleting data with quotas

Stack Traces | 0.017s run time

Invalid response body while trying to fetch http://127.0.0.1:8000/quota-test-bucket8/?quota=true: Premature close

should not increase the inflights when the object is being rewritten with a smaller object::quota evaluation with scuba metrics should not increase the inflights when the object is being rewritten with a smaller object

Stack Traces | 0.017s run time

Invalid response body while trying to fetch http://127.0.0.1:8000/quota-test-bucket9/?quota=true: Premature close

should only evaluate quota and not update inflights for PutObject with the x-scal-s3-version-id header::quota evaluation with scuba metrics should only evaluate quota and not update inflights for PutObject with the x-scal-s3-version-id header

Stack Traces | 0.017s run time

Invalid response body while trying to fetch http://127.0.0.1:8000/quota-test-bucket13/?quota=true: Premature close

should decrease the inflights when performing multi object delete::quota evaluation with scuba metrics should decrease the inflights when performing multi object delete

Stack Traces | 0.018s run time

Invalid response body while trying to fetch http://127.0.0.1:8000/quota-test-bucket10/?quota=true: Premature close

should return QuotaExceeded when trying to CopyObject in a bucket with quota::quota evaluation with scuba metrics should return QuotaExceeded when trying to CopyObject in a bucket with quota

Stack Traces | 0.021s run time

Invalid response body while trying to fetch http://127.0.0.1:8000/quota-test-bucket2/?quota=true: Premature close

should return QuotaExceeded when trying to complete MPU in a bucket with quota::quota evaluation with scuba metrics should return QuotaExceeded when trying to complete MPU in a bucket with quota

Stack Traces | 0.021s run time

Invalid response body while trying to fetch http://127.0.0.1:8000/quota-test-bucket3/?quota=true: Premature close

should not return QuotaExceeded if the quota is not exceeded::quota evaluation with scuba metrics should not return QuotaExceeded if the quota is not exceeded

Stack Traces | 0.023s run time

Invalid response body while trying to fetch http://127.0.0.1:8000/quota-test-bucket4/?quota=true: Premature close

should not update the inflights if the API errored after evaluating quotas (deletion)::quota evaluation with scuba metrics should not update the inflights if the API errored after evaluating quotas (deletion)

Stack Traces | 0.023s run time

Invalid response body while trying to fetch http://127.0.0.1:8000/quota-test-bucket11/?quota=true: Premature close

should not update the inflights if the quota check is passing but the object is already restored::quota evaluation with scuba metrics should not update the inflights if the quota check is passing but the object is already restored

Stack Traces | 0.025s run time

Invalid response body while trying to fetch http://127.0.0.1:8000/quota-test-bucket14/?quota=true: Premature close

should return QuotaExceeded when trying to restore an object in a bucket with quota::quota evaluation with scuba metrics should return QuotaExceeded when trying to restore an object in a bucket with quota

Stack Traces | 0.029s run time

Invalid response body while trying to fetch http://127.0.0.1:8000/quota-test-bucket7/?quota=true: Premature close

should return QuotaExceeded when trying to copy a part in a bucket with quota::quota evaluation with scuba metrics should return QuotaExceeded when trying to copy a part in a bucket with quota

Stack Traces | 0.03s run time

Invalid response body while trying to fetch http://127.0.0.1:8000/quota-test-bucket6/?quota=true: Premature close

should return QuotaExceeded when trying to copyObject in a versioned bucket with quota::quota evaluation with scuba metrics should return QuotaExceeded when trying to copyObject in a versioned bucket with quota

Stack Traces | 0.034s run time

Invalid response body while trying to fetch http://127.0.0.1:8000/quota-test-bucket12/?quota=true: Premature close

should return QuotaExceeded when trying to PutObject in a bucket with quota::quota evaluation with scuba metrics should return QuotaExceeded when trying to PutObject in a bucket with quota

Stack Traces | 0.078s run time

Invalid response body while trying to fetch http://127.0.0.1:8000/quota-test-bucket1/?quota=true: Premature close

"after each" hook for "should create a bunch of objects and their versions"::put and get object with versioning With v4 signature "after each" hook for "should create a bunch of objects and their versions"

Stack Traces | 0.958s run time

The bucket you tried to delete is not empty.

should not evaluate quotas if the backend is not available::quota evaluation with scuba metrics should not evaluate quotas if the backend is not available

Stack Traces | 30s run time

Timeout of 30000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves. (.../tests/sur/quota.js)

View the full list of 6 ❄️ flaky test(s)

"after each" hook for "should fail if trying to overwrite a delete marker"::MPU with x-scal-s3-version-id header With default signature "after each" hook for "should fail if trying to overwrite a delete marker"

Flake rate in main: 100.00% (Passed 0 times, Failed 89 times)

Stack Traces | 0.015s run time

We encountered an internal error. Please try again.

should accept large quota::Test update bucket quota should accept large quota
Flake rate in main: 4.17% (Passed 23 times, Failed 1 times)
Stack Traces | 0.003s run time
Invalid response body while trying to fetch http://127.0.0.1:8000/updatequotatestbucket/?quota=true: Premature close

should return the quota::Test get bucket quota should return the quota
Flake rate in main: 13.81% (Passed 668 times, Failed 107 times)
Stack Traces | 0.006s run time
Invalid response body while trying to fetch http://127.0.0.1:8000/getquotatestbucket/?quota=true: Premature close

should update quota with XML format::Test update bucket quota should update quota with XML format

Flake rate in main: 4.17% (Passed 23 times, Failed 1 times)

Stack Traces | 0.004s run time

Expected no error, but got FetchError: Invalid response body while trying to fetch http://127.0.0.1:8000/updatequotatestbucket/?quota=true: Premature close

should update quota with explicit JSON content-type::Test update bucket quota should update quota with explicit JSON content-type

Flake rate in main: 4.17% (Passed 23 times, Failed 1 times)

Stack Traces | 0.004s run time

Invalid response body while trying to fetch http://127.0.0.1:8000/updatequotatestbucket/?quota=true: Premature close

should update the quota, using json parsing by default::Test update bucket quota should update the quota, using json parsing by default

Flake rate in main: 4.17% (Passed 23 times, Failed 1 times)

Stack Traces | 0.005s run time

Invalid response body while trying to fetch http://127.0.0.1:8000/updatequotatestbucket/?quota=true: Premature close

To view more test analytics, go to the Test Analytics Dashboard
_{📋 Got 3 mins? Take this short survey to help us improve Test Analytics.}

claude · 2026-05-29T18:40:47Z

+});
+
+describe('CRR cascade — putMetadata', () => {
+    it('second write with the same microVersionId returns loop-detected', async () => {


Test names should start with "should" per project conventions (e.g. should return loop-detected on second write with the same microVersionId).

— Claude Code

claude · 2026-05-29T18:40:55Z

        "@hapi/joi": "^17.1.1",
        "@smithy/node-http-handler": "^3.0.0",
-        "arsenal": "git+https://github.com/scality/Arsenal#8.4.2",
+        "arsenal": "file:../Arsenal",


arsenal and @scality/cloudserverclient use file: local path references. These must be pinned to a tag or version before merging — file: references will break any install outside of your local workstation.

— Claude Code

claude · 2026-05-29T18:41:03Z

package.json: arsenal and @scality/cloudserverclient use file: local path references instead of pinned tags/versions — these will break installs for anyone else and must be updated before merge
- Pin arsenal to the Arsenal PR tag once merged (was 8.4.2)
- Pin @scality/cloudserverclient to a published version (was 1.0.7)
- tests/functional/backbeat/crrCascade.js: test names in it() blocks should start with should

Review by Claude Code

claude · 2026-06-01T11:54:15Z

    }
+
+    const encodedMicroVersionId = request.headers['x-scal-micro-version-id'];
+    const incomingRaw = encodedMicroVersionId ? decodeMicroVersionId(encodedMicroVersionId) : null;


The decode error handling here differs from putMetadata (line 574-575), which normalises the result immediately so incomingRaw is guaranteed non-Error. Here incomingRaw can hold an Error object, then is checked later with !(incomingRaw instanceof Error). Consider aligning with the putMetadata pattern for consistency.

— Claude Code

claude · 2026-06-01T11:54:29Z

        "@hapi/joi": "^17.1.1",
        "@smithy/node-http-handler": "^3.0.0",
-        "arsenal": "git+https://github.com/scality/Arsenal#8.4.2",
+        "arsenal": "file:../Arsenal",


arsenal and @scality/cloudserverclient point to local file:../ paths. These must be pinned to git tags before merging — anyone cloning this repo won't have these sibling directories.

— Claude Code

claude · 2026-06-01T11:54:40Z

package.json: arsenal and @scality/cloudserverclient point to local file:../ paths — must be pinned to git tags before merging.
- lib/utilities/collectResponseHeaders.js:103: Cascade replicas without a next hop have status: '' and isReplica: true, but the outer if only checks status (falsy for empty string), so x-amz-replication-status: REPLICA header will be missing from S3 responses for these objects.
- lib/routes/routeBackbeat.js:438: putData and putMetadata handle decodeMicroVersionId error cases differently — consider aligning for consistency.
- tests/functional/backbeat/crrCascade.js: Missing after() hook to clean up the three test buckets created in before().

Review by Claude Code

claude · 2026-06-17T14:16:43Z

Dependency pinning: arsenal is pinned to a raw commit hash (f6b6e2a...) instead of a tag. Git-based deps should pin to tags per project conventions.
- Tag the Arsenal commit and reference the tag instead.
- Duplicate binary file: cloudserverclient.tgz is byte-identical to scality-cloudserverclient-v1.0.9.tgz but is not referenced anywhere. It adds 721 KB of dead weight to the repo.
- Remove cloudserverclient.tgz.
- cloudserverclient moved to production dependency as a local tgz: Previously @scality/cloudserverclient was a dev-only registry dependency. It is now a production dependency pointing at a checked-in tarball. This is fragile for security patches and a significant scope change.
- Consider publishing to the registry or using a git-based reference like other Scality deps.
- Test naming convention: Functional test names in crrCascade.js are missing the should prefix required by project conventions (e.g. throw CascadeLoopDetectedException... → should throw CascadeLoopDetectedException...).
- Prefix all it() descriptions with should.
- Missing test cleanup: The functional test before() creates three buckets but there is no after() hook to delete them.
- Add an after() block to clean up test buckets.

Review by Claude Code

claude · 2026-06-18T14:52:25Z

Arsenal is pinned to a raw commit hash (f6b6e2a…) instead of a semver tag — violates the project's dependency pinning convention for git-based deps.
- Pin to a tag once the Arsenal release containing the required changes is cut.
cloudserverclient.tgz is committed but never referenced — only scality-cloudserverclient-v1.0.9.tgz is used in package.json and Dockerfile.
- Remove the orphan file.
Both .tgz binary artifacts are checked into git, inflating history permanently.
- Consider publishing @scality/cloudserverclient to the registry or using a git ref like other Scality deps.
Unused output variable in tests/functional/backbeat/crrCascade.js:228.
- Drop the assignment: await putMetadata(key, newerMvId);
Inconsistent error-assertion pattern in the same describe block: line 139 uses assert.rejects, line 158 uses try/catch + assert.fail.
- Use assert.rejects in both tests for consistency.

Review by Claude Code

claude · 2026-06-18T15:22:08Z

package.json:40 — Arsenal is pinned to a commit hash instead of a tag. Git-based deps should pin to a tag per project conventions.
- cloudserverclient.tgz — This file is added but never referenced anywhere (package.json and Dockerfile only reference scality-cloudserverclient-v1.0.9.tgz). Remove the orphan file.
- lib/routes/routeBackbeat.js:638 — Inconsistent isReplica check uses truthy (isReplica) while lines 654, 671, and 750 use strict equality (isReplica === true). Use strict equality for consistency.
- tests/functional/backbeat/crrCascade.js:228 — output variable assigned but never used.

Review by Claude Code

maeldonn · 2026-06-24T07:54:47Z

+            // as cascade targets because they use the MultiBackend S3 path which
+            // bypasses the putData/putMetadata routes, so loop detection cannot fire
+            // on those destinations.
+            const BLOCKED_LOCATION_TYPES = ['location-scality-ring-s3-v1', 'location-scality-artesca-s3-v1'];


Could it be tested on a real lab ?

should be honestly, i think i need to double check this together with the design and the code

maeldonn · 2026-06-24T07:55:52Z

        replicationInfo: getReplicationInfo(config,
            objectKey, bucketMD, false, size, null, null, authInfo),
        overheadField,
+        updateMicroVersionId: true,


Do we really need this ?

yes, actually this is important, I added this after making my tests in codespaces :
Without this :
You put an object : object isn't assigned an micro version Id
You setup the whole replication, backbeat picks up that source object, it doesn't have any microversion id so cloudserver just doesn't try to process it as a potential cascade replication

maeldonn · 2026-06-24T07:58:35Z

+            request.resume();
+            return _respondWithHeaders(
+                response,
+                { code: MicroVersionIdAlreadyStoredException.name,
+                    message: 'incoming microVersionId already at destination' },
+                {},
+                log,
+                callback,
+                409,
+            );


could you remove duplication to simplify here ?

Ok I did something, you take a look but honestly it's really not that great 🤔

Issue: CLDSRV-897

SylvainSenechal commented May 29, 2026

View reviewed changes

Comment thread lib/api/apiUtils/object/versioning.js Outdated

SylvainSenechal commented May 29, 2026

View reviewed changes

Comment thread lib/routes/routeBackbeat.js

claude Bot reviewed May 29, 2026

View reviewed changes

SylvainSenechal commented May 29, 2026

View reviewed changes

Comment thread lib/routes/routeBackbeat.js

SylvainSenechal commented May 29, 2026

View reviewed changes

Comment thread lib/routes/routeBackbeat.js Outdated

SylvainSenechal commented May 29, 2026

View reviewed changes

SylvainSenechal mentioned this pull request May 29, 2026

Upgrade microVersionId to timestamp-ordered format and add isReplica scality/Arsenal#2628

Merged

SylvainSenechal changed the title ~~Improvement/cldsrv 897~~ Add CRR Cascaded capabilities May 29, 2026

SylvainSenechal requested review from a team, delthas and maeldonn May 29, 2026 18:11

SylvainSenechal force-pushed the improvement/CLDSRV-897 branch from c15fb2b to 7912480 Compare May 29, 2026 18:38

claude Bot reviewed May 29, 2026

View reviewed changes

SylvainSenechal mentioned this pull request May 29, 2026

Added headers for CRR Cascaded scality/cloudserverclient#24

Merged

SylvainSenechal force-pushed the improvement/CLDSRV-897 branch from 7912480 to 973261a Compare June 1, 2026 11:51

claude Bot reviewed Jun 1, 2026

View reviewed changes

Comment thread tests/functional/backbeat/crrCascade.js

claude Bot reviewed Jun 1, 2026

View reviewed changes

Comment thread lib/utilities/collectResponseHeaders.js Outdated

claude Bot reviewed Jun 17, 2026

View reviewed changes

Comment thread package.json Outdated

claude Bot reviewed Jun 17, 2026

View reviewed changes

Comment thread tests/functional/backbeat/crrCascade.js Outdated

claude Bot reviewed Jun 17, 2026

View reviewed changes

Comment thread tests/functional/backbeat/crrCascade.js

SylvainSenechal requested a review from maeldonn June 17, 2026 14:22

SylvainSenechal force-pushed the improvement/CLDSRV-897 branch from eb0241a to 92f01c0 Compare June 18, 2026 14:46