mattbaird / elastigo

A Go (golang) based Elasticsearch client library.
Apache License 2.0
943 stars 240 forks source link

Handle multiple ES Hosts #22

Open araddon opened 11 years ago

araddon commented 11 years ago

Currently this Public Static var is used to configure ES which has a number of consequences: 1) can't do multiple hosts (in same cluster), 2) can't do multiple es "clusters".
https://github.com/linohh/elastigo/blob/master/api/request.go#L35

What about extracting it out and allowing a Connection Manager? Something that could someday wrap the Cluster Status/Health info? Potentially keep backwards compatibility in the Api but supporting a single global ConnectionManager, or allow people to create and manage their own (to address the 2nd above).

kenkeiter commented 10 years ago

:+1: Yeah, this would be useful...

jbenet commented 10 years ago

:+1: Yes. The interface is otherwise broken.

mattbaird commented 10 years ago

I have the code to allow for this in my gochimp project, I just need to implement. I'll try to get to it soon, sorry real job is super busy these days!

kenkeiter commented 10 years ago

Totally fair! Is the gochimp project open source? Would it be possible for me to extract the code? I'm actually blocking on this, and I've had to wrap parts of elastic search myself -- it's miserable. Thanks a ton!

kenkeiter commented 10 years ago

@araddon Have you solved this for yourself?

mattbaird commented 10 years ago

Ken - check out the Hailocab fork, they haven't submitted a pull request, but they've implemented the feature you want: https://github.com/hailocab/elastigo/commit/48572e16e02feb4493cb1729d4bd486c7f1b4ff9

Can you test and submit a PR on master? I'd be happy to merge.

cheers!

jbenet commented 10 years ago

@mattbaird this doesn't actually implement the same thing. That's host-pooling (trying multiple hosts). What we --or at least I-- mean is that the API should expose an object (representing a Host, pooled or not), and calls happen on it. This would allow multiple different hosts to work in the same program, e.g.:

a := elastigo.NewClient("localhost", 9300)
b := elastigo.NewClient("example.com", 9300)

// search _different_ instances
a.Search(...)
b.Search(...)

// transfer objects
o, _ := a.Get(...)
b.Put(...o...)

You could be backwards compatible (by initializing a static client, like net/http does), but i think it critical to allow the creation of entirely distinct instances.

kisielk commented 10 years ago

:+1: to this. I am working on a project right now where I need to talk to multiple ES servers, but I can't use this library in its current form.

mattbaird commented 10 years ago

ok, this seems to be pretty straight forward. I might get to it shortly (sorry, new startup, time is precious). Willing to review/accept good PRs!

balboah commented 10 years ago

:+1:

elentar commented 10 years ago

I'd like to see this too, parallel queries to multiple ES clusters would be great. :+1:

mattbaird commented 10 years ago

Unfortunately I believe I cannot use the net/http method because of separate packages. This might require a major refactoring. Any suggestions?

mattbaird commented 10 years ago

I am sad. The packaging structure is turning out to be a very big PITA.

I might flatten the whole thing, but that's going to be somewhat disruptive for people using this. Anyone care to comment?

shutej commented 10 years ago

This is what major version numbers are for. This was the very first thing I noticed about this package, no manual connection.

Also, it would be nice if the more canonical "Dial" style of connection were supported via a URL.

mattbaird commented 10 years ago

I agree 100% with the need to fix this. I spent some time trying to retrofit it and keep things compatible but I don't think it's possible.

Fixing it in a non backwards compatible way should be relatively easy. I'm getting started on that shortly.

shutej commented 10 years ago

I have it halfway done. The big issue seems to be the name collisions from putting it all in one module.

mattbaird commented 10 years ago

that's great! thank you. If you want to put it on a branch, we can collaborate.

shutej commented 10 years ago

I'm find/replacing like crazy, gimme a few more minutes.

shutej commented 10 years ago

OK, my fork seems to compile.