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 an option to distribute reads among all nodes #3

Merged
merged 2 commits into from
May 27, 2020

Conversation

o1egl
Copy link
Contributor

@o1egl o1egl commented May 26, 2020

In my current setup I have 1 master and 1 slave with 90% of read operations and 10% of write operations. This means that the resources of the host are not being used properly. The idea is to have and option to distribute read requests between the master and slave.

@linxGnu
Copy link
Owner

linxGnu commented May 26, 2020

Hello @o1egl
Thank you for your consideration. May I ask you for the purpose/use case clearly?

As your PR shows up, additional option targets the case that we could query to one of masters or one of slaves. The balancer only picks one that succeed. Is it your purpose?

@coveralls
Copy link

coveralls commented May 26, 2020

Coverage Status

Coverage increased (+0.1%) to 91.56% when pulling 64846d7 on o1egl:master into 5bac59b on linxGnu:master.

@o1egl
Copy link
Contributor Author

o1egl commented May 26, 2020

@linxGnu I added a description

Copy link
Owner

@linxGnu linxGnu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have small comments for your PR. Most of comments are naming only (naming is hard)
Thank you @o1egl

// ConnectMasterSlaves to master-slave databases, healthchecks will ensure they are working
// driverName: mysql, postgres, etc.
// masterDSNs: data source names of Masters.
// slaveDSNs: data source names of Slaves.
// args: args[0] = true to indicates galera/wsrep cluster.
func ConnectMasterSlaves(driverName string, masterDSNs []string, slaveDSNs []string, args ...interface{}) (*DBs, []error) {
func ConnectMasterSlaves(driverName string, masterDSNs []string, slaveDSNs []string, options ...Option) (*DBs, []error) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love currying options like these. Thank you!

mssqlx.go Outdated Show resolved Hide resolved
mssqlx.go Outdated Show resolved Hide resolved
mssqlx.go Outdated Show resolved Hide resolved
mssqlx.go Outdated Show resolved Hide resolved
mssqlx_test.go Outdated Show resolved Hide resolved
@o1egl o1egl force-pushed the master branch 2 times, most recently from 21252c8 to 7db7567 Compare May 27, 2020 11:59
@o1egl
Copy link
Contributor Author

o1egl commented May 27, 2020

@linxGnu thank you for your review. I addressed your comments.

@linxGnu
Copy link
Owner

linxGnu commented May 27, 2020

@o1egl Thank you very much for your contribution. It's great to see the PR.
👍

@linxGnu linxGnu merged commit 73cc499 into linxGnu:master May 27, 2020
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