New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

idleTimeoutMillis is not working with generic-pool 3.x #178

Open
IshanCSE opened this Issue Feb 2, 2017 · 17 comments

Comments

Projects
None yet
5 participants
@IshanCSE

IshanCSE commented Feb 2, 2017

Hi Team,
I have upgraded generic-pool library from 2.x to 3.x.
I have created a pool using

genericPool.createPool({ 
create: function(){
          return new promise(function(resolve,reject){
           // create client 
          resolve(client);
});
},
destroy: function(client){
         return new Promise(function(resolve){
           client.on('end', function(){
            resolve()
          })
          client.end()
});
},{configParams});

idleTimeoutMillis timeout is set to 30ms. genericpool is not creating new resourse after 30ms.

@sandfox sandfox added the bug label Feb 2, 2017

@sandfox

This comment has been minimized.

Show comment
Hide comment
@sandfox

sandfox Feb 2, 2017

Collaborator

Hi there,
I think this is problem of the docs not being very clear! Unless evictionRunIntervalMillis is set to number greater than 0, neither softIdleTimeoutMillis nor idleTimeoutMillis have any effect.

evictionRunIntervalMillis sets how how often idle resources are checked and by default it's zero, i.e. resources are never checked.

I should do something to make this more clear!

Collaborator

sandfox commented Feb 2, 2017

Hi there,
I think this is problem of the docs not being very clear! Unless evictionRunIntervalMillis is set to number greater than 0, neither softIdleTimeoutMillis nor idleTimeoutMillis have any effect.

evictionRunIntervalMillis sets how how often idle resources are checked and by default it's zero, i.e. resources are never checked.

I should do something to make this more clear!

@IshanCSE

This comment has been minimized.

Show comment
Hide comment
@IshanCSE

IshanCSE Feb 2, 2017

Thanks for your quick reply

IshanCSE commented Feb 2, 2017

Thanks for your quick reply

@IshanCSE

This comment has been minimized.

Show comment
Hide comment
@IshanCSE

IshanCSE Feb 2, 2017

Hi,
idleTimeout is working now. But there is a problem with the pool.drain().
case 1.
Lets say I am not releasing resource to pool and keep calling pool.acquire(). I have set acquireTimeoutMillis to 30s , thus After 30s pool.acquire will throw timeout error. After receiving timeout error, I am calling pool.drain()

pool.drain().then(function() {
console.log('pool drained :::::::::::');
return pool.clear();
});
But, callback specified in then never called , "pool drained :::::::::::" has not printed. Am I missing something?

IshanCSE commented Feb 2, 2017

Hi,
idleTimeout is working now. But there is a problem with the pool.drain().
case 1.
Lets say I am not releasing resource to pool and keep calling pool.acquire(). I have set acquireTimeoutMillis to 30s , thus After 30s pool.acquire will throw timeout error. After receiving timeout error, I am calling pool.drain()

pool.drain().then(function() {
console.log('pool drained :::::::::::');
return pool.clear();
});
But, callback specified in then never called , "pool drained :::::::::::" has not printed. Am I missing something?

@sandfox

This comment has been minimized.

Show comment
Hide comment
@sandfox

sandfox Feb 2, 2017

Collaborator

If you borrow a resource from the pool, but then never return it, the pool will not drain,
or to it put another way, pool.drain will wait for all outstanding resources to be returned before resolving.

Currently there isn't concept of "abandoned resources" (I.e those which have been borrowed, then lost/forgotten about and not returned) so the pool can't just ignore them. It's vaguely on the roadmap to add but I've not given it any serious planning yet.

Collaborator

sandfox commented Feb 2, 2017

If you borrow a resource from the pool, but then never return it, the pool will not drain,
or to it put another way, pool.drain will wait for all outstanding resources to be returned before resolving.

Currently there isn't concept of "abandoned resources" (I.e those which have been borrowed, then lost/forgotten about and not returned) so the pool can't just ignore them. It's vaguely on the roadmap to add but I've not given it any serious planning yet.

@IshanCSE

This comment has been minimized.

Show comment
Hide comment
@IshanCSE

IshanCSE Feb 3, 2017

Problem with the node pool is when pool.acquire throws Timeout error, pool still contains resources and thus when I throw err node process doesnot exit. This scenario occurs only when pool is exhausted.

IshanCSE commented Feb 3, 2017

Problem with the node pool is when pool.acquire throws Timeout error, pool still contains resources and thus when I throw err node process doesnot exit. This scenario occurs only when pool is exhausted.

@sandfox

This comment has been minimized.

Show comment
Hide comment
@sandfox

sandfox Feb 5, 2017

Collaborator

I think the behaviour you are talking about it is by design so you can decide you how you want your application to behave.
The pool throwing timeoutError may not be fatal for you code, you might just log out the error and keep the pool/app running, or you might want to close the app.
If you want the pool to close after a timeoutError you will need to manually call drain etc.

Collaborator

sandfox commented Feb 5, 2017

I think the behaviour you are talking about it is by design so you can decide you how you want your application to behave.
The pool throwing timeoutError may not be fatal for you code, you might just log out the error and keep the pool/app running, or you might want to close the app.
If you want the pool to close after a timeoutError you will need to manually call drain etc.

@IshanCSE

This comment has been minimized.

Show comment
Hide comment
@IshanCSE

IshanCSE Feb 5, 2017

My issue is after getting timeoutError , Pool doesnot allow node process to terminate (in case All pool resources are acquired and never released).

IshanCSE commented Feb 5, 2017

My issue is after getting timeoutError , Pool doesnot allow node process to terminate (in case All pool resources are acquired and never released).

@sandfox

This comment has been minimized.

Show comment
Hide comment
@sandfox

sandfox Feb 6, 2017

Collaborator

If you want the process to exit without first returning all the borrowed resources to the pool, your only option is to just crash the process. There is no way to "force close" the pool, and even if there was it would probably just end up with the same result

Collaborator

sandfox commented Feb 6, 2017

If you want the process to exit without first returning all the borrowed resources to the pool, your only option is to just crash the process. There is no way to "force close" the pool, and even if there was it would probably just end up with the same result

@George0406

This comment has been minimized.

Show comment
Hide comment
@George0406

George0406 Sep 4, 2017

Hi, I'm confused about idleTimeoutMillis and evictionRunIntervalMillis, I had read the README.md but I still can't distinguish it. Can anyone explain the difference...?

George0406 commented Sep 4, 2017

Hi, I'm confused about idleTimeoutMillis and evictionRunIntervalMillis, I had read the README.md but I still can't distinguish it. Can anyone explain the difference...?

@sandfox

This comment has been minimized.

Show comment
Hide comment
@sandfox

sandfox Sep 5, 2017

Collaborator

@George0406 sure:

idleTimeoutMillis specifies the threshold for deciding if an resource has been idle for too long. However, crossing this threshold does not directly cause the resource to be evicted/destroyed. The actual eviction/destruction process is handled separately by the evictor (lol), which runs periodically every evictionRunIntervalMillis.

Or to put it another way, the evictor runs every evictionRunIntervalMillis, and checks numTestsPerRun resources inside the pool that are waiting to be borrowed, and if a resource has been idle for longer than idleTimeoutMillis, destroys it.

Collaborator

sandfox commented Sep 5, 2017

@George0406 sure:

idleTimeoutMillis specifies the threshold for deciding if an resource has been idle for too long. However, crossing this threshold does not directly cause the resource to be evicted/destroyed. The actual eviction/destruction process is handled separately by the evictor (lol), which runs periodically every evictionRunIntervalMillis.

Or to put it another way, the evictor runs every evictionRunIntervalMillis, and checks numTestsPerRun resources inside the pool that are waiting to be borrowed, and if a resource has been idle for longer than idleTimeoutMillis, destroys it.

@George0406

This comment has been minimized.

Show comment
Hide comment
@George0406

George0406 Sep 5, 2017

@sandfox Thanks for the quick reply

Sorry, I had typed wrong paramters in the previous question, I would like to ask what is the difference between idleTimeoutMillis and softIdleTimeoutMillis, and thanks for your reply again.

George0406 commented Sep 5, 2017

@sandfox Thanks for the quick reply

Sorry, I had typed wrong paramters in the previous question, I would like to ask what is the difference between idleTimeoutMillis and softIdleTimeoutMillis, and thanks for your reply again.

@sandfox

This comment has been minimized.

Show comment
Hide comment
@sandfox

sandfox Sep 5, 2017

Collaborator

@George0406 no problem.

softIdleTimeoutMillis is pretty much like idleTimeoutMillis except it only applies to "excess" resources in the pool. If the pool has 10 resources in it which have been idle too long and a min size of 4, then only 6 of the resources will eligible to be destroyed.

Does that help?

Collaborator

sandfox commented Sep 5, 2017

@George0406 no problem.

softIdleTimeoutMillis is pretty much like idleTimeoutMillis except it only applies to "excess" resources in the pool. If the pool has 10 resources in it which have been idle too long and a min size of 4, then only 6 of the resources will eligible to be destroyed.

Does that help?

@George0406

This comment has been minimized.

Show comment
Hide comment
@George0406

George0406 Sep 6, 2017

@sandfox , It's clear to me now, and thanks for your clear explanation.

George0406 commented Sep 6, 2017

@sandfox , It's clear to me now, and thanks for your clear explanation.

@J0NNYZER0

This comment has been minimized.

Show comment
Hide comment
@J0NNYZER0

J0NNYZER0 Apr 3, 2018

@sandfox:
If my my pool configuration is set like so, would not explicitly setting the evict property cause ETIMEDOUT errors?

Here is my configuration:

pool: {
max: 10,
min: 1,
idle: 10000
}

etimedout

J0NNYZER0 commented Apr 3, 2018

@sandfox:
If my my pool configuration is set like so, would not explicitly setting the evict property cause ETIMEDOUT errors?

Here is my configuration:

pool: {
max: 10,
min: 1,
idle: 10000
}

etimedout

@sandfox sandfox self-assigned this Apr 4, 2018

@zxc7922927

This comment has been minimized.

Show comment
Hide comment
@zxc7922927

zxc7922927 Jun 26, 2018

@sandfox
softIdleTimeoutMillis is pretty much like idleTimeoutMillis except it only applies to "excess" resources in the pool. If the pool has 10 resources in it which have been idle too long and a min size of 4, then only 6 of the resources will eligible to be destroyed.

=====================================
After I try both of then, it is seen like idleTimeoutMillis more like above description?
When I setting idleTimeoutMillis 10000, after 10 seconds those connection(max - min) will be kill,
but softIdleTimeoutMillis not. Can you check this? thanks!!

zxc7922927 commented Jun 26, 2018

@sandfox
softIdleTimeoutMillis is pretty much like idleTimeoutMillis except it only applies to "excess" resources in the pool. If the pool has 10 resources in it which have been idle too long and a min size of 4, then only 6 of the resources will eligible to be destroyed.

=====================================
After I try both of then, it is seen like idleTimeoutMillis more like above description?
When I setting idleTimeoutMillis 10000, after 10 seconds those connection(max - min) will be kill,
but softIdleTimeoutMillis not. Can you check this? thanks!!

@sandfox

This comment has been minimized.

Show comment
Hide comment
@sandfox

sandfox Jun 28, 2018

Collaborator

@zxc7922927 could supply some example code? Something to bear in mind is that the as soonas you set evictionRunIntervalMillis to any non-zero value, the default setting for idleTimeoutMillis (30 seconds) will take effect along with any value you supply for softIdleTimeoutMillis

Collaborator

sandfox commented Jun 28, 2018

@zxc7922927 could supply some example code? Something to bear in mind is that the as soonas you set evictionRunIntervalMillis to any non-zero value, the default setting for idleTimeoutMillis (30 seconds) will take effect along with any value you supply for softIdleTimeoutMillis

@zxc7922927

This comment has been minimized.

Show comment
Hide comment
@zxc7922927

zxc7922927 Jun 28, 2018

@sandfox
here is my setting:
{
refreshIdle: false,
connection_min: 1,
connection_max: 50,
acquire_timeout: 5000,
idleTimeoutMillis: 10000
}
After using apachebench stress test, when I check process list, there has some sleep connections.
After 10 seconds, they has been destroyed but keep one connection in there.
But when I replace idleTimeoutMillis with softIdleTimeoutMillis, those connections just keep in there, won't be destroyed.

zxc7922927 commented Jun 28, 2018

@sandfox
here is my setting:
{
refreshIdle: false,
connection_min: 1,
connection_max: 50,
acquire_timeout: 5000,
idleTimeoutMillis: 10000
}
After using apachebench stress test, when I check process list, there has some sleep connections.
After 10 seconds, they has been destroyed but keep one connection in there.
But when I replace idleTimeoutMillis with softIdleTimeoutMillis, those connections just keep in there, won't be destroyed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment