search

sequelize / sequelize-pool

Resource pool implementation. It can be used to throttle expensive resources.
Other
39 stars 17 forks source link

make destroy() call its' side-effects once #5

Closed itai-sagi closed 5 years ago

itai-sagi commented 5 years ago

multiple calls to destroy will only result in its' side effects being called once. this is a fix for https://github.com/sequelize/sequelize/issues/10902

sushantdhiman commented 5 years ago

So calling destroy two or more times on same resource cause pool to cross pool.max limit?

itai-sagi commented 5 years ago

not exactly. because 2 errors are thrown when a connection is killed, the count is decreased by 2 for every connection that is killed, reducing the count to zero and thus allowing more connections to be opened.

sushantdhiman commented 5 years ago

Then we should decrement count only when filter removed something from _availableObjects and _inUseObjects. What do you think, I think that should fix this issue

itai-sagi commented 5 years ago

Sounds good.

sushantdhiman commented 5 years ago

Can you update this PR and a test after this one. Just copy linked test and assert count is not decreased by more than one when destroy is called multiple times on same resource

sushantdhiman commented 5 years ago

Should be fixed by https://github.com/sequelize/sequelize-pool/pull/6, let me know if you got any suggestions :)