SOLR-17234: LBHttp2SolrClient does not skip "zombie" endpoints (#2411)
diff --git a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBSolrClient.java b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBSolrClient.java
index 90566e4..8bb0ccd 100644
--- a/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBSolrClient.java
+++ b/solr/solrj/src/java/org/apache/solr/client/solrj/impl/LBSolrClient.java
@@ -265,8 +265,7 @@
while (it.hasNext()) {
endpoint = it.next();
// if the server is currently a zombie, just skip to the next one
- // TODO: zombieServers key is String not Endpoint.
- EndpointWrapper wrapper = zombieServers.get(endpoint);
+ EndpointWrapper wrapper = zombieServers.get(endpoint.toString());
if (wrapper != null) {
final int numDeadServersToTry = req.getNumDeadServersToTry();
if (numDeadServersToTry > 0) {
@@ -488,8 +487,7 @@
req.getRequest().setBasePath(baseUrl.toString());
rsp.rsp = getClient(baseUrl).request(req.getRequest(), (String) null);
if (isZombie) {
- // TODO: zombieServers key is String not Endpoint.
- zombieServers.remove(baseUrl);
+ zombieServers.remove(baseUrl.toString());
}
} catch (BaseHttpSolrClient.RemoteExecutionException e) {
throw e;
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientTest.java
index 203f050..367184e 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBHttp2SolrClientTest.java
@@ -108,8 +108,16 @@
} catch (TimeoutException | ExecutionException e) {
fail(iterMessage + " Response ended in failure: " + e);
}
- if (j == 0) {
- // The first endpoint gives an exception, so it retries.
+ if (i == 0) {
+ // When j=0, "endpoint one" fails.
+ // The first time around (i) it tries the first, then the second.
+ //
+ // With j=0 and i>0, it only tries "endpoint two".
+ //
+ // When j=1 and i=0, "endpoint two" starts failing.
+ // So it tries both it and "endpoint one"
+ //
+ // With j=1 and i>0, it only tries "endpoint one".
assertEquals(iterMessage, 2, client.lastBasePaths.size());
String failedBasePath = client.lastBasePaths.remove(0);
diff --git a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBSolrClientTest.java b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBSolrClientTest.java
index b29629b..60caf70 100644
--- a/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBSolrClientTest.java
+++ b/solr/solrj/src/test/org/apache/solr/client/solrj/impl/LBSolrClientTest.java
@@ -103,11 +103,13 @@
assertTrue(endpointIterator.hasNext());
assertEquals(new LBSolrClient.Endpoint("1"), endpointIterator.nextOrError());
assertTrue(endpointIterator.hasNext());
- assertEquals(new LBSolrClient.Endpoint("2"), endpointIterator.nextOrError());
- assertTrue(endpointIterator.hasNext());
assertEquals(new LBSolrClient.Endpoint("3"), endpointIterator.nextOrError());
assertTrue(endpointIterator.hasNext());
assertEquals(new LBSolrClient.Endpoint("4"), endpointIterator.nextOrError());
+
+ // Try those on the Zombie list after all other possibilities are exhausted.
+ assertTrue(endpointIterator.hasNext());
+ assertEquals(new LBSolrClient.Endpoint("2"), endpointIterator.nextOrError());
}
@Test
