generic-pool - npm Package Compare versions

Comparing version 3.1.2 to 3.1.3

CHANGELOG.md

 # Change Log
 
+## [3.1.3] - November 26 2016
+- internal refactoring and comment improvements
+- fix #159 so draining and closing don't leave resources behind
+- stop the evictor from keeping the event loop open after draining
 
@@ -138,2 +142,3 @@ ## [3.1.2] - November 22 2016
 [unreleased]: https://github.com/coopernurse/node-pool/compare/v3.1.2...HEAD
+[3.1.3]: https://github.com/coopernurse/node-pool/compare/v3.1.2...v3.1.3
 [3.1.2]: https://github.com/coopernurse/node-pool/compare/v3.1.1...v3.1.2
@@ -140,0 +145,0 @@ [3.1.1]: https://github.com/coopernurse/node-pool/compare/v3.1.0...v3.1.1

lib/Pool.js

@@ -11,2 +11,4 @@ 'use strict'
 
+const reflector = require('./utils').reflector
+
 /**
@@ -130,11 +132,7 @@ * TODO: move me
     const wrappedDestroyPromise = this._Promise.resolve(destroyPromise)
-    this._factoryDestroyOperations.add(wrappedDestroyPromise)
 
-    wrappedDestroyPromise
+    this._trackOperation(wrappedDestroyPromise, this._factoryDestroyOperations)
     .catch((reason) => {
       this.emit(FACTORY_DESTROY_ERROR, reason)
     })
-    .then(() => {
-      this._factoryDestroyOperations.delete(wrappedDestroyPromise)
-    })
 
@@ -160,6 +158,5 @@ // TODO: maybe ensuring minimum pool size should live outside here
     const wrappedValidationPromise = this._Promise.resolve(validationPromise)
-    this._validationOperations.add(wrappedValidationPromise)
 
-    wrappedValidationPromise.then((isValid) => {
-      this._validationOperations.delete(wrappedValidationPromise)
+    this._trackOperation(wrappedValidationPromise, this._validationOperations)
+    .then((isValid) => {
       this._testOnBorrowResources.delete(pooledResource)
@@ -212,2 +209,9 @@
 
+    const resourceShortfall = numWaitingClients - this._potentiallyAllocableResourceCount
+
+    const actualNumberOfResourcesToCreate = Math.min(this.spareResourceCapacity, resourceShortfall)
+    for (let i = 0; actualNumberOfResourcesToCreate > i; i++) {
+      this._createResource()
+    }
+
     // If we are doing test-on-borrow see how many more resources need to be moved into test
@@ -224,16 +228,2 @@ // to help satisfy waitingClients
 
-    // Do we still have outstanding requests that can't be fulfilled by resources in-test and available
-    const potentiallyAllocableResources = this._availableObjects.length +
-      this._testOnBorrowResources.size +
-      this._testOnReturnResources.size +
-      this._factoryCreateOperations.size
-
-    const resourceShortfall = numWaitingClients - potentiallyAllocableResources
-
-    const spareResourceCapacity = this._config.max - (this._allObjects.size + this._factoryCreateOperations.size)
-    const actualNumberOfResourcesToCreate = Math.min(spareResourceCapacity, resourceShortfall)
-    for (let i = 0; actualNumberOfResourcesToCreate > i; i++) {
-      this._createResource()
-    }
-
     // if we aren't testing-on-borrow then lets try to allocate what we can
@@ -271,2 +261,21 @@ if (this._config.testOnBorrow === false) {
   /**
+   * tracks on operation using given set
+   * handles adding/removing from the set and resolve/rejects the value/reason
+   * @param  {Promise} operation
+   * @param  {Set} set       Set holding operations
+   * @return {Promise}       Promise that resolves once operation has been removed from set
+   */
+  _trackOperation (operation, set) {
+    set.add(operation)
+
+    return operation.then((v) => {
+      set.delete(operation)
+      return this._Promise.resolve(v)
+    }, (e) => {
+      set.delete(operation)
+      return this._Promise.reject(e)
+    })
+  }
+
+  /**
    * @private
@@ -278,11 +287,8 @@ */
     const wrappedFactoryPromise = this._Promise.resolve(factoryPromise)
-    this._factoryCreateOperations.add(wrappedFactoryPromise)
 
-    wrappedFactoryPromise
+    this._trackOperation(wrappedFactoryPromise, this._factoryCreateOperations)
     .then((resource) => {
-      this._factoryCreateOperations.delete(wrappedFactoryPromise)
       this._handleNewResource(resource)
     })
     .catch((reason) => {
-      this._factoryCreateOperations.delete(wrappedFactoryPromise)
       this.emit(FACTORY_CREATE_ERROR, reason)
@@ -307,7 +313,5 @@ this._dispense()
     }
-    if (this._count < this._config.min) {
-      const diff = this._config.min - this._count
-      for (let i = 0; i < diff; i++) {
-        this._createResource()
-      }
+    const minShortfall = this._config.min - this._count
+    for (let i = 0; i < minShortfall; i++) {
+      this._createResource()
     }
@@ -476,7 +480,9 @@ }
    * Disallow any new acquire calls and let the request backlog dissapate.
-   * Resolves once all resources are returned to pool and available...
+   * The Pool will no longer attempt to maintain a "min" number of resources
+   * and will only make new resources on demand.
+   * Resolves once all resource requests are fulfilled and all resources are returned to pool and available...
+   * Should probably be called "drain work"
    * @returns {Promise}
    */
   drain () {
-    // disable the ability to put more work on the queue.
     this._draining = true
@@ -487,2 +493,5 @@ return this.__allResourceRequestsSettled()
       })
+      .then(() => {
+        this._descheduleEvictorRun()
+      })
   }
@@ -494,3 +503,3 @@
       // FIXME: what if they can "resolve" out of order....?
-      return this._waitingClientsQueue.tail.promise.then(() => {}, () => {})
+      return reflector(this._waitingClientsQueue.tail.promise)
     }
@@ -502,6 +511,5 @@ return this._Promise.resolve()
   __allResourcesReturned () {
-    const ps = []
-    this._resourceLoans.forEach(function (loan) {
-      ps.push(loan.promise)
-    })
+    const ps = Array.from(this._resourceLoans.values)
+    .map((loan) => loan.promise)
+    .map(reflector)
     return this._Promise.all(ps)
@@ -515,3 +523,3 @@ }
    *
-   * Note that if factory.min > 0, the pool will destroy all idle resources
+   * Note that if factory.min > 0 and the pool isn't "draining", the pool will destroy all idle resources
    * in the pool, but replace them with newly created resources up to the
@@ -523,11 +531,32 @@ * specified factory.min value.  If this is not desired, set factory.min
   clear () {
-    this._descheduleEvictorRun()
-    for (const resource of this._availableObjects) {
-      this._destroy(resource)
-    }
-    // TODO: we should handle factory.destroy rejecting...
-    return this._Promise.all(this._factoryDestroyOperations)
+    const reflectedCreatePromises = Array.from(this._factoryCreateOperations)
+    .map(reflector)
+
+    // wait for outstanding factory.create to complete
+    return this._Promise.all(reflectedCreatePromises)
+    .then(() => {
+      // Destroy existing resources
+      for (const resource of this._availableObjects) {
+        this._destroy(resource)
+      }
+      const reflectedDestroyPromises = Array.from(this._factoryDestroyOperations)
+      .map(reflector)
+      return this._Promise.all(reflectedDestroyPromises)
+    })
   }
 
   /**
+   * How many resources are available to allocated
+   * (includes resources that have not been tested and may faul validation)
+   * NOTE: internal for now as the name is awful and might not be useful to anyone
+   * @return {Number} number of resources the pool has to allocate
+   */
+  get _potentiallyAllocableResourceCount () {
+    return this._availableObjects.length +
+      this._testOnBorrowResources.size +
+      this._testOnReturnResources.size +
+      this._factoryCreateOperations.size
+  }
+
+  /**
    * The combined count of the currently created objects and those in the
@@ -544,2 +573,10 @@ * process of being created
   /**
+   * How many more resources does the pool have room for
+   * @return {Number} number of resources the pool could create before hitting any limits
+   */
+  get spareResourceCapacity () {
+    return this._config.max - (this._allObjects.size + this._factoryCreateOperations.size)
+  }
+
+  /**
    * see _count above
@@ -546,0 +583,0 @@ * @return {Number} [description]

package.json

 {
   "name": "generic-pool",
   "description": "Generic resource pooling for Node.JS",
-  "version": "3.1.2",
+  "version": "3.1.3",
   "author": "James Cooper <[e-mail]>",
@@ -6,0 +6,0 @@ "contributors": [

README.md

@@ -43,3 +43,3 @@ [![build status](https://secure.travis-ci.org/coopernurse/node-pool.png)](http://travis-ci.org/coopernurse/node-pool)
     create: function(){
-		 return new Promise(function(resolve, reject{
+		 return new Promise(function(resolve, reject){
 	        var client = DbDriver.createClient()
@@ -50,3 +50,3 @@ client.on('connected', function(){
 	    })
-    }
+    },
     destroy: function(client){
@@ -53,0 +53,0 @@ return new Promise(function(resolve){

New alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Fixed alerts

License Policy Violation

License

This package is not allowed per your license policy. Review the package's license to ensure compliance.

Found 1 instance in 1 package

Improved metrics

Total package byte prevSize

85258

increased by2.11%

Number of package files

32

increased by3.23%

Lines of code

2021

increased by2.17%

No dependency changes