Skip to content
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

Connection Pool #233

Open
scottrobertson opened this issue Dec 13, 2016 · 14 comments
Open

Connection Pool #233

scottrobertson opened this issue Dec 13, 2016 · 14 comments
Labels
feature New functionalities

Comments

@scottrobertson
Copy link

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
Copy link
Collaborator

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
Copy link
Author

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
Copy link
Author

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
Copy link
Collaborator

@scottrobertson cool! 👍

@meenie
Copy link

meenie commented Jun 30, 2017

@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
Copy link
Author

scottrobertson commented Jun 30, 2017

@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
Copy link

meenie commented Jun 30, 2017

Awesome, thanks for the fast response!

@zedtux
Copy link
Collaborator

zedtux commented Feb 25, 2021

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?

@zedtux zedtux added the feature New functionalities label Feb 25, 2021
@scottrobertson
Copy link
Author

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
Copy link
Collaborator

zedtux commented Feb 25, 2021

Thank you for your quick answer 🙏

@zedtux
Copy link
Collaborator

zedtux commented Oct 25, 2021

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
Copy link
Collaborator

zedtux commented Nov 4, 2021

@scottrobertson any input please?

@scottrobertson
Copy link
Author

@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
Copy link
Collaborator

zedtux commented Nov 4, 2021

Thank you for your feedback.

@zedtux zedtux reopened this Nov 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionalities
Projects
None yet
Development

No branches or pull requests

4 participants