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

Open() to connect to multiple db servers #43

Closed
pkieltyka opened this issue Oct 7, 2014 · 13 comments
Closed

Open() to connect to multiple db servers #43

pkieltyka opened this issue Oct 7, 2014 · 13 comments
Assignees
Milestone

Comments

@pkieltyka
Copy link
Member

I really like the design of upper/db, it's super clean and minimal. However, I believe db.Open() should accept an array of settings objects. For example, it's common with mongodb to pass a bunch of hosts which are in a cluster and then the client will determine the master, etc. etc. I presume MySQL / Postgresl would have something similar in a master/slave arrangement. Thoughts?

@pkieltyka
Copy link
Member Author

I ended up just instantiating the Source myself and passing in a mgo.Session / mgo.Database to create a db.Database object.

It works great. Except I had to fork/write a mongo.NewSource() function since the Source fields are unexported.

See:
pkieltyka@5859178

..I looked at the mysql, postgres, ql and sqlite adapters as well to see if upper/db could reasonably support someone instantiating their own connection as long as it adheres to db.Database. It can definitely be done, but the Use() methods in the other adapters always call .Open(), which requires the use of the passed config. I feel it could get hairy, so perhaps a better way is for db.Settings to support a ConnectionUrl of sorts that could be parsed, etc. and allow for more flexibility to open connections with the respective lower-level adapters. For now I'll use my fork, but this is very important IMHO since most large scale applications connect to more then just a single database host.

@xiam
Copy link
Member

xiam commented Oct 8, 2014

Hi Peter!

I agree with you on the importance of this proposal.

I was thinking on modifying the db.Settings struct so it can hold an array of read-only replicas, in order to close #36:

type Settings struct {
  ...
  Replica []Settings
}

All SELECT/find()-like queries executed on the current database session would get distributed (using a round robin approach) among these read-only replicas and write operations would get delivered to the master with no additional effort.

I believe this master -> slave concept can also be mapped to MongoDB, as we see here: http://docs.mongodb.org/manual/core/replica-set-members/, so it definitely can be done.

We could also add a adapter.RegisterSecondary(db.Setting) (db.Secondary) method to the adapters in order to add a replica. Read and write queries would be mapped to master or slave too.

I'm not sure about adding a ConnectionURL as that would be definitely incompatible among different databases.

I think I like the idea of adapter.RegisterSecondary() as it's more explicit.

What do you think? Would any of these options solve your current case?

@xiam xiam self-assigned this Oct 8, 2014
@pkieltyka
Copy link
Member Author

Ah yea issue #36 is the same to this, exactly.

The issue with your proposal of Replica []Settings is that some databases (for sure Mongo is this way) just want to get a list of hosts in a cluster, and not distinguishing one as a master or slave when opening a connection. Once the db client connects, the server tells the client who is a master and who is a slave. Then you can do the read-balancing, but in a cluster environment, if the master goes away, then one of the slaves becomes the master and should start accepting writes. This is how mongo works and mgo supports this with mgo.Session.

I'm not sure how the postgres and mysql drivers work, and if they have this "cluster" configuration ability.

what if you made the Open() func in wrapper.go like this:

func Open(adapter string, settings ...Settings) (Database, error)

then someone could just do:
db.Open("mongo", db.Settings{Host:"localhost"})
or
db.Open("mongo", db.Settings{Host:"host1"}, db.Settings{Host:"host2"})

.. so both are supported actually :) and even backward compatible. You could add a ReadOnly bool if you REALLY had to for mysql and postgres.. but ideally all databases have a cluster mode which can tell clients to switch to master/slave modes.

@pkieltyka
Copy link
Member Author

the other option I see.. is BYOC (Bring Your Own Connection) .. and allow developers to create a Connection interface (aka Source) object to that adapter, and connect themselves like I did with the hack above..

@xiam
Copy link
Member

xiam commented Oct 8, 2014

Sounds great, I really like your suggestion for Open() and it does not require changing the API.

As for MySQL and PostgreSQL, there are lots of solutions for this problem, but the most common pattern that does not require additional middleware is routing the queries to master or slave in each case

I'll work on the Open() modifications for MySQL and PostgreSQL, it would be great if you could help providing a testing environment for debugging the MongoDB replication.

@xiam
Copy link
Member

xiam commented Oct 8, 2014

BTW, if you'd like to test something you could use this vm: https://github.com/upper/db-vagrant

@pkieltyka
Copy link
Member Author

👍

@xiam xiam added this to the future milestone Nov 14, 2014
@xiam
Copy link
Member

xiam commented Nov 18, 2014

We're advancing a bit here, now we have support for a ConnectionURL{} interface.

If you're using the mongo adapter and you'd like to connect to multiple host you can now define a mongo.ConnectionURL value like this:

var settings = mongo.ConnectionURL{
  Address: db.Cluster(db.Host("1.2.3.4"), db.Host("5.6.7.8"), ...),
  Database: "dbname",
}

There is also a mongo.ParseURL() function that takes a string and builds the equivalent mongo.ConnectionURL struct:

var settings = mongo.ParseURL(`mongodb://foo:bar@1.2.3.4,5.6.7.8/dbname`)

The ConnectionURL{} interface is now supported on all adapters, but multiple host support is still pending for MySQL and PostgreSQL.

@pkieltyka
Copy link
Member Author

I see.. I guess db.Cluster() was a new function added for the purposes of configuring these hosts..? what is the form to specify a node being read-only?

@pkieltyka
Copy link
Member Author

I would have to think about it more.. but why not go with func Open(adapter string, settings ...Settings) (Database, error) instead like I suggested above? it will cut down all the code from adding string parsers of the connection url, and more obvious on how to add read-only. I'm not sure what is standard for connection urls to define a slave, perhaps ?slave or ?readonly

@pkieltyka
Copy link
Member Author

I guess the connection string url is a single point, but offers flexibility in the configuration via a url to a particular db.. I guess if there would be so many options this could be better.. but how many kind of options would there be..

@xiam
Copy link
Member

xiam commented Nov 19, 2014

Hi Peter,

The recently introduced db.ConnectionURL interface allows an user to use the ConnectionURL struct provided by the adapter, or to create a custom struct that creates custom DSNs, that solves only the first part of the problem, db.Settings was too inflexible.

Support for func Open(adapter string, settings ...db.ConnectionURL) (Database, error) is still required in order to work with multiple database connections, such as read only slaves in SQL databases.

The db.Cluster() function was introduced as an special helper for creating MongoDB DSNs that needed replicas (mongodb://foo:bar@server1,server2,server3/dbname), but now I think I made I mistake placing it as a function of the db package instead of a function of the mongo package. I'll have to fix that immediately before updating Open().

@xiam
Copy link
Member

xiam commented Jul 25, 2015

I forgot to close this one, but this has been in mongodb and upper for some time now:

mongo.Cluster(db.Host("localhost"), db.Host("1.2.3.4"), db.HostPort("example.org", 1234))

@xiam xiam closed this as completed Jul 25, 2015
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

No branches or pull requests

2 participants