From 16f15861c2d98db48c8c5a8b02542bf022dd2e1e Mon Sep 17 00:00:00 2001 From: Daria Pardue Date: Fri, 16 Sep 2022 15:42:58 -0400 Subject: [PATCH 01/10] refactor: give connection pool access to server --- src/cmap/connection_pool.ts | 8 +++++++- src/sdam/server.ts | 2 +- test/tools/cmap_spec_runner.ts | 23 ++++++++++++++++------- test/unit/cmap/connection_pool.test.js | 17 ++++++++++------- 4 files changed, 34 insertions(+), 16 deletions(-) diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts index 65bbe5c7ea..2d61a770b3 100644 --- a/src/cmap/connection_pool.ts +++ b/src/cmap/connection_pool.ts @@ -19,6 +19,7 @@ import { import { MongoError, MongoInvalidArgumentError, MongoRuntimeError } from '../error'; import { Logger } from '../logger'; import { CancellationToken, TypedEventEmitter } from '../mongo_types'; +import type { Server } from '../sdam/server'; import { Callback, eachAsync, makeCounter } from '../utils'; import { connect } from './connect'; import { Connection, ConnectionEvents, ConnectionOptions } from './connection'; @@ -38,6 +39,8 @@ import { import { PoolClearedError, PoolClosedError, WaitQueueTimeoutError } from './errors'; import { ConnectionPoolMetrics } from './metrics'; +/** @internal */ +const kServer = Symbol('server'); /** @internal */ const kLogger = Symbol('logger'); /** @internal */ @@ -126,6 +129,8 @@ export class ConnectionPool extends TypedEventEmitter { /** @internal */ [kPoolState]: typeof PoolState[keyof typeof PoolState]; /** @internal */ + [kServer]: Server; + /** @internal */ [kLogger]: Logger; /** @internal */ [kConnections]: Denque; @@ -212,7 +217,7 @@ export class ConnectionPool extends TypedEventEmitter { static readonly CONNECTION_CHECKED_IN = CONNECTION_CHECKED_IN; /** @internal */ - constructor(options: ConnectionPoolOptions) { + constructor(server: Server, options: ConnectionPoolOptions) { super(); this.options = Object.freeze({ @@ -234,6 +239,7 @@ export class ConnectionPool extends TypedEventEmitter { } this[kPoolState] = PoolState.paused; + this[kServer] = server; this[kLogger] = new Logger('ConnectionPool'); this[kConnections] = new Denque(); this[kPending] = 0; diff --git a/src/sdam/server.ts b/src/sdam/server.ts index 3560ef9cfe..f62e5de796 100644 --- a/src/sdam/server.ts +++ b/src/sdam/server.ts @@ -151,7 +151,7 @@ export class Server extends TypedEventEmitter { logger: new Logger('Server'), state: STATE_CLOSED, topology, - pool: new ConnectionPool(poolOptions), + pool: new ConnectionPool(this, poolOptions), operationCount: 0 }; diff --git a/test/tools/cmap_spec_runner.ts b/test/tools/cmap_spec_runner.ts index 81c081e857..bee6f74002 100644 --- a/test/tools/cmap_spec_runner.ts +++ b/test/tools/cmap_spec_runner.ts @@ -3,7 +3,7 @@ import { EventEmitter } from 'events'; import { clearTimeout, setTimeout } from 'timers'; import { promisify } from 'util'; -import { Connection, HostAddress, MongoClient } from '../../src'; +import { Connection, HostAddress, MongoClient, Server } from '../../src'; import { ConnectionPool, ConnectionPoolOptions } from '../../src/cmap/connection_pool'; import { CMAP_EVENTS } from '../../src/constants'; import { makeClientMetadata, shuffle } from '../../src/utils'; @@ -253,11 +253,12 @@ export class ThreadContext { threads: Map = new Map(); connections: Map = new Map(); orphans: Set = new Set(); - poolEvents = []; + poolEvents: any[] = []; poolEventsEventEmitter = new EventEmitter(); #poolOptions: Partial; #hostAddress: HostAddress; + #server: Server; #supportedOperations: ReturnType; #injectPoolStats = false; @@ -267,12 +268,14 @@ export class ThreadContext { * @param poolOptions - Allows the test to pass in extra options to the pool not specified by the spec test definition, such as the environment-dependent "loadBalanced" */ constructor( + server: Server, hostAddress: HostAddress, poolOptions: Partial = {}, contextOptions: { injectPoolStats: boolean } ) { this.#poolOptions = poolOptions; this.#hostAddress = hostAddress; + this.#server = server; this.#supportedOperations = getTestOpDefinitions(this); this.#injectPoolStats = contextOptions.injectPoolStats; } @@ -292,7 +295,7 @@ export class ThreadContext { } createPool(options) { - this.pool = new ConnectionPool({ + this.pool = new ConnectionPool(this.#server, { ...this.#poolOptions, ...options, hostAddress: this.#hostAddress @@ -438,7 +441,10 @@ export function runCmapTestSuite( ) { for (const test of tests) { describe(test.name, function () { - let hostAddress: HostAddress, threadContext: ThreadContext, client: MongoClient; + let hostAddress: HostAddress, + server: Server, + threadContext: ThreadContext, + client: MongoClient; beforeEach(async function () { let utilClient: MongoClient; @@ -479,11 +485,14 @@ export function runCmapTestSuite( } try { - const serverMap = utilClient.topology.s.description.servers; - const hosts = shuffle(serverMap.keys()); + const serverDescriptionMap = utilClient.topology?.s.description.servers; + const serverMap = utilClient.topology?.s.servers; + const hosts = shuffle(serverDescriptionMap.keys()); const selectedHostUri = hosts[0]; - hostAddress = serverMap.get(selectedHostUri).hostAddress; + hostAddress = serverDescriptionMap.get(selectedHostUri).hostAddress; + server = serverMap.get(selectedHostUri); threadContext = new ThreadContext( + server, hostAddress, this.configuration.isLoadBalanced ? { loadBalanced: true } : {}, { injectPoolStats: !!options?.injectPoolStats } diff --git a/test/unit/cmap/connection_pool.test.js b/test/unit/cmap/connection_pool.test.js index 3f2b5176d7..5b4ac7c5e7 100644 --- a/test/unit/cmap/connection_pool.test.js +++ b/test/unit/cmap/connection_pool.test.js @@ -26,7 +26,7 @@ describe('Connection Pool', function () { } }); - const pool = new ConnectionPool({ maxPoolSize: 1, hostAddress: server.hostAddress() }); + const pool = new ConnectionPool(server, { maxPoolSize: 1, hostAddress: server.hostAddress() }); pool.ready(); const events = []; @@ -69,7 +69,7 @@ describe('Connection Pool', function () { } }); - const pool = new ConnectionPool({ + const pool = new ConnectionPool(server, { maxPoolSize: 1, socketTimeoutMS: 200, hostAddress: server.hostAddress() @@ -99,7 +99,7 @@ describe('Connection Pool', function () { } }); - const pool = new ConnectionPool({ + const pool = new ConnectionPool(server, { maxPoolSize: 1, waitQueueTimeoutMS: 200, hostAddress: server.hostAddress() @@ -137,7 +137,7 @@ describe('Connection Pool', function () { } }); - const pool = new ConnectionPool({ hostAddress: server.hostAddress() }); + const pool = new ConnectionPool(server, { hostAddress: server.hostAddress() }); pool.ready(); const callback = (err, result) => { @@ -169,7 +169,7 @@ describe('Connection Pool', function () { } }); - const pool = new ConnectionPool({ + const pool = new ConnectionPool(server, { waitQueueTimeoutMS: 200, hostAddress: server.hostAddress() }); @@ -201,7 +201,7 @@ describe('Connection Pool', function () { } }); - const pool = new ConnectionPool({ hostAddress: server.hostAddress() }); + const pool = new ConnectionPool(server, { hostAddress: server.hostAddress() }); pool.ready(); const callback = (err, result) => { @@ -229,7 +229,10 @@ describe('Connection Pool', function () { } }); - const pool = new ConnectionPool({ maxPoolSize: 1, hostAddress: server.hostAddress() }); + const pool = new ConnectionPool(server, { + maxPoolSize: 1, + hostAddress: server.hostAddress() + }); pool.ready(); const events = []; From cfeeed14c9e40c4487d1d849f25819da86d71b7e Mon Sep 17 00:00:00 2001 From: Daria Pardue Date: Fri, 16 Sep 2022 18:30:04 -0400 Subject: [PATCH 02/10] feat: add original error message to PoolClearedError --- src/cmap/connection_pool.ts | 4 ++++ src/cmap/errors.ts | 6 +++--- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts index 2d61a770b3..0b53cb6cb8 100644 --- a/src/cmap/connection_pool.ts +++ b/src/cmap/connection_pool.ts @@ -310,6 +310,10 @@ export class ConnectionPool extends TypedEventEmitter { return this[kServiceGenerations]; } + get serverError() { + return this[kServer].description.error; + } + /** * Get the metrics information for the pool when a wait queue timeout occurs. */ diff --git a/src/cmap/errors.ts b/src/cmap/errors.ts index b2892715ad..4e0d25a27c 100644 --- a/src/cmap/errors.ts +++ b/src/cmap/errors.ts @@ -28,9 +28,9 @@ export class PoolClearedError extends MongoNetworkError { address: string; constructor(pool: ConnectionPool) { - // TODO(NODE-3135): pass in original pool-clearing error and use in message - // "failed with: " - super(`Connection pool for ${pool.address} was cleared because another operation failed`); + super( + `Connection pool for ${pool.address} was cleared because another operation failed with: "${pool.serverError?.message}"` + ); this.address = pool.address; } From 87ea6d43efe114c4f26c563890bbd19d1b3dddde Mon Sep 17 00:00:00 2001 From: Daria Pardue Date: Tue, 27 Sep 2022 15:24:03 -0400 Subject: [PATCH 03/10] refactor: separate sdam error handling from session error handling --- src/sdam/server.ts | 64 +++++++++++++++++++++++++--------------------- 1 file changed, 35 insertions(+), 29 deletions(-) diff --git a/src/sdam/server.ts b/src/sdam/server.ts index f62e5de796..29929bc5ac 100644 --- a/src/sdam/server.ts +++ b/src/sdam/server.ts @@ -444,6 +444,39 @@ function isRetryableWritesEnabled(topology: Topology) { return topology.s.options.retryWrites !== false; } +function handleError(server: Server, error: MongoError, connection: Connection) { + if (error instanceof MongoNetworkError) { + if (!(error instanceof MongoNetworkTimeoutError) || isNetworkErrorBeforeHandshake(error)) { + // In load balanced mode we never mark the server as unknown and always + // clear for the specific service id. + + if (!server.loadBalanced) { + error.addErrorLabel(MongoErrorLabel.ResetPool); + markServerUnknown(server, error); + } else { + server.s.pool.clear(connection.serviceId); + } + } + } else { + if (isSDAMUnrecoverableError(error)) { + if (shouldHandleStateChangeError(server, error)) { + const shouldClearPool = maxWireVersion(server) <= 7 || isNodeShuttingDownError(error); + if (server.loadBalanced && shouldClearPool) { + server.s.pool.clear(connection.serviceId); + } + + if (!server.loadBalanced) { + if (shouldClearPool) { + error.addErrorLabel(MongoErrorLabel.ResetPool); + } + markServerUnknown(server, error); + process.nextTick(() => server.requestCheck()); + } + } + } + } +} + function makeOperationHandler( server: Server, connection: Connection, @@ -490,18 +523,6 @@ function makeOperationHandler( ) { error.addErrorLabel(MongoErrorLabel.RetryableWriteError); } - - if (!(error instanceof MongoNetworkTimeoutError) || isNetworkErrorBeforeHandshake(error)) { - // In load balanced mode we never mark the server as unknown and always - // clear for the specific service id. - - if (!server.loadBalanced) { - error.addErrorLabel(MongoErrorLabel.ResetPool); - markServerUnknown(server, error); - } else { - server.s.pool.clear(connection.serviceId); - } - } } else { if ( (isRetryableWritesEnabled(server.s.topology) || isTransactionCommand(cmd)) && @@ -510,23 +531,6 @@ function makeOperationHandler( ) { error.addErrorLabel(MongoErrorLabel.RetryableWriteError); } - - if (isSDAMUnrecoverableError(error)) { - if (shouldHandleStateChangeError(server, error)) { - const shouldClearPool = maxWireVersion(server) <= 7 || isNodeShuttingDownError(error); - if (server.loadBalanced && shouldClearPool) { - server.s.pool.clear(connection.serviceId); - } - - if (!server.loadBalanced) { - if (shouldClearPool) { - error.addErrorLabel(MongoErrorLabel.ResetPool); - } - markServerUnknown(server, error); - process.nextTick(() => server.requestCheck()); - } - } - } } if ( @@ -537,6 +541,8 @@ function makeOperationHandler( session.unpin({ force: true }); } + handleError(server, error, connection); + return callback(error); }; } From ba32a464dd39994e0f5816b584cf06d4cdd1ce4c Mon Sep 17 00:00:00 2001 From: Daria Pardue Date: Tue, 27 Sep 2022 15:29:19 -0400 Subject: [PATCH 04/10] refactor: make handleError a server class method --- src/sdam/server.ts | 72 ++++++++++++++++++++++++---------------------- 1 file changed, 38 insertions(+), 34 deletions(-) diff --git a/src/sdam/server.ts b/src/sdam/server.ts index 29929bc5ac..c2e836dda6 100644 --- a/src/sdam/server.ts +++ b/src/sdam/server.ts @@ -376,6 +376,43 @@ export class Server extends TypedEventEmitter { callback ); } + + /** + * Handle SDAM error + * @internal + */ + handleError(error: MongoError, connection: Connection) { + if (error instanceof MongoNetworkError) { + if (!(error instanceof MongoNetworkTimeoutError) || isNetworkErrorBeforeHandshake(error)) { + // In load balanced mode we never mark the server as unknown and always + // clear for the specific service id. + + if (!this.loadBalanced) { + error.addErrorLabel(MongoErrorLabel.ResetPool); + markServerUnknown(this, error); + } else { + this.s.pool.clear(connection.serviceId); + } + } + } else { + if (isSDAMUnrecoverableError(error)) { + if (shouldHandleStateChangeError(this, error)) { + const shouldClearPool = maxWireVersion(this) <= 7 || isNodeShuttingDownError(error); + if (this.loadBalanced && shouldClearPool) { + this.s.pool.clear(connection.serviceId); + } + + if (!this.loadBalanced) { + if (shouldClearPool) { + error.addErrorLabel(MongoErrorLabel.ResetPool); + } + markServerUnknown(this, error); + process.nextTick(() => this.requestCheck()); + } + } + } + } + } } function calculateRoundTripTime(oldRtt: number, duration: number): number { @@ -444,39 +481,6 @@ function isRetryableWritesEnabled(topology: Topology) { return topology.s.options.retryWrites !== false; } -function handleError(server: Server, error: MongoError, connection: Connection) { - if (error instanceof MongoNetworkError) { - if (!(error instanceof MongoNetworkTimeoutError) || isNetworkErrorBeforeHandshake(error)) { - // In load balanced mode we never mark the server as unknown and always - // clear for the specific service id. - - if (!server.loadBalanced) { - error.addErrorLabel(MongoErrorLabel.ResetPool); - markServerUnknown(server, error); - } else { - server.s.pool.clear(connection.serviceId); - } - } - } else { - if (isSDAMUnrecoverableError(error)) { - if (shouldHandleStateChangeError(server, error)) { - const shouldClearPool = maxWireVersion(server) <= 7 || isNodeShuttingDownError(error); - if (server.loadBalanced && shouldClearPool) { - server.s.pool.clear(connection.serviceId); - } - - if (!server.loadBalanced) { - if (shouldClearPool) { - error.addErrorLabel(MongoErrorLabel.ResetPool); - } - markServerUnknown(server, error); - process.nextTick(() => server.requestCheck()); - } - } - } - } -} - function makeOperationHandler( server: Server, connection: Connection, @@ -541,7 +545,7 @@ function makeOperationHandler( session.unpin({ force: true }); } - handleError(server, error, connection); + server.handleError(error, connection); return callback(error); }; From 02a59480e2695f426db972f73e19277238822ff0 Mon Sep 17 00:00:00 2001 From: Daria Pardue Date: Tue, 27 Sep 2022 15:46:59 -0400 Subject: [PATCH 05/10] refactor: call sdam error handling on minPoolSize population --- src/cmap/connection_pool.ts | 7 +++++-- src/sdam/server.ts | 10 +++++++--- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts index 0b53cb6cb8..9641e6243a 100644 --- a/src/cmap/connection_pool.ts +++ b/src/cmap/connection_pool.ts @@ -661,18 +661,21 @@ export class ConnectionPool extends TypedEventEmitter { // connection permits because that potentially delays the availability of // the connection to a checkout request this.createConnection((err, connection) => { + if (err) { + this[kServer].handleError(err); + } if (!err && connection) { this[kConnections].push(connection); process.nextTick(() => this.processWaitQueue()); } if (this[kPoolState] === PoolState.ready) { clearTimeout(this[kMinPoolSizeTimer]); - this[kMinPoolSizeTimer] = setTimeout(() => this.ensureMinPoolSize(), 10); + this[kMinPoolSizeTimer] = setTimeout(() => this.ensureMinPoolSize(), 5000); } }); } else { clearTimeout(this[kMinPoolSizeTimer]); - this[kMinPoolSizeTimer] = setTimeout(() => this.ensureMinPoolSize(), 100); + this[kMinPoolSizeTimer] = setTimeout(() => this.ensureMinPoolSize(), 10000); } } diff --git a/src/sdam/server.ts b/src/sdam/server.ts index c2e836dda6..3ef81a3490 100644 --- a/src/sdam/server.ts +++ b/src/sdam/server.ts @@ -20,6 +20,7 @@ import { } from '../constants'; import type { AutoEncrypter } from '../deps'; import { + AnyError, isNetworkErrorBeforeHandshake, isNodeShuttingDownError, isSDAMUnrecoverableError, @@ -381,7 +382,10 @@ export class Server extends TypedEventEmitter { * Handle SDAM error * @internal */ - handleError(error: MongoError, connection: Connection) { + handleError(error: AnyError, connection?: Connection) { + if (!(error instanceof MongoError)) { + return; + } if (error instanceof MongoNetworkError) { if (!(error instanceof MongoNetworkTimeoutError) || isNetworkErrorBeforeHandshake(error)) { // In load balanced mode we never mark the server as unknown and always @@ -390,7 +394,7 @@ export class Server extends TypedEventEmitter { if (!this.loadBalanced) { error.addErrorLabel(MongoErrorLabel.ResetPool); markServerUnknown(this, error); - } else { + } else if (connection) { this.s.pool.clear(connection.serviceId); } } @@ -398,7 +402,7 @@ export class Server extends TypedEventEmitter { if (isSDAMUnrecoverableError(error)) { if (shouldHandleStateChangeError(this, error)) { const shouldClearPool = maxWireVersion(this) <= 7 || isNodeShuttingDownError(error); - if (this.loadBalanced && shouldClearPool) { + if (this.loadBalanced && connection && shouldClearPool) { this.s.pool.clear(connection.serviceId); } From dbf45e4cce7d3492e7394ffa243d70b1c0e98671 Mon Sep 17 00:00:00 2001 From: Daria Pardue Date: Tue, 27 Sep 2022 17:50:55 -0400 Subject: [PATCH 06/10] test: fix cmap spec runner implementation --- src/cmap/connection_pool.ts | 4 ++-- test/tools/cmap_spec_runner.ts | 33 +++++++++++++++++++++------------ 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts index 9641e6243a..0d8225a73d 100644 --- a/src/cmap/connection_pool.ts +++ b/src/cmap/connection_pool.ts @@ -670,12 +670,12 @@ export class ConnectionPool extends TypedEventEmitter { } if (this[kPoolState] === PoolState.ready) { clearTimeout(this[kMinPoolSizeTimer]); - this[kMinPoolSizeTimer] = setTimeout(() => this.ensureMinPoolSize(), 5000); + this[kMinPoolSizeTimer] = setTimeout(() => this.ensureMinPoolSize(), 10); } }); } else { clearTimeout(this[kMinPoolSizeTimer]); - this[kMinPoolSizeTimer] = setTimeout(() => this.ensureMinPoolSize(), 10000); + this[kMinPoolSizeTimer] = setTimeout(() => this.ensureMinPoolSize(), 100); } } diff --git a/test/tools/cmap_spec_runner.ts b/test/tools/cmap_spec_runner.ts index bee6f74002..340d4a1c4c 100644 --- a/test/tools/cmap_spec_runner.ts +++ b/test/tools/cmap_spec_runner.ts @@ -259,6 +259,7 @@ export class ThreadContext { #poolOptions: Partial; #hostAddress: HostAddress; #server: Server; + #originalServerPool: ConnectionPool; #supportedOperations: ReturnType; #injectPoolStats = false; @@ -300,6 +301,8 @@ export class ThreadContext { ...options, hostAddress: this.#hostAddress }); + this.#originalServerPool = this.#server.s.pool; + this.#server.s.pool = this.pool; ALL_POOL_EVENTS.forEach(eventName => { this.pool.on(eventName, event => { if (this.#injectPoolStats) { @@ -315,6 +318,7 @@ export class ThreadContext { } closePool() { + this.#server.s.pool = this.#originalServerPool; return new Promise(resolve => { ALL_POOL_EVENTS.forEach(ev => this.pool.removeAllListeners(ev)); this.pool.close(resolve); @@ -486,27 +490,32 @@ export function runCmapTestSuite( try { const serverDescriptionMap = utilClient.topology?.s.description.servers; - const serverMap = utilClient.topology?.s.servers; const hosts = shuffle(serverDescriptionMap.keys()); const selectedHostUri = hosts[0]; hostAddress = serverDescriptionMap.get(selectedHostUri).hostAddress; - server = serverMap.get(selectedHostUri); + + client = this.configuration.newClient( + `mongodb://${hostAddress}/${ + this.configuration.isLoadBalanced ? '?loadBalanced=true' : '?directConnection=true' + }` + ); + await client.connect(); + if (test.failPoint) { + await client.db('admin').command(test.failPoint); + } + + const serverMap = client.topology?.s.servers; + server = serverMap?.get(selectedHostUri); + if (!server) { + throw new Error('Failed to retrieve server for test'); + } + threadContext = new ThreadContext( server, hostAddress, this.configuration.isLoadBalanced ? { loadBalanced: true } : {}, { injectPoolStats: !!options?.injectPoolStats } ); - - if (test.failPoint) { - client = this.configuration.newClient( - `mongodb://${hostAddress}/${ - this.configuration.isLoadBalanced ? '?loadBalanced=true' : '?directConnection=true' - }` - ); - await client.connect(); - await client.db('admin').command(test.failPoint); - } } finally { await utilClient.close(); } From 74862b16de37bee6db9ea5c71cc3d03e45c54f81 Mon Sep 17 00:00:00 2001 From: Daria Pardue Date: Tue, 27 Sep 2022 17:54:57 -0400 Subject: [PATCH 07/10] fix: emit connectionClosed event on establishment error --- src/cmap/connection_pool.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts index 0d8225a73d..409d0b6361 100644 --- a/src/cmap/connection_pool.ts +++ b/src/cmap/connection_pool.ts @@ -597,6 +597,10 @@ export class ConnectionPool extends TypedEventEmitter { if (err || !connection) { this[kLogger].debug(`connection attempt failed with error [${JSON.stringify(err)}]`); this[kPending]--; + this.emit( + ConnectionPool.CONNECTION_CLOSED, + new ConnectionClosedEvent(this, { id: connectOptions.id } as Connection, 'error') + ); callback(err ?? new MongoRuntimeError('Connection creation failed without error')); return; } From 3c3eaddb83312677ecb005e72d680d6d5a3ad935 Mon Sep 17 00:00:00 2001 From: Daria Pardue Date: Tue, 27 Sep 2022 17:56:24 -0400 Subject: [PATCH 08/10] test: unskip implemented test --- ...ection_monitoring_and_pooling.spec.test.ts | 27 ++++++------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/test/integration/connection-monitoring-and-pooling/connection_monitoring_and_pooling.spec.test.ts b/test/integration/connection-monitoring-and-pooling/connection_monitoring_and_pooling.spec.test.ts index 8ee4487673..525d25d700 100644 --- a/test/integration/connection-monitoring-and-pooling/connection_monitoring_and_pooling.spec.test.ts +++ b/test/integration/connection-monitoring-and-pooling/connection_monitoring_and_pooling.spec.test.ts @@ -18,28 +18,17 @@ const LB_SKIP_TESTS: SkipDescription[] = [ skipReason: 'cannot run against a load balanced environment' })); -const POOL_PAUSED_SKIP_TESTS: SkipDescription[] = [ - 'error during minPoolSize population clears pool' -].map(description => ({ - description, - skipIfCondition: 'always', - skipReason: 'TODO(NODE-3135): make connection pool SDAM aware' -})); - describe('Connection Monitoring and Pooling Spec Tests (Integration)', function () { const tests: CmapTest[] = loadSpecTests('connection-monitoring-and-pooling'); runCmapTestSuite(tests, { - testsToSkip: LB_SKIP_TESTS.concat( - [ - { - description: 'waiting on maxConnecting is limited by WaitQueueTimeoutMS', - skipIfCondition: 'always', - skipReason: - 'not applicable: waitQueueTimeoutMS limits connection establishment time in our driver' - } - ], - POOL_PAUSED_SKIP_TESTS - ) + testsToSkip: LB_SKIP_TESTS.concat([ + { + description: 'waiting on maxConnecting is limited by WaitQueueTimeoutMS', + skipIfCondition: 'always', + skipReason: + 'not applicable: waitQueueTimeoutMS limits connection establishment time in our driver' + } + ]) }); }); From 0f8e6d8d3dc095052e963af935a7a091d7ba923a Mon Sep 17 00:00:00 2001 From: Daria Pardue Date: Wed, 28 Sep 2022 10:19:01 -0400 Subject: [PATCH 09/10] test: skip incompatible pool clear test on lb --- .../connection_monitoring_and_pooling.spec.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/integration/connection-monitoring-and-pooling/connection_monitoring_and_pooling.spec.test.ts b/test/integration/connection-monitoring-and-pooling/connection_monitoring_and_pooling.spec.test.ts index 525d25d700..d0eb20858d 100644 --- a/test/integration/connection-monitoring-and-pooling/connection_monitoring_and_pooling.spec.test.ts +++ b/test/integration/connection-monitoring-and-pooling/connection_monitoring_and_pooling.spec.test.ts @@ -11,7 +11,8 @@ const LB_SKIP_TESTS: SkipDescription[] = [ 'pool clear halts background minPoolSize establishments', 'clearing a paused pool emits no events', 'after clear, cannot check out connections until pool ready', - 'readying a ready pool emits no events' + 'readying a ready pool emits no events', + 'error during minPoolSize population clears pool' ].map(description => ({ description, skipIfCondition: 'loadBalanced', From ac95f81e679dce8a83b7855183c0d5f31aa11358 Mon Sep 17 00:00:00 2001 From: Daria Pardue Date: Wed, 28 Sep 2022 11:04:37 -0400 Subject: [PATCH 10/10] refactor: narrow ConnectionClosedEvent param type --- src/cmap/connection_pool.ts | 2 +- src/cmap/connection_pool_events.ts | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts index 409d0b6361..0a0c1f417d 100644 --- a/src/cmap/connection_pool.ts +++ b/src/cmap/connection_pool.ts @@ -599,7 +599,7 @@ export class ConnectionPool extends TypedEventEmitter { this[kPending]--; this.emit( ConnectionPool.CONNECTION_CLOSED, - new ConnectionClosedEvent(this, { id: connectOptions.id } as Connection, 'error') + new ConnectionClosedEvent(this, { id: connectOptions.id, serviceId: undefined }, 'error') ); callback(err ?? new MongoRuntimeError('Connection creation failed without error')); return; diff --git a/src/cmap/connection_pool_events.ts b/src/cmap/connection_pool_events.ts index 90c4826d58..9fd0f08ee9 100644 --- a/src/cmap/connection_pool_events.ts +++ b/src/cmap/connection_pool_events.ts @@ -106,7 +106,11 @@ export class ConnectionClosedEvent extends ConnectionPoolMonitoringEvent { serviceId?: ObjectId; /** @internal */ - constructor(pool: ConnectionPool, connection: Connection, reason: string) { + constructor( + pool: ConnectionPool, + connection: Pick, + reason: string + ) { super(pool); this.connectionId = connection.id; this.reason = reason || 'unknown';