Skip to content

Commit 3d296b7

Browse files
durrandurran
authored
feat(NODE-7020): remove ping on connect (#4607)
1 parent a8338ad commit 3d296b7

File tree

8 files changed

+47
-76
lines changed

8 files changed

+47
-76
lines changed

‎src/sdam/topology.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ import {
4646
makeStateMachine,
4747
noop,
4848
now,
49-
ns,
5049
promiseWithResolvers,
5150
shuffle
5251
} from '../utils';
@@ -459,7 +458,7 @@ export class Topology extends TypedEventEmitter<TopologyEvents> {
459458
waitQueueTimeoutMS: this.client.s.options.waitQueueTimeoutMS
460459
});
461460
const selectServerOptions = {
462-
operationName: 'ping',
461+
operationName: 'handshake',
463462
...options,
464463
timeoutContext
465464
};
@@ -469,9 +468,11 @@ export class Topology extends TypedEventEmitter<TopologyEvents> {
469468
readPreferenceServerSelector(readPreference),
470469
selectServerOptions
471470
);
471+
472472
const skipPingOnConnect = this.s.options.__skipPingOnConnect === true;
473473
if (!skipPingOnConnect && this.s.credentials) {
474-
await server.command(ns('admin.$cmd'), { ping: 1 }, { timeoutContext });
474+
const connection = await server.pool.checkOut({ timeoutContext: timeoutContext });
475+
server.pool.checkIn(connection);
475476
stateTransition(this, STATE_CONNECTED);
476477
this.emit(Topology.OPEN, this);
477478
this.emit(Topology.CONNECT, this);

‎test/integration/node-specific/abort_signal.test.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -631,7 +631,7 @@ describe('AbortSignal support', () => {
631631
mongodbLogComponentSeverities: { serverSelection: 'debug' },
632632
mongodbLogPath: {
633633
write: log => {
634-
if (log.c === 'serverSelection' && log.operation === 'ping') {
634+
if (log.c === 'serverSelection' && log.operation === 'handshake') {
635635
controller.abort();
636636
promise.resolve();
637637
}
@@ -676,7 +676,7 @@ describe('AbortSignal support', () => {
676676
mongodbLogComponentSeverities: { serverSelection: 'debug' },
677677
mongodbLogPath: {
678678
write: log => {
679-
if (log.c === 'serverSelection' && log.operation === 'ping') {
679+
if (log.c === 'serverSelection' && log.operation === 'handshake') {
680680
controller.abort();
681681
promise.resolve();
682682
}

‎test/integration/node-specific/auto_connect.test.ts‎

Lines changed: 1 addition & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ import {
1313
Topology,
1414
TopologyType
1515
} from '../../mongodb';
16-
import { type FailPoint, sleep } from '../../tools/utils';
16+
import { sleep } from '../../tools/utils';
1717

1818
describe('When executing an operation for the first time', () => {
1919
let client: MongoClient;
@@ -842,32 +842,6 @@ describe('When executing an operation for the first time', () => {
842842
});
843843
});
844844

845-
describe(
846-
'when the server requires auth and ping is delayed',
847-
{ requires: { auth: 'enabled', mongodb: '>=4.4' } },
848-
function () {
849-
beforeEach(async function () {
850-
// set failpoint to delay ping
851-
// create new util client to avoid affecting the test client
852-
const utilClient = this.configuration.newClient();
853-
await utilClient.db('admin').command({
854-
configureFailPoint: 'failCommand',
855-
mode: { times: 1 },
856-
data: { failCommands: ['ping'], blockConnection: true, blockTimeMS: 1000 }
857-
} as FailPoint);
858-
await utilClient.close();
859-
});
860-
861-
it('timeoutMS from the client is not used for the internal `ping`', async function () {
862-
const start = performance.now();
863-
const returnedClient = await client.connect();
864-
const end = performance.now();
865-
expect(returnedClient).to.equal(client);
866-
expect(end - start).to.be.within(1000, 1500); // timeoutMS is 1000, did not apply.
867-
});
868-
}
869-
);
870-
871845
describe(
872846
'when server selection takes longer than the timeout',
873847
{ requires: { auth: 'enabled', mongodb: '>=4.4' } },

‎test/integration/node-specific/mongo_client.test.ts‎

Lines changed: 23 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import {
1818
ServerDescription,
1919
Topology
2020
} from '../../mongodb';
21-
import { clearFailPoint, configureFailPoint, runLater } from '../../tools/utils';
21+
import { clearFailPoint, configureFailPoint } from '../../tools/utils';
2222
import { setupDatabase } from '../shared';
2323

2424
describe('class MongoClient', function () {
@@ -588,30 +588,27 @@ describe('class MongoClient', function () {
588588
});
589589

590590
it(
591-
'creates topology and send ping when auth is enabled',
591+
'creates topology and checks out connection when auth is enabled',
592592
{ requires: { auth: 'enabled' } },
593593
async function () {
594-
const commandToBeStarted = once(client, 'commandStarted');
594+
const checkoutStarted = once(client, 'connectionCheckOutStarted');
595595
await client.connect();
596-
const [pingOnConnect] = await commandToBeStarted;
597-
expect(pingOnConnect).to.have.property('commandName', 'ping');
596+
const checkout = await checkoutStarted;
597+
expect(checkout).to.exist;
598598
expect(client).to.have.property('topology').that.is.instanceOf(Topology);
599599
}
600600
);
601601

602602
it(
603-
'does not send ping when authentication is disabled',
603+
'does not checkout connection when authentication is disabled',
604604
{ requires: { auth: 'disabled' } },
605605
async function () {
606-
const commandToBeStarted = once(client, 'commandStarted');
606+
const checkoutStartedEvents = [];
607+
client.on('connectionCheckOutStarted', event => {
608+
checkoutStartedEvents.push(event);
609+
});
607610
await client.connect();
608-
const delayedFind = runLater(async () => {
609-
await client.db().collection('test').findOne();
610-
}, 300);
611-
const [findOneOperation] = await commandToBeStarted;
612-
// Proves that the first command started event that is emitted is a find and not a ping
613-
expect(findOneOperation).to.have.property('commandName', 'find');
614-
await delayedFind;
611+
expect(checkoutStartedEvents).to.be.empty;
615612
expect(client).to.have.property('topology').that.is.instanceOf(Topology);
616613
}
617614
);
@@ -620,15 +617,15 @@ describe('class MongoClient', function () {
620617
'permits operations to be run after connect is called',
621618
{ requires: { auth: 'enabled' } },
622619
async function () {
623-
const pingCommandToBeStarted = once(client, 'commandStarted');
620+
const checkoutStarted = once(client, 'connectionCheckOutStarted');
624621
await client.connect();
625-
const [pingOnConnect] = await pingCommandToBeStarted;
622+
const checkout = await checkoutStarted;
623+
expect(checkout).to.exist;
626624

627625
const findCommandToBeStarted = once(client, 'commandStarted');
628626
await client.db('test').collection('test').findOne();
629627
const [findCommandStarted] = await findCommandToBeStarted;
630628

631-
expect(pingOnConnect).to.have.property('commandName', 'ping');
632629
expect(findCommandStarted).to.have.property('commandName', 'find');
633630
expect(client).to.have.property('topology').that.is.instanceOf(Topology);
634631
}
@@ -1186,24 +1183,28 @@ describe('class MongoClient', function () {
11861183

11871184
const tests = [
11881185
// only skipInitialPing=true will have no events upon connect
1189-
{ description: 'should skip ping command when set to true', value: true, expectEvents: 0 },
11901186
{
1191-
description: 'should not skip ping command when set to false',
1187+
description: 'should skip connection checkout when set to true',
1188+
value: true,
1189+
expectEvents: 0
1190+
},
1191+
{
1192+
description: 'should not skip connection checkout when set to false',
11921193
value: false,
11931194
expectEvents: 1
11941195
},
11951196
{
1196-
description: 'should not skip ping command when unset',
1197+
description: 'should not skip connection checkout command when unset',
11971198
value: undefined,
11981199
expectEvents: 1
11991200
}
12001201
];
12011202
for (const { description, value, expectEvents } of tests) {
12021203
it(description, async function () {
12031204
const options = value === undefined ? {} : { __skipPingOnConnect: value };
1204-
const client = this.configuration.newClient({}, { ...options, monitorCommands: true });
1205+
const client = this.configuration.newClient({}, { ...options });
12051206
const events = [];
1206-
client.on('commandStarted', event => events.push(event));
1207+
client.on('connectionCheckOutStarted', event => events.push(event));
12071208

12081209
try {
12091210
await client.connect();
@@ -1212,11 +1213,6 @@ describe('class MongoClient', function () {
12121213
}
12131214

12141215
expect(events).to.have.lengthOf(expectEvents);
1215-
if (expectEvents > 1) {
1216-
for (const event of events) {
1217-
expect(event).to.have.property('commandName', 'ping');
1218-
}
1219-
}
12201216
});
12211217
}
12221218
});

‎test/integration/server-selection/unified-server-selection-node-specs-logging/load-balanced.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@
7979
"selector": {
8080
"$$exists": true
8181
},
82-
"operation": "ping",
82+
"operation": "handshake",
8383
"topologyDescription": {
8484
"$$exists": true
8585
}
@@ -93,7 +93,7 @@
9393
"selector": {
9494
"$$exists": true
9595
},
96-
"operation": "ping",
96+
"operation": "handshake",
9797
"topologyDescription": {
9898
"$$exists": true
9999
}

‎test/integration/server-selection/unified-server-selection-node-specs-logging/replica-set.json‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,7 +100,7 @@
100100
"selector": {
101101
"$$exists": true
102102
},
103-
"operation": "ping",
103+
"operation": "handshake",
104104
"topologyDescription": {
105105
"$$exists": true
106106
}
@@ -114,7 +114,7 @@
114114
"selector": {
115115
"$$exists": true
116116
},
117-
"operation": "ping",
117+
"operation": "handshake",
118118
"topologyDescription": {
119119
"$$exists": true
120120
},
@@ -134,7 +134,7 @@
134134
"selector": {
135135
"$$exists": true
136136
},
137-
"operation": "ping",
137+
"operation": "handshake",
138138
"topologyDescription": {
139139
"$$exists": true
140140
},

‎test/integration/server-selection/unified-server-selection-node-specs-logging/sharded.json‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@
8787
"selector": {
8888
"$$exists": true
8989
},
90-
"operation": "ping",
90+
"operation": "handshake",
9191
"topologyDescription": {
9292
"$$exists": true
9393
}
@@ -101,7 +101,7 @@
101101
"selector": {
102102
"$$exists": true
103103
},
104-
"operation": "ping",
104+
"operation": "handshake",
105105
"topologyDescription": {
106106
"$$exists": true
107107
},
@@ -121,7 +121,7 @@
121121
"selector": {
122122
"$$exists": true
123123
},
124-
"operation": "ping",
124+
"operation": "handshake",
125125
"topologyDescription": {
126126
"$$exists": true
127127
},
@@ -244,7 +244,7 @@
244244
"selector": {
245245
"$$exists": true
246246
},
247-
"operation": "ping",
247+
"operation": "handshake",
248248
"topologyDescription": {
249249
"$$exists": true
250250
}
@@ -258,7 +258,7 @@
258258
"selector": {
259259
"$$exists": true
260260
},
261-
"operation": "ping",
261+
"operation": "handshake",
262262
"topologyDescription": {
263263
"$$exists": true
264264
},
@@ -278,7 +278,7 @@
278278
"selector": {
279279
"$$exists": true
280280
},
281-
"operation": "ping",
281+
"operation": "handshake",
282282
"topologyDescription": {
283283
"$$exists": true
284284
},

‎test/integration/server-selection/unified-server-selection-node-specs-logging/standalone.json‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@
8484
"selector": {
8585
"$$exists": true
8686
},
87-
"operation": "ping",
87+
"operation": "handshake",
8888
"topologyDescription": {
8989
"$$exists": true
9090
}
@@ -98,7 +98,7 @@
9898
"selector": {
9999
"$$exists": true
100100
},
101-
"operation": "ping",
101+
"operation": "handshake",
102102
"topologyDescription": {
103103
"$$exists": true
104104
},
@@ -118,7 +118,7 @@
118118
"selector": {
119119
"$$exists": true
120120
},
121-
"operation": "ping",
121+
"operation": "handshake",
122122
"topologyDescription": {
123123
"$$exists": true
124124
},
@@ -241,7 +241,7 @@
241241
"selector": {
242242
"$$exists": true
243243
},
244-
"operation": "ping",
244+
"operation": "handshake",
245245
"topologyDescription": {
246246
"$$exists": true
247247
}
@@ -255,7 +255,7 @@
255255
"selector": {
256256
"$$exists": true
257257
},
258-
"operation": "ping",
258+
"operation": "handshake",
259259
"topologyDescription": {
260260
"$$exists": true
261261
},
@@ -275,7 +275,7 @@
275275
"selector": {
276276
"$$exists": true
277277
},
278-
"operation": "ping",
278+
"operation": "handshake",
279279
"topologyDescription": {
280280
"$$exists": true
281281
},

0 commit comments

Comments
 (0)