From 75227d95f4688f20386dd2ad0d4960be18f4ad83 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Tue, 8 Jul 2025 12:18:58 -0400 Subject: [PATCH 01/11] feat(NODE-7020): remove ping on connect --- src/sdam/topology.ts | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/src/sdam/topology.ts b/src/sdam/topology.ts index 4da824d059a..97290393b54 100644 --- a/src/sdam/topology.ts +++ b/src/sdam/topology.ts @@ -46,7 +46,6 @@ import { makeStateMachine, noop, now, - ns, promiseWithResolvers, shuffle } from '../utils'; @@ -469,15 +468,9 @@ export class Topology extends TypedEventEmitter { readPreferenceServerSelector(readPreference), selectServerOptions ); - const skipPingOnConnect = this.s.options.__skipPingOnConnect === true; - if (!skipPingOnConnect && this.s.credentials) { - await server.command(ns('admin.$cmd'), { ping: 1 }, { timeoutContext }); - stateTransition(this, STATE_CONNECTED); - this.emit(Topology.OPEN, this); - this.emit(Topology.CONNECT, this); - - return this; - } + + const connection = await server.pool.checkOut({ timeoutContext: timeoutContext }); + server.pool.checkIn(connection); stateTransition(this, STATE_CONNECTED); this.emit(Topology.OPEN, this); From 9b4bb6e8a34ee379b9828d37413c84a42cd70ecf Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Wed, 16 Jul 2025 16:35:49 +0200 Subject: [PATCH 02/11] test: dont connect client --- test/tools/unified-spec-runner/entities.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/tools/unified-spec-runner/entities.ts b/test/tools/unified-spec-runner/entities.ts index 34d5aa886d9..6e0ea988ea2 100644 --- a/test/tools/unified-spec-runner/entities.ts +++ b/test/tools/unified-spec-runner/entities.ts @@ -621,9 +621,7 @@ export class EntitiesMap extends Map { const client = new UnifiedMongoClient(uri, entity.client, config); try { new EntityEventRegistry(client, entity.client, map).register(); - await client.connect(); } catch (error) { - console.error('failed to connect entity', entity); // In the case where multiple clients are defined in the test and any one of them failed // to connect, but others did succeed, we need to ensure all open clients are closed. const clients = map.mapOf('client'); From 6197f4f5637fe5ff0123f3a7ee11e8d99b9c021e Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Fri, 1 Aug 2025 11:40:48 +0200 Subject: [PATCH 03/11] Revert "test: dont connect client" This reverts commit 8021743c016238748402d50b7fee66d3670b3ad8. --- test/tools/unified-spec-runner/entities.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/tools/unified-spec-runner/entities.ts b/test/tools/unified-spec-runner/entities.ts index 6e0ea988ea2..34d5aa886d9 100644 --- a/test/tools/unified-spec-runner/entities.ts +++ b/test/tools/unified-spec-runner/entities.ts @@ -621,7 +621,9 @@ export class EntitiesMap extends Map { const client = new UnifiedMongoClient(uri, entity.client, config); try { new EntityEventRegistry(client, entity.client, map).register(); + await client.connect(); } catch (error) { + console.error('failed to connect entity', entity); // In the case where multiple clients are defined in the test and any one of them failed // to connect, but others did succeed, we need to ensure all open clients are closed. const clients = map.mapOf('client'); From 24f5fe3d9ea06634512c769e78ad8975a617ed0d Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Mon, 4 Aug 2025 14:29:33 +0200 Subject: [PATCH 04/11] chore: keep option --- src/sdam/topology.ts | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/sdam/topology.ts b/src/sdam/topology.ts index 97290393b54..a7ca443f6d9 100644 --- a/src/sdam/topology.ts +++ b/src/sdam/topology.ts @@ -469,8 +469,16 @@ export class Topology extends TypedEventEmitter { selectServerOptions ); - const connection = await server.pool.checkOut({ timeoutContext: timeoutContext }); - server.pool.checkIn(connection); + const skipPingOnConnect = this.s.options.__skipPingOnConnect === true; + if (!skipPingOnConnect && this.s.credentials) { + const connection = await server.pool.checkOut({ timeoutContext: timeoutContext }); + server.pool.checkIn(connection); + stateTransition(this, STATE_CONNECTED); + this.emit(Topology.OPEN, this); + this.emit(Topology.CONNECT, this); + + return this; + } stateTransition(this, STATE_CONNECTED); this.emit(Topology.OPEN, this); From aa8a2806ac8265692643e9cbb2f9f5fb565a0fb2 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Mon, 4 Aug 2025 14:56:54 +0200 Subject: [PATCH 05/11] test: fix tests --- .../node-specific/mongo_client.test.ts | 40 ++++++++----------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/test/integration/node-specific/mongo_client.test.ts b/test/integration/node-specific/mongo_client.test.ts index dd3d012fdb8..28bb66b64c5 100644 --- a/test/integration/node-specific/mongo_client.test.ts +++ b/test/integration/node-specific/mongo_client.test.ts @@ -588,30 +588,25 @@ describe('class MongoClient', function () { }); it( - 'creates topology and send ping when auth is enabled', + 'creates topology and checks out connection when auth is enabled', { requires: { auth: 'enabled' } }, async function () { - const commandToBeStarted = once(client, 'commandStarted'); + const checkoutStarted = once(client, 'connectionCheckOutStarted'); await client.connect(); - const [pingOnConnect] = await commandToBeStarted; - expect(pingOnConnect).to.have.property('commandName', 'ping'); + const checkout = await checkoutStarted; + expect(checkout).to.exist; expect(client).to.have.property('topology').that.is.instanceOf(Topology); } ); it( - 'does not send ping when authentication is disabled', + 'does not checkout connection when authentication is disabled', { requires: { auth: 'disabled' } }, async function () { - const commandToBeStarted = once(client, 'commandStarted'); + const checkoutStarted = once(client, 'connectionCheckOutStarted'); await client.connect(); - const delayedFind = runLater(async () => { - await client.db().collection('test').findOne(); - }, 300); - const [findOneOperation] = await commandToBeStarted; - // Proves that the first command started event that is emitted is a find and not a ping - expect(findOneOperation).to.have.property('commandName', 'find'); - await delayedFind; + const checkout = await checkoutStarted; + expect(checkout).to.not.exist; expect(client).to.have.property('topology').that.is.instanceOf(Topology); } ); @@ -1186,14 +1181,18 @@ describe('class MongoClient', function () { const tests = [ // only skipInitialPing=true will have no events upon connect - { description: 'should skip ping command when set to true', value: true, expectEvents: 0 }, { - description: 'should not skip ping command when set to false', + description: 'should skip connection checkout when set to true', + value: true, + expectEvents: 0 + }, + { + description: 'should not skip connection checkout when set to false', value: false, expectEvents: 1 }, { - description: 'should not skip ping command when unset', + description: 'should not skip connection checkout command when unset', value: undefined, expectEvents: 1 } @@ -1201,9 +1200,9 @@ describe('class MongoClient', function () { for (const { description, value, expectEvents } of tests) { it(description, async function () { const options = value === undefined ? {} : { __skipPingOnConnect: value }; - const client = this.configuration.newClient({}, { ...options, monitorCommands: true }); + const client = this.configuration.newClient({}, { ...options }); const events = []; - client.on('commandStarted', event => events.push(event)); + client.on('connectionCheckOutStarted', event => events.push(event)); try { await client.connect(); @@ -1212,11 +1211,6 @@ describe('class MongoClient', function () { } expect(events).to.have.lengthOf(expectEvents); - if (expectEvents > 1) { - for (const event of events) { - expect(event).to.have.property('commandName', 'ping'); - } - } }); } }); From 890d02c5ac6b44d297d62a6b2a3e55035ace5e6c Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Mon, 4 Aug 2025 15:14:29 +0200 Subject: [PATCH 06/11] test: fix more tests --- .../node-specific/auto_connect.test.ts | 26 ------------------- .../node-specific/mongo_client.test.ts | 6 ++--- 2 files changed, 3 insertions(+), 29 deletions(-) diff --git a/test/integration/node-specific/auto_connect.test.ts b/test/integration/node-specific/auto_connect.test.ts index 3e56b69fbef..6f472531c32 100644 --- a/test/integration/node-specific/auto_connect.test.ts +++ b/test/integration/node-specific/auto_connect.test.ts @@ -842,32 +842,6 @@ describe('When executing an operation for the first time', () => { }); }); - describe( - 'when the server requires auth and ping is delayed', - { requires: { auth: 'enabled', mongodb: '>=4.4' } }, - function () { - beforeEach(async function () { - // set failpoint to delay ping - // create new util client to avoid affecting the test client - const utilClient = this.configuration.newClient(); - await utilClient.db('admin').command({ - configureFailPoint: 'failCommand', - mode: { times: 1 }, - data: { failCommands: ['ping'], blockConnection: true, blockTimeMS: 1000 } - } as FailPoint); - await utilClient.close(); - }); - - it('timeoutMS from the client is not used for the internal `ping`', async function () { - const start = performance.now(); - const returnedClient = await client.connect(); - const end = performance.now(); - expect(returnedClient).to.equal(client); - expect(end - start).to.be.within(1000, 1500); // timeoutMS is 1000, did not apply. - }); - } - ); - describe( 'when server selection takes longer than the timeout', { requires: { auth: 'enabled', mongodb: '>=4.4' } }, diff --git a/test/integration/node-specific/mongo_client.test.ts b/test/integration/node-specific/mongo_client.test.ts index 28bb66b64c5..945ae9dedf6 100644 --- a/test/integration/node-specific/mongo_client.test.ts +++ b/test/integration/node-specific/mongo_client.test.ts @@ -615,15 +615,15 @@ describe('class MongoClient', function () { 'permits operations to be run after connect is called', { requires: { auth: 'enabled' } }, async function () { - const pingCommandToBeStarted = once(client, 'commandStarted'); + const checkoutStarted = once(client, 'connectionCheckOutStarted'); await client.connect(); - const [pingOnConnect] = await pingCommandToBeStarted; + const checkout = await checkoutStarted; + expect(checkout).to.not.exist; const findCommandToBeStarted = once(client, 'commandStarted'); await client.db('test').collection('test').findOne(); const [findCommandStarted] = await findCommandToBeStarted; - expect(pingOnConnect).to.have.property('commandName', 'ping'); expect(findCommandStarted).to.have.property('commandName', 'find'); expect(client).to.have.property('topology').that.is.instanceOf(Topology); } From 976f4a5a60a900aa64ce4a8b2d588392fd83e39b Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Mon, 4 Aug 2025 15:30:03 +0200 Subject: [PATCH 07/11] test: fix client test --- test/integration/node-specific/mongo_client.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/integration/node-specific/mongo_client.test.ts b/test/integration/node-specific/mongo_client.test.ts index 945ae9dedf6..18601589bff 100644 --- a/test/integration/node-specific/mongo_client.test.ts +++ b/test/integration/node-specific/mongo_client.test.ts @@ -618,7 +618,7 @@ describe('class MongoClient', function () { const checkoutStarted = once(client, 'connectionCheckOutStarted'); await client.connect(); const checkout = await checkoutStarted; - expect(checkout).to.not.exist; + expect(checkout).to.exist; const findCommandToBeStarted = once(client, 'commandStarted'); await client.db('test').collection('test').findOne(); From b44db82716cb13e270a9a1794bf4fcdcff7c604a Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Mon, 4 Aug 2025 15:50:45 +0200 Subject: [PATCH 08/11] test: fix noauth test --- test/integration/node-specific/mongo_client.test.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/test/integration/node-specific/mongo_client.test.ts b/test/integration/node-specific/mongo_client.test.ts index 18601589bff..9e0394013cd 100644 --- a/test/integration/node-specific/mongo_client.test.ts +++ b/test/integration/node-specific/mongo_client.test.ts @@ -18,7 +18,7 @@ import { ServerDescription, Topology } from '../../mongodb'; -import { clearFailPoint, configureFailPoint, runLater } from '../../tools/utils'; +import { clearFailPoint, configureFailPoint } from '../../tools/utils'; import { setupDatabase } from '../shared'; describe('class MongoClient', function () { @@ -603,10 +603,12 @@ describe('class MongoClient', function () { 'does not checkout connection when authentication is disabled', { requires: { auth: 'disabled' } }, async function () { - const checkoutStarted = once(client, 'connectionCheckOutStarted'); + const checkoutStartedEvents = []; + client.on('connectionCheckOutStarted', event => { + checkoutStartedEvents.push(event); + }); await client.connect(); - const checkout = await checkoutStarted; - expect(checkout).to.not.exist; + expect(checkoutStartedEvents).to.be.empty; expect(client).to.have.property('topology').that.is.instanceOf(Topology); } ); From 11ad1846540727ab0408b41c7eebefa3ce3820ca Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Tue, 5 Aug 2025 13:41:38 +0200 Subject: [PATCH 09/11] chore: use handshake instead of ping --- src/sdam/topology.ts | 2 +- test/integration/node-specific/auto_connect.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/sdam/topology.ts b/src/sdam/topology.ts index a7ca443f6d9..02077a4f069 100644 --- a/src/sdam/topology.ts +++ b/src/sdam/topology.ts @@ -458,7 +458,7 @@ export class Topology extends TypedEventEmitter { waitQueueTimeoutMS: this.client.s.options.waitQueueTimeoutMS }); const selectServerOptions = { - operationName: 'ping', + operationName: 'handshake', ...options, timeoutContext }; diff --git a/test/integration/node-specific/auto_connect.test.ts b/test/integration/node-specific/auto_connect.test.ts index 6f472531c32..f0850049632 100644 --- a/test/integration/node-specific/auto_connect.test.ts +++ b/test/integration/node-specific/auto_connect.test.ts @@ -13,7 +13,7 @@ import { Topology, TopologyType } from '../../mongodb'; -import { type FailPoint, sleep } from '../../tools/utils'; +import { sleep } from '../../tools/utils'; describe('When executing an operation for the first time', () => { let client: MongoClient; From 3f1eb76195c0ea1dc75073aa101f8bdd71863d74 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Tue, 5 Aug 2025 14:20:06 +0200 Subject: [PATCH 10/11] test: move ping to handshake --- .../load-balanced.json | 4 ++-- .../replica-set.json | 6 +++--- .../sharded.json | 12 ++++++------ .../standalone.json | 12 ++++++------ 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/test/integration/server-selection/unified-server-selection-node-specs-logging/load-balanced.json b/test/integration/server-selection/unified-server-selection-node-specs-logging/load-balanced.json index df30a43ebb5..125766ad1e4 100644 --- a/test/integration/server-selection/unified-server-selection-node-specs-logging/load-balanced.json +++ b/test/integration/server-selection/unified-server-selection-node-specs-logging/load-balanced.json @@ -79,7 +79,7 @@ "selector": { "$$exists": true }, - "operation": "ping", + "operation": "handshake", "topologyDescription": { "$$exists": true } @@ -93,7 +93,7 @@ "selector": { "$$exists": true }, - "operation": "ping", + "operation": "handshake", "topologyDescription": { "$$exists": true } diff --git a/test/integration/server-selection/unified-server-selection-node-specs-logging/replica-set.json b/test/integration/server-selection/unified-server-selection-node-specs-logging/replica-set.json index 304688bc054..b462f3b80e3 100644 --- a/test/integration/server-selection/unified-server-selection-node-specs-logging/replica-set.json +++ b/test/integration/server-selection/unified-server-selection-node-specs-logging/replica-set.json @@ -100,7 +100,7 @@ "selector": { "$$exists": true }, - "operation": "ping", + "operation": "handshake", "topologyDescription": { "$$exists": true } @@ -114,7 +114,7 @@ "selector": { "$$exists": true }, - "operation": "ping", + "operation": "handshake", "topologyDescription": { "$$exists": true }, @@ -134,7 +134,7 @@ "selector": { "$$exists": true }, - "operation": "ping", + "operation": "handshake", "topologyDescription": { "$$exists": true }, diff --git a/test/integration/server-selection/unified-server-selection-node-specs-logging/sharded.json b/test/integration/server-selection/unified-server-selection-node-specs-logging/sharded.json index ee8094be4e3..345fc84bc00 100644 --- a/test/integration/server-selection/unified-server-selection-node-specs-logging/sharded.json +++ b/test/integration/server-selection/unified-server-selection-node-specs-logging/sharded.json @@ -87,7 +87,7 @@ "selector": { "$$exists": true }, - "operation": "ping", + "operation": "handshake", "topologyDescription": { "$$exists": true } @@ -101,7 +101,7 @@ "selector": { "$$exists": true }, - "operation": "ping", + "operation": "handshake", "topologyDescription": { "$$exists": true }, @@ -121,7 +121,7 @@ "selector": { "$$exists": true }, - "operation": "ping", + "operation": "handshake", "topologyDescription": { "$$exists": true }, @@ -244,7 +244,7 @@ "selector": { "$$exists": true }, - "operation": "ping", + "operation": "handshake", "topologyDescription": { "$$exists": true } @@ -258,7 +258,7 @@ "selector": { "$$exists": true }, - "operation": "ping", + "operation": "handshake", "topologyDescription": { "$$exists": true }, @@ -278,7 +278,7 @@ "selector": { "$$exists": true }, - "operation": "ping", + "operation": "handshake", "topologyDescription": { "$$exists": true }, diff --git a/test/integration/server-selection/unified-server-selection-node-specs-logging/standalone.json b/test/integration/server-selection/unified-server-selection-node-specs-logging/standalone.json index c79a1f1f65c..2b5ee88338b 100644 --- a/test/integration/server-selection/unified-server-selection-node-specs-logging/standalone.json +++ b/test/integration/server-selection/unified-server-selection-node-specs-logging/standalone.json @@ -84,7 +84,7 @@ "selector": { "$$exists": true }, - "operation": "ping", + "operation": "handshake", "topologyDescription": { "$$exists": true } @@ -98,7 +98,7 @@ "selector": { "$$exists": true }, - "operation": "ping", + "operation": "handshake", "topologyDescription": { "$$exists": true }, @@ -118,7 +118,7 @@ "selector": { "$$exists": true }, - "operation": "ping", + "operation": "handshake", "topologyDescription": { "$$exists": true }, @@ -241,7 +241,7 @@ "selector": { "$$exists": true }, - "operation": "ping", + "operation": "handshake", "topologyDescription": { "$$exists": true } @@ -255,7 +255,7 @@ "selector": { "$$exists": true }, - "operation": "ping", + "operation": "handshake", "topologyDescription": { "$$exists": true }, @@ -275,7 +275,7 @@ "selector": { "$$exists": true }, - "operation": "ping", + "operation": "handshake", "topologyDescription": { "$$exists": true }, From 5ee10961c744793f515f59fd055e495cf1e615e7 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Tue, 5 Aug 2025 14:54:32 +0200 Subject: [PATCH 11/11] test: fix abort signal tests --- test/integration/node-specific/abort_signal.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/integration/node-specific/abort_signal.test.ts b/test/integration/node-specific/abort_signal.test.ts index 40ad3b3414d..c128156f2e2 100644 --- a/test/integration/node-specific/abort_signal.test.ts +++ b/test/integration/node-specific/abort_signal.test.ts @@ -631,7 +631,7 @@ describe('AbortSignal support', () => { mongodbLogComponentSeverities: { serverSelection: 'debug' }, mongodbLogPath: { write: log => { - if (log.c === 'serverSelection' && log.operation === 'ping') { + if (log.c === 'serverSelection' && log.operation === 'handshake') { controller.abort(); promise.resolve(); } @@ -676,7 +676,7 @@ describe('AbortSignal support', () => { mongodbLogComponentSeverities: { serverSelection: 'debug' }, mongodbLogPath: { write: log => { - if (log.c === 'serverSelection' && log.operation === 'ping') { + if (log.c === 'serverSelection' && log.operation === 'handshake') { controller.abort(); promise.resolve(); }