Provide option to set `maxLifespanTime` for the connections

[subeeshcbabu-zz]

• The released connections should be auto validated against a maxLifespanTime and should be destroyed after this lifespan (if not in use).

[sandfox]

Agreed, there is some pre-existing stuff in projects like apache-pool which have the concept of " abandoned object removal" which should probably be a starting point for this

https://commons.apache.org/proper/commons-pool/apidocs/org/apache/commons/pool2/impl/AbandonedConfig.html

(though we can definitely skip a lot of those config settings)

[poshest]

+1

If node-pool and then node-postgres (which is built on top of it) would implement a connection TTL or maxLifespanTime as @subeeshcbabu calls it, it will solve my problem over here.

It seems to me that this is essential connection pooling functionality.

By the way, I don't think "Abandoned" management is what @subeeshcbabu means. I don't think he's talking about objects that were lost in space, but rather, destroying un-abandoned objects in this._availableObjects.

[sandfox]

@poshest as far as I can tell *un-abandonded" objects that are in this._availableObjects should be covered by idleTimeoutMillis config option?

[poshest]

@sandfox no, that destroys connections that have been idle for the specified time. I'm talking about destroying them at any point that they become idle after the TTL has expired.

[sandfox]

@poshest ahh, I get what you mean now, thanks. Off the top of my head not sure how to do it, but I think it should be reasonably easy if it's implemented in roughly the same way as idle is.

How about something like this:

const Pool = require('generic-pool').Pool;
const myPool = new Pool({
  maxLifespanMillis: 1000 * 60 * 5 //5 mins
  // all the usual other options
})

If a resource is in the pool and the TTL passes the resource is destroyed using the destroy function passed in a pool creation time (as normal) Else if the resource is in use and the TTL passes, once it gets returned to the pool, it is destroyed as above.

Once a resource is destroyed through TTL expiration another will be created to replace it? (i'm assuming yes in the same way refreshIdle option does by default)

refreshIdle - boolean that specifies whether idle resources at or below the min threshold should be destroyed/re-created. optional (default=true)

not to self: this has nothing to do with "abandonded" resources

[poshest]

@sandfox yes, sounds great :)

May I suggest timeoutMillis as the name of the property? I think it helps to "group" that functionality with idleTimeoutMillis - since they're very similar, but the addition of idle makes the difference between them obvious.

I don't understand the description of refreshIdle. What is the "min threshold" in that sentence? The way I think you interpreted it, it should read

boolean that specifies whether idle resources expired after idleTimeoutMillis should be automatically re-created. optional (default=true)

But yes, there needs to be some logic about refreshing, and to be complete, maxLifespanMillis/timeoutMillis should have its own configurable refreshTimedout or something.

Also a decision needs to be made about whether reapIntervalMillis is used as the polling interval for this. I think it makes sense to re-use this variable for that purpose. The documentation could be

reapIntervalMillis : frequency to check for idle resources (default 1000) for the purposes of expiring resources based on both maxLifespanMillis/timeoutMillis and idleTimeoutMillis

[sandfox]

Turns out I misread the logic around refreshIdle slightly when I wrote the above. When refreshIdle is false idleTimeoutMillis is never used and resource do not ever time out while waiting, (it basically disables the idle timeout behaviour). By default it is set to true and resources idleTimeout after 30 seconds.

So we should probably have an equivalent flag for maxLifespanMillis (placeholder name for now to avoid ambiguity) to enable / disable this feature.

re reapIntervalMillis, I'm all for re-using the existing code/stuff here.

[poshest]

Sounds fair to me. :)

[olalonde]

Also need this feature... Is a PR on the way?

[sandfox]

Yes... sorry for the lack of movement on this. It'll most likely turn up in version 3 once I've got the big internal rewrite out of the way to make supporting this easier

[olalonde]

I ended up writing my own pool with TTL support: https://gist.github.com/olalonde/2cd3e1a6ea54454da16878e5cfb03a9f

doubt it can be useful to anyone in this state but sharing it here just in case

[t3hmrman]

So it looks like the custom Evictor stuff runs pretty close to this feature -- maybe it makes sense to just ship some Evictors with features like this with the library and make them easy to use?