SOLR-7566: Search requests should return the shard name that is down

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1685153 13f79535-47bb-0310-9956-ffa450edef68

solr/CHANGES.txt

@@ -124,6 +124,8 @@ Bug Fixes
 * SOLR-6835: ReRankQueryParserPlugin checks now whether the reRankQuery parameter is present and not empty.
   (帅广应, Marius Grama via shalin)
 
+* SOLR-7566: Search requests should return the shard name that is down. (Marius Grama, shalin)
+
 Optimizations
 ----------------------
 * SOLR-7660: Avoid redundant 'exists' calls made to ZK while fetching cluster state updates. (shalin)

solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java

@@ -58,8 +58,6 @@ import org.apache.solr.core.SolrCore;
 import org.apache.solr.request.SolrQueryRequest;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-
-import com.google.common.base.Strings;
 import org.slf4j.MDC;
 
 public class HttpShardHandler extends ShardHandler {
@@ -469,6 +467,15 @@ public class HttpShardHandler extends ShardHandler {
               sliceShardsStr.append(url);
             }
 
+            if (sliceShardsStr.length() == 0) {
+              boolean tolerant = rb.req.getParams().getBool(ShardParams.SHARDS_TOLERANT, false);
+              if (!tolerant) {
+                // stop the check when there are no replicas available for a shard
+                throw new SolrException(SolrException.ErrorCode.SERVICE_UNAVAILABLE,
+                    "no servers hosting shard: " + rb.slices[i]);
+              }
+            }
+
             rb.shards[i] = sliceShardsStr.toString();
           }
         }

solr/core/src/test/org/apache/solr/cloud/TestDownShardTolerantSearch.java

@@ -0,0 +1,69 @@
+package org.apache.solr.cloud;
+
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import java.io.IOException;
+
+import org.apache.solr.client.solrj.SolrQuery;
+import org.apache.solr.client.solrj.SolrServerException;
+import org.apache.solr.client.solrj.response.QueryResponse;
+import org.apache.solr.common.params.ShardParams;
+import org.junit.Test;
+
+import static org.hamcrest.CoreMatchers.is;
+
+/**
+ * Test which asserts that shards.tolerant=true works even if one shard is down
+ * and also asserts that a meaningful exception is thrown when shards.tolerant=false
+ * See SOLR-7566
+ */
+public class TestDownShardTolerantSearch extends AbstractFullDistribZkTestBase {
+
+  public TestDownShardTolerantSearch() {
+    sliceCount = 2;
+  }
+
+  @Test
+  @ShardsFixed(num = 2)
+  public void test() throws Exception {
+    waitForRecoveriesToFinish(true);
+
+    indexAbunchOfDocs();
+    commit();
+    QueryResponse response = cloudClient.query(new SolrQuery("*:*").setRows(1));
+    assertThat(response.getStatus(), is(0));
+    assertThat(response.getResults().getNumFound(), is(66L));
+
+    ChaosMonkey.kill(shardToJetty.get(SHARD1).get(0));
+
+    response = cloudClient.query(new SolrQuery("*:*").setRows(1).setParam(ShardParams.SHARDS_TOLERANT, true));
+    assertThat(response.getStatus(), is(0));
+    assertTrue(response.getResults().getNumFound() > 0);
+
+    try {
+      response = cloudClient.query(new SolrQuery("*:*").setRows(1).setParam(ShardParams.SHARDS_TOLERANT, false));
+      fail("Request should have failed because we killed shard1 jetty");
+    } catch (SolrServerException e) {
+      log.info("error from server", e);
+      assertNotNull(e.getCause());
+      assertTrue("Error message from server should have the name of the down shard", e.getCause().getMessage().contains("shard1"));
+    } catch (IOException e) {
+      e.printStackTrace();
+    }
+  }
+}