SOLR-7493: Requests aren't distributed evenly if the collection isn't present locally

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

solr/CHANGES.txt

@@ -114,6 +114,9 @@ Bug Fixes
   previously, Solr would leave old index.yyyyMMddHHmmssSSS directories left behind after failed recoveries
   in the data directory, which unnecessarily consumes disk space. (Mark Miller, Timothy Potter)
 
+* SOLR-7493: Requests aren't distributed evenly if the collection isn't present locally.
+  (Jeff Wartes, shalin)
+
 
 Optimizations
 ----------------------

solr/core/src/java/org/apache/solr/servlet/HttpSolrCall.java

@@ -30,6 +30,7 @@ import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Random;
 import java.util.Set;
 
 /*
@@ -116,6 +117,12 @@ import org.slf4j.LoggerFactory;
 public class HttpSolrCall {
   protected static Logger log = LoggerFactory.getLogger(HttpSolrCall.class);
 
+  protected static Random random;
+  static {
+    String seed = System.getProperty("tests.seed");
+    random = new Random(seed != null ? Long.parseLong(seed) : System.currentTimeMillis());
+  }
+
   protected final SolrDispatchFilter solrDispatchFilter;
   protected final CoreContainer cores;
   protected final HttpServletRequest req;
@@ -836,9 +843,16 @@ public class HttpSolrCall {
                             boolean byCoreName, boolean activeReplicas) {
     String coreUrl;
     Set<String> liveNodes = clusterState.getLiveNodes();
-    for (Slice slice : slices) {
-      Map<String, Replica> sliceShards = slice.getReplicasMap();
-      for (Replica replica : sliceShards.values()) {
+    List<Slice> randomizedSlices = new ArrayList<>(slices.size());
+    randomizedSlices.addAll(slices);
+    Collections.shuffle(randomizedSlices, random);
+
+    for (Slice slice : randomizedSlices) {
+      List<Replica> randomizedReplicas = new ArrayList<>();
+      randomizedReplicas.addAll(slice.getReplicas());
+      Collections.shuffle(randomizedReplicas, random);
+
+      for (Replica replica : randomizedReplicas) {
         if (!activeReplicas || (liveNodes.contains(replica.getNodeName())
             && replica.getState() == Replica.State.ACTIVE)) {
 

solr/core/src/java/org/apache/solr/servlet/SolrDispatchFilter.java

@@ -93,7 +93,7 @@ public class SolrDispatchFilter extends BaseSolrFilter {
     String exclude = config.getInitParameter("excludePatterns");
     if(exclude != null) {
       String[] excludeArray = exclude.split(",");
-      excludePatterns = new ArrayList();
+      excludePatterns = new ArrayList<>();
       for (String element : excludeArray) {
         excludePatterns.add(Pattern.compile(element));
       }

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

@@ -0,0 +1,112 @@
+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.util.ArrayList;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.solr.BaseDistributedSearchTestCase;
+import org.apache.solr.SolrTestCaseJ4;
+import org.apache.solr.client.solrj.SolrQuery;
+import org.apache.solr.client.solrj.embedded.JettySolrRunner;
+import org.apache.solr.client.solrj.impl.HttpSolrClient;
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.client.solrj.request.CoreAdminRequest;
+import org.apache.solr.common.cloud.ClusterState;
+import org.apache.solr.common.cloud.DocCollection;
+import org.apache.solr.common.cloud.Replica;
+import org.apache.solr.common.cloud.ZkStateReader;
+import org.apache.solr.core.CoreContainer;
+import org.apache.solr.core.SolrCore;
+import org.apache.solr.handler.component.ShardHandlerFactory;
+import org.apache.solr.handler.component.TrackingShardHandlerFactory;
+import org.apache.solr.request.SolrRequestHandler;
+import org.apache.solr.servlet.SolrDispatchFilter;
+import org.junit.Test;
+
+/**
+ * Asserts that requests aren't always sent to the same poor node. See SOLR-7493
+ */
+@SolrTestCaseJ4.SuppressSSL
+public class TestRandomRequestDistribution extends AbstractFullDistribZkTestBase {
+
+  @Test
+  @BaseDistributedSearchTestCase.ShardsFixed(num = 3)
+  public void testRequestTracking() throws Exception {
+    waitForThingsToLevelOut(30);
+
+    List<String> nodeNames = new ArrayList<>(3);
+    for (CloudJettyRunner cloudJetty : cloudJettys) {
+      nodeNames.add(cloudJetty.nodeName);
+    }
+    assertEquals(3, nodeNames.size());
+
+    new CollectionAdminRequest.Create()
+        .setCollectionName("a1x2")
+        .setNumShards(1)
+        .setReplicationFactor(2)
+        .setCreateNodeSet(nodeNames.get(0) + ',' + nodeNames.get(1))
+        .process(cloudClient);
+
+    new CollectionAdminRequest.Create()
+        .setCollectionName("b1x1")
+        .setNumShards(1)
+        .setReplicationFactor(1)
+        .setCreateNodeSet(nodeNames.get(2))
+        .process(cloudClient);
+
+    waitForRecoveriesToFinish("a1x2", true);
+    waitForRecoveriesToFinish("b1x1", true);
+
+    cloudClient.getZkStateReader().updateClusterState(true);
+
+    ClusterState clusterState = cloudClient.getZkStateReader().getClusterState();
+    DocCollection b1x1 = clusterState.getCollection("b1x1");
+    Collection<Replica> replicas = b1x1.getSlice("shard1").getReplicas();
+    assertEquals(1, replicas.size());
+    String baseUrl = replicas.iterator().next().getStr(ZkStateReader.BASE_URL_PROP);
+    if (!baseUrl.endsWith("/")) baseUrl += "/";
+    HttpSolrClient client = new HttpSolrClient(baseUrl + "a1x2");
+    client.setSoTimeout(5000);
+    client.setConnectionTimeout(2000);
+
+    log.info("Making requests to " + baseUrl + "a1x2");
+    for (int i=0; i < 10; i++)  {
+      client.query(new SolrQuery("*:*"));
+    }
+
+    Map<String, Integer> shardVsCount = new HashMap<>();
+    for (JettySolrRunner runner : jettys) {
+      CoreContainer container = ((SolrDispatchFilter) runner.getDispatchFilter().getFilter()).getCores();
+      for (SolrCore core : container.getCores()) {
+        SolrRequestHandler select = core.getRequestHandler("");
+        long c = (long) select.getStatistics().get("requests");
+        shardVsCount.put(core.getName(), (int) c);
+      }
+    }
+
+    log.info("Shard count map = " + shardVsCount);
+
+    for (Map.Entry<String, Integer> entry : shardVsCount.entrySet()) {
+      assertTrue("Shard " + entry.getKey() + " received all 10 requests", entry.getValue() != 10);
+    }
+  }
+}