Revert "Count pending connects when considering max pool size"

Author: zhenlineo

src/internal/pool.js

@@ -56,7 +56,6 @@ class Pool {
     this._maxSize = config.maxSize
     this._acquisitionTimeout = config.acquisitionTimeout
     this._pools = {}
-    this._pendingCreates = {}
     this._acquireRequests = {}
     this._activeResourceCounts = {}
     this._release = this._release.bind(this)
@@ -74,11 +73,18 @@ class Pool {
       const key = address.asKey()
 
       if (resource) {
-        resourceAcquired(key, this._activeResourceCounts)
-        if (this._log.isDebugEnabled()) {
-          this._log.debug(`${resource} acquired from the pool ${key}`)
+        if (
+          this._maxSize &&
+          this.activeResourceCount(address) >= this._maxSize
+        ) {
+          this._destroy(resource)
+        } else {
+          resourceAcquired(key, this._activeResourceCounts)
+          if (this._log.isDebugEnabled()) {
+            this._log.debug(`${resource} acquired from the pool ${key}`)
+          }
+          return resource
         }
-        return resource
       }
 
       // We're out of resources and will try to acquire later on when an existing resource is released.
@@ -179,7 +185,6 @@ class Pool {
     if (!pool) {
       pool = []
       this._pools[key] = pool
-      this._pendingCreates[key] = 0
     }
     while (pool.length) {
       const resource = pool.pop()
@@ -196,29 +201,12 @@ class Pool {
       }
     }
 
-    // Ensure requested max pool size
-    if (this._maxSize > 0) {
-      // Include pending creates when checking pool size since these probably will add
-      // to the number when fulfilled.
-      const numConnections =
-        this.activeResourceCount(address) + this._pendingCreates[key]
-      if (numConnections >= this._maxSize) {
-        // Will put this request in queue instead since the pool is full
-        return null
-      }
+    if (this._maxSize && this.activeResourceCount(address) >= this._maxSize) {
+      return null
     }
 
     // there exist no idle valid resources, create a new one for acquisition
-    // Keep track of how many pending creates there are to avoid making too many connections.
-    this._pendingCreates[key] = this._pendingCreates[key] + 1
-    let resource
-    try {
-      // Invoke callback that creates actual connection
-      resource = await this._create(address, this._release)
-    } finally {
-      this._pendingCreates[key] = this._pendingCreates[key] - 1
-    }
-    return resource
+    return await this._create(address, this._release)
   }
 
   async _release (address, resource) {

test/internal/pool.test.js

@@ -562,59 +562,10 @@ describe('#unit Pool', async () => {
     expectNumberOfAcquisitionRequests(pool, address, 0)
   })
 
-  const address = ServerAddress.fromUrl('bolt://localhost:7687')
-
-  it('should consider pending connects when evaluating max pool size', async () => {
-    const conns = []
-    const pool = new Pool({
-      // Hook into connection creation to track when and what connections that are
-      // created.
-      create: (server, release) => {
-        // Create a fake connection that makes it possible control when it's connected
-        // and released from the outer scope.
-        const conn = {
-          server: server,
-          release: release
-        }
-        const promise = new Promise((resolve, reject) => {
-          conn.resolve = resolve
-          conn.reject = reject
-        })
-        // Put the connection in a list in outer scope even though there only should be
-        // one when the test is succeeding.
-        conns.push(conn)
-        return promise
-      },
-      // Setup pool to only allow one connection
-      config: new PoolConfig(1, 100000)
-    })
-
-    // Make the first request for a connection, this will be hanging waiting for the
-    // connect promise to be resolved.
-    const req1 = pool.acquire(address)
-    expect(conns.length).toEqual(1)
-
-    // Make another request to the same server, this should not try to acquire another
-    // connection since the pool will be full when the connection for the first request
-    // is resolved.
-    const req2 = pool.acquire(address)
-    expect(conns.length).toEqual(1)
-
-    // Let's fulfill the connect promise belonging to the first request.
-    conns[0].resolve(conns[0])
-    await expectAsync(req1).toBeResolved()
-
-    // Release the connection, it should be picked up by the second request.
-    conns[0].release(address, conns[0])
-    await expectAsync(req2).toBeResolved()
-
-    // Just to make sure that there hasn't been any new connection.
-    expect(conns.length).toEqual(1)
-  })
-
   it('should not time out if max pool size is not set', async () => {
     let counter = 0
 
+    const address = ServerAddress.fromUrl('bolt://localhost:7687')
     const pool = new Pool({
       create: (server, release) =>
         Promise.resolve(new Resource(server, counter++, release)),