From dac32c41422bd7bc7c7d6aac437252e54d20168e Mon Sep 17 00:00:00 2001 From: Daria Pardue Date: Thu, 29 Sep 2022 15:16:17 -0400 Subject: [PATCH 1/7] feat: add minPoolSizeCheckIntervalMS option to pool --- src/cmap/connection_pool.ts | 13 +++++++++++-- test/tools/cmap_spec_runner.ts | 7 +++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts index 6bd65cce562..5bd4da746d9 100644 --- a/src/cmap/connection_pool.ts +++ b/src/cmap/connection_pool.ts @@ -90,6 +90,8 @@ export interface ConnectionPoolOptions extends Omit { maxConnecting: options.maxConnecting ?? 2, maxIdleTimeMS: options.maxIdleTimeMS ?? 0, waitQueueTimeoutMS: options.waitQueueTimeoutMS ?? 0, + minPoolSizeCheckIntervalMS: options.minPoolSizeCheckIntervalMS ?? 100, autoEncrypter: options.autoEncrypter, metadata: options.metadata }); @@ -683,12 +686,18 @@ export class ConnectionPool extends TypedEventEmitter { } if (this[kPoolState] === PoolState.ready) { clearTimeout(this[kMinPoolSizeTimer]); - this[kMinPoolSizeTimer] = setTimeout(() => this.ensureMinPoolSize(), 10); + this[kMinPoolSizeTimer] = setTimeout( + () => this.ensureMinPoolSize(), + this.options.minPoolSizeCheckIntervalMS + ); } }); } else { clearTimeout(this[kMinPoolSizeTimer]); - this[kMinPoolSizeTimer] = setTimeout(() => this.ensureMinPoolSize(), 100); + this[kMinPoolSizeTimer] = setTimeout( + () => this.ensureMinPoolSize(), + this.options.minPoolSizeCheckIntervalMS + ); } } diff --git a/test/tools/cmap_spec_runner.ts b/test/tools/cmap_spec_runner.ts index 340d4a1c4c6..ee775c49953 100644 --- a/test/tools/cmap_spec_runner.ts +++ b/test/tools/cmap_spec_runner.ts @@ -351,8 +351,11 @@ async function runCmapTest(test: CmapTest, threadContext: ThreadContext) { const poolOptions = test.poolOptions || {}; expect(CMAP_POOL_OPTION_NAMES).to.include.members(Object.keys(poolOptions)); - // TODO(NODE-3255): update condition to only remove option if set to -1 + let minPoolSizeCheckIntervalMS; if (poolOptions.backgroundThreadIntervalMS) { + if (poolOptions.backgroundThreadIntervalMS !== -1) { + minPoolSizeCheckIntervalMS = poolOptions.backgroundThreadIntervalMS; + } delete poolOptions.backgroundThreadIntervalMS; } @@ -373,7 +376,7 @@ async function runCmapTest(test: CmapTest, threadContext: ThreadContext) { const mainThread = threadContext.getThread(MAIN_THREAD_KEY); mainThread.start(); - threadContext.createPool({ ...poolOptions, metadata }); + threadContext.createPool({ ...poolOptions, metadata, minPoolSizeCheckIntervalMS }); // yield control back to the event loop so that the ConnectionPoolCreatedEvent // has a chance to be fired before any synchronously-emitted events from // the queued operations From cc0f63ca8542f9df68b74dc0144d3a9e4571d5fc Mon Sep 17 00:00:00 2001 From: Daria Pardue Date: Thu, 29 Sep 2022 16:43:34 -0400 Subject: [PATCH 2/7] test: add unit test for minPoolSizeCheckIntervalMS --- test/unit/cmap/connection_pool.test.js | 88 ++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/test/unit/cmap/connection_pool.test.js b/test/unit/cmap/connection_pool.test.js index 5b4ac7c5e73..fbfae63c668 100644 --- a/test/unit/cmap/connection_pool.test.js +++ b/test/unit/cmap/connection_pool.test.js @@ -9,6 +9,7 @@ const { expect } = require('chai'); const { setImmediate } = require('timers'); const { ns, isHello } = require('../../../src/utils'); const { LEGACY_HELLO_COMMAND } = require('../../../src/constants'); +const { createTimerSandbox } = require('../timer_sandbox'); describe('Connection Pool', function () { let server; @@ -128,6 +129,93 @@ describe('Connection Pool', function () { }); }); + describe('minPoolSize population', function () { + let clock, timerSandbox; + beforeEach(() => { + timerSandbox = createTimerSandbox(); + clock = sinon.useFakeTimers(); + }); + + afterEach(() => { + if (clock) { + timerSandbox.restore(); + clock.restore(); + clock = undefined; + } + }); + + it('should respect the minPoolSizeCheckIntervalMS option', function () { + const pool = new ConnectionPool(server, { + minPoolSize: 2, + minPoolSizeCheckIntervalMS: 42, + hostAddress: server.hostAddress() + }); + const ensureSpy = sinon.spy(pool, 'ensureMinPoolSize'); + + // return a fake connection that won't get identified as perished + const createConnStub = sinon + .stub(pool, 'createConnection') + .yields(null, { destroy: () => null, generation: 0 }); + + pool.ready(); + + // expect ensureMinPoolSize to execute immediately + expect(ensureSpy).to.have.been.calledOnce; + expect(createConnStub).to.have.been.calledOnce; + + // check that the successful connection return schedules another run + clock.tick(42); + expect(ensureSpy).to.have.been.calledTwice; + expect(createConnStub).to.have.been.calledTwice; + + // check that the 2nd successful connection return schedules another run + // but don't expect to get a new connection since we are at minPoolSize + clock.tick(42); + expect(ensureSpy).to.have.been.calledThrice; + expect(createConnStub).to.have.been.calledTwice; + + // check that the next scheduled check runs even after we're at minPoolSize + clock.tick(42); + expect(ensureSpy).to.have.callCount(4); + expect(createConnStub).to.have.been.calledTwice; + }); + + it('should default minPoolSizeCheckIntervalMS to 100ms', function () { + const pool = new ConnectionPool(server, { + minPoolSize: 2, + hostAddress: server.hostAddress() + }); + const ensureSpy = sinon.spy(pool, 'ensureMinPoolSize'); + + // return a fake connection that won't get identified as perished + const createConnStub = sinon + .stub(pool, 'createConnection') + .yields(null, { destroy: () => null, generation: 0 }); + + pool.ready(); + + // expect ensureMinPoolSize to execute immediately + expect(ensureSpy).to.have.been.calledOnce; + expect(createConnStub).to.have.been.calledOnce; + + // check that the successful connection return schedules another run + clock.tick(100); + expect(ensureSpy).to.have.been.calledTwice; + expect(createConnStub).to.have.been.calledTwice; + + // check that the 2nd successful connection return schedules another run + // but don't expect to get a new connection since we are at minPoolSize + clock.tick(100); + expect(ensureSpy).to.have.been.calledThrice; + expect(createConnStub).to.have.been.calledTwice; + + // check that the next scheduled check runs even after we're at minPoolSize + clock.tick(100); + expect(ensureSpy).to.have.callCount(4); + expect(createConnStub).to.have.been.calledTwice; + }); + }); + describe('withConnection', function () { it('should manage a connection for a successful operation', function (done) { server.setMessageHandler(request => { From 6b5576a4fd19dca451c09b6a1cb9b9cf973a17ed Mon Sep 17 00:00:00 2001 From: Daria Pardue Date: Thu, 29 Sep 2022 16:55:08 -0400 Subject: [PATCH 3/7] fix: make option optional --- src/cmap/connection_pool.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts index 5bd4da746d9..83778c624fa 100644 --- a/src/cmap/connection_pool.ts +++ b/src/cmap/connection_pool.ts @@ -91,7 +91,7 @@ export interface ConnectionPoolOptions extends Omit Date: Fri, 30 Sep 2022 11:30:57 -0400 Subject: [PATCH 4/7] fix: update new prop ts type to internal --- src/cmap/connection_pool.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts index 83778c624fa..68db03d48ab 100644 --- a/src/cmap/connection_pool.ts +++ b/src/cmap/connection_pool.ts @@ -90,7 +90,7 @@ export interface ConnectionPoolOptions extends Omit Date: Fri, 30 Sep 2022 12:23:10 -0400 Subject: [PATCH 5/7] test: update test to reliably await connection pool minsize --- ...er_selection.prose.operation_count.test.ts | 22 ++++++------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/test/integration/server-selection/server_selection.prose.operation_count.test.ts b/test/integration/server-selection/server_selection.prose.operation_count.test.ts index 21b41ad4230..c3720026d27 100644 --- a/test/integration/server-selection/server_selection.prose.operation_count.test.ts +++ b/test/integration/server-selection/server_selection.prose.operation_count.test.ts @@ -1,6 +1,5 @@ import { expect } from 'chai'; -import { setTimeout } from 'timers'; -import { promisify } from 'util'; +import { on } from 'events'; import { CommandStartedEvent } from '../../../src'; import { Collection } from '../../../src/collection'; @@ -27,19 +26,12 @@ async function runTaskGroup(collection: Collection, count: 10 | 100 | 1000) { async function ensurePoolIsFull(client: MongoClient) { let connectionCount = 0; - const onConnectionCreated = () => connectionCount++; - client.on('connectionCreated', onConnectionCreated); - - // 250ms should be plenty of time to fill the connection pool, - // but just in case we'll loop a couple of times. - for (let i = 0; connectionCount < POOL_SIZE * 2 && i < 10; ++i) { - await promisify(setTimeout)(250); - } - - client.removeListener('connectionCreated', onConnectionCreated); - - if (connectionCount !== POOL_SIZE * 2) { - throw new Error('Connection pool did not fill up'); + // eslint-disable-next-line @typescript-eslint/no-unused-vars + for await (const _event of on(client, 'connectionCreated')) { + connectionCount++; + if (connectionCount === POOL_SIZE * 2) { + break; + } } } From 8d6641a3a1a252b20aefb48a1b52fbc9fb4ab15d Mon Sep 17 00:00:00 2001 From: Daria Pardue Date: Fri, 30 Sep 2022 14:21:40 -0400 Subject: [PATCH 6/7] refactor: update option name --- src/cmap/connection_pool.ts | 8 ++++---- test/tools/cmap_spec_runner.ts | 6 +++--- test/unit/cmap/connection_pool.test.js | 6 +++--- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/cmap/connection_pool.ts b/src/cmap/connection_pool.ts index 68db03d48ab..d5daf8b186c 100644 --- a/src/cmap/connection_pool.ts +++ b/src/cmap/connection_pool.ts @@ -91,7 +91,7 @@ export interface ConnectionPoolOptions extends Omit { maxConnecting: options.maxConnecting ?? 2, maxIdleTimeMS: options.maxIdleTimeMS ?? 0, waitQueueTimeoutMS: options.waitQueueTimeoutMS ?? 0, - minPoolSizeCheckIntervalMS: options.minPoolSizeCheckIntervalMS ?? 100, + minPoolSizeCheckFrequencyMS: options.minPoolSizeCheckFrequencyMS ?? 100, autoEncrypter: options.autoEncrypter, metadata: options.metadata }); @@ -688,7 +688,7 @@ export class ConnectionPool extends TypedEventEmitter { clearTimeout(this[kMinPoolSizeTimer]); this[kMinPoolSizeTimer] = setTimeout( () => this.ensureMinPoolSize(), - this.options.minPoolSizeCheckIntervalMS + this.options.minPoolSizeCheckFrequencyMS ); } }); @@ -696,7 +696,7 @@ export class ConnectionPool extends TypedEventEmitter { clearTimeout(this[kMinPoolSizeTimer]); this[kMinPoolSizeTimer] = setTimeout( () => this.ensureMinPoolSize(), - this.options.minPoolSizeCheckIntervalMS + this.options.minPoolSizeCheckFrequencyMS ); } } diff --git a/test/tools/cmap_spec_runner.ts b/test/tools/cmap_spec_runner.ts index ee775c49953..9919ab82238 100644 --- a/test/tools/cmap_spec_runner.ts +++ b/test/tools/cmap_spec_runner.ts @@ -351,10 +351,10 @@ async function runCmapTest(test: CmapTest, threadContext: ThreadContext) { const poolOptions = test.poolOptions || {}; expect(CMAP_POOL_OPTION_NAMES).to.include.members(Object.keys(poolOptions)); - let minPoolSizeCheckIntervalMS; + let minPoolSizeCheckFrequencyMS; if (poolOptions.backgroundThreadIntervalMS) { if (poolOptions.backgroundThreadIntervalMS !== -1) { - minPoolSizeCheckIntervalMS = poolOptions.backgroundThreadIntervalMS; + minPoolSizeCheckFrequencyMS = poolOptions.backgroundThreadIntervalMS; } delete poolOptions.backgroundThreadIntervalMS; } @@ -376,7 +376,7 @@ async function runCmapTest(test: CmapTest, threadContext: ThreadContext) { const mainThread = threadContext.getThread(MAIN_THREAD_KEY); mainThread.start(); - threadContext.createPool({ ...poolOptions, metadata, minPoolSizeCheckIntervalMS }); + threadContext.createPool({ ...poolOptions, metadata, minPoolSizeCheckFrequencyMS }); // yield control back to the event loop so that the ConnectionPoolCreatedEvent // has a chance to be fired before any synchronously-emitted events from // the queued operations diff --git a/test/unit/cmap/connection_pool.test.js b/test/unit/cmap/connection_pool.test.js index fbfae63c668..29ae69a6615 100644 --- a/test/unit/cmap/connection_pool.test.js +++ b/test/unit/cmap/connection_pool.test.js @@ -144,10 +144,10 @@ describe('Connection Pool', function () { } }); - it('should respect the minPoolSizeCheckIntervalMS option', function () { + it('should respect the minPoolSizeCheckFrequencyMS option', function () { const pool = new ConnectionPool(server, { minPoolSize: 2, - minPoolSizeCheckIntervalMS: 42, + minPoolSizeCheckFrequencyMS: 42, hostAddress: server.hostAddress() }); const ensureSpy = sinon.spy(pool, 'ensureMinPoolSize'); @@ -180,7 +180,7 @@ describe('Connection Pool', function () { expect(createConnStub).to.have.been.calledTwice; }); - it('should default minPoolSizeCheckIntervalMS to 100ms', function () { + it('should default minPoolSizeCheckFrequencyMS to 100ms', function () { const pool = new ConnectionPool(server, { minPoolSize: 2, hostAddress: server.hostAddress() From 588f704506804a314a09b34d44441ba63cc208e5 Mon Sep 17 00:00:00 2001 From: Daria Pardue Date: Fri, 30 Sep 2022 15:07:24 -0400 Subject: [PATCH 7/7] test: add timeout error --- .../server_selection.prose.operation_count.test.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/integration/server-selection/server_selection.prose.operation_count.test.ts b/test/integration/server-selection/server_selection.prose.operation_count.test.ts index c3720026d27..fef521dd245 100644 --- a/test/integration/server-selection/server_selection.prose.operation_count.test.ts +++ b/test/integration/server-selection/server_selection.prose.operation_count.test.ts @@ -4,6 +4,7 @@ import { on } from 'events'; import { CommandStartedEvent } from '../../../src'; import { Collection } from '../../../src/collection'; import { MongoClient } from '../../../src/mongo_client'; +import { sleep } from '../../tools/utils'; const failPoint = { configureFailPoint: 'failCommand', @@ -24,7 +25,7 @@ async function runTaskGroup(collection: Collection, count: 10 | 100 | 1000) { } } -async function ensurePoolIsFull(client: MongoClient) { +async function ensurePoolIsFull(client: MongoClient): Promise { let connectionCount = 0; // eslint-disable-next-line @typescript-eslint/no-unused-vars for await (const _event of on(client, 'connectionCreated')) { @@ -74,7 +75,10 @@ describe('operationCount-based Selection Within Latency Window - Prose Test', fu await client.connect(); // Step 4: Using CMAP events, ensure the client's connection pools for both mongoses have been saturated - await poolIsFullPromise; + const poolIsFull = Promise.race([poolIsFullPromise, sleep(30 * 1000)]); + if (!poolIsFull) { + throw new Error('Timed out waiting for connection pool to fill to minPoolSize'); + } seeds = client.topology.s.seedlist.map(address => address.toString());