From 09d141a86fa11b6d5abb074359c8021407ef020b Mon Sep 17 00:00:00 2001 From: Bailey Pearson Date: Fri, 18 Nov 2022 14:41:21 -0500 Subject: [PATCH] address comments --- src/cmap/connection_pool.ts | 45 +++++-------------- ...ection_monitoring_and_pooling.spec.test.ts | 7 +++ 2 files changed, 18 insertions(+), 34 deletions(-) diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts index de99c76918..97c6c06364 100644 --- a/src/cmap/connection_pool.ts +++ b/src/cmap/connection_pool.ts @@ -415,7 +415,6 @@ export class ConnectionPool extends TypedEventEmitter { */ clear(options: { serviceId?: ObjectId; interruptInUseConnections?: boolean } = {}): void { const { serviceId } = options; - const interruptInUseConnections = options.interruptInUseConnections ?? false; if (this.closed) { return; } @@ -438,10 +437,9 @@ export class ConnectionPool extends TypedEventEmitter { ); return; } - - const oldGeneration = this[kGeneration]; - // handle non load-balanced case + const interruptInUseConnections = options.interruptInUseConnections ?? false; + const oldGeneration = this[kGeneration]; this[kGeneration] += 1; const alreadyPaused = this[kPoolState] === PoolState.paused; this[kPoolState] = PoolState.paused; @@ -454,9 +452,9 @@ export class ConnectionPool extends TypedEventEmitter { ); } - process.nextTick(() => - this.pruneConnections({ minGeneration: oldGeneration, interruptInUseConnections }) - ); + if (interruptInUseConnections) { + process.nextTick(() => this.interruptInUseConnections(oldGeneration)); + } this.processWaitQueue(); } @@ -468,41 +466,20 @@ export class ConnectionPool extends TypedEventEmitter { * Only connections where `connection.generation <= minGeneration` are killed. Connections are closed with a * resumable PoolClearedOnNetworkTimeoutError. */ - private pruneConnections({ - interruptInUseConnections, - minGeneration - }: { - interruptInUseConnections: boolean; - minGeneration: number; - }) { - this[kConnections].prune(connection => { + private interruptInUseConnections(minGeneration: number) { + for (const connection of this[kCheckedOut]) { if (connection.generation <= minGeneration) { + this[kCheckedOut].delete(connection); connection.onError(new PoolClearedOnNetworkError(this)); this.emit( ConnectionPool.CONNECTION_CLOSED, new ConnectionClosedEvent(this, connection, 'stale') ); - - return true; } - return false; - }); - - if (interruptInUseConnections) { - for (const connection of this[kCheckedOut]) { - if (connection.generation <= minGeneration) { - this[kCheckedOut].delete(connection); - connection.onError(new PoolClearedOnNetworkError(this)); - this.emit( - ConnectionPool.CONNECTION_CLOSED, - new ConnectionClosedEvent(this, connection, 'stale') - ); - } - } - - // TODO(NODE-4784): track pending connections and cancel - // this[kCancellationToken].emit('cancel'); } + + // TODO(NODE-4784): track pending connections and cancel + // this[kCancellationToken].emit('cancel'); } /** Close the pool */ 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 9113b23aff..1d166eb614 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 @@ -24,6 +24,13 @@ const INTERRUPT_IN_USE_SKIPPED_TESTS: SkipDescription[] = [ description: 'clear with interruptInUseConnections = true closes pending connections', skipIfCondition: 'always', skipReason: 'TODO(NODE-4784): track and kill pending connections' + }, + { + description: + 'Pool clear SHOULD schedule the next background thread run immediately (interruptInUseConnections: false)', + skipIfCondition: 'always', + skipReason: + 'NodeJS does not have a background thread responsible for managing connections, and so already checked in connections are not pruned when in-use connections are interrupted.' } ];