NoBrainerORM / nobrainer

Ruby ORM for RethinkDB
http://nobrainer.io/
Other
388 stars 49 forks source link

Connection Pool #233

Open scottrobertson opened 7 years ago

scottrobertson commented 7 years ago

Hey @nviennot

We are at the point that we could really do with a connection pool (we are hitting timeout issues at the moment, and are trying to reduce the load on the DB), so it's something I could work on implementing into NoBrainer. But first of all, I was just wondering, have you ever started work on implementing it into NoBrainer? If so, where did you land on it?

We have one in place using https://github.com/mperham/connection_pool for our none NoBrainer queries and it's been working super well for us in production.

nviennot commented 7 years ago

hey Scott,

I haven't implemented any connection pool feature.

The gem you are referring to has an assumption: "There is no provision for repairing or checking the health of a connection; connections should be self-repairing. This is true of the Dalli and Redis clients." This is not so true with NoBrainer. Upon failure, NoBrainer issues a disconnect (https://github.com/nviennot/nobrainer/blob/master/lib/no_brainer/query_runner/reconnect.rb#L5) and a new connection is instantiated on the next try (https://github.com/nviennot/nobrainer/blob/master/lib/no_brainer/connection_manager.rb#L60) A reconnection tries to connect to the next RethinkDB URL (in case many RethinkDB servers are specified).

My advice is to implement our own pool instead of relying on an external gem, but feel free to implement it however you see fit. I'm available for questions.

scottrobertson commented 7 years ago

That is basically exactly how we have implemented connection_pool. If the driver raises one of the errors in is_connection_error_exception then we shutdown the pool, and a new one is connected on the next try. So not a lot of the logic should have to change. I will have a play around and let you know what i come up with.

scottrobertson commented 7 years ago

Just an update on this. We put our basic implementation into production about 2 weeks ago, and it's been working very well for us. We have had very few connection issues since then. I will try and put together a PR when i get time.

nviennot commented 7 years ago

@scottrobertson cool! :+1:

meenie commented 7 years ago

@scottrobertson Would you be willing to post the code you are using? I'm interested in doing the same thing, and a head start would be great :).

scottrobertson commented 7 years ago

@meenie hey.

This is basically what we are doing:

https://github.com/Baremetrics/nobrainer/blob/8c30f57a6252cbffc75730fb93a3f993d61f89ba/lib/no_brainer/query_runner/driver.rb#L4-L9

And the RDB::Pool is essentially this https://gist.github.com/scottrobertson/390792a5ed76a64d07ff7ab9ab028249

There will be a cleaner way to do this, but it works well for us even though it's hacky :)

meenie commented 7 years ago

Awesome, thanks for the fast response!

zedtux commented 3 years ago

Sorry for coming so late on this one, but I'd like to know if you have any code to implement this feature in this gem?

scottrobertson commented 3 years ago

Sorry for coming so late on this one, but I'd like to know if you have any code to implement this feature in this gem?

We stopped using Rethink a long time ago, so I don’t have anything sorry.

zedtux commented 3 years ago

Thank you for your quick answer :pray:

zedtux commented 2 years ago

Well, the connection pool is still a thing this gem should have, I would have kept this issue open. Could you please tell why you've closed this issue ?

zedtux commented 2 years ago

@scottrobertson any input please?

scottrobertson commented 2 years ago

@zedtux feel free to reopen it if you want. I've not used this in a long time and I just closed all my stale/inactive issues in bulk.

zedtux commented 2 years ago

Thank you for your feedback.