searchbox-io/Jest — pull request #38 by tootedom (2013-05-06 18:51:25+00:00)
Title: When Discovery is enabled, the list of servers passed to the client are not published in a thread safe manner

Hi there,

I noticed that when Node Discovery is enabled, the list of servers generated by the NodeChecker and set on the AbstractJestClient, aren't guaranteed to be published to the all threads the sub-sequentially using the getElasticSearchServer method.

The setServers method isn't synchronised, so there is no guarantee that the two variables, are published to to those threads calling the getElasticSearchServer method.

public void setServers(LinkedHashSet servers) {
this.servers = servers;
this.roundRobinIterator = Iterators.cycle(servers);
}

I've introduced an immutable ServerList implementation RoundRobinServerList that uses a CircularArray array buffer of server names and CAS operations to provide what I think are the same semantics (I hope, that was my intention) as the google iterator.cycle. Only the CAS implementation uses a non blocking approach so the synchronized on getElasticSearchServer is no longer needed; which should make the client faster. I also moved the checking of if the server host ends with '/' to the ServerList rather than on each call to the getRequestURL(). I hope these changes make sense.

I also had to change a couple of other things as the test's weren't compiling (CountIntegrationTest), updated to 0.90.0 over the RC2 (Template test was failing due to this), changed CreateIndexTest to expect "" or "{}" (wasn't sure what was to be expected here).

I hope that all makes sense. Let me know if you have any questions.
thanks
/dom