Skip to content

Commit cc85ebf

Browse files
baileympearsonbaileympearson
authored
fix(NODE-7232): only send endSessions during client close if the topology supports sessions (#4722)
1 parent 9a1bc65 commit cc85ebf

7 files changed

Lines changed: 159 additions & 110 deletions

File tree

‎src/mongo_client.ts‎

Lines changed: 30 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { promises as fs } from 'fs';
22
import type { TcpNetConnectOpts } from 'net';
33
import type { ConnectionOptions as TLSConnectionOptions, TLSSocketOptions } from 'tls';
44

5-
import { type ServerCommandOptions, type TimeoutContext } from '.';
5+
import { TopologyType } from '.';
66
import { type BSONSerializeOptions, type Document, resolveBSONOptions } from './bson';
77
import { ChangeStream, type ChangeStreamDocument, type ChangeStreamOptions } from './change_stream';
88
import type { AutoEncrypter, AutoEncryptionOptions } from './client-side-encryption/auto_encrypter';
@@ -21,7 +21,6 @@ import {
2121
makeClientMetadata
2222
} from './cmap/handshake/client_metadata';
2323
import type { CompressorName } from './cmap/wire_protocol/compression';
24-
import { MongoDBResponse } from './cmap/wire_protocol/responses';
2524
import { parseOptions, resolveSRVRecord } from './connection_string';
2625
import { MONGO_CLIENT_EVENTS } from './constants';
2726
import { type AbstractCursor } from './cursor/abstract_cursor';
@@ -43,8 +42,8 @@ import {
4342
type ClientBulkWriteResult
4443
} from './operations/client_bulk_write/common';
4544
import { ClientBulkWriteExecutor } from './operations/client_bulk_write/executor';
45+
import { EndSessionsOperation } from './operations/end_sessions';
4646
import { executeOperation } from './operations/execute_operation';
47-
import { AbstractOperation } from './operations/operation';
4847
import type { ReadConcern, ReadConcernLevel, ReadConcernLike } from './read_concern';
4948
import { ReadPreference, type ReadPreferenceMode } from './read_preference';
5049
import type { ServerMonitoringMode } from './sdam/monitor';
@@ -61,7 +60,7 @@ import {
6160
type HostAddress,
6261
hostMatchesWildcards,
6362
isHostMatch,
64-
MongoDBNamespace,
63+
type MongoDBNamespace,
6564
noop,
6665
ns,
6766
resolveOptions,
@@ -763,40 +762,12 @@ export class MongoClient extends TypedEventEmitter<MongoClientEvents> implements
763762
return;
764763
}
765764

766-
// If we would attempt to select a server and get nothing back we short circuit
767-
// to avoid the server selection timeout.
768-
const selector = readPreferenceServerSelector(ReadPreference.primaryPreferred);
769-
const topologyDescription = this.topology.description;
770-
const serverDescriptions = Array.from(topologyDescription.servers.values());
771-
const servers = selector(topologyDescription, serverDescriptions);
772-
if (servers.length !== 0) {
773-
const endSessions = Array.from(this.s.sessionPool.sessions, ({ id }) => id);
774-
if (endSessions.length !== 0) {
775-
try {
776-
class EndSessionsOperation extends AbstractOperation<void> {
777-
override ns = MongoDBNamespace.fromString('admin.$cmd');
778-
override SERVER_COMMAND_RESPONSE_TYPE = MongoDBResponse;
779-
override buildCommand(_connection: Connection, _session?: ClientSession): Document {
780-
return {
781-
endSessions
782-
};
783-
}
784-
override buildOptions(timeoutContext: TimeoutContext): ServerCommandOptions {
785-
return {
786-
timeoutContext,
787-
readPreference: ReadPreference.primaryPreferred,
788-
noResponse: true
789-
};
790-
}
791-
override get commandName(): string {
792-
return 'endSessions';
793-
}
794-
}
795-
await executeOperation(this, new EndSessionsOperation());
796-
} catch (error) {
797-
squashError(error);
798-
}
799-
}
765+
const supportsSessions =
766+
this.topology.description.type === TopologyType.LoadBalanced ||
767+
this.topology.description.logicalSessionTimeoutMinutes != null;
768+
769+
if (supportsSessions) {
770+
await endSessions(this, this.topology);
800771
}
801772

802773
// clear out references to old topology
@@ -809,6 +780,27 @@ export class MongoClient extends TypedEventEmitter<MongoClientEvents> implements
809780
if (encrypter) {
810781
await encrypter.close(this);
811782
}
783+
784+
async function endSessions(
785+
client: MongoClient,
786+
{ description: topologyDescription }: Topology
787+
) {
788+
// If we would attempt to select a server and get nothing back we short circuit
789+
// to avoid the server selection timeout.
790+
const selector = readPreferenceServerSelector(ReadPreference.primaryPreferred);
791+
const serverDescriptions = Array.from(topologyDescription.servers.values());
792+
const servers = selector(topologyDescription, serverDescriptions);
793+
if (servers.length !== 0) {
794+
const endSessions = Array.from(client.s.sessionPool.sessions, ({ id }) => id);
795+
if (endSessions.length !== 0) {
796+
try {
797+
await executeOperation(client, new EndSessionsOperation(endSessions));
798+
} catch (error) {
799+
squashError(error);
800+
}
801+
}
802+
}
803+
}
812804
}
813805

814806
/**

‎src/operations/end_sessions.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
import {
2+
type ClientSession,
3+
type Connection,
4+
type ServerCommandOptions,
5+
type ServerSessionId,
6+
type TimeoutContext
7+
} from '..';
8+
import { type Document } from '../bson';
9+
import { MongoDBResponse } from '../cmap/wire_protocol/responses';
10+
import { ReadPreference } from '../read_preference';
11+
import { MongoDBNamespace } from '../utils';
12+
import { AbstractOperation } from './operation';
13+
14+
export class EndSessionsOperation extends AbstractOperation<void> {
15+
override ns = MongoDBNamespace.fromString('admin.$cmd');
16+
override SERVER_COMMAND_RESPONSE_TYPE = MongoDBResponse;
17+
18+
private sessions: Array<ServerSessionId>;
19+
20+
constructor(sessions: Array<ServerSessionId>) {
21+
super();
22+
this.sessions = sessions;
23+
}
24+
25+
override buildCommand(_connection: Connection, _session?: ClientSession): Document {
26+
return {
27+
endSessions: this.sessions
28+
};
29+
}
30+
31+
override buildOptions(timeoutContext: TimeoutContext): ServerCommandOptions {
32+
return {
33+
timeoutContext,
34+
readPreference: ReadPreference.primaryPreferred,
35+
noResponse: true
36+
};
37+
}
38+
39+
override get commandName(): string {
40+
return 'endSessions';
41+
}
42+
}

‎test/integration/client-side-operations-timeout/client_side_operations_timeout.prose.test.ts‎

Lines changed: 2 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,8 @@
11
/* Specification prose tests */
22

3-
import { type ChildProcess, spawn } from 'node:child_process';
43
import { Readable } from 'node:stream';
54

65
import { expect } from 'chai';
7-
import * as os from 'os';
8-
import * as path from 'path';
96
import * as semver from 'semver';
107
import * as sinon from 'sinon';
118
import { pipeline } from 'stream/promises';
@@ -27,6 +24,7 @@ import {
2724
import {
2825
clearFailPoint,
2926
configureFailPoint,
27+
configureMongocryptdSpawnHooks,
3028
type FailCommandFailPoint,
3129
makeMultiBatchWrite,
3230
measureDuration
@@ -127,28 +125,16 @@ describe('CSOT spec prose tests', function () {
127125

128126
let client: MongoClient;
129127
const mongocryptdTestPort = '23000';
130-
let childProcess: ChildProcess;
128+
configureMongocryptdSpawnHooks({ port: mongocryptdTestPort });
131129

132130
beforeEach(async function () {
133-
const pidFile = path.join(os.tmpdir(), new ObjectId().toHexString());
134-
childProcess = spawn(
135-
'mongocryptd',
136-
['--port', mongocryptdTestPort, '--ipv6', '--pidfilepath', pidFile],
137-
{
138-
stdio: 'ignore',
139-
detached: true
140-
}
141-
);
142-
143-
childProcess.on('error', error => console.warn(this.currentTest?.fullTitle(), error));
144131
client = new MongoClient(`mongodb://localhost:${mongocryptdTestPort}/?timeoutMS=1000`, {
145132
monitorCommands: true
146133
});
147134
});
148135

149136
afterEach(async function () {
150137
await client.close();
151-
childProcess.kill('SIGKILL');
152138
sinon.restore();
153139
});
154140

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

Lines changed: 48 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,14 @@ import * as events from 'node:events';
33
import { expect } from 'chai';
44

55
import { getCSFLEKMSProviders } from '../../csfle-kms-providers';
6-
import { type Collection, type FindCursor, type MongoClient } from '../../mongodb';
6+
import {
7+
type Collection,
8+
type CommandStartedEvent,
9+
type FindCursor,
10+
type MongoClient
11+
} from '../../mongodb';
12+
import { configureMongocryptdSpawnHooks } from '../../tools/utils';
13+
import { filterForCommands } from '../shared';
714
import { runScriptAndGetProcessInfo } from './resource_tracking_script_builder';
815

916
describe('MongoClient.close() Integration', () => {
@@ -490,20 +497,49 @@ describe('MongoClient.close() Integration', () => {
490497
});
491498

492499
describe('when MongoClient.close is called', function () {
493-
it('sends an endSessions command', async function () {
494-
await client.db('a').collection('a').insertOne({ a: 1 });
495-
await client.db('a').collection('a').insertOne({ a: 1 });
496-
await client.db('a').collection('a').insertOne({ a: 1 });
497-
const endSessionsStarted = events.once(client, 'commandStarted');
498-
const willEndSessions = events.once(client, 'commandSucceeded');
500+
describe('when sessions are supported', function () {
501+
it('sends an endSessions command', async function () {
502+
await client.db('a').collection('a').insertOne({ a: 1 });
503+
await client.db('a').collection('a').insertOne({ a: 1 });
504+
await client.db('a').collection('a').insertOne({ a: 1 });
505+
const endSessionsStarted = events.once(client, 'commandStarted');
506+
const willEndSessions = events.once(client, 'commandSucceeded');
499507

500-
await client.close();
508+
await client.close();
509+
510+
const [startedEv] = await endSessionsStarted;
511+
expect(startedEv).to.have.nested.property('command.endSessions').that.has.lengthOf(1);
501512

502-
const [startedEv] = await endSessionsStarted;
503-
expect(startedEv).to.have.nested.property('command.endSessions').that.has.lengthOf(1);
513+
const [commandEv] = await willEndSessions;
514+
expect(commandEv).to.have.property('commandName', 'endSessions');
515+
});
516+
});
504517

505-
const [commandEv] = await willEndSessions;
506-
expect(commandEv).to.have.property('commandName', 'endSessions');
518+
describe('when sessions are not supported', function () {
519+
const mongocryptdTestPort = '27022';
520+
let client: MongoClient;
521+
const commands: Array<CommandStartedEvent> = [];
522+
523+
configureMongocryptdSpawnHooks({ port: mongocryptdTestPort });
524+
525+
beforeEach('configure cryptd client and prepopulate session pool', async function () {
526+
client = this.configuration.newClient(`mongodb://localhost:${mongocryptdTestPort}`, {
527+
monitorCommands: true
528+
});
529+
530+
client.on('commandStarted', filterForCommands('endSessions', commands));
531+
532+
// run an operation to instantiate an implicit session (which should be omitted) from the
533+
// actual command but still instantiated by the client. See session prose test 18.
534+
await client.db().command({ hello: true });
535+
expect(client.s.sessionPool.sessions).to.have.length.greaterThan(0);
536+
});
537+
538+
it('does not execute endSessions', async function () {
539+
await client.close();
540+
541+
expect(commands).to.deep.equal([]);
542+
});
507543
});
508544
});
509545
});

‎test/integration/server-discovery-and-monitoring/server_description.test.ts‎

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,20 @@
1-
import { type ChildProcess, spawn } from 'node:child_process';
2-
31
import { expect } from 'chai';
4-
import * as os from 'os';
5-
import * as path from 'path';
62

7-
import { MongoClient, ObjectId } from '../../mongodb';
3+
import { MongoClient } from '../../mongodb';
4+
import { configureMongocryptdSpawnHooks } from '../../tools/utils';
85

96
describe('class ServerDescription', function () {
107
describe('when connecting to mongocryptd', { requires: { mongodb: '>=4.4' } }, function () {
118
let client: MongoClient;
12-
const mongocryptdTestPort = '27022';
13-
let childProcess: ChildProcess;
9+
10+
const { port: mongocryptdTestPort } = configureMongocryptdSpawnHooks();
1411

1512
beforeEach(async function () {
16-
const pidFile = path.join(os.tmpdir(), new ObjectId().toHexString());
17-
childProcess = spawn(
18-
'mongocryptd',
19-
['--port', mongocryptdTestPort, '--ipv6', '--pidfilepath', pidFile],
20-
{
21-
stdio: 'ignore',
22-
detached: true
23-
}
24-
);
25-
26-
childProcess.on('error', error => console.warn(this.currentTest?.fullTitle(), error));
2713
client = new MongoClient(`mongodb://localhost:${mongocryptdTestPort}`);
2814
});
2915

3016
afterEach(async function () {
3117
await client?.close();
32-
childProcess.kill('SIGKILL');
3318
});
3419

3520
it('iscryptd is set to true ', async function () {

‎test/integration/sessions/sessions.prose.test.ts‎

Lines changed: 2 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,11 @@
1-
import { ObjectId } from 'bson';
21
import { expect } from 'chai';
3-
import { type ChildProcess, spawn } from 'child_process';
42
import { once } from 'events';
5-
import * as os from 'os';
6-
import * as path from 'path';
73

84
import { type CommandStartedEvent } from '../../../src/cmap/command_monitoring_events';
95
import { type Collection } from '../../../src/collection';
106
import { MongoDriverError, MongoInvalidArgumentError } from '../../../src/error';
117
import { MongoClient } from '../../../src/mongo_client';
12-
import { sleep } from '../../tools/utils';
8+
import { configureMongocryptdSpawnHooks, sleep } from '../../tools/utils';
139

1410
describe('Sessions Prose Tests', () => {
1511
describe('5. Session argument is for the right client', () => {
@@ -128,23 +124,8 @@ describe('Sessions Prose Tests', () => {
128124
*/
129125
const mongocryptdTestPort = '27022';
130126
let client: MongoClient;
131-
let childProcess: ChildProcess;
132-
133-
before(() => {
134-
const pidFile = path.join(os.tmpdir(), new ObjectId().toHexString());
135-
childProcess = spawn(
136-
'mongocryptd',
137-
['--port', mongocryptdTestPort, '--ipv6', '--pidfilepath', pidFile],
138-
{
139-
stdio: 'ignore',
140-
detached: true
141-
}
142-
);
143127

144-
childProcess.on('error', err => {
145-
console.warn('Sessions prose mongocryptd error:', err);
146-
});
147-
});
128+
configureMongocryptdSpawnHooks({ port: mongocryptdTestPort });
148129

149130
beforeEach(async () => {
150131
client = new MongoClient(`mongodb://localhost:${mongocryptdTestPort}`, {
@@ -160,10 +141,6 @@ describe('Sessions Prose Tests', () => {
160141
await client?.close();
161142
});
162143

163-
after(() => {
164-
childProcess.kill();
165-
});
166-
167144
it(
168145
'18. Implicit session is ignored if connection does not support sessions',
169146
{

0 commit comments

Comments
 (0)