Bump microVersionId and add isReplica handling by maeldonn · Pull Request #6178 · scality/cloudserver

maeldonn · 2026-05-29T08:13:24Z

Cloudserver-side prep for cascaded CRR (design): bumps arsenal to the improvement/ARSN-578/micro-version-id branch, bumps microVersionId on every user-facing object metadata write through a single bumpMicroVersionId helper (gated on replicationInfo presence to keep storage tight, with a force flag preserving the existing overwriting-version path), sources x-amz-replication-status from the new ReplicationInfo.isReplica field with a legacy status === 'REPLICA' fallback, and clears isReplica on direct user writes (tagging / ACL / retention / legal hold) so a modified replica stops looking like one. The backbeat-route side of the design (setting isReplica = true on incoming replicas, cascade comparison, MultiBackend guard) lands in a follow-up ticket — until then the new fields are read but never set, so observable behavior is unchanged.

Issue: CLDSRV-906

bert-e · 2026-05-29T08:13:28Z

Hello maeldonn,

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-29T08:13:38Z

Incorrect fix version

The Fix Version/s in issue CLDSRV-906 contains:

None

Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:

9.4.0

Please check the Fix Version/s of CLDSRV-906, or the target
branch of this pull request.

claude · 2026-05-29T08:15:43Z

arsenal is pinned to a branch instead of a tag in package.json — git-based deps must pin to a tag for reproducible builds.
- Test names in tests/unit/api/apiUtils/object/bumpMicroVersionId.js and tests/unit/api/objectReplicationMD.js should start with "should".

Review by Claude Code

codecov · 2026-05-29T08:17:14Z

❌ 1 Tests Failed:

Tests completed	Failed	Passed	Skipped
3739	1	3738	0

View the full list of 1 ❄️ 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 v4 signature "after each" hook for "should fail if trying to overwrite a delete marker"

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

Stack Traces | 0.015s 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-06-03T14:13:17Z

Arsenal dependency is pinned to a branch, not a tag — pin to a tagged release before merging to production
- Test names in bumpMicroVersionId.js and objectReplicationMD.js should start with should per project convention

Review by Claude Code

bert-e · 2026-06-03T14:16:00Z

Waiting for approval

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

the author
2 peers

claude · 2026-06-03T14:17:30Z

Arsenal dependency pinned to a commit SHA instead of a release tag (package.json)
- Pin to a tagged release before merging; the previous value was 8.4.4
- Test names in it() blocks don't start with "should" (bumpMicroVersionId.js, objectReplicationMD.js)
- Prefix test descriptions with "should" per project conventions

Review by Claude Code

claude · 2026-06-03T15:16:29Z

Arsenal dependency pinned to a commit hash instead of a release tag (package.json:36). Update to a tag once the arsenal improvement/ARSN-578/micro-version-id branch changes are released.

The logic changes (bumpMicroVersionId helper, isReplica clearing on user writes, isReplica-aware replication-status header) are correct and well-tested.

Review by Claude Code

claude · 2026-06-04T14:00:58Z

Arsenal dependency pinned to a commit hash instead of a version tag, violating the project convention for git-based deps. - Pin to a tagged release before merging.

The functional changes (bumpMicroVersionId helper, isReplica handling in collectResponseHeaders with legacy fallback, isReplica clearing on user writes, microVersionId bumping in tagging/ACL/legal-hold/retention operations) look correct and are well-covered by tests.

Review by Claude Code

claude · 2026-06-04T14:11:30Z

Arsenal is pinned to a commit hash instead of a release tag (package.json:36). Git-based deps should reference a tag so the dependency is a reviewed, versioned release.
- Pin to a proper arsenal tag once the upstream branch is released.

Review by Claude Code

bert-e · 2026-06-11T14:37:05Z

Conflict

There is a conflict between your branch improvement/CLDSRV-906/micro-version-id and the
destination branch development/9.4.

Please resolve the conflict on the feature branch (improvement/CLDSRV-906/micro-version-id).

git fetch && \
git checkout origin/improvement/CLDSRV-906/micro-version-id && \
git merge origin/development/9.4

Resolve merge conflicts and commit

git push origin HEAD:improvement/CLDSRV-906/micro-version-id

Issue: CLDSRV-906

Prefer ReplicationInfo.isReplica over replicationInfo.status when producing x-amz-replication-status. The legacy status === 'REPLICA' branch is kept as a fallback so objects written before the feature still surface the correct value. Issue: CLDSRV-906

Bump microVersionId on every user write that changes object metadata other than replicationInfo, providing a unique revision identifier needed by upcoming cascaded CRR loop detection. Issue: CLDSRV-906

bert-e · 2026-06-18T09:57:34Z

Waiting for approval

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

the author
2 peers

claude · 2026-06-18T10:00:17Z

        "@opentelemetry/instrumentation-mongodb": "~0.69.0",
        "@smithy/node-http-handler": "^3.0.0",
-        "arsenal": "git+https://github.com/scality/Arsenal#8.4.7",
+        "arsenal": "git+https://github.com/scality/Arsenal#f6b6e2a37675e76186326e22c17b7a4cef77f5e0",


Arsenal is pinned to a commit hash (f6b6e2a...) instead of a release tag. Per project conventions, git-based deps should pin to a tag for traceability and reproducibility. If this is intentional for development/integration before the arsenal release, please add a tracking comment or TODO so it does not ship to production on a floating commit.

— Claude Code

claude · 2026-06-18T10:00:28Z

Arsenal dependency is pinned to a commit hash instead of a release tag, which does not meet the project convention for git-based deps.
- Pin to a tagged Arsenal release before merging, or add a TODO/tracking comment if this is intentional pre-release integration work.

The rest of the PR looks solid: the bumpMicroVersionId helper is clean and well-gated, isReplica handling in collectResponseHeaders correctly implements the legacy fallback, the isReplica clearing via Object.assign with getReplicationInfo works as intended, and the test coverage across all metadata-only write paths (tagging, legal hold, retention, ACL) is thorough.

Review by Claude Code

SylvainSenechal · 2026-06-18T13:58:03Z

nit : I think putting it in versioning.js instead of creating a new file could have been enough 🤔

I'd rather keep it as its own file: versioning.js is already 585 lines and handles versionId semantics, whereas microVersionId is a separate revision marker (used by cascaded CRR loop detection). Six call sites import it, so a focused helper file felt clearer than growing versioning.js. Happy to move it if you feel strongly though.

SylvainSenechal · 2026-06-18T14:00:45Z

        content,
        role: ReplicationConfiguration.resolveSourceRole(config.role),
        isNFS: bucketMD.isNFS(),
+        isReplica: false,


mixed feeling on this : I say let's keep it, it makes sense as we return status pending, but also weird because we have objectMd available in the function, so we should return objectMd.isReplica 🤔

Intentional, same as hardcoding status: 'PENDING': this builds a fresh outgoing replicationInfo for a user write, which by definition isn't a replica. The hardcoded false is precisely what flips the previous isReplica: true to false when merged onto objectMD.replicationInfo at the call sites. Reading objectMD.isReplica would defeat that.

true, other fields are also hardcoded

SylvainSenechal · 2026-06-18T14:06:07Z

                if (replicationInfo) {
                    // eslint-disable-next-line no-param-reassign
                    objectMD.replicationInfo = Object.assign({}, objectMD.replicationInfo, replicationInfo);
+                    bumpMicroVersionId(objectMD);


On my implementation, i was also resetting isReplica to false during these metadata updates, shouldn't we do it ?

Already handled: getReplicationInfo returns isReplica: false and the Object.assign merge applies it.

SylvainSenechal · 2026-06-18T14:10:47Z

-        // update microVersionId when overwriting metadata.
        if (updateMicroVersionId) {
-            md.updateMicroVersionId();
+            md.updateMicroVersionId(config.instanceId, config.replicationGroupId);


On my version, i removed the if statement, and the params update
microVersionId.

I found this param was only set in overwritingVersioning() at the beginning :
metadataStoreParams.updateMicroVersionId = true;
which I also removed.

I did it because I think microVersionId should always be updated when metadataStoreObject is called what do you think 🤔

not fully sure though, because maybe we wanna avoid useless microVersionId storage in mondoDb, and also bump microVersionId on more specific path ?

metadataStoreObject is called by multiple operations (PUT, COPY, MPU complete, etc.) where we don't want to bump microVersionId. Keeping the existing gated behavior.

For now we want to avoid unnecessary metadata writes since microVersionId is only consumed by CRR. That doesn't mean it's coupled to CRR conceptually, just that there's no other consumer today, so bumping it unconditionally would be wasted writes.

When a user updates tags, ACL, retention, or legal-hold on an object that arrived via replication (isReplica=true), the resulting object is no longer a replica - clear the flag so the x-amz-replication-status response header reflects the new state. Issue: CLDSRV-906

claude · 2026-06-18T15:15:51Z

        "@opentelemetry/instrumentation-mongodb": "~0.69.0",
        "@smithy/node-http-handler": "^3.0.0",
-        "arsenal": "git+https://github.com/scality/Arsenal#8.4.7",
+        "arsenal": "git+https://github.com/scality/Arsenal#f6b6e2a37675e76186326e22c17b7a4cef77f5e0",


Arsenal is pinned to a commit hash (f6b6e2a...) instead of a tag. Per project conventions, git-based deps (arsenal, vaultclient, bucketclient, etc.) must pin to a tag, not a branch or bare commit hash. This should be updated to a tag once the arsenal branch is released.

— Claude Code

claude · 2026-06-18T15:15:59Z

Arsenal dependency is pinned to a commit hash instead of a tag. Git-based deps must pin to a tag per project conventions. Update to a tag once the arsenal branch is released.

The rest of the PR looks solid: the bumpMicroVersionId helper is clean and well-gated, the isReplica field is correctly cleared on direct user writes via the Object.assign merge with getReplicationInfo (which now returns isReplica: false), and the collectResponseHeaders change preserves backward compatibility with legacy status === 'REPLICA' metadata. Tests cover the new behavior well.

Review by Claude Code

maeldonn requested review from a team, benzekrimaha and delthas May 29, 2026 08:13

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/unit/api/apiUtils/object/bumpMicroVersionId.js Outdated

claude Bot reviewed May 29, 2026

View reviewed changes

Comment thread tests/unit/api/objectReplicationMD.js Outdated

francoisferrand reviewed May 29, 2026

View reviewed changes

Comment thread lib/api/objectDeleteTagging.js Outdated

Comment thread lib/api/objectPutLegalHold.js Outdated

Comment thread lib/api/apiUtils/object/bumpMicroVersionId.js

Comment thread lib/services.js Outdated

maeldonn force-pushed the improvement/CLDSRV-906/micro-version-id branch from 3b4110b to 6ba0ebe Compare June 3, 2026 14:09