You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ho...@apache.org on 2013/04/26 19:34:09 UTC
svn commit: r1476310 - in /lucene/dev/trunk/solr: CHANGES.txt
core/src/java/org/apache/solr/handler/component/HttpShardHandler.java
core/src/test/org/apache/solr/cloud/BasicDistributedZkTest.java
Author: hossman
Date: Fri Apr 26 17:34:08 2013
New Revision: 1476310
URL: http://svn.apache.org/r1476310
Log:
SOLR-4705: Fixed bug causing NPE when querying a single replica in SolrCloud using the shards param
Modified:
lucene/dev/trunk/solr/CHANGES.txt
lucene/dev/trunk/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java
lucene/dev/trunk/solr/core/src/test/org/apache/solr/cloud/BasicDistributedZkTest.java
Modified: lucene/dev/trunk/solr/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/CHANGES.txt?rev=1476310&r1=1476309&r2=1476310&view=diff
==============================================================================
--- lucene/dev/trunk/solr/CHANGES.txt (original)
+++ lucene/dev/trunk/solr/CHANGES.txt Fri Apr 26 17:34:08 2013
@@ -78,6 +78,9 @@ Bug Fixes
* SOLR-4752: There are some minor bugs in the Collections API parameter
validation. (Mark Miller)
+* SOLR-4705: Fixed bug causing NPE when querying a single replica in SolrCloud
+ using the shards param (Raintung Li, hossman)
+
Other Changes
----------------------
Modified: lucene/dev/trunk/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java?rev=1476310&r1=1476309&r2=1476310&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java (original)
+++ lucene/dev/trunk/solr/core/src/java/org/apache/solr/handler/component/HttpShardHandler.java Fri Apr 26 17:34:08 2013
@@ -324,7 +324,7 @@ public class HttpShardHandler extends Sh
// and make it a non-distributed request.
String ourSlice = cloudDescriptor.getShardId();
String ourCollection = cloudDescriptor.getCollectionName();
- if (rb.slices.length == 1
+ if (rb.slices.length == 1 && rb.slices[0] != null
&& ( rb.slices[0].equals(ourSlice) || rb.slices[0].equals(ourCollection + "_" + ourSlice) ) // handle the <collection>_<slice> format
&& ZkStateReader.ACTIVE.equals(cloudDescriptor.getLastPublished()) )
{
@@ -405,4 +405,4 @@ public class HttpShardHandler extends Sh
-}
\ No newline at end of file
+}
Modified: lucene/dev/trunk/solr/core/src/test/org/apache/solr/cloud/BasicDistributedZkTest.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/core/src/test/org/apache/solr/cloud/BasicDistributedZkTest.java?rev=1476310&r1=1476309&r2=1476310&view=diff
==============================================================================
--- lucene/dev/trunk/solr/core/src/test/org/apache/solr/cloud/BasicDistributedZkTest.java (original)
+++ lucene/dev/trunk/solr/core/src/test/org/apache/solr/cloud/BasicDistributedZkTest.java Fri Apr 26 17:34:08 2013
@@ -26,6 +26,7 @@ import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
+import java.util.Collections;
import java.util.concurrent.Callable;
import java.util.concurrent.CompletionService;
import java.util.concurrent.ExecutorCompletionService;
@@ -34,6 +35,7 @@ import java.util.concurrent.SynchronousQ
import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
+import org.apache.commons.lang.StringUtils;
import org.apache.lucene.util.LuceneTestCase.Slow;
import org.apache.solr.JSONTestUtil;
import org.apache.solr.client.solrj.SolrQuery;
@@ -319,6 +321,7 @@ public class BasicDistributedZkTest exte
// would be better if these where all separate tests - but much, much
// slower
doOptimisticLockingAndUpdating();
+ testShardParamVariations();
testMultipleCollections();
testANewCollectionInOneInstance();
testSearchByCollectionName();
@@ -336,6 +339,109 @@ public class BasicDistributedZkTest exte
}
}
+ private void testShardParamVariations() throws Exception {
+ SolrQuery query = new SolrQuery("*:*");
+ Map<String,Long> shardCounts = new HashMap<String,Long>();
+
+ for (String shard : shardToJetty.keySet()) {
+ // every client should give the same numDocs for this shard
+ // shffle the clients in a diff order for each shard
+ List<SolrServer> solrclients = new ArrayList<SolrServer>(this.clients);
+ Collections.shuffle(solrclients, random());
+ for (SolrServer client : solrclients) {
+ query.set("shards", shard);
+ long numDocs = client.query(query).getResults().getNumFound();
+ assertTrue("numDocs < 0 for shard "+shard+" via "+client,
+ 0 <= numDocs);
+ if (!shardCounts.containsKey(shard)) {
+ shardCounts.put(shard, numDocs);
+ }
+ assertEquals("inconsitent numDocs for shard "+shard+" via "+client,
+ shardCounts.get(shard).longValue(), numDocs);
+
+ List<CloudJettyRunner> replicaJetties
+ = new ArrayList<CloudJettyRunner>(shardToJetty.get(shard));
+ Collections.shuffle(replicaJetties, random());
+
+ // each replica should also give the same numDocs
+ ArrayList<String> replicaAlts = new ArrayList<String>(replicaJetties.size() * 2);
+ for (CloudJettyRunner replicaJetty : shardToJetty.get(shard)) {
+ String replica = removeProtocol(replicaJetty.url);
+ query.set("shards", replica);
+
+ // replicas already shuffled, use this in the alternative check below
+ if (0 == random().nextInt(3) || replicaAlts.size() < 2) {
+ replicaAlts.add(replica);
+ }
+
+ numDocs = client.query(query).getResults().getNumFound();
+ assertTrue("numDocs < 0 for replica "+replica+" via "+client,
+ 0 <= numDocs);
+ assertEquals("inconsitent numDocs for shard "+shard+
+ " in replica "+replica+" via "+client,
+ shardCounts.get(shard).longValue(), numDocs);
+ }
+
+ // any combination of replica alternatives should give same numDocs
+ String replicas = StringUtils.join(replicaAlts.toArray(), "|");
+ query.set("shards", replicas);
+ numDocs = client.query(query).getResults().getNumFound();
+ assertTrue("numDocs < 0 for replicas "+replicas+" via "+client,
+ 0 <= numDocs);
+ assertEquals("inconsitent numDocs for replicas "+replicas+
+ " via "+client,
+ shardCounts.get(shard).longValue(), numDocs);
+ }
+ }
+
+ // sums of multiple shards should add up regardless of how we
+ // query those shards or which client we use
+ long randomShardCountsExpected = 0;
+ ArrayList<String> randomShards = new ArrayList<String>(shardCounts.size());
+ for (Map.Entry<String,Long> shardData : shardCounts.entrySet()) {
+ if (random().nextBoolean() || randomShards.size() < 2) {
+ String shard = shardData.getKey();
+ randomShardCountsExpected += shardData.getValue();
+ if (random().nextBoolean()) {
+ // use shard id
+ randomShards.add(shard);
+ } else {
+ // use some set explicit replicas
+ ArrayList<String> replicas = new ArrayList<String>(7);
+ for (CloudJettyRunner replicaJetty : shardToJetty.get(shard)) {
+ if (0 == random().nextInt(3) || 0 == replicas.size()) {
+ replicas.add(removeProtocol(replicaJetty.url));
+ }
+ }
+ Collections.shuffle(replicas, random());
+ randomShards.add(StringUtils.join(replicas, "|"));
+ }
+ }
+ }
+ String randShards = StringUtils.join(randomShards, ",");
+ query.set("shards", randShards);
+ for (SolrServer client : this.clients) {
+ assertEquals("numDocs for "+randShards+" via "+client,
+ randomShardCountsExpected,
+ client.query(query).getResults().getNumFound());
+ }
+
+ // total num docs must match sum of every shard's numDocs
+ query = new SolrQuery("*:*");
+ long totalShardNumDocs = 0;
+ for (Long c : shardCounts.values()) {
+ totalShardNumDocs += c;
+ }
+ for (SolrServer client : clients) {
+ assertEquals("sum of shard numDocs on client: " + client,
+ totalShardNumDocs,
+ client.query(query).getResults().getNumFound());
+ }
+ assertTrue("total numDocs <= 0, WTF? Test is useless",
+ 0 < totalShardNumDocs);
+
+ }
+
private void testStopAndStartCoresInOneInstance() throws Exception {
SolrServer client = clients.get(0);
String url3 = getBaseUrl(client);
@@ -980,4 +1086,11 @@ public class BasicDistributedZkTest exte
// insurance
DirectUpdateHandler2.commitOnClose = true;
}
+
+ /**
+ * Given a URL as a string, removes the leading protocol from that string
+ */
+ private static String removeProtocol(String url) {
+ return url.replaceFirst("^[^:/]{1,20}:/+","");
+ }
}