Skip to content

feat(NODE-7020): remove ping on connect #4607

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.

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
merged 11 commits into from
Aug 5, 2025
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
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
5 changes: 3 additions & 2 deletions src/sdam/topology.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ import {
makeStateMachine,
noop,
now,
ns,
promiseWithResolvers,
shuffle
} from '../utils';
Expand Down Expand Up @@ -469,9 +468,11 @@ export class Topology extends TypedEventEmitter<TopologyEvents> {
readPreferenceServerSelector(readPreference),
selectServerOptions
);

const skipPingOnConnect = this.s.options.__skipPingOnConnect === true;
if (!skipPingOnConnect && this.s.credentials) {
await server.command(ns('admin.$cmd'), { ping: 1 }, { timeoutContext });
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);
Expand Down
26 changes: 0 additions & 26 deletions test/integration/node-specific/auto_connect.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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' } },
Expand Down
50 changes: 23 additions & 27 deletions test/integration/node-specific/mongo_client.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () {
Expand Down Expand Up @@ -588,30 +588,27 @@ 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 checkoutStartedEvents = [];
client.on('connectionCheckOutStarted', event => {
checkoutStartedEvents.push(event);
});
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;
expect(checkoutStartedEvents).to.be.empty;
expect(client).to.have.property('topology').that.is.instanceOf(Topology);
}
);
Expand All @@ -620,15 +617,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.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);
}
Expand Down Expand Up @@ -1186,24 +1183,28 @@ 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
}
];
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();
Expand All @@ -1212,11 +1213,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');
}
}
});
}
});
Expand Down