neumino / rethinkdbdash

An advanced Node.js driver for RethinkDB with a connection pool, support for streams etc.
MIT License
848 stars 109 forks source link

Connection doesn't automatically close #368

Open Sven65 opened 6 years ago

Sven65 commented 6 years ago

Using the connection pool in an app that pulls data from a RDB server based on user activity doesn't work as intented.

The library inflates the connection pool to the extremes making the application crash due to a TCP Read error by never closing the connection after the query has been ran.

mxstbr commented 5 years ago

This is biting us in the butt really hard right now in production, has anybody found a workaround?

mxstbr commented 5 years ago

This code is very suspicious:

https://github.com/neumino/rethinkdbdash/blob/7421553ea98a0874a54980505966383e75d2973f/lib/pool.js#L159-L169

Maybe that's the culprit?

benfletcher commented 5 years ago

FWIW, Spectrum.chat forked rethinkdbdash for the apparent sole purpose of uncommenting that code:

https://github.com/withspectrum/spectrum/commit/245aeae61ad8c8814db015b9215e694186fdcd34

The source for the fork is not on Github and only on npm. Doing a diff, the only other difference I saw was the addition of an .npmignore file and resulting absence of node_modules in the package.

The forked package is rethinkhaberdashery.

mxstbr commented 5 years ago

That is correct, that was us! 👍 Feel free to use the package, but it is unlikely we will do any more maintenance on it.

The real fix in our case was to limit the number of connections much lower than the defaults and making the buffer and max connections the same amount to avoid constantly opening and closing connections. We also talked with our DB hosting provider (compose.io) to make sure we were not exceeding the max amount of connections their proxy could handle (2000).

https://github.com/withspectrum/spectrum/blob/alpha/shared/db/db.js#L12-L19