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:38:25Z

    RequestUids: String,

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


as discussed (late) in the design, not an input parameter : PutData should minimize work/logic related to metadata, and should simply return the current microVersionId (instead of "x-scal-cascade-loop-detected")

this will reduce coupling, while giving all flexibility for backbeat to take all decisions.

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

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.

francoisferrand · 2026-06-03T17:40:16Z

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


nit: when it exists, should it return just a bool or actually the value itself ?(previous or current, not sure what we need)

Should be a bool

Bool works, although yeah, we could return a microVersionId and let backbeat decide for itself what it should do with it

francoisferrand · 2026-06-03T17:41:27Z

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


just MicroVersionId, consistent with the x-scal header

yeah and actually i did some rework to integrate this with the error of handling of smithy, this is defined and returned with the VersionIdCollisionException now

francoisferrand · 2026-06-03T17:42:23Z

    @httpHeader("X-Scal-Request-Uids")
    RequestUids: String,

+    @httpHeader("x-scal-source-version-id")


why "source" ? you usual header is x-scal-version-id I think, should stick to it ?

In a code that's already not super easy to read, I think its easy to mix up ids, and being able to write this on cloudserver just make thing very clear 🤔

const incomingVersionIdEncoded = request.headers['x-scal-source-version-id'];

edit: Yeah actually in cloudserver, we already have similar pattern in routebackbeat (x-scal-source-version-id, x-scal-source-bucket), so I prefer to keep it like this

francoisferrand · 2026-06-03T17:42:55Z

    @httpHeader("X-Scal-Request-Uids")
    RequestUids: String,

+    @httpHeader("x-scal-source-version-id")


should be marked as "optional" : if not set, we should keep previous/existing behavior (no conflict detection etc)

To be confirmed, but I think it's optional by default.

thats right, there is an @required field
that will produce this for example.
Else the field is optional

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:20:47Z

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


Should be a bool

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

Issue: CLDSRVCLT-14

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

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

maeldonn requested changes Jun 5, 2026

View reviewed changes

SylvainSenechal added 2 commits June 9, 2026 21:53

Add new headers for CRR Cascaded

c153b6e

Issue: CLDSRVCLT-14

Bump version to 1.0.9

ec98192

Issue: CLDSRVCLT-14

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

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



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

Conversation

SylvainSenechal commented May 21, 2026 • edited 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!

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!

francoisferrand Jun 3, 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!

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!

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!

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!

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!

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!

bert-e commented Jun 5, 2026

Waiting for approval

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

SylvainSenechal commented May 21, 2026 •

edited

Loading

SylvainSenechal May 29, 2026 •

edited

Loading

francoisferrand Jun 3, 2026 •

edited

Loading

SylvainSenechal Jun 8, 2026 •

edited

Loading

SylvainSenechal Jun 8, 2026 •

edited

Loading