Skip to content

INTEGRATION [PR#2672 > development/8.5] ARSN-613: Properly return already exists error #2677

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bert-e merged 3 commits into from
Jul 23, 2026
Merged

INTEGRATION [PR#2672 > development/8.5] ARSN-613: Properly return already exists error #2677

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
dbbc68b
Return BucketAlreadyExists from Mongo createBucket on duplicates
benzekrimaha Jul 23, 2026
83072fd
Bump package version to 8.4.21
benzekrimaha Jul 23, 2026
fd1354b
Merge remote-tracking branch 'origin/improvement/ARSN-613-return-alre…
benzekrimaha Jul 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
133 changes: 65 additions & 68 deletions lib/storage/metadata/mongoclient/MongoClientInterface.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -373,16 +373,20 @@ class MongoClientInterface {
usersBucket to have attributes, we pre-create the
usersBucket attributes here (see bucketCreation.js line
36)*/
const log = this.logger;
const usersBucketAttr = new BucketInfo(
constants.usersBucket,
'admin',
'admin',
new Date().toJSON(),
BucketInfo.currentModelVersion(),
);
return this.createBucket(constants.usersBucket, usersBucketAttr, this.logger, err => {
if (err) {
this.logger.fatal('error writing usersBucket ' + 'attributes to metastore', { error: err });
return this.createBucket(constants.usersBucket, usersBucketAttr, log, createErr => {
if (createErr?.is?.BucketAlreadyExists) {
return cb();
}
if (createErr) {
log.fatal('error writing usersBucket attributes to metastore', { error: createErr });
throw errors.InternalError;
}
return cb();
Expand Down Expand Up @@ -424,8 +428,17 @@ class MongoClientInterface {
).makeSerializable();
const m = this.getCollection<BucketMetastoreDocument>(METASTORE);

let vFormat = this.defaultBucketKeyFormat;
if (
bucketName === constants.usersBucket ||
bucketName === PENSIEVE ||
bucketName.startsWith(constants.mpuBucketPrefix)
) {
vFormat = BUCKET_VERSIONS.v0;
}

const payload = {
$set: {
$setOnInsert: {
_id: bucketName,
value: {
...newBucketMD,
Expand All @@ -443,96 +456,80 @@ class MongoClientInterface {
},
},
},
vFormat: this.defaultBucketKeyFormat,
vFormat,
},
};
if (
bucketName !== constants.usersBucket &&
bucketName !== PENSIEVE &&
!bucketName.startsWith(constants.mpuBucketPrefix)
) {
payload.$set.vFormat = this.defaultBucketKeyFormat;
} else {
payload.$set.vFormat = BUCKET_VERSIONS.v0;
}

// we don't have to test bucket existence here as it is done
// on the upper layers
m.updateOne(
{
_id: bucketName,
},
payload,
{
upsert: true,
},
)
return m
.updateOne(
{
_id: bucketName,
},
payload,
{
upsert: true,
},
)
.then(result => {
if (result.matchedCount === 0 && result.modifiedCount === 0 && result.upsertedCount === 0) {
log.debug('createBucket: failed to create bucket', { bucketName, result });
return cb(errors.InternalError);
throw errors.InternalError;
}
if (result.matchedCount > 0 && result.upsertedCount === 0) {
log.debug('createBucket: bucket already exists in metastore', { bucketName, result });
throw errors.BucketAlreadyExists;
}
// caching bucket vFormat
this.bucketVFormatCache.add(bucketName, payload.$set.vFormat);
this.bucketVFormatCache.add(bucketName, vFormat);
// NOTE: We do not need to create a collection for
// "constants.usersBucket" and "PENSIEVE" since it has already
// been created
if (bucketName !== constants.usersBucket && bucketName !== PENSIEVE) {
return this.db!.createCollection(bucketName)
.catch(err => {
// MongoDB returns NamespaceExists (code 48) when
// the collection already exists, e.g. on
// concurrent create/drop/create sequences on MPU
// shadow buckets. The collection being there is
// the desired outcome, so treat it as success:
// this mirrors deleteBucket, which ignores
// NamespaceNotFound when dropping the collection.
if (err.codeName !== 'NamespaceExists') {
throw err;
log.error('createBucket: error creating collection', {
bucketName,
error: err.message,
});
throw errors.InternalError;
}
// Metastore insert succeeded: an orphaned backing
// collection is not a bucket-level duplicate.
log.debug('createBucket: collection already exists', { bucketName });
})
.then(() => {
if (this.shardCollections) {
const cmd = {
shardCollection: `${this.database}.${bucketName}`,
key: { _id: 1 },
};
return this.adminDb!.command(cmd, {})
.catch(err => {
// Concurrent createBucket calls may
// race on shardCollection: sharding
// an already-sharded collection fails
// with AlreadyInitialized. The
// collection is sharded (with the
// same {_id: 1} key) either way.
if (err.codeName !== 'AlreadyInitialized') {
throw err;
}
log.debug('createBucket: collection already sharded', { bucketName });
})
.then(() => cb(null))
.catch(err => {
log.error('createBucket: enabling sharding', { error: err });
return cb(errors.InternalError);
});
if (!this.shardCollections) {
return undefined;
}
return cb(null);
})
.catch(err => {
log.error('createBucket: error creating collection', {
bucketName,
error: err.message,
const cmd = {
shardCollection: `${this.database}.${bucketName}`,
key: { _id: 1 },
};
return this.adminDb!.command(cmd, {}).catch(err => {
if (err.codeName !== 'AlreadyInitialized') {
log.error('createBucket: enabling sharding', { error: err });
throw errors.InternalError;
}
log.debug('createBucket: collection already sharded', { bucketName });
return undefined;
});
return cb(errors.InternalError);
});
}
return cb(null);
return undefined;
})
.catch(err => {
log.error('createBucket: error creating bucket', { error: err.message });
return cb(errors.InternalError);
});
.then(
() => cb(null),
err => {
if (err?.is) {
return cb(err);
}
log.error('createBucket: error creating bucket', { error: err.message });
return cb(errors.InternalError);
},
);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
"engines": {
"node": ">=20"
},
"version": "8.5.11",
"version": "8.5.12",
"config": {
"mongodbMemoryServer": {
"version": "8.0.23"
Expand Down
142 changes: 98 additions & 44 deletions tests/unit/storage/metadata/mongoclient/MongoClientInterface.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ describe('MongoClientInterface::createBucket', () => {
return Object.assign(new Error(codeName), { code, codeName });
}

function setupMockClient(shardCollections) {
function setupMockClient(shardCollections, updateOneResult) {
client = new MongoClientInterface({
logger,
replicaSetHosts: 'localhost:27017',
Expand All @@ -384,70 +384,115 @@ describe('MongoClientInterface::createBucket', () => {
commandStub = sinon.stub().resolves({});
client.db = {
collection: sinon.stub().returns({
updateOne: sinon.stub().resolves({
matchedCount: 0,
modifiedCount: 0,
upsertedCount: 1,
}),
updateOne: sinon.stub().resolves(
updateOneResult || {
matchedCount: 0,
modifiedCount: 0,
upsertedCount: 1,
},
),
}),
createCollection: createCollectionStub,
};
client.adminDb = { command: commandStub };
client.client = {};
}

it('should succeed when the collection already exists (NamespaceExists)', done => {
function createBucketPromised(bucketName, bucketMD = baseBucket) {
return new Promise((resolve, reject) => {
client.createBucket(bucketName, bucketMD, logger, err => {
if (err) {
return reject(err);
}
return resolve();
});
});
}

it('should return BucketAlreadyExists when metastore entry already exists', async () => {
setupMockClient(false, {
matchedCount: 1,
modifiedCount: 0,
upsertedCount: 0,
});
await assert.rejects(
() => createBucketPromised('test-bucket'),
err => err?.is?.BucketAlreadyExists,
);
assert.strictEqual(createCollectionStub.called, false);
const updateOne = client.db.collection().updateOne;
assert(updateOne.calledOnce);
assert(updateOne.firstCall.args[1].$setOnInsert);
assert.strictEqual(updateOne.firstCall.args[1].$set, undefined);
});

it('should succeed when the backing collection already exists (NamespaceExists)', async () => {
setupMockClient(false);
createCollectionStub.rejects(makeMongoError('NamespaceExists', 48));
client.createBucket('test-bucket', baseBucket, logger, err => {
assert.ifError(err);
assert(createCollectionStub.calledOnceWith('test-bucket'));
done();
});
await createBucketPromised('test-bucket');
assert(createCollectionStub.calledOnceWith('test-bucket'));
assert.strictEqual(commandStub.called, false);
});

it('should shard when the backing collection already exists (NamespaceExists)', async () => {
setupMockClient(true);
createCollectionStub.rejects(makeMongoError('NamespaceExists', 48));
await createBucketPromised('test-bucket');
assert(createCollectionStub.calledOnceWith('test-bucket'));
assert(
commandStub.calledOnceWith({
shardCollection: 'test.test-bucket',
key: { _id: 1 },
}),
);
});

it('should return InternalError on other createCollection errors', done => {
it('should return InternalError on other createCollection errors', async () => {
setupMockClient(false);
createCollectionStub.rejects(makeMongoError('HostUnreachable', 6));
client.createBucket('test-bucket', baseBucket, logger, err => {
assert(err);
assert(err.is.InternalError);
done();
});
await assert.rejects(
() => createBucketPromised('test-bucket'),
err => err?.is?.InternalError,
);
});

it('should still shard the collection when it already exists', done => {
it('should shard the collection after a successful createCollection', async () => {
setupMockClient(true);
createCollectionStub.rejects(makeMongoError('NamespaceExists', 48));
client.createBucket('test-bucket', baseBucket, logger, err => {
assert.ifError(err);
assert(
commandStub.calledOnceWith({
shardCollection: 'test.test-bucket',
key: { _id: 1 },
}),
);
done();
});
await createBucketPromised('test-bucket');
assert(createCollectionStub.calledOnceWith('test-bucket'));
assert(
commandStub.calledOnceWith({
shardCollection: 'test.test-bucket',
key: { _id: 1 },
}),
);
});

it('should succeed when the collection is already sharded (AlreadyInitialized)', done => {
it('should succeed when the collection is already sharded (AlreadyInitialized)', async () => {
setupMockClient(true);
commandStub.rejects(makeMongoError('AlreadyInitialized', 23));
client.createBucket('test-bucket', baseBucket, logger, err => {
assert.ifError(err);
done();
});
await createBucketPromised('test-bucket');
});

it('should return InternalError on other shardCollection errors', done => {
it('should return InternalError on other shardCollection errors', async () => {
setupMockClient(true);
commandStub.rejects(makeMongoError('HostUnreachable', 6));
client.createBucket('test-bucket', baseBucket, logger, err => {
assert(err);
assert(err.is.InternalError);
done();
});
await assert.rejects(
() => createBucketPromised('test-bucket'),
err => err?.is?.InternalError,
);
});

it('should not invoke the callback again when it throws', async () => {
setupMockClient(false);
const callbackError = new Error('callback failure');
const callback = sinon.stub().throws(callbackError);

await assert.rejects(
() => client.createBucket('test-bucket', baseBucket, logger, callback),
err => err === callbackError,
);
assert(callback.calledOnce);
});
});

Expand Down Expand Up @@ -626,7 +671,7 @@ describe('MongoClientInterface, tests', () => {
});

it('should create a bucket whose backing collection already exists', done => {
const bucketName = 'test-bucket-collection-exists';
const bucketName = `test-bucket-collection-exists-${Date.now()}`;
async.waterfall(
[
next => {
Expand All @@ -648,13 +693,22 @@ describe('MongoClientInterface, tests', () => {
);
});

it('should succeed on concurrent createBucket calls for the same bucket', done => {
it('should return BucketAlreadyExists on concurrent createBucket calls', done => {
const bucketName = 'test-bucket-concurrent-create';
const results = [];
async.times(
5,
(n, next) => createBucket(client, bucketName, false, next),
(n, next) =>
createBucket(client, bucketName, false, err => {
results.push(err);
next();
}),
err => {
assert.ifError(err);
const successes = results.filter(e => !e).length;
const alreadyExists = results.filter(e => e?.is?.BucketAlreadyExists).length;
assert.strictEqual(successes, 1);
assert.strictEqual(alreadyExists, 4);
client.deleteBucket(bucketName, logger, done);
},
);
Expand Down
Loading