fix: session pool should only create session if pending<=waiters (#791)

* fix: session pool should only create session if pending<=waiters

The session pool would always initiate the creation of a new session
if one was not available at the moment that an application requested
a session, even when a large number of sessions were already pending
creation. This could cause the pool to create more sessions than
necessary for an application.

Fixes #790.

* fix: add clarifying comment

Author: olavloite

src/session-pool.ts

@@ -235,6 +235,8 @@ export class SessionPool extends EventEmitter implements SessionPoolInterface {
   _inventory: SessionInventory;
   _onClose!: Promise<void>;
   _pending = 0;
+  _pendingPrepare = 0;
+  _numWaiters = 0;
   _pingHandle!: NodeJS.Timer;
   _requests: PQueue;
   _traces: Map<string, trace.StackFrame[]>;
@@ -459,9 +461,13 @@ export class SessionPool extends EventEmitter implements SessionPoolInterface {
     // back into the pool. This will prevent the session to be marked as leaked if
     // the pool is closed while the session is being prepared.
     this._traces.delete(session.id);
+    this._pendingPrepare++;
     this._prepareTransaction(session)
       .catch(() => (session.type = types.ReadOnly))
-      .then(() => this._release(session));
+      .then(() => {
+        this._pendingPrepare--;
+        this._release(session);
+      });
   }
 
   /**
@@ -772,7 +778,12 @@ export class SessionPool extends EventEmitter implements SessionPoolInterface {
       );
     }
 
-    if (!this.isFull) {
+    // Only create a new session if there are more waiters than sessions already
+    // being created. The current requester will be waiter number _numWaiters+1.
+    if (
+      !this.isFull &&
+      this._pending + this._pendingPrepare <= this._numWaiters
+    ) {
       promises.push(
         new Promise((_, reject) => {
           this._createSession(type).catch(reject);
@@ -781,10 +792,13 @@ export class SessionPool extends EventEmitter implements SessionPoolInterface {
     }
 
     try {
+      this._numWaiters++;
       await Promise.race(promises);
     } catch (e) {
       removeListener!();
       throw e;
+    } finally {
+      this._numWaiters--;
     }
 
     return this._borrowNextAvailableSession(type);

test/session-pool.ts

@@ -1186,6 +1186,28 @@ describe('SessionPool', () => {
       assert.strictEqual(stub.callCount, 1);
     });
 
+    it('should wait for a pending session to become available', async () => {
+      const fakeSession = createSession();
+
+      sessionPool.options.max = 2;
+      sessionPool._pending = 1;
+      const stub = sandbox
+        .stub(sessionPool, '_createSession')
+        .withArgs(types.ReadOnly)
+        .callsFake(() => {
+          return Promise.reject(new Error('should not be called'));
+        });
+      sandbox
+        .stub(sessionPool, '_borrowNextAvailableSession')
+        .withArgs(types.ReadOnly)
+        .returns(fakeSession);
+      setTimeout(() => sessionPool.emit('available'), 100);
+
+      const session = await sessionPool._getSession(types.ReadOnly, startTime);
+      assert.strictEqual(session, fakeSession);
+      assert.strictEqual(stub.callCount, 0);
+    });
+
     it('should return any create errors', async () => {
       const error = new Error('err');