Added headers for CRR Cascaded by SylvainSenechal · Pull Request #24 · scality/cloudserverclient

SylvainSenechal · 2026-05-21T15:46:37Z

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

Related PRs :
Arsenal : scality/Arsenal#2628
Cloudserver : scality/cloudserver#6179
Backbeat : scality/backbeat#2747
S3utils : scality/s3utils#395

bert-e · 2026-05-21T15:46:41Z

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.

bert-e · 2026-05-21T15:46:49Z

Waiting for approval

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

the author
2 peers

SylvainSenechal · 2026-05-29T18:40:52Z

First I added some tests in this codebase, but they were too functional and not really oriented toward just testing the client itself. + we already have test testing the client itself for putData/putMetadata

So I added the tests in Cloudserver : They are functional tests using CloudserverClient with the micro version id, testing 409 error, actual api behavior etc. I think they are better in cloudserver than in here

francoisferrand · 2026-06-01T11:41:18Z

+    @httpHeader("x-scal-cascade-loop-detected")
+    CascadeLoopDetected: Boolean


PutMetadata is a generic api, used in many more cases than CRR.
→ api should stay generic, no reason to introduce "crr cascaded" vocabulary : stick to some generic concept, like "conflict" or "already have microVersionId"

I thought about this but I thought it would be better for future maintainers to understand what this field is used for.
I dont wanna go with conflict as this would already be used for microVersionId that are too old, but I'll find something for the equality around "already have micro version id"

edit: name updated, let me know if you have other name ideas

I thought it would be better for future maintainers to understand what this field is used for.

this is exactly my point : it will be easier to maintain a simple contract (return a conflict error if micro version id is older; also return previous micro version id), and be reusable ; instead of having a contract which is specific to a specific use case (when doing cascaded crr, backbeat will make this call and cloudserver should do this specific logic, so that ...)

i.e. we should avoid "specific use case" in API, and "simple, generic, intelligible contracts"

francoisferrand · 2026-06-01T11:43:23Z

-
+
+    @httpHeader("x-scal-micro-version-id")
+    MicroVersionId: String,


nit: to make the semantic explicit, field could be name something like IfMicroVersionIdOlderThan (i.e. semantics is part of the API)

Leaving for now, what do other ppl think about the naming ?

@scality/raving-robots WDYT?

x-scal-micro-version-id is simpler and easier to understand.

🤝ok.

We however should document the semantics of this field here: i.e. it will conflict (and not write metadata) if micro version id is older than the one already stored

maeldonn · 2026-06-05T10:18:41Z

-
+
+    @httpHeader("x-scal-micro-version-id")
+    MicroVersionId: String,


x-scal-micro-version-id is simpler and easier to understand.

maeldonn · 2026-06-05T10:20:29Z

+    versionId: String,
+
+    @httpHeader("x-scal-micro-version-id-exists")
+    MicroVersionIdExists: Boolean


Should be x-scal-replication-loop to differentiate between a success write and a loop skip.

I used
if data.MicroVersionIdExists to check if we had a success or a loop

edit:
nvm yeah I reviewed the naming and used
x-scal-replication-loop
replicationLoop

maeldonn · 2026-06-05T10:21:39Z

    input: PutMetadataInput,
-    output: PutMetadataOutput
+    output: PutMetadataOutput,
+    errors: [ConflictException]


Should also be added to putData

I removed it from putData, it doesn't raise a 409 anymore, and instead just return 200 with micro version id, and backbeat do the check.

But honestly this is all a bit complicated, having to deal with both error code and returned values that are either booleans or microVersionId..

Yeah i'm a bit starting to question the current implementation, as again it works but it's tricky to mix error code, with boolean flag, with string returned value.

Would probably be wiser and easier to maintain to just have a "CrrCascadeOutcome" = 'dataAlreadyExists' | 'loop' | 'stale' or whatever we need, and just have backbeat switch on all cases.

Because here in backbeat it gets a bit dirty 🤔

Forget about the 2 other messages,
I did some rework to properly integrate the smithy errors

Now backbeat will be able to check errors like this :

err instanceof StaleMicroVersionIdException

bert-e · 2026-06-05T10:21:50Z

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:

@maeldonn

benzekrimaha · 2026-06-10T07:05:41Z

+    ReplicationLoop: Boolean
+}
+
+@error("client")


can we add a test for this changes the generated client contract, but nothing checks that MicroVersionId / versionId are serialized as headers, nor that the new 409 errors are deserialized with their headers

When testing cloudserver client, there is some kind of circular dependency where I can test the client either here, in cloudserver or in backbeat.

I have functional tests in both backbeat and cloudserver that triggers these errors

francoisferrand · 2026-06-10T21:25:37Z

+@httpError(409)
+structure StaleMicroVersionIdException {
+    @required
+    message: String


should the micro-version-id be returned in that case? Maybe we don't use it today, but could be used?

would also allow to use the same Exception for PutData and PutMetadata 409 errors

I'm adding the micro version id as its not really costing us anything

I don't think we should merge the 2 error exception, that would force backbeat to have more logic to reclassify to first verify 409, then classify the 409 based on the microversion id
With the current setup, the classification is done directly in cloudserver and the backbeat code handling the error reads easily

maeldonn · 2026-06-11T08:31:38Z

+
+@error("client")
+@httpError(409)
+structure VersionIdCollisionException {


Backbeat will pattern-match on this (err.name == VersionIdCollisionException). Consider documenting the error:

/// Returned by PutData when the destination already has an object at this /// versionId. The existing microVersionId is included in the response header /// so the caller can run the cascade loop/stale/proceed classification. @error("client") @httpError(409) structure VersionIdCollisionException { ... }

maeldonn · 2026-06-11T08:31:38Z

 structure PutMetadataOutput {
    /// Version ID of the stored metadata
-    versionId: String
+    versionId: String,


Align all fields to camelCase or PascalCase

Actually, versionId is an existing field, i think it was already lowered case even before we started using cloudserverclient.
Looking at many other apis, their output is almost always using lowered case versionIds, so I think instead i will switch ReplicationLoop to camelCase

maeldonn · 2026-06-11T08:31:38Z

+
+@error("client")
+@httpError(409)
+structure StaleMicroVersionIdException {


Same doc-comment suggestion as VersionIdCollisionException:

/// Returned by PutMetadata when the incoming microVersionId is older than /// the destination's current value (stale cascade event). The write is not /// applied; the caller should mark the replication as FAILED (non-retryable). @error("client") @httpError(409) structure StaleMicroVersionIdException { ... }

Issue: CLDSRVCLT-14

francoisferrand · 2026-06-12T16:29:16Z

+    versionId: String,
+
+    @httpHeader("x-scal-replication-loop")
+    replicationLoop: Boolean


when do we have 200 and "replicationLoop" ?
is this not just a StaleMicroVersionIdException as well (with the same micro version id) ?

maeldonn reviewed May 22, 2026

View reviewed changes

Comment thread models/backbeatRoutes/putdata.smithy

SylvainSenechal force-pushed the improvement/CLDSRVCLT-14 branch 3 times, most recently from e2a81bf to 798a647 Compare May 29, 2026 18:39

SylvainSenechal commented May 29, 2026

View reviewed changes

SylvainSenechal marked this pull request as ready for review May 29, 2026 18:41

SylvainSenechal requested review from a team, benzekrimaha and delthas May 29, 2026 18:41

This was referenced May 29, 2026

Add CRR Cascaded capabilities scality/cloudserver#6179

Open

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

Open

francoisferrand reviewed Jun 1, 2026

View reviewed changes

SylvainSenechal force-pushed the improvement/CLDSRVCLT-14 branch from 798a647 to a072957 Compare June 2, 2026 17:54

SylvainSenechal requested review from francoisferrand and maeldonn June 2, 2026 19:42

This was referenced Jun 3, 2026

Add Crr Cascade capabilities to backbeat crr replication scality/backbeat#2747

Open

Adopt timestamp-ordered microVersionId in CRR resync tool scality/s3utils#395

Open

francoisferrand reviewed Jun 3, 2026

View reviewed changes

Comment thread models/backbeatRoutes/putMetadata.smithy Outdated

francoisferrand reviewed Jun 3, 2026

View reviewed changes

Comment thread models/backbeatRoutes/putdata.smithy Outdated

Comment thread models/backbeatRoutes/putdata.smithy Outdated

Comment thread models/backbeatRoutes/putdata.smithy Outdated

maeldonn requested changes Jun 5, 2026

View reviewed changes

SylvainSenechal force-pushed the improvement/CLDSRVCLT-14 branch from a072957 to ec98192 Compare June 9, 2026 20:00

SylvainSenechal requested review from francoisferrand and maeldonn June 9, 2026 20:00

benzekrimaha reviewed Jun 10, 2026

View reviewed changes

francoisferrand reviewed Jun 10, 2026

View reviewed changes

maeldonn reviewed Jun 11, 2026

View reviewed changes

delthas removed their request for review June 11, 2026 14:28

SylvainSenechal added 2 commits June 12, 2026 00:00

Add new headers for CRR Cascaded

bdedc59

Issue: CLDSRVCLT-14

Bump version to 1.0.9

94c822c

Issue: CLDSRVCLT-14

SylvainSenechal force-pushed the improvement/CLDSRVCLT-14 branch from ec98192 to 94c822c Compare June 11, 2026 22:00

SylvainSenechal requested review from benzekrimaha, francoisferrand and maeldonn June 11, 2026 22:00

francoisferrand reviewed Jun 12, 2026

View reviewed changes

		@httpHeader("x-scal-cascade-loop-detected")
		CascadeLoopDetected: Boolean



		@httpHeader("x-scal-micro-version-id")
		MicroVersionId: String,

Conversation

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

Uh oh!

Uh oh!

bert-e commented May 21, 2026

Hello sylvainsenechal,

Uh oh!

bert-e commented May 21, 2026

Waiting for approval

Uh oh!

Uh oh!

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

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!

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!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

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!

Choose a reason for hiding this comment

Uh oh!

bert-e commented Jun 5, 2026

Waiting for approval

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!

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!

Reviewers

Assignees

SylvainSenechal commented May 21, 2026 •

edited by francoisferrand

Loading

SylvainSenechal May 29, 2026 •

edited

Loading

SylvainSenechal Jun 8, 2026 •

edited

Loading