Upgrade microVersionId to timestamp-ordered format and add isReplica by maeldonn · Pull Request #2628 · scality/Arsenal

maeldonn · 2026-05-22T14:11:04Z

Replace the random 8-byte hex microVersionId with a 20-character timestamp-ordered identifier matching the versionId scheme, and add encode/decode/compare helpers in a new MicroVersionID module. Add an optional isReplica flag to ReplicationInfo to distinguish replica writes from user writes, enabling cascaded CRR.

Issue: ARSN-578

Crr cascaded design : https://github.com/scality/citadel/pull/349
Related PRs :
Cloudserver : scality/cloudserver#6179
CloudserverClient : scality/cloudserverclient#24
Backbeat : scality/backbeat#2747
S3utils : scality/s3utils#395

bert-e · 2026-05-22T14:11:09Z

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-22T14:11:14Z

Incorrect fix version

The Fix Version/s in issue ARSN-578 contains:

None

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

8.4.3

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

claude · 2026-05-22T14:13:46Z

LGTM

Review by Claude Code

claude · 2026-05-22T14:58:02Z

Breaking API change: updateMicroVersionId() now requires a replicationGroupId argument, which will break all existing callers in downstream repos (CloudServer, Backbeat, etc.). Coordinate updates or consider an optional parameter with a fallback.
- Make the parameter optional with a random default, or ensure all consumers are updated in the same release cycle.
updateMicroVersionId JSDoc says @return itself but the method body has no return this statement.
- Add return this; for consistency with other ObjectMD setters.

Review by Claude Code

claude · 2026-05-22T15:26:01Z

LGTM

The changes are well-structured: the new MicroVersionId module correctly mirrors the VersionId timestamp-ordering scheme with its own isolated sequencing state, the isReplica flag is cleanly additive and backward compatible, and the test coverage is thorough (uniqueness, ordering, encode/decode round-trip, compare, padding/truncation).

Review by Claude Code

bert-e · 2026-05-22T18:03:11Z

Waiting for approval

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

the author
2 peers

claude · 2026-05-22T18:23:17Z

updateMicroVersionId JSDoc says @return itself but the method doesn't return this, breaking the chaining convention used by all other setters in ObjectMD. Since the method is being rewritten, add return this;.

Review by Claude Code

claude · 2026-05-22T18:28:34Z

LGTM — one breaking change to flag:

- updateMicroVersionId() signature changed to require a replicationGroupId parameter — downstream callers (CloudServer, Backbeat) must be updated in lockstep

The new MicroVersionId module, isReplica field, and timestamp-ordered format all look correct. Backward compatibility for existing stored metadata (old 16-char random hex microVersionIds) is preserved since getMicroVersionId() returns the raw value and _updateFromParsedJSON copies it as-is.

Review by Claude Code

SylvainSenechal · 2026-06-01T11:39:40Z

            storageType: storageType || '',
            dataStoreVersionId: dataStoreVersionId || '',
            isNFS,
+            isReplica: isReplica !== undefined ? isReplica : (status === 'REPLICA' ? true : undefined),


Suggested change

isReplica: isReplica !== undefined ? isReplica : (status === 'REPLICA' ? true : undefined),

isReplica: isReplica !== undefined ? isReplica : (status === 'REPLICA' ? true : false),

not sure if undefined or false when status not REPLICA

Since it's a boolean i would say false. But if we can save some storage in database, we should use undefined.

I would say false too, avoid as much undefined variable as possible 🤔 What do other think ?

bert-e · 2026-06-02T17:58:12Z

Incorrect fix version

The Fix Version/s in issue ARSN-578 contains:

8.4.4

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

8.4.5

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

claude · 2026-06-02T18:00:56Z

checkCrrCascadeEvent does not validate incomingRaw with isMicroVersionIdComparable, so comparing a legacy 16-char hex microVersionId against a time-ordered ID can produce incorrect stale/proceed results.
- Add !isMicroVersionIdComparable(incomingRaw) to the guard clause at line 393 of VersionID.ts.
- getMicroVersionId() return value changed from null to undefined — a silent breaking change for downstream callers checking === null.
- Document this as a breaking change, or keep returning null for backward compatibility.
- No tests for isMicroVersionIdComparable and checkCrrCascadeEvent — these new exported functions have four code paths that need coverage.
- Add unit tests covering non-comparable input, loop, stale, and proceed cases.

Review by Claude Code

codecov · 2026-06-02T18:06:14Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.96%. Comparing base (6a5ee64) to head (83682d1).

Additional details and impacted files

@@                 Coverage Diff                 @@
##           development/8.4    #2628      +/-   ##
===================================================
+ Coverage            73.92%   73.96%   +0.04%     
===================================================
  Files                  229      229              
  Lines                18480    18501      +21     
  Branches              3822     3830       +8     
===================================================
+ Hits                 13661    13685      +24     
+ Misses                4814     4811       -3     
  Partials                 5        5

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

claude · 2026-06-02T19:27:25Z

getMicroVersionId() return value changed from null to undefined when unset — this is a silent breaking change for any caller using === null
- Either keep returning null for backward compatibility, or confirm all downstream callers (CloudServer, Backbeat, S3utils) have been updated to handle undefined

- updateMicroVersionId() signature changed from zero args to two required args (instanceId, replicationGroupId) — coordinated breaking change
- PR body notes Backbeat and S3utils PRs are "Not done yet"; ensure those are updated before merging this

Review by Claude Code

maeldonn · 2026-06-03T13:26:07Z

+ *   - the legacy 16-char random-hex value written by old ObjectMD code
+ *   - any other string shorter than the minimum valid raw length (27 chars)
+ */
+export function isMicroVersionIdComparable(id: string | null | undefined): boolean {


Suggested change

export function isMicroVersionIdComparable(id: string | null | undefined): boolean {

export function isComparable(id?: string | null): boolean {

Can the field really be null ? 🤔

yeah
But I think the real mess here is : We are calling typescript code with javascript code. This would be easier to deal with if it was all typescript, here I did something defensive

Re-using this comment to ask :
you want some name changes
isMicroVersionIdComparable -> isComparable
checkCrrCascadeEvent -> compare

I say why not but its a bit annoying that the repo using this code
has to do :

const { versioning } = require('arsenal');
versioning.VersionID.checkCrrCascadeEvent()

Or
const { decode, encode, checkCrrCascadeEvent } = versioning.VersionID;
checkCrrCascadeEvent()

Like I mean the namespace isnt microversionId exactly 🤔

maeldonn · 2026-06-03T13:26:56Z

+ * 16-char hex, or otherwise too short) : a legacy incoming ID must never
+ * produce a false 'stale' or 'loop' event
+ */
+export function checkCrrCascadeEvent(


Suggested change

export function checkCrrCascadeEvent(

export function compare(

'loop' | 'stale' | 'proceed' are specific to CRR not to versionID format. We should just expose a simple compare function in this file

ah yeah something that would return

'equal' | 'older' | 'younger' 🤔

It's a bit annoying though, because in the first if, it's just not comparable and we return proceed.
Feels like we would need another utility function on top of this utility function if we want something nice usable for crr cascade 🤔 Or need to return a fourth 'not comparable' value, then it can work

Like in backbeat I would prefer to call
if checkCrrCascadeEvent(id1, id2) === 'proceed' {do proceed logic}
than
if compare(id1,id1) === ('older' || 'not comparable') {do proceed logic} 🤔

I want some more THOUGHTS before making a change

maeldonn · 2026-06-03T13:29:44Z


    setReplicationStatus(status: string) {
        this._data.replicationInfo.status = status;
+        if (status === 'REPLICA') {


I think this is useless since we are not going anymore to set status to 'REPLICA'

Shouldn't we at least keep it to avoid weird state ?
Really not sure about this, maybe we can get thoughts from somebody else.

Also are you sure we aren't gonna use setReplicationStatus ? In backbeat, in ObjectQueueEntry.js, I see some setReplicationStatus('REPLICA'), I haven't touched them for crrCascade, maybe you did for multi destination, but not sure.

I'd also say lets keep it to avoid breaking any behavior

maeldonn · 2026-06-03T13:31:39Z

+     * @return true if this object is a replica
+     */
+    getReplicationIsReplica(): boolean {
+        return this._data.replicationInfo.isReplica === true


Not sure if we need logic in this getter. Is it used somewhere ?

I don't understand the comment 🤔

What logic in this getter is not needed, the || this._data.replicationInfo.status === 'REPLICA'; ?

Do you mean we should only return isReplica ?

Replace the random 8-byte hex microVersionId the timestamp-ordered identifier matching the versionId scheme, and add helpers for the crr cascade feature. Add an optional isReplica flag to ReplicationInfo to distinguish replica writes from user writes, enabling cascaded CRR. Issue: ARSN-578

Issue: ARSN-578

bert-e · 2026-06-09T20:54:57Z

Waiting for approval

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

the author
2 peers

claude · 2026-06-09T20:58:23Z

getMicroVersionId() return value changed from null to undefined when unset — callers using === null checks will silently break
- Audit downstream consumers (CloudServer, Backbeat) in the related PRs, or keep the null return for backward compatibility

Review by Claude Code

benzekrimaha · 2026-06-10T06:57:18Z

+    if (incomingRaw === existing) {
+        return 'loop';
+    }
+    if (incomingRaw > existing) {


is comparing the full id intentional?
the ordering after timestamp/seq includes rg/instance data, so same-slot writes can be classified as stale based on ids, not time

benzekrimaha · 2026-06-10T06:58:16Z

+ *   - any other string shorter than the minimum valid raw length (27 chars)
+ */
+export function isMicroVersionIdComparable(id: string | null | undefined): boolean {
+    return typeof id === 'string' && id.length >= LEGACY_BASE62_DECODED_LENGTH;


is length really enough to decide this is comparable?
if this can come from metadata or headers, a malformed 27+ chars string may produce a stale decision..

maeldonn requested review from a team, SylvainSenechal and benzekrimaha May 22, 2026 14:11

SylvainSenechal reviewed May 22, 2026

View reviewed changes

Comment thread lib/models/ObjectMD.ts

SylvainSenechal reviewed May 22, 2026

View reviewed changes

Comment thread lib/models/ObjectMD.ts

SylvainSenechal reviewed May 22, 2026

View reviewed changes

Comment thread lib/models/ObjectMD.ts Outdated

SylvainSenechal reviewed May 22, 2026

View reviewed changes

Comment thread lib/versioning/MicroVersionId.ts Outdated

maeldonn force-pushed the improvement/ARSN-578/micro-version-id branch from caf9a51 to 868cae8 Compare May 22, 2026 14:55

SylvainSenechal reviewed May 22, 2026

View reviewed changes

Comment thread lib/versioning/MicroVersionId.ts Outdated

claude Bot reviewed May 22, 2026

View reviewed changes

Comment thread lib/models/ObjectMD.ts Outdated

claude Bot reviewed May 22, 2026

View reviewed changes

Comment thread lib/models/ObjectMD.ts Outdated

SylvainSenechal reviewed May 22, 2026

View reviewed changes

Comment thread lib/versioning/MicroVersionId.ts Outdated

maeldonn force-pushed the improvement/ARSN-578/micro-version-id branch from 868cae8 to 2eae0c5 Compare May 22, 2026 15:22

maeldonn marked this pull request as ready for review May 22, 2026 15:24

maeldonn commented May 22, 2026

View reviewed changes

Comment thread lib/versioning/MicroVersionId.ts Outdated

SylvainSenechal reviewed May 22, 2026

View reviewed changes

Comment thread lib/versioning/MicroVersionId.ts Outdated

maeldonn requested a review from SylvainSenechal May 22, 2026 18:03

claude Bot reviewed May 22, 2026

View reviewed changes

Comment thread lib/models/ObjectMD.ts

maeldonn force-pushed the improvement/ARSN-578/micro-version-id branch from fbaf3fe to 3160d6b Compare May 22, 2026 18:25

claude Bot reviewed May 22, 2026

View reviewed changes

Comment thread lib/models/ObjectMD.ts Outdated

maeldonn force-pushed the improvement/ARSN-578/micro-version-id branch from 3160d6b to 5f4839a Compare May 22, 2026 18:31

claude Bot reviewed May 22, 2026

View reviewed changes

Comment thread lib/models/ObjectMD.ts

SylvainSenechal reviewed Jun 1, 2026

View reviewed changes

Comment thread lib/models/ObjectMD.ts Outdated

SylvainSenechal reviewed Jun 1, 2026

View reviewed changes

Comment thread lib/models/ObjectMD.ts

SylvainSenechal force-pushed the improvement/ARSN-578/micro-version-id branch from 2d68077 to 031c259 Compare June 1, 2026 11:37

SylvainSenechal reviewed Jun 1, 2026

View reviewed changes

SylvainSenechal force-pushed the improvement/ARSN-578/micro-version-id branch 2 times, most recently from 110b0c8 to e166386 Compare June 2, 2026 17:58

claude Bot reviewed Jun 2, 2026

View reviewed changes

Comment thread lib/models/ObjectMD.ts

claude Bot reviewed Jun 2, 2026

View reviewed changes

Comment thread lib/versioning/VersionID.ts

claude Bot reviewed Jun 2, 2026

View reviewed changes

Comment thread lib/versioning/VersionID.ts Outdated

SylvainSenechal force-pushed the improvement/ARSN-578/micro-version-id branch from e166386 to 2c429ab Compare June 2, 2026 19:24

claude Bot reviewed Jun 2, 2026

View reviewed changes

Comment thread lib/models/ObjectMD.ts

SylvainSenechal requested a review from francoisferrand 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

SylvainSenechal added the claude-review-retro Identify PR with claude review that have issues (like duplicated comments) label Jun 3, 2026

maeldonn requested a review from SylvainSenechal June 3, 2026 13:31

maeldonn commented Jun 3, 2026

View reviewed changes

maeldonn added 2 commits June 9, 2026 22:54

Reformat with prettier

83682d1

Issue: ARSN-578

SylvainSenechal force-pushed the improvement/ARSN-578/micro-version-id branch from 2c429ab to 83682d1 Compare June 9, 2026 20:54

	isReplica: isReplica !== undefined ? isReplica : (status === 'REPLICA' ? true : undefined),
	isReplica: isReplica !== undefined ? isReplica : (status === 'REPLICA' ? true : false),

	export function isMicroVersionIdComparable(id: string \| null \| undefined): boolean {
	export function isComparable(id?: string \| null): boolean {

	export function checkCrrCascadeEvent(
	export function compare(

Conversation

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

Uh oh!

Uh oh!

bert-e commented May 22, 2026

Hello maeldonn,

Uh oh!

bert-e commented May 22, 2026

Incorrect fix version

Uh oh!

claude Bot commented May 22, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

claude Bot commented May 22, 2026

Uh oh!

Uh oh!

claude Bot commented May 22, 2026

Uh oh!

Uh oh!

Uh oh!

bert-e commented May 22, 2026

Waiting for approval

Uh oh!

Uh oh!

claude Bot commented May 22, 2026

Uh oh!

Uh oh!

claude Bot commented May 22, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maeldonn 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!

bert-e commented Jun 2, 2026

Incorrect fix version

Uh oh!

Uh oh!

Uh oh!

Uh oh!

claude Bot commented Jun 2, 2026

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

Uh oh!

claude Bot commented Jun 2, 2026

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

maeldonn commented May 22, 2026 •

edited by SylvainSenechal

Loading

maeldonn Jun 3, 2026 •

edited

Loading

codecov Bot commented Jun 2, 2026 •

edited

Loading

SylvainSenechal Jun 8, 2026 •

edited

Loading

maeldonn Jun 3, 2026 •

edited

Loading