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

Add Clustered Database Connections [WIP] #1673

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

hodgesds
Copy link

@hodgesds hodgesds commented May 2, 2018

This PR is a work in progress and before going too far I'd like to get some feedback. The feature that I'm looking to implement is the ability to use a clustered database connection(s). The idea is that it would be configurable so that reads work on replicas and writes get directed to the master. If this is a feature that sounds reasonable to implement I'd be more than willing to continue on this, but I'd like the blessing of the maintainers first. Thanks!

@hodgesds hodgesds changed the title Add Cluster Database Connections [WIP] Add Clustered Database Connections [WIP] May 2, 2018
@weiznich
Copy link
Member

weiznich commented May 9, 2018

In my opinion this is something that should be handled at connection pooling level.
So implementing this would require to rewrite the current pooling architecture. There are other issues in the current pooling system so this is something on our backlog. I'm not sure when the rewrite takes place. (Maybe we should first collect some ideas what the new system should be able to handle.)

@hodgesds
Copy link
Author

hodgesds commented May 9, 2018

I'm going to open an issue for now to track this as that seems to be the right place for continuing what needs to be discussed.

@sgrif
Copy link
Member

sgrif commented May 9, 2018

Worth noting that this can't be handled entirely at the connection pooling level, since any implementation of this would have to ensure all reads get directed to the primary when a transaction is open. That said, I do generally agree that this should get handled outside of Diesel.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants