Specifying the name of the replica with the distribute_reads method

[fabienpiette]

Issue: #37

[ankane]

Hey @fabienpiette, thanks for the PR, including tests 💯

As I was going reviewing/compiling feedback for this, I think I found a slightly different way that doesn't require a custom strategy (which would allow it to work with any strategy). I don't typically do this, but I put together a branch to try it out. I'd be curious to hear your thoughts. https://github.com/ankane/distribute_reads/compare/named_connections

For this PR, I believe the approach isn't thread-safe, as @slave_pool.current_name will be shared across threads.

All that being said, both approaches add a bit more complexity than I was hoping for, so still on the fence.

[fabienpiette]

Hi!
Indeed the current_name wasn't thread safe, I find your approach much more elegant (especially since we don't have to define a new Makara strategy).
I've tested it locally on my project and it seems to work well.
It does add a bit of complexity but the code is clean and the code overload is only on the DistributeReads::Pool/Makara::Pool class and is quite minimal in my opinion.
Anyway my original problem is solved, well done 👍

[ankane]

Great, glad it's working, and fwiw, your PR helped out a lot - was much easier to make some tweaks than start from scratch.

Going to leave this feature in a branch for now. Let me know if you run into any issues with it. Will feel more comfortable if it's running successfully in production for a while, as well as if others are interested in it.

[fabienpiette]

No problem, thanks to you 👍

[GaudamThiyagarajan]

Great, glad it's working, and fwiw, your PR helped out a lot - was much easier to make some tweaks than start from scratch.

Going to leave this feature in a branch for now. Let me know if you run into any issues with it. Will feel more comfortable if it's running successfully in production for a while, as well as if others are interested in it.

Are we merging this anytime sooner?

[fabienpiette]

Great, glad it's working, and fwiw, your PR helped out a lot - was much easier to make some tweaks than start from scratch.
Going to leave this feature in a branch for now. Let me know if you run into any issues with it. Will feel more comfortable if it's running successfully in production for a while, as well as if others are interested in it.

Are we merging this anytime sooner?

Hey ! I've been able to test these modifications in production, no problems to report for the moment, I think it can be merged soon.