From 3c6cc7c79423bd1d5db24bab2f541336168ca6fe Mon Sep 17 00:00:00 2001 From: Warren James Date: Tue, 4 Feb 2025 16:02:21 -0500 Subject: [PATCH 1/7] Update messages --- src/error.ts | 5 ++++- src/sdam/topology_description.ts | 12 +++++++++++- .../rs/new_primary.json | 3 ++- .../rs/new_primary.yml | 3 ++- .../rs/new_primary_new_electionid.json | 2 +- .../rs/new_primary_new_electionid.yml | 2 +- .../rs/new_primary_new_setversion.json | 2 +- .../rs/new_primary_new_setversion.yml | 2 +- .../rs/primary_disconnect_electionid.json | 3 ++- .../rs/primary_disconnect_electionid.yml | 3 ++- .../rs/primary_disconnect_setversion.json | 3 ++- .../rs/primary_disconnect_setversion.yml | 3 ++- ...etversion_greaterthan_max_without_electionid.json | 2 +- ...setversion_greaterthan_max_without_electionid.yml | 2 +- .../rs/setversion_without_electionid-pre-6.0.json | 2 +- .../rs/setversion_without_electionid-pre-6.0.yml | 2 +- .../use_setversion_without_electionid-pre-6.0.json | 2 +- .../rs/use_setversion_without_electionid-pre-6.0.yml | 2 +- 18 files changed, 37 insertions(+), 18 deletions(-) diff --git a/src/error.ts b/src/error.ts index 6d41087e3f5..6038e660197 100644 --- a/src/error.ts +++ b/src/error.ts @@ -359,10 +359,13 @@ export class MongoStalePrimaryError extends MongoRuntimeError { serverDescription: ServerDescription, maxSetVersion: number | null, maxElectionId: ObjectId | null, + message?: string, options?: { cause?: Error } ) { + console.log(message); super( - `primary marked stale due to electionId/setVersion mismatch: server setVersion: ${serverDescription.setVersion}, server electionId: ${serverDescription.electionId}, topology setVersion: ${maxSetVersion}, topology electionId: ${maxElectionId}`, + message ?? + `primary marked stale due to electionId/setVersion mismatch: server setVersion: ${serverDescription.setVersion}, server electionId: ${serverDescription.electionId}, topology setVersion: ${maxSetVersion}, topology electionId: ${maxElectionId}`, options ); } diff --git a/src/sdam/topology_description.ts b/src/sdam/topology_description.ts index d4f7af11477..2e127540d8e 100644 --- a/src/sdam/topology_description.ts +++ b/src/sdam/topology_description.ts @@ -398,6 +398,7 @@ function updateRsFromPrimary( } else { // Stale primary // replace serverDescription with a default ServerDescription of type "Unknown" + console.log(`marking newly discovered primary: ${serverDescription.address} as stale`); serverDescriptions.set( serverDescription.address, new ServerDescription(serverDescription.address, undefined, { @@ -416,6 +417,7 @@ function updateRsFromPrimary( compareObjectId(maxElectionId, electionId) > 0 ) { // this primary is stale, we must remove it + console.log(`marking newly discovered primary: ${serverDescription.address} as stale`); serverDescriptions.set( serverDescription.address, new ServerDescription(serverDescription.address, undefined, { @@ -441,11 +443,19 @@ function updateRsFromPrimary( // We've heard from the primary. Is it the same primary as before? for (const [address, server] of serverDescriptions) { if (server.type === ServerType.RSPrimary && server.address !== serverDescription.address) { + console.log( + `server.address: ${server.address}; serverDescription.address: ${serverDescription.address}` + ); // Reset old primary's type to Unknown. serverDescriptions.set( address, new ServerDescription(server.address, undefined, { - error: new MongoStalePrimaryError(serverDescription, maxSetVersion, maxElectionId) + error: new MongoStalePrimaryError( + serverDescription, + maxSetVersion, + maxElectionId, + 'primary marked stale due to discovery of newer primary' + ) }) ); diff --git a/test/spec/server-discovery-and-monitoring/rs/new_primary.json b/test/spec/server-discovery-and-monitoring/rs/new_primary.json index 1a84c69c919..69b07516b99 100644 --- a/test/spec/server-discovery-and-monitoring/rs/new_primary.json +++ b/test/spec/server-discovery-and-monitoring/rs/new_primary.json @@ -58,7 +58,8 @@ "servers": { "a:27017": { "type": "Unknown", - "setName": null + "setName": null, + "error": "primary marked stale due to discovery of newer primary" }, "b:27017": { "type": "RSPrimary", diff --git a/test/spec/server-discovery-and-monitoring/rs/new_primary.yml b/test/spec/server-discovery-and-monitoring/rs/new_primary.yml index f2485a18633..50c996f52c2 100644 --- a/test/spec/server-discovery-and-monitoring/rs/new_primary.yml +++ b/test/spec/server-discovery-and-monitoring/rs/new_primary.yml @@ -63,7 +63,8 @@ phases: [ "a:27017": { type: "Unknown", - setName: + setName:, + error: "primary marked stale due to discovery of newer primary" }, "b:27017": { diff --git a/test/spec/server-discovery-and-monitoring/rs/new_primary_new_electionid.json b/test/spec/server-discovery-and-monitoring/rs/new_primary_new_electionid.json index ec6e736d55c..90ef0ce8dc3 100644 --- a/test/spec/server-discovery-and-monitoring/rs/new_primary_new_electionid.json +++ b/test/spec/server-discovery-and-monitoring/rs/new_primary_new_electionid.json @@ -77,7 +77,7 @@ "type": "Unknown", "setName": null, "electionId": null, - "error": "primary marked stale due to electionId/setVersion mismatch" + "error": "primary marked stale due to discovery of newer primary" }, "b:27017": { "type": "RSPrimary", diff --git a/test/spec/server-discovery-and-monitoring/rs/new_primary_new_electionid.yml b/test/spec/server-discovery-and-monitoring/rs/new_primary_new_electionid.yml index 945930a8f5f..6418301c084 100644 --- a/test/spec/server-discovery-and-monitoring/rs/new_primary_new_electionid.yml +++ b/test/spec/server-discovery-and-monitoring/rs/new_primary_new_electionid.yml @@ -64,7 +64,7 @@ phases: [ type: "Unknown", setName: , electionId: , - error: "primary marked stale due to electionId/setVersion mismatch" + error: "primary marked stale due to discovery of newer primary" }, "b:27017": { type: "RSPrimary", diff --git a/test/spec/server-discovery-and-monitoring/rs/new_primary_new_setversion.json b/test/spec/server-discovery-and-monitoring/rs/new_primary_new_setversion.json index 1db05cbd5a1..9c1e2d4bddc 100644 --- a/test/spec/server-discovery-and-monitoring/rs/new_primary_new_setversion.json +++ b/test/spec/server-discovery-and-monitoring/rs/new_primary_new_setversion.json @@ -77,7 +77,7 @@ "type": "Unknown", "setName": null, "electionId": null, - "error": "primary marked stale due to electionId/setVersion mismatch" + "error": "primary marked stale due to discovery of newer primary" }, "b:27017": { "type": "RSPrimary", diff --git a/test/spec/server-discovery-and-monitoring/rs/new_primary_new_setversion.yml b/test/spec/server-discovery-and-monitoring/rs/new_primary_new_setversion.yml index c46e987927f..7abf69a8c0d 100644 --- a/test/spec/server-discovery-and-monitoring/rs/new_primary_new_setversion.yml +++ b/test/spec/server-discovery-and-monitoring/rs/new_primary_new_setversion.yml @@ -64,7 +64,7 @@ phases: [ type: "Unknown", setName: , electionId:, - error: "primary marked stale due to electionId/setVersion mismatch" + error: "primary marked stale due to discovery of newer primary" }, "b:27017": { type: "RSPrimary", diff --git a/test/spec/server-discovery-and-monitoring/rs/primary_disconnect_electionid.json b/test/spec/server-discovery-and-monitoring/rs/primary_disconnect_electionid.json index d87f7a3612d..b030bd2c538 100644 --- a/test/spec/server-discovery-and-monitoring/rs/primary_disconnect_electionid.json +++ b/test/spec/server-discovery-and-monitoring/rs/primary_disconnect_electionid.json @@ -49,7 +49,7 @@ "type": "Unknown", "setName": null, "electionId": null, - "error": "primary marked stale due to electionId/setVersion mismatch" + "error": "primary marked stale due to discovery of newer primary" }, "b:27017": { "type": "RSPrimary", @@ -125,6 +125,7 @@ "a:27017": { "type": "Unknown", "setName": null, + "error": "primary marked stale due to electionId/setVersion mismatch", "electionId": null }, "b:27017": { diff --git a/test/spec/server-discovery-and-monitoring/rs/primary_disconnect_electionid.yml b/test/spec/server-discovery-and-monitoring/rs/primary_disconnect_electionid.yml index 7a2b9572093..4ee8612019c 100644 --- a/test/spec/server-discovery-and-monitoring/rs/primary_disconnect_electionid.yml +++ b/test/spec/server-discovery-and-monitoring/rs/primary_disconnect_electionid.yml @@ -37,7 +37,7 @@ phases: [ type: "Unknown", setName: , electionId:, - error: "primary marked stale due to electionId/setVersion mismatch" + error: "primary marked stale due to discovery of newer primary" }, "b:27017": { type: "RSPrimary", @@ -100,6 +100,7 @@ phases: [ "a:27017": { type: "Unknown", setName: , + error: "primary marked stale due to electionId/setVersion mismatch", electionId: }, "b:27017": { diff --git a/test/spec/server-discovery-and-monitoring/rs/primary_disconnect_setversion.json b/test/spec/server-discovery-and-monitoring/rs/primary_disconnect_setversion.json index 0a59fd9ce62..653a5f29e81 100644 --- a/test/spec/server-discovery-and-monitoring/rs/primary_disconnect_setversion.json +++ b/test/spec/server-discovery-and-monitoring/rs/primary_disconnect_setversion.json @@ -49,7 +49,7 @@ "type": "Unknown", "setName": null, "electionId": null, - "error": "primary marked stale due to electionId/setVersion mismatch" + "error": "primary marked stale due to discovery of newer primary" }, "b:27017": { "type": "RSPrimary", @@ -125,6 +125,7 @@ "a:27017": { "type": "Unknown", "setName": null, + "error": "primary marked stale due to electionId/setVersion mismatch", "electionId": null }, "b:27017": { diff --git a/test/spec/server-discovery-and-monitoring/rs/primary_disconnect_setversion.yml b/test/spec/server-discovery-and-monitoring/rs/primary_disconnect_setversion.yml index 1592dfff1a5..bc6c538e930 100644 --- a/test/spec/server-discovery-and-monitoring/rs/primary_disconnect_setversion.yml +++ b/test/spec/server-discovery-and-monitoring/rs/primary_disconnect_setversion.yml @@ -37,7 +37,7 @@ phases: [ type: "Unknown", setName: , electionId:, - error: "primary marked stale due to electionId/setVersion mismatch" + error: "primary marked stale due to discovery of newer primary" }, "b:27017": { type: "RSPrimary", @@ -100,6 +100,7 @@ phases: [ "a:27017": { type: "Unknown", setName: , + error: "primary marked stale due to electionId/setVersion mismatch", electionId: }, "b:27017": { diff --git a/test/spec/server-discovery-and-monitoring/rs/setversion_greaterthan_max_without_electionid.json b/test/spec/server-discovery-and-monitoring/rs/setversion_greaterthan_max_without_electionid.json index e3451659472..06c89609f54 100644 --- a/test/spec/server-discovery-and-monitoring/rs/setversion_greaterthan_max_without_electionid.json +++ b/test/spec/server-discovery-and-monitoring/rs/setversion_greaterthan_max_without_electionid.json @@ -66,7 +66,7 @@ "type": "Unknown", "setName": null, "electionId": null, - "error": "primary marked stale due to electionId/setVersion mismatch" + "error": "primary marked stale due to discovery of newer primary" }, "b:27017": { "type": "RSPrimary", diff --git a/test/spec/server-discovery-and-monitoring/rs/setversion_greaterthan_max_without_electionid.yml b/test/spec/server-discovery-and-monitoring/rs/setversion_greaterthan_max_without_electionid.yml index 07b98e8af53..622597809e7 100644 --- a/test/spec/server-discovery-and-monitoring/rs/setversion_greaterthan_max_without_electionid.yml +++ b/test/spec/server-discovery-and-monitoring/rs/setversion_greaterthan_max_without_electionid.yml @@ -62,7 +62,7 @@ phases: [ type: "Unknown", setName: , electionId:, - error: "primary marked stale due to electionId/setVersion mismatch" + error: "primary marked stale due to discovery of newer primary" }, "b:27017": { type: "RSPrimary", diff --git a/test/spec/server-discovery-and-monitoring/rs/setversion_without_electionid-pre-6.0.json b/test/spec/server-discovery-and-monitoring/rs/setversion_without_electionid-pre-6.0.json index 6b6cd39255c..87029e578b7 100644 --- a/test/spec/server-discovery-and-monitoring/rs/setversion_without_electionid-pre-6.0.json +++ b/test/spec/server-discovery-and-monitoring/rs/setversion_without_electionid-pre-6.0.json @@ -66,7 +66,7 @@ "type": "Unknown", "setName": null, "electionId": null, - "error": "primary marked stale due to electionId/setVersion mismatch" + "error": "primary marked stale due to discovery of newer primary" }, "b:27017": { "type": "RSPrimary", diff --git a/test/spec/server-discovery-and-monitoring/rs/setversion_without_electionid-pre-6.0.yml b/test/spec/server-discovery-and-monitoring/rs/setversion_without_electionid-pre-6.0.yml index c186dfe3744..0fd735dcc5f 100644 --- a/test/spec/server-discovery-and-monitoring/rs/setversion_without_electionid-pre-6.0.yml +++ b/test/spec/server-discovery-and-monitoring/rs/setversion_without_electionid-pre-6.0.yml @@ -62,7 +62,7 @@ phases: [ type: "Unknown", setName: , electionId:, - error: "primary marked stale due to electionId/setVersion mismatch" + error: "primary marked stale due to discovery of newer primary" }, "b:27017": { type: "RSPrimary", diff --git a/test/spec/server-discovery-and-monitoring/rs/use_setversion_without_electionid-pre-6.0.json b/test/spec/server-discovery-and-monitoring/rs/use_setversion_without_electionid-pre-6.0.json index 50c6666ee53..a63efeac128 100644 --- a/test/spec/server-discovery-and-monitoring/rs/use_setversion_without_electionid-pre-6.0.json +++ b/test/spec/server-discovery-and-monitoring/rs/use_setversion_without_electionid-pre-6.0.json @@ -74,7 +74,7 @@ "type": "Unknown", "setName": null, "electionId": null, - "error": "primary marked stale due to electionId/setVersion mismatch" + "error": "primary marked stale due to discovery of newer primary" }, "b:27017": { "type": "RSPrimary", diff --git a/test/spec/server-discovery-and-monitoring/rs/use_setversion_without_electionid-pre-6.0.yml b/test/spec/server-discovery-and-monitoring/rs/use_setversion_without_electionid-pre-6.0.yml index 799075c1999..d02fba5d522 100644 --- a/test/spec/server-discovery-and-monitoring/rs/use_setversion_without_electionid-pre-6.0.yml +++ b/test/spec/server-discovery-and-monitoring/rs/use_setversion_without_electionid-pre-6.0.yml @@ -63,7 +63,7 @@ phases: [ type: "Unknown", setName: , electionId:, - error: "primary marked stale due to electionId/setVersion mismatch" + error: "primary marked stale due to discovery of newer primary" }, "b:27017": { type: "RSPrimary", From 3205ddd1a44c1076b9d53d8a7ad51dc2708c6a48 Mon Sep 17 00:00:00 2001 From: Warren James Date: Tue, 4 Feb 2025 16:21:28 -0500 Subject: [PATCH 2/7] remove log statements --- src/sdam/topology_description.ts | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/sdam/topology_description.ts b/src/sdam/topology_description.ts index 2e127540d8e..7be0d0bdb9a 100644 --- a/src/sdam/topology_description.ts +++ b/src/sdam/topology_description.ts @@ -398,7 +398,6 @@ function updateRsFromPrimary( } else { // Stale primary // replace serverDescription with a default ServerDescription of type "Unknown" - console.log(`marking newly discovered primary: ${serverDescription.address} as stale`); serverDescriptions.set( serverDescription.address, new ServerDescription(serverDescription.address, undefined, { @@ -417,7 +416,6 @@ function updateRsFromPrimary( compareObjectId(maxElectionId, electionId) > 0 ) { // this primary is stale, we must remove it - console.log(`marking newly discovered primary: ${serverDescription.address} as stale`); serverDescriptions.set( serverDescription.address, new ServerDescription(serverDescription.address, undefined, { @@ -443,9 +441,6 @@ function updateRsFromPrimary( // We've heard from the primary. Is it the same primary as before? for (const [address, server] of serverDescriptions) { if (server.type === ServerType.RSPrimary && server.address !== serverDescription.address) { - console.log( - `server.address: ${server.address}; serverDescription.address: ${serverDescription.address}` - ); // Reset old primary's type to Unknown. serverDescriptions.set( address, From 16efd860dc516208585030cf389ea415d9a8e938 Mon Sep 17 00:00:00 2001 From: Warren James Date: Tue, 4 Feb 2025 17:40:18 -0500 Subject: [PATCH 3/7] remove console.log --- src/error.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/error.ts b/src/error.ts index 6038e660197..57c607a6655 100644 --- a/src/error.ts +++ b/src/error.ts @@ -362,7 +362,6 @@ export class MongoStalePrimaryError extends MongoRuntimeError { message?: string, options?: { cause?: Error } ) { - console.log(message); super( message ?? `primary marked stale due to electionId/setVersion mismatch: server setVersion: ${serverDescription.setVersion}, server electionId: ${serverDescription.electionId}, topology setVersion: ${maxSetVersion}, topology electionId: ${maxElectionId}`, From 669c6d1ddfcd424b2e5c50530126f89ef2f9bddd Mon Sep 17 00:00:00 2001 From: Warren James Date: Wed, 5 Feb 2025 11:49:15 -0500 Subject: [PATCH 4/7] code organization --- src/error.ts | 18 ++++-------------- src/sdam/topology_description.ts | 24 +++++++++++++++++++----- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/src/error.ts b/src/error.ts index 57c607a6655..2dc382ed4c2 100644 --- a/src/error.ts +++ b/src/error.ts @@ -1,10 +1,10 @@ -import type { Document, ObjectId } from './bson'; +import type { Document } from './bson'; import { type ClientBulkWriteError, type ClientBulkWriteResult } from './operations/client_bulk_write/common'; import type { ServerType } from './sdam/common'; -import type { ServerDescription, TopologyVersion } from './sdam/server_description'; +import type { TopologyVersion } from './sdam/server_description'; import type { TopologyDescription } from './sdam/topology_description'; /** @public */ @@ -355,18 +355,8 @@ export class MongoStalePrimaryError extends MongoRuntimeError { * * @public **/ - constructor( - serverDescription: ServerDescription, - maxSetVersion: number | null, - maxElectionId: ObjectId | null, - message?: string, - options?: { cause?: Error } - ) { - super( - message ?? - `primary marked stale due to electionId/setVersion mismatch: server setVersion: ${serverDescription.setVersion}, server electionId: ${serverDescription.electionId}, topology setVersion: ${maxSetVersion}, topology electionId: ${maxElectionId}`, - options - ); + constructor(message: string, options?: { cause?: Error }) { + super(message, options); } override get name(): string { diff --git a/src/sdam/topology_description.ts b/src/sdam/topology_description.ts index 7be0d0bdb9a..55eb48bb35f 100644 --- a/src/sdam/topology_description.ts +++ b/src/sdam/topology_description.ts @@ -376,6 +376,19 @@ function updateRsFromPrimary( maxSetVersion: number | null = null, maxElectionId: ObjectId | null = null ): [TopologyType, string | null, number | null, ObjectId | null] { + const setVersionElectionIdMismatch = ( + serverDescription: ServerDescription, + maxSetVersion: number | null, + maxElectionId: ObjectId | null + ) => { + return ( + `primary marked stale due to electionId/setVersion mismatch:` + + ` server setVersion: ${serverDescription.setVersion},` + + ` server electionId: ${serverDescription.electionId},` + + ` topology setVersion: ${maxSetVersion},` + + ` topology electionId: ${maxElectionId}` + ); + }; setName = setName || serverDescription.setName; if (setName !== serverDescription.setName) { serverDescriptions.delete(serverDescription.address); @@ -401,7 +414,9 @@ function updateRsFromPrimary( serverDescriptions.set( serverDescription.address, new ServerDescription(serverDescription.address, undefined, { - error: new MongoStalePrimaryError(serverDescription, maxSetVersion, maxElectionId) + error: new MongoStalePrimaryError( + setVersionElectionIdMismatch(serverDescription, maxSetVersion, maxElectionId) + ) }) ); @@ -419,7 +434,9 @@ function updateRsFromPrimary( serverDescriptions.set( serverDescription.address, new ServerDescription(serverDescription.address, undefined, { - error: new MongoStalePrimaryError(serverDescription, maxSetVersion, maxElectionId) + error: new MongoStalePrimaryError( + setVersionElectionIdMismatch(serverDescription, maxSetVersion, maxElectionId) + ) }) ); @@ -446,9 +463,6 @@ function updateRsFromPrimary( address, new ServerDescription(server.address, undefined, { error: new MongoStalePrimaryError( - serverDescription, - maxSetVersion, - maxElectionId, 'primary marked stale due to discovery of newer primary' ) }) From 392615815775136c7f349cac4a3335638088da17 Mon Sep 17 00:00:00 2001 From: Warren James Date: Thu, 6 Feb 2025 15:54:09 -0500 Subject: [PATCH 5/7] add unit test --- .../server_discovery_and_monitoring.test.ts | 123 ++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100644 test/unit/assorted/server_discovery_and_monitoring.test.ts diff --git a/test/unit/assorted/server_discovery_and_monitoring.test.ts b/test/unit/assorted/server_discovery_and_monitoring.test.ts new file mode 100644 index 00000000000..8daae6082b3 --- /dev/null +++ b/test/unit/assorted/server_discovery_and_monitoring.test.ts @@ -0,0 +1,123 @@ +import { expect } from 'chai'; +import { type TopologyDescription } from 'mongodb-legacy'; +import * as sinon from 'sinon'; + +import { + type MongoClient, + ObjectId, + Server, + ServerDescription, + Topology, + TOPOLOGY_DESCRIPTION_CHANGED, + type TopologyDescriptionChangedEvent +} from '../../mongodb'; + +const SDAM_EVENTS = [ + // Topology events + TOPOLOGY_DESCRIPTION_CHANGED +]; + +describe('Server Discovery and Monitoring', function () { + let serverConnect: sinon.SinonStub; + let topologySelectServer: sinon.SinonStub; + let client: MongoClient; + let events: TopologyDescriptionChangedEvent[] = []; + + function getNewDescription() { + const [topologyDescriptionChanged] = events.filter( + x => x.name === 'topologyDescriptionChanged' + ); + events = []; + return topologyDescriptionChanged.newDescription; + } + + before(async function () { + serverConnect = sinon.stub(Server.prototype, 'connect').callsFake(function () { + this.s.state = 'connected'; + this.emit('connect'); + }); + topologySelectServer = sinon + .stub(Topology.prototype, 'selectServer') + .callsFake(async function (_selector, _options) { + topologySelectServer.restore(); + + const fakeServer = { s: { state: 'connected' }, removeListener: () => true }; + return fakeServer; + }); + const events = []; + client.on('topologyDescriptionChanged', event => events.push(event)); + await client.connect(); + }); + + after(function () { + serverConnect.restore(); + }); + + describe('when a newer primary is detected', function () { + it('steps down original primary to unknown server description with appropriate error message', async function () { + let newDescription: TopologyDescription; + // Start with a as primary + client.topology.serverUpdateHandler( + new ServerDescription('a:27017', { + ok: 1, + helloOk: true, + isWritablePrimary: true, + hosts: ['a:27017', 'b:27017'], + setName: 'rs', + setVersion: 1, + electionId: ObjectId.createFromHexString('000000000000000000000001'), + minWireVersion: 0, + maxWireVersion: 21 + }) + ); + + newDescription = getNewDescription(); + + expect(newDescription.type).to.equal('ReplicaSetWithPrimary'); + + // b is elected as primary, a gets marked stale + client.topology.serverUpdateHandler( + new ServerDescription('b:27017', { + ok: 1, + helloOk: true, + isWritablePrimary: true, + hosts: ['a:27017', 'b:27017'], + setName: 'rs', + setVersion: 2, + electionId: ObjectId.createFromHexString('000000000000000000000001'), + minWireVersion: 0, + maxWireVersion: 21 + }) + ); + + newDescription = getNewDescription(); + + let aOutcome = newDescription.servers.get('a:27017'); + expect(aOutcome.error).to.match(/primary marked stale due to discovery of newer primary/); + + // a still incorrectly reports as primary + client.topology.serverUpdateHandler( + new ServerDescription('a:27017', { + ok: 1, + helloOk: true, + isWritablePrimary: true, + hosts: ['a:27017', 'b:27017'], + setName: 'rs', + setVersion: 1, + electionId: ObjectId.createFromHexString('000000000000000000000001'), + minWireVersion: 0, + maxWireVersion: 21 + }) + ); + + newDescription = getNewDescription(); + + aOutcome = newDescription.servers.get('a:27017'); + + expect(aOutcome.type).to.equal('Unknown'); + expect(aOutcome.error).to.match( + /primary marked stale due to electionId\/setVersion mismatch: server setVersion: \d+, server electionId: \d{24}, topology setVersion: \d, topology electionId: \d{24}/ + ); + }); + }); +}); From 8efe1efea5ceca85c96a25ecc600298a21afb660 Mon Sep 17 00:00:00 2001 From: Warren James Date: Thu, 6 Feb 2025 16:08:29 -0500 Subject: [PATCH 6/7] test cleanup --- .../server_discovery_and_monitoring.test.ts | 106 +++++++++--------- 1 file changed, 53 insertions(+), 53 deletions(-) diff --git a/test/unit/assorted/server_discovery_and_monitoring.test.ts b/test/unit/assorted/server_discovery_and_monitoring.test.ts index 8daae6082b3..824dbd4bf54 100644 --- a/test/unit/assorted/server_discovery_and_monitoring.test.ts +++ b/test/unit/assorted/server_discovery_and_monitoring.test.ts @@ -3,39 +3,31 @@ import { type TopologyDescription } from 'mongodb-legacy'; import * as sinon from 'sinon'; import { - type MongoClient, + MongoClient, ObjectId, Server, ServerDescription, Topology, - TOPOLOGY_DESCRIPTION_CHANGED, type TopologyDescriptionChangedEvent } from '../../mongodb'; -const SDAM_EVENTS = [ - // Topology events - TOPOLOGY_DESCRIPTION_CHANGED -]; - describe('Server Discovery and Monitoring', function () { let serverConnect: sinon.SinonStub; let topologySelectServer: sinon.SinonStub; let client: MongoClient; - let events: TopologyDescriptionChangedEvent[] = []; + let events: TopologyDescriptionChangedEvent[]; function getNewDescription() { - const [topologyDescriptionChanged] = events.filter( - x => x.name === 'topologyDescriptionChanged' - ); - events = []; + const topologyDescriptionChanged = events[events.length - 1]; return topologyDescriptionChanged.newDescription; } - before(async function () { + beforeEach(async function () { serverConnect = sinon.stub(Server.prototype, 'connect').callsFake(function () { this.s.state = 'connected'; this.emit('connect'); }); + topologySelectServer = sinon .stub(Topology.prototype, 'selectServer') .callsFake(async function (_selector, _options) { @@ -44,57 +36,65 @@ describe('Server Discovery and Monitoring', function () { const fakeServer = { s: { state: 'connected' }, removeListener: () => true }; return fakeServer; }); - const events = []; + + events = []; + client = new MongoClient('mongodb://a/?replicaSet=rs'); client.on('topologyDescriptionChanged', event => events.push(event)); await client.connect(); + + // Start with a as primary + client.topology.serverUpdateHandler( + new ServerDescription('a:27017', { + ok: 1, + helloOk: true, + isWritablePrimary: true, + hosts: ['a:27017', 'b:27017'], + setName: 'rs', + setVersion: 1, + electionId: ObjectId.createFromHexString('000000000000000000000001'), + minWireVersion: 0, + maxWireVersion: 21 + }) + ); + + // b is elected as primary, a gets marked stale + client.topology.serverUpdateHandler( + new ServerDescription('b:27017', { + ok: 1, + helloOk: true, + isWritablePrimary: true, + hosts: ['a:27017', 'b:27017'], + setName: 'rs', + setVersion: 2, + electionId: ObjectId.createFromHexString('000000000000000000000001'), + minWireVersion: 0, + maxWireVersion: 21 + }) + ); }); - after(function () { + afterEach(async function () { serverConnect.restore(); + await client.close().catch(() => null); }); - describe('when a newer primary is detected', function () { - it('steps down original primary to unknown server description with appropriate error message', async function () { - let newDescription: TopologyDescription; - // Start with a as primary - client.topology.serverUpdateHandler( - new ServerDescription('a:27017', { - ok: 1, - helloOk: true, - isWritablePrimary: true, - hosts: ['a:27017', 'b:27017'], - setName: 'rs', - setVersion: 1, - electionId: ObjectId.createFromHexString('000000000000000000000001'), - minWireVersion: 0, - maxWireVersion: 21 - }) - ); - - newDescription = getNewDescription(); - - expect(newDescription.type).to.equal('ReplicaSetWithPrimary'); - - // b is elected as primary, a gets marked stale - client.topology.serverUpdateHandler( - new ServerDescription('b:27017', { - ok: 1, - helloOk: true, - isWritablePrimary: true, - hosts: ['a:27017', 'b:27017'], - setName: 'rs', - setVersion: 2, - electionId: ObjectId.createFromHexString('000000000000000000000001'), - minWireVersion: 0, - maxWireVersion: 21 - }) - ); + let newDescription: TopologyDescription; + describe('when a newer primary is detected', function () { + it('steps down original primary to unknown server description with appropriate error message', function () { newDescription = getNewDescription(); - let aOutcome = newDescription.servers.get('a:27017'); + const aOutcome = newDescription.servers.get('a:27017'); + const bOutcome = newDescription.servers.get('b:27017'); + expect(aOutcome.type).to.equal('Unknown'); expect(aOutcome.error).to.match(/primary marked stale due to discovery of newer primary/); + expect(bOutcome.type).to.equal('RSPrimary'); + }); + }); + + describe('when a stale primary still reports itself as primary', function () { + it('gets marked as unknown with an error message with the new and old replicaSetVersion and electionId', function () { // a still incorrectly reports as primary client.topology.serverUpdateHandler( new ServerDescription('a:27017', { @@ -112,7 +112,7 @@ describe('Server Discovery and Monitoring', function () { newDescription = getNewDescription(); - aOutcome = newDescription.servers.get('a:27017'); + const aOutcome = newDescription.servers.get('a:27017'); expect(aOutcome.type).to.equal('Unknown'); expect(aOutcome.error).to.match( From a85a5fa7135852deef21dc3dea0f2e4750a272ac Mon Sep 17 00:00:00 2001 From: Warren James Date: Thu, 6 Feb 2025 16:28:29 -0500 Subject: [PATCH 7/7] update regex --- test/unit/assorted/server_discovery_and_monitoring.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/unit/assorted/server_discovery_and_monitoring.test.ts b/test/unit/assorted/server_discovery_and_monitoring.test.ts index 824dbd4bf54..1c58591fb85 100644 --- a/test/unit/assorted/server_discovery_and_monitoring.test.ts +++ b/test/unit/assorted/server_discovery_and_monitoring.test.ts @@ -116,7 +116,7 @@ describe('Server Discovery and Monitoring', function () { expect(aOutcome.type).to.equal('Unknown'); expect(aOutcome.error).to.match( - /primary marked stale due to electionId\/setVersion mismatch: server setVersion: \d+, server electionId: \d{24}, topology setVersion: \d, topology electionId: \d{24}/ + /primary marked stale due to electionId\/setVersion mismatch: server setVersion: \d+, server electionId: \d{24}, topology setVersion: \d+, topology electionId: \d{24}/ ); }); });