search

byzhang / terrastore

Automatically exported from code.google.com/p/terrastore
Other
0 stars 0 forks source link

Support for multiple server URLs/endpoints in Java API #68

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
Implement a way to provide the TerrastoreClient with multiple server URLs + 
fail-over/blacklisting when a server stops responding.

Original issue reported on code.google.com by johansso...@gmail.com on 21 Jun 2010 at 9:25

GoogleCodeExporter commented 9 years ago

Original comment by johansso...@gmail.com on 21 Jun 2010 at 9:33

GoogleCodeExporter commented 9 years ago

Original comment by sergio.b...@gmail.com on 24 Jul 2010 at 10:44

GoogleCodeExporter commented 9 years ago 
First implementation based on the "HostManager" abstraction, providing the host 
to connect to depending on implementation strategies.
Currently provided "single server" and "ordered server list" implementations.

See commit: 
http://code.google.com/p/terrastore/source/detail?r=f71d9d2c5c68ad88d1db84bd91b1
7564fb0b3d69&repo=javaclient

Sven, do you mind to review for feedback?

Original comment by sergio.b...@gmail.com on 26 Aug 2010 at 11:58

GoogleCodeExporter commented 9 years ago 
I'll get back with comments shortly.

Original comment by johansso...@gmail.com on 26 Aug 2010 at 12:17

GoogleCodeExporter commented 9 years ago 
Looks solid to me. Well done!

Just a couple of comments on details in OrderedHostManager,
see draft comment on that class.

Original comment by johansso...@gmail.com on 26 Aug 2010 at 12:54

GoogleCodeExporter commented 9 years ago 
Thanks Sven.

I just can't find any comments on OrderedHostManager ... am I missing something?

Original comment by sergio.b...@gmail.com on 26 Aug 2010 at 1:49

GoogleCodeExporter commented 9 years ago 
Weird. I've never used that comment function before. I see it if I open you 
changeset on the site, and open the file from there.

Copy-paste, starting at line 43:

"Why the iteration?
'boolean found = hosts.contains(suspected)' ought to be enough. 

At first I didn't see the logic behind removing and then immediately 
adding the suspsected server URL. For clarity I would consider to
factor out those two lines to a method named something akin to

private moveToEndOfList(String suspected) {
    hosts.remove(suspected);
    hosts.add(suspected);
}"

Original comment by johansso...@gmail.com on 26 Aug 2010 at 1:54

GoogleCodeExporter commented 9 years ago 
Ooops, you're right, that stupid iteration comes from a previous implementation 
where there was an actual body, but now contains() is good enough: thanks for 
spotting that stupid piece of code.

Fixed and pushed ;)

Original comment by sergio.b...@gmail.com on 26 Aug 2010 at 2:13

GoogleCodeExporter commented 9 years ago

Original comment by sergio.b...@gmail.com on 11 Sep 2010 at 8:36