From 761f5cc6fc1ea3b844eddbd303ec83144188ef2f Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Mon, 1 Jun 2026 14:56:16 -0500 Subject: [PATCH 1/4] fix(mongodb-runner): handle arbiter RS convergence race in metadata check When a replica set is initiated, the code only waits for a PRIMARY to appear before proceeding to assertAllServersHaveInsertedLocalMetadata(). An arbiter node may still be transitioning to ARBITER state at that point, so hello.arbiterOnly can be undefined while this.isArbiter is already true, causing an immediate "Arbiter flag mismatch" throw. Fix _ensureMatchingMetadataColl to retry via eventually() when isArbiter is true but hello has not yet confirmed arbiterOnly, giving the member time to converge. Also handle the deserialize() ordering where isArbiter is set after _populateBuildInfo runs by skipping the check when hello.arbiterOnly is true regardless of the local flag. Add unit tests covering the retry path, immediate-success path, timeout propagation, and the deserialize ordering edge case. --- .../mongodb-runner/src/mongoserver.spec.ts | 82 +++++++++++++++++++ packages/mongodb-runner/src/mongoserver.ts | 37 ++++++--- 2 files changed, 109 insertions(+), 10 deletions(-) create mode 100644 packages/mongodb-runner/src/mongoserver.spec.ts diff --git a/packages/mongodb-runner/src/mongoserver.spec.ts b/packages/mongodb-runner/src/mongoserver.spec.ts new file mode 100644 index 00000000..be0a493c --- /dev/null +++ b/packages/mongodb-runner/src/mongoserver.spec.ts @@ -0,0 +1,82 @@ +import { expect } from 'chai'; +import { MongoServer } from './mongoserver'; +import type { MongoClient } from 'mongodb'; +import sinon from 'sinon'; + +describe('MongoServer._ensureMatchingMetadataColl', function () { + let server: MongoServer; + + beforeEach(function () { + server = new MongoServer(); + }); + + afterEach(function () { + sinon.restore(); + }); + + function makeClient(responses: Array>): { + client: MongoClient; + commandStub: sinon.SinonStub; + } { + let callCount = 0; + const commandStub = sinon + .stub() + .callsFake( + async () => responses[Math.min(callCount++, responses.length - 1)], + ); + const client = { + db: sinon.stub().returns({ command: commandStub }), + } as unknown as MongoClient; + return { client, commandStub }; + } + + it('skips metadata check immediately when hello already reports arbiterOnly', async function () { + server.isArbiter = true; + const { client, commandStub } = makeClient([{ arbiterOnly: true }]); + + await (server as any)._ensureMatchingMetadataColl(client, 'insert-new'); + + // Only the initial hello call; no retries needed. + expect(commandStub).to.have.been.calledOnce; + }); + + it('retries hello until arbiterOnly is confirmed when arbiter has not yet converged', async function () { + server.isArbiter = true; + // First hello returns nothing; second (inside eventually) returns arbiterOnly. + const { client, commandStub } = makeClient([{}, { arbiterOnly: true }]); + + await (server as any)._ensureMatchingMetadataColl(client, 'insert-new'); + + // Two hello calls: the initial one and one successful retry. + expect(commandStub.callCount).to.equal(2); + }); + + it('throws after timeout when isArbiter is true but server never reports arbiterOnly', async function () { + server.isArbiter = true; + const { client } = makeClient([{}]); // never returns arbiterOnly: true + + let err: Error | undefined; + try { + await (server as any)._ensureMatchingMetadataColl(client, 'insert-new', { + intervalMs: 10, + timeoutMs: 50, + }); + } catch (e) { + err = e as Error; + } + + expect(err?.message).to.include( + 'Arbiter flag mismatch -- server should be arbiter but hello indicates it is not', + ); + }); + + it('skips metadata check when hello reports arbiterOnly but isArbiter is not yet set', async function () { + // Simulates the deserialize() ordering: isArbiter is assigned after _populateBuildInfo runs. + server.isArbiter = false; + const { client, commandStub } = makeClient([{ arbiterOnly: true }]); + + await (server as any)._ensureMatchingMetadataColl(client, 'restore-check'); + + expect(commandStub).to.have.been.calledOnce; + }); +}); diff --git a/packages/mongodb-runner/src/mongoserver.ts b/packages/mongodb-runner/src/mongoserver.ts index ec88dd13..94b911c0 100644 --- a/packages/mongodb-runner/src/mongoserver.ts +++ b/packages/mongodb-runner/src/mongoserver.ts @@ -21,6 +21,7 @@ import { jsonClone, makeConnectionString, sleep, + eventually, } from './util'; /** @@ -311,12 +312,12 @@ export class MongoServer extends EventEmitter { }); filterLogStreamForBuildInfo(logEntryStream).then( (buildInfo) => { - ((srv.buildInfo = buildInfo), + (srv.buildInfo = buildInfo), debug( 'got server build info from log', srv.serverVersion, srv.serverVariant, - )); + ); }, () => { /* ignore error */ @@ -421,17 +422,33 @@ export class MongoServer extends EventEmitter { private async _ensureMatchingMetadataColl( client: MongoClient, mode: 'insert-new' | 'restore-check', + retryOptions?: { intervalMs?: number; timeoutMs?: number }, ): Promise { - const hello = await client.db('admin').command({ hello: 1 }); - const { arbiterOnly } = hello; - if (arbiterOnly === this.isArbiter) { - debug('skipping metadata check for arbiter'); + let hello = await client.db('admin').command({ hello: 1 }); + if (this.isArbiter) { + // RS convergence: hello.arbiterOnly may lag behind the RS config while the + // member transitions to ARBITER state. Retry until confirmed or timeout. + if (!hello.arbiterOnly) { + await eventually( + async () => { + hello = await client.db('admin').command({ hello: 1 }); + if (!hello.arbiterOnly) { + throw new Error( + 'Arbiter flag mismatch -- server should be arbiter but hello indicates it is not', + ); + } + }, + retryOptions ?? { intervalMs: 500, timeoutMs: 30_000 }, + ); + } + debug('skipping metadata check for confirmed arbiter'); return; } - if (this.isArbiter) { - throw new Error( - 'Arbiter flag mismatch -- server should be arbiter but hello indicates it is not', - ); + if (hello.arbiterOnly) { + // isArbiter may not be set yet (e.g. during deserialize where it is + // assigned after _populateBuildInfo); skip metadata check. + debug('skipping metadata check for arbiter'); + return; } const isMongoS = hello.msg === 'isdbgrid'; From 9c392be5809161dc28c5b0fb94cd557d1961c765 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Mon, 1 Jun 2026 15:49:51 -0500 Subject: [PATCH 2/4] fix(mongodb-runner): remove spurious async from test stub callsFake --- packages/mongodb-runner/src/mongoserver.spec.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/mongodb-runner/src/mongoserver.spec.ts b/packages/mongodb-runner/src/mongoserver.spec.ts index be0a493c..f20ee2bc 100644 --- a/packages/mongodb-runner/src/mongoserver.spec.ts +++ b/packages/mongodb-runner/src/mongoserver.spec.ts @@ -21,9 +21,7 @@ describe('MongoServer._ensureMatchingMetadataColl', function () { let callCount = 0; const commandStub = sinon .stub() - .callsFake( - async () => responses[Math.min(callCount++, responses.length - 1)], - ); + .callsFake(() => responses[Math.min(callCount++, responses.length - 1)]); const client = { db: sinon.stub().returns({ command: commandStub }), } as unknown as MongoClient; From e492993d7ab8dbf59cf127918562d584457b0f67 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Mon, 1 Jun 2026 16:16:40 -0500 Subject: [PATCH 3/4] chore(mongodb-runner): apply prettier 3.8.3 formatting From 844062b28f403b3392cb49ed603bf758216aa9e7 Mon Sep 17 00:00:00 2001 From: Steven Silvester Date: Mon, 1 Jun 2026 16:17:33 -0500 Subject: [PATCH 4/4] chore(mongodb-runner): apply prettier 3.8.3 formatting --- packages/mongodb-runner/src/mongoserver.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/mongodb-runner/src/mongoserver.ts b/packages/mongodb-runner/src/mongoserver.ts index 94b911c0..8cf5b9d1 100644 --- a/packages/mongodb-runner/src/mongoserver.ts +++ b/packages/mongodb-runner/src/mongoserver.ts @@ -312,12 +312,12 @@ export class MongoServer extends EventEmitter { }); filterLogStreamForBuildInfo(logEntryStream).then( (buildInfo) => { - (srv.buildInfo = buildInfo), + ((srv.buildInfo = buildInfo), debug( 'got server build info from log', srv.serverVersion, srv.serverVariant, - ); + )); }, () => { /* ignore error */