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.

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

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

❌ 1 Tests Failed:

Tests completed	Failed	Passed	Skipped
9693	1	9692	0

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

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

Stack Traces | 0.017s run time

We encountered an internal error. Please try again.

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: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: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

bert-e · 2026-07-03T17:27:31Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
2 peers

The following reviewers are expecting changes from the author, or must review again:

@francoisferrand

delthas · 2026-07-06T08:39:30Z

Removing myself as reviewer since alrdy covered by 2 people; and quite complex. Re-add me if needed.

SylvainSenechal · 2026-07-08T16:37:58Z

Btw I will deal with prettier at the end

francoisferrand · 2026-07-09T13:18:34Z

    }
+
+    const incomingVersionIdEncoded = request.headers['x-scal-version-id'];
+    if (incomingVersionIdEncoded != null && incomingVersionIdEncoded !== '') {


should we not check incomingVersionIdEncoded against undefined to detect the absence/presence of field ?

I don't have a big opinion on this.
VersionID was already always passed from backbeat before, what's sure is right here for the collision check we need it

But I don't really know whether this function should work without a versionID provided.
What I know is In cloudserverClient, version id header field is not mandatory, and in backbeat, the api is called in only one place with the versionId (and I doubt it's used anywhere else)

francoisferrand · 2026-07-09T13:29:20Z

+            current !== null &&
+            (incoming === null || incoming > current);


kind of hard to read (and validate that it works), would be best not to see this: i.e. use existing compareVersionId...

looking at the code of compare versionId in arsenal : it seems that function does not support comparing "null" versionID → should be reworked to support that case

Will need to standardize on what "null" is: null, '' or falsish... Falsish is more flexible (does not require null coalescence after loading document...) but means there may not be an actual equality when the function return 0 (null != undefined)

We have this doc, although its not referenced directly in the code 🤔 https://scality.atlassian.net/wiki/spaces/OS/pages/edit-v2/4141449224

francoisferrand · 2026-07-09T13:38:54Z

            omVal.isDeleteMarker &&
            omVal.replicationInfo &&
-            omVal.replicationInfo.status === 'REPLICA' &&
+            (omVal.replicationInfo.isReplica === true || omVal.replicationInfo.status === 'REPLICA') &&


nit: should we introduce a helper function? or maybe "standardize" omVal.replicationInfo.isReplica after the JSON.parse() call on line 658 ?

try { omVal = JSON.parse(payload); omVal.replicationInfo.isReplica ||= omVal.replicationInfo.status === 'REPLICA'; } catch { return callback(errors.MalformedPOSTRequest); }

francoisferrand · 2026-07-09T14:02:47Z

+                status: '',
+                backends: [],
+                content: [],
+                destination: '',
+                storageClass: '',
+                role: '',
+                storageType: '',
+                dataStoreVersionId: '',


nit: do we need to put an empty replicationInfo? can't we keep the field undefined?

Prefer to keep, anyways we will always have at least
omVal.replicationInfo.isReplica = true;

francoisferrand · 2026-07-16T12:44:19Z

+        const isIncomingOlderThanCurrent = (incoming, current) => {
+            if (current === null) { // nothing to be stale against
+                return false; 
+            }
+            // null incoming : source has no cascade revision => oldest possible state
+            // larger string = older revision (reverse-chronological order)
+            return incoming === null || incoming > current;
+        };


is this not compareMicroVersionId ? Or is that "refactor" moved to later commit?

(still quite hard to read, even with the comments... but somehow can't see the original comment about this...)

francoisferrand · 2026-07-16T12:50:31Z

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


there is a bit of duplication everywhere:

2x 10 lines here (as initially pointed by Mael)

matching 2 different errors in backbeat

doing 2 separate comparison (== then strict older)

code here may be dedup by using compareVersionId, something like:

const diff = compareVersionId(current, incoming): if (diff <= 0) { const err := diff === 0 ? MicroVersionIdAlreadyStoredException.name : StaleMicroVersionIdException.name; log.debug(....) request.resume(); return _respondWithHeaderCrrConflict( ..., err, ... ) }

bert-e · 2026-07-16T12:54:39Z

Waiting for approval

The following approvals are needed before I can proceed with the merge:

the author
2 peers

claude · 2026-07-16T15:30:35Z


        try {
            omVal = JSON.parse(payload);
+            if (omVal.replicationInfo.status === 'REPLICA') {


Accessing omVal.replicationInfo.status inside the JSON.parse() try/catch means a missing replicationInfo field throws TypeError, which the catch converts to MalformedPOSTRequest. The payload is valid JSON — the error is misleading and will make debugging harder. Use optional chaining so only actual parse failures trigger MalformedPOSTRequest.

Suggested change

if (omVal.replicationInfo.status === 'REPLICA') {

if (omVal.replicationInfo?.status === 'REPLICA') {

Issue: CLDSRV-897

SylvainSenechal · 2026-07-16T17:20:45Z

/approve

bert-e · 2026-07-16T17:20:58Z

I have successfully merged the changeset of this pull request
into targetted development branches:

✔️ development/9.4

The following branches have NOT changed:

development/7.10
development/7.4
development/7.70
development/8.8
development/9.0
development/9.1
development/9.2
development/9.3

This pull request did not target the following hotfix branch(es) so they
were left untouched:

hotfix/7.70.21
hotfix/7.10.27
hotfix/7.4.5
hotfix/9.0.32
hotfix/7.70.73
hotfix/7.10.3
hotfix/7.10.2
hotfix/7.4.7
hotfix/7.10.15
hotfix/7.4.8
hotfix/8.8.45
hotfix/7.10.8
hotfix/7.2.0
hotfix/7.9.0
hotfix/7.4.0
hotfix/6.4.7
hotfix/7.70.51
hotfix/7.10.49
hotfix/7.6.0
hotfix/7.70.11
hotfix/7.4.10
hotfix/7.10.28
hotfix/7.10.4
hotfix/7.10.1
hotfix/7.4.2
hotfix/7.70.45
hotfix/7.4.4
hotfix/7.8.0
hotfix/9.2.36
hotfix/9.0.7
hotfix/7.10.0
hotfix/9.2.24
hotfix/7.10.30
hotfix/7.4.9
hotfix/7.7.0
hotfix/7.4.6
hotfix/7.4.1
hotfix/7.4.3

Please check the status of the associated issue CLDSRV-897.

Goodbye sylvainsenechal.

The following options are set: approve

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 Outdated

SylvainSenechal commented May 29, 2026

View reviewed changes

Comment thread lib/routes/routeBackbeat.js Outdated

claude Bot reviewed May 29, 2026

View reviewed changes

Comment thread package.json Outdated

claude Bot reviewed May 29, 2026

View reviewed changes

Comment thread package.json Outdated

claude Bot reviewed May 29, 2026

View reviewed changes

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

claude Bot reviewed May 29, 2026

View reviewed changes

Comment thread tests/functional/backbeat/putMetadata.js

SylvainSenechal commented May 29, 2026

View reviewed changes

Comment thread lib/routes/routeBackbeat.js Outdated

SylvainSenechal commented May 29, 2026

View reviewed changes

Comment thread lib/routes/routeBackbeat.js Outdated

SylvainSenechal commented May 29, 2026

View reviewed changes

Comment thread package.json Outdated

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

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

claude Bot reviewed May 29, 2026

View reviewed changes

Comment thread package.json Outdated

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 lib/routes/routeBackbeat.js Outdated

claude Bot reviewed Jun 1, 2026

View reviewed changes

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

claude Bot reviewed Jun 1, 2026

View reviewed changes

Comment thread package.json Outdated

claude Bot reviewed Jun 1, 2026

View reviewed changes

Comment thread lib/utilities/collectResponseHeaders.js Outdated

delthas removed their request for review July 6, 2026 08:39

SylvainSenechal force-pushed the improvement/CLDSRV-897 branch 2 times, most recently from dc58e7d to f54b101 Compare July 7, 2026 20:04

SylvainSenechal requested a review from francoisferrand July 7, 2026 20:04

claude Bot reviewed Jul 7, 2026

View reviewed changes

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

SylvainSenechal force-pushed the improvement/CLDSRV-897 branch from f54b101 to 14aa3a0 Compare July 8, 2026 16:29

francoisferrand reviewed Jul 9, 2026

View reviewed changes

SylvainSenechal force-pushed the improvement/CLDSRV-897 branch 2 times, most recently from 8526cd3 to 7888d79 Compare July 15, 2026 16:01

SylvainSenechal requested a review from francoisferrand July 15, 2026 16:07

SylvainSenechal force-pushed the improvement/CLDSRV-897 branch from 7888d79 to f1f8e09 Compare July 16, 2026 00:15

claude Bot reviewed Jul 16, 2026

View reviewed changes

Comment thread tests/unit/routes/routeBackbeat.js Outdated

francoisferrand approved these changes Jul 16, 2026

View reviewed changes

SylvainSenechal force-pushed the improvement/CLDSRV-897 branch from f1f8e09 to 7e1eeb1 Compare July 16, 2026 15:25

claude Bot reviewed Jul 16, 2026

View reviewed changes

SylvainSenechal force-pushed the improvement/CLDSRV-897 branch from 7e1eeb1 to 5bc97d1 Compare July 16, 2026 15:36

SylvainSenechal added 3 commits July 16, 2026 18:39

Implement CRR Cascaded feature

a424363

Issue: CLDSRV-897

apply prettier

77140ea

Issue: CLDSRV-897

bump to 9.4.0-preview.6

e65838c

Issue: CLDSRV-897

SylvainSenechal force-pushed the improvement/CLDSRV-897 branch from 5bc97d1 to e65838c Compare July 16, 2026 16:40

bert-e merged commit e65838c into development/9.4 Jul 16, 2026
59 of 61 checks passed

bert-e deleted the improvement/CLDSRV-897 branch July 16, 2026 17:21

eve-ci-cd Bot had a problem deploying to zenko/improvement/ZENKO-5263@2.16 July 16, 2026 17:49 Failure

eve-ci-cd Bot had a problem deploying to zenko/improvement/ZENKO-5263@2.16 July 16, 2026 18:04 Failure

		current !== null &&
		(incoming === null \|\| incoming > current);

	if (omVal.replicationInfo.status === 'REPLICA') {
	if (omVal.replicationInfo?.status === 'REPLICA') {

Uh oh!

Conversation

SylvainSenechal commented May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bert-e commented May 29, 2026

Hello sylvainsenechal,

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

claude Bot commented May 29, 2026

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov Bot commented May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ 1 Tests Failed:

Uh oh!

Uh oh!

Uh oh!

claude Bot commented May 29, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

claude Bot commented Jun 1, 2026

Uh oh!

bert-e commented Jul 3, 2026

Waiting for approval

Uh oh!

delthas commented Jul 6, 2026

Uh oh!

Uh oh!

SylvainSenechal commented Jul 8, 2026

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bert-e commented Jul 16, 2026

Waiting for approval

Uh oh!

claude Bot Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

SylvainSenechal commented Jul 16, 2026

Uh oh!

bert-e commented Jul 16, 2026

Uh oh!

Uh oh!

Reviewers

SylvainSenechal commented May 29, 2026 •

edited

Loading

codecov Bot commented May 29, 2026 •

edited

Loading