Handle BucketAlreadyExists races and bump arsenal for MPU afterEach fix by benzekrimaha · Pull Request #6222 · scality/cloudserver

benzekrimaha · 2026-07-08T15:25:52Z

Summary

Fix MPU shadow-bucket creation races in cloudserver callers and bump arsenal to ARSN-613 at 07aa045b for paired testing with Arsenal PR #2672.

ARSN-613 makes MongoClientInterface.createBucket() return BucketAlreadyExists on concurrent duplicates instead of silent idempotence. Cloudserver must handle that contract:

getMPUBucket: on BucketAlreadyExists, return the local mpuBucket (no extra getBucket). Fixes the intermittent afterEach failure in mpuVersion.js ("We encountered an internal error") when ListMultipartUploads races on MPU shadow bucket creation during test cleanup.
freshStartCreateBucket: on BucketAlreadyExists, only run addToUsersBucket — skip updateBucket with the local BucketInfo so a concurrent winner's metadata is not overwritten. The winner path still runs full cleanUpBucket.

Arsenal pin is a commit hash (not #8.4.20) to pick up in-flight ARSN-613 fixes ($setOnInsert, NamespaceExists) before release tagging.

Issue: CLDSRV-919

bert-e · 2026-07-08T15:25:57Z

Hello benzekrimaha,

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-07-08T15:26:02Z

Incorrect fix version

The Fix Version/s in issue CLDSRV-919 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-919, or the target
branch of this pull request.

bert-e · 2026-07-08T15:34:23Z

Request integration branches

Waiting for integration branch creation to be requested by the user.

To request integration branches, please comment on this pull request with the following command:

/create_integration_branches

Alternatively, the /approve and /create_pull_requests commands will automatically
create the integration branches.

bert-e · 2026-07-08T15:36:41Z

Waiting for approval

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

the author
2 peers

The following options are set: create_integration_branches

bert-e · 2026-07-08T15:38:05Z

Waiting for approval

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

the author
2 peers

The following options are set: create_pull_requests, create_integration_branches

codecov · 2026-07-08T15:38:59Z

Codecov Report

❌ Patch coverage is 83.97790% with 29 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.93%. Comparing base (055ae1b) to head (84a0d65).
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
lib/services.js	78.33%	26 Missing ⚠️
lib/api/apiUtils/bucket/bucketCreation.js	95.08%	3 Missing ⚠️

Additional details and impacted files

Files with missing lines	Coverage Δ
lib/api/apiUtils/bucket/bucketCreation.js	`94.21% <95.08%> (+0.82%)`	⬆️
lib/services.js	`87.53% <78.33%> (+0.09%)`	⬆️

... and 1 file with indirect coverage changes

@@                 Coverage Diff                 @@
##           development/9.3    #6222      +/-   ##
===================================================
- Coverage            84.93%   84.93%   -0.01%     
===================================================
  Files                  206      206              
  Lines                13366    13369       +3     
===================================================
+ Hits                 11353    11355       +2     
- Misses                2013     2014       +1

Flag	Coverage Δ
file-ft-tests-null-compat	`63.00% <69.06%> (-5.91%)`	⬇️
kmip-ft-tests	`28.31% <37.01%> (+0.02%)`	⬆️
mongo-v0-ft-tests	`69.62% <72.37%> (-0.01%)`	⬇️
mongo-v1-ft-tests	`69.60% <71.82%> (-0.02%)`	⬇️
multiple-backend	`36.78% <33.70%> (+0.01%)`	⬆️
s3c-ft-tests-v0	`64.13% <75.13%> (+0.01%)`	⬆️
s3c-ft-tests-v0-null-compat	`64.19% <75.13%> (+0.01%)`	⬆️
s3c-ft-tests-v1	`64.11% <75.13%> (+0.01%)`	⬆️
sur-tests	`35.84% <51.93%> (-0.81%)`	⬇️
sur-tests-inflights	`37.69% <53.59%> (+0.02%)`	⬆️
unit	`70.53% <79.00%> (+0.02%)`	⬆️
utapi-v2-tests	`34.57% <56.90%> (+0.02%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

bert-e · 2026-07-09T15:33:27Z

Request integration branches

Waiting for integration branch creation to be requested by the user.

To request integration branches, please comment on this pull request with the following command:

/create_integration_branches

Alternatively, the /approve and /create_pull_requests commands will automatically
create the integration branches.

benzekrimaha · 2026-07-13T10:33:11Z

+                    if (err) {
+                        return callback(err);
+                    }
+                    return callback(null, data);
+                });
+            },
        },
-    },
-    // Function to run upon finishing both parallel requests
-    (err, results) => {
-        if (err) {
-            return cb(err);


Out of scope for CLDSRV-919 — pre-existing callback style in this file. Can dismiss.

francoisferrand

nit: this is exactly the kind of PR where prettier is really adding noise... 5-10 lines of "real" change, but prettier fully rewrote 2 large files...

francoisferrand · 2026-07-10T12:38:27Z

+        } else {
+            log.trace('created bucket in metadata');
+        }
        return addToUsersBucket(canonicalID, bucketName, bucket, log, err => {


should not be be done if bucket already exists, to avoid race and/or incorrect content?

I also don't understand why bucket already exists error does not cause the function to return early with an error ?

More generally i don't understand why this wasn't catched before in the err ?

I would've expect current behavior to be :

CreateBucket(bucketName) => return err because collision in the db on bucket name, then return callback(err)

I think I'm missing some context information about : what trigger this bug precisely, cuz the pr talks about MPU but here we are fixing a function called freshStartCreateBucket, is this not called all the time to create any bucket ?

On BucketAlreadyExists we now only run addToUsersBucket and skip removeTransientOrDeletedLabel / updateBucket with our local BucketInfo, so we don't overwrite metadata the concurrent winner already wrote. The winner path still runs full cleanUpBucket when we actually created the metastore entry. No extra metastore read.

ARSN-613 changes metadata.createBucket() to return BucketAlreadyExists on a metastore duplicate instead of silent success. Without handling it we surface InternalError. Previously cloudserver always continued because there was no err to catch.

The MPU flake is in getMPUBucket (InitiateMultipartUpload / ListMultipartUploads). freshStartCreateBucket is the normal CreateBucket path when two requests both see NoBucketYet and race on metadata.createBucket — same contract change.

We don't return BucketAlreadyExists to the S3 client here: the parallel check already decided this is our bucket being created. On duplicate we only ensure the users-bucket listing; we skip updateBucket with our local object.

francoisferrand · 2026-07-10T12:47:00Z

                return metadata.createBucket(MPUBucketName, mpuBucket, log,
                    err => {
+                        if (err?.is?.BucketAlreadyExists) {
+                            return metadata.getBucket(MPUBucketName, log,


is this function getMPUBucket() called a lot (it is not an s3 api, not sure if it is called everywhere in mpu code, or just in a few places) : if that is the case, maybe we should plan another improvement to let createBucket return the bucket data as well it case it already exists.

...or maybe we could just return the (existing) mpuBucket variable? for actual buckets, there may be more in the BucketInfo -tags, lifecycle rules...-, but likely not the case for mpu's BucketInfo ?

getMPUBucket is only called from initiateMultipartUpload and listMultipartUploads. On BucketAlreadyExists we now return the local mpuBucket , MPU shadow BucketInfo is minimal (no tags/lifecycle), so that should be enough and avoids an extra getBucket. Longer term, Arsenal could return existing bucket data on duplicate so all callers benefit.

SylvainSenechal · 2026-07-10T17:41:55Z

     *                        upload bucket or error if any
     * @return {undefined}
     */
    getMPUBucket(destinationBucket, bucketName, log, cb) {


Mhh. I find this very weird :

It's called
GetMpuBucket
It currently does :

GetBucket, if not found, create it.
So ok this is like a "get or create if not found".

But then with the new code changes, this function becomes

GetBucket, if not found, create it. When creating it, if it already exists, getBucket
But if the bucket was not found in the first place, why would we get an "alreadyExists" error when creating it 🤔 ? Race condition ? But then, this is infinite, because in the last getBucket you can also get a notExist error 🤔

Two concurrent MPU calls can both get NoSuchBucket, one wins createBucket, the other gets BucketAlreadyExists. We return the local mpuBucket we built for the create attempt — same owner/displayName as the winner's shadow bucket, no second read. Not a loop: BucketAlreadyExists means the metastore entry exists; we don't refetch.

Issue: CLDSRV-919

claude · 2026-07-13T12:16:01Z

    const bucketName = bucket.getName();
    metadata.createBucket(bucketName, bucket, log, err => {
-        if (err) {
+        if (err && !err.is.BucketAlreadyExists) {


Unsafe access: if err is not an arsenal error (e.g. a raw Error from a network failure), err.is is undefined and err.is.BucketAlreadyExists throws TypeError. The safe pattern err?.is?.BucketAlreadyExists is already used 4 lines below and in getMPUBucket. Reorder to match:

Suggested change

if (err && !err.is.BucketAlreadyExists) {

if (err?.is?.BucketAlreadyExists) {

log.trace('bucket already exists in metadata');

// Ensure users-bucket listing only; do not updateBucket with our

// local BucketInfo when a concurrent request won createBucket.

return addToUsersBucket(canonicalID, bucketName, bucket, log, callback);

}

if (err) {

log.debug('error from metadata', { error: err });

return callback(err);

}

Issue: CLDSRV-919

benzekrimaha force-pushed the improvement/CLDSRV-919-afterEach-flakiness-internal-error-fix branch 2 times, most recently from ab63d85 to 2cae9bc Compare July 8, 2026 15:31

benzekrimaha changed the base branch from development/9.4 to development/9.3 July 8, 2026 15:31

benzekrimaha changed the title ~~CLDSRV-919: bump arsenal to 8.5.7 for MPU afterEach flake fix~~ Bump arsenal with MPU afterEach flakiness fix Jul 8, 2026

scality deleted a comment from bert-e Jul 8, 2026

benzekrimaha requested review from a team, SylvainSenechal and delthas July 8, 2026 15:37

bert-e mentioned this pull request Jul 8, 2026

INTEGRATION [PR#6222 > development/9.4] Bump arsenal with MPU afterEach flakiness fix #6223

Closed

benzekrimaha requested a review from tcarmet July 8, 2026 15:39

SylvainSenechal approved these changes Jul 8, 2026

View reviewed changes

tcarmet approved these changes Jul 8, 2026

View reviewed changes

delthas approved these changes Jul 9, 2026

View reviewed changes

benzekrimaha force-pushed the improvement/CLDSRV-919-afterEach-flakiness-internal-error-fix branch from 2cae9bc to 6178ad8 Compare July 9, 2026 15:30

benzekrimaha changed the title ~~Bump arsenal with MPU afterEach flakiness fix~~ Handle BucketAlreadyExists races and bump arsenal for MPU afterEach fix Jul 9, 2026

scality deleted a comment from bert-e Jul 9, 2026

benzekrimaha force-pushed the improvement/CLDSRV-919-afterEach-flakiness-internal-error-fix branch from 71b7d71 to 0a09689 Compare July 9, 2026 16:10

github-advanced-security AI found potential problems Jul 10, 2026

View reviewed changes

benzekrimaha force-pushed the improvement/CLDSRV-919-afterEach-flakiness-internal-error-fix branch 2 times, most recently from b8c7b85 to 19a2bd2 Compare July 10, 2026 06:51

benzekrimaha requested review from SylvainSenechal, delthas, francoisferrand and tcarmet July 10, 2026 07:18

francoisferrand reviewed Jul 10, 2026

View reviewed changes

SylvainSenechal reviewed Jul 10, 2026

View reviewed changes

SylvainSenechal self-requested a review July 10, 2026 17:44

claude Bot reviewed Jul 13, 2026

View reviewed changes

Comment thread package.json Outdated

claude Bot reviewed Jul 13, 2026

View reviewed changes

Comment thread tests/unit/lib/services.spec.js Outdated

benzekrimaha requested a review from francoisferrand July 13, 2026 12:08

benzekrimaha force-pushed the improvement/CLDSRV-919-afterEach-flakiness-internal-error-fix branch from 37f6c40 to 922f83d Compare July 13, 2026 12:10

benzekrimaha added 4 commits July 13, 2026 14:13

Handle BucketAlreadyExists races in cloudserver callers.

3598f59

Issue: CLDSRV-919

Add unit tests for BucketAlreadyExists race handling.

41d9765

Issue: CLDSRV-919

Bump arsenal to ARSN-613 at 07aa045b for paired testing.

da31c7c

Issue: CLDSRV-919

Prettify files touched by PR.

c478b7c

Issue: CLDSRV-919

benzekrimaha force-pushed the improvement/CLDSRV-919-afterEach-flakiness-internal-error-fix branch from 922f83d to e9a6dbe Compare July 13, 2026 12:13

claude Bot reviewed Jul 13, 2026

View reviewed changes

Avoid updateBucket on BucketAlreadyExists create races.

84a0d65

Issue: CLDSRV-919

benzekrimaha force-pushed the improvement/CLDSRV-919-afterEach-flakiness-internal-error-fix branch from e9a6dbe to 84a0d65 Compare July 13, 2026 12:19

-        if (err && !err.is.BucketAlreadyExists) {
+        if (err?.is?.BucketAlreadyExists) {
+            log.trace('bucket already exists in metadata');
+            // Ensure users-bucket listing only; do not updateBucket with our
+            // local BucketInfo when a concurrent request won createBucket.
+            return addToUsersBucket(canonicalID, bucketName, bucket, log, callback);
+        }
+        if (err) {
+            log.debug('error from metadata', { error: err });
+            return callback(err);
+        }

Uh oh!

Conversation

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

Uh oh!

Summary

Uh oh!

bert-e commented Jul 8, 2026

Hello benzekrimaha,

Uh oh!

bert-e commented Jul 8, 2026

Incorrect fix version

Uh oh!

bert-e commented Jul 8, 2026

Request integration branches

Uh oh!

bert-e commented Jul 8, 2026

Waiting for approval

Uh oh!

bert-e commented Jul 8, 2026

Waiting for approval

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

bert-e commented Jul 9, 2026

Request integration branches

Uh oh!

benzekrimaha Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

francoisferrand left a comment

Choose a reason for hiding this comment

Uh oh!

francoisferrand Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

SylvainSenechal Jul 10, 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!

benzekrimaha Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

benzekrimaha Jul 13, 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!

francoisferrand Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

benzekrimaha Jul 13, 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!

SylvainSenechal Jul 10, 2026

Choose a reason for hiding this comment

Uh oh!

benzekrimaha Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

claude Bot Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

benzekrimaha commented Jul 8, 2026 •

edited

Loading

codecov Bot commented Jul 8, 2026 •

edited

Loading

SylvainSenechal Jul 10, 2026 •

edited

Loading

benzekrimaha Jul 13, 2026 •

edited

Loading

benzekrimaha Jul 13, 2026 •

edited

Loading