Fixed a bug that was caused by specifying routing on a multi percolat…

…e request causing an ArrayIndexOutOfBoundsException. The multi percolate shard responses are collected in an atomic array which uses the shard id is used as index, but the number of shards the multi percolate request was meant to go to was used as size of this array instead the total number of shards an index has. This caused the exception when routing was used. Closes #6214
elastic · Jan 7, 2015 · c94d056 · c94d056
1 parent 7ec8973
commit c94d056
Show file tree

Hide file tree

Showing 2 changed files with 83 additions and 2 deletions.
diff --git a/src/main/java/org/elasticsearch/action/percolate/TransportMultiPercolateAction.java b/src/main/java/org/elasticsearch/action/percolate/TransportMultiPercolateAction.java
@@ -136,7 +136,7 @@ public void onFailure(Throwable e) {
 
     }
 
-    private class ASyncAction {
+    private final class ASyncAction {
 
         final ActionListener<MultiPercolateResponse> finalListener;
         final Map<ShardId, TransportShardMultiPercolateAction.Request> requestsByShard;
@@ -188,7 +188,9 @@ private class ASyncAction {
                         continue;
                     }
 
-                    responsesByItemAndShard.set(slot, new AtomicReferenceArray(shards.size()));
+                    // The shard id is used as index in the atomic ref array, so we need to find out how many shards there are regardless of routing:
+                    int numShards = clusterService.operationRouting().searchShardsCount(clusterState, percolateRequest.indices(), concreteIndices, null, null);
+                    responsesByItemAndShard.set(slot, new AtomicReferenceArray(numShards));
                     expectedOperationsPerItem.set(slot, new AtomicInteger(shards.size()));
                     for (ShardIterator shard : shards) {
                         ShardId shardId = shard.shardId();

diff --git a/src/test/java/org/elasticsearch/percolator/MultiPercolatorTests.java b/src/test/java/org/elasticsearch/percolator/MultiPercolatorTests.java
@@ -114,6 +114,85 @@ public void testBasics() throws Exception {
         assertThat(item.errorMessage(), containsString("document missing"));
     }
 
+    @Test
+    public void testWithRouting() throws Exception {
+        assertAcked(prepareCreate("test").addMapping("type", "field1", "type=string"));
+        ensureGreen();
+
+        logger.info("--> register a queries");
+        client().prepareIndex("test", PercolatorService.TYPE_NAME, "1")
+                .setRouting("a")
+                .setSource(jsonBuilder().startObject().field("query", matchQuery("field1", "b")).field("a", "b").endObject())
+                .execute().actionGet();
+        client().prepareIndex("test", PercolatorService.TYPE_NAME, "2")
+                .setRouting("a")
+                .setSource(jsonBuilder().startObject().field("query", matchQuery("field1", "c")).endObject())
+                .execute().actionGet();
+        client().prepareIndex("test", PercolatorService.TYPE_NAME, "3")
+                .setRouting("a")
+                .setSource(jsonBuilder().startObject().field("query", boolQuery()
+                                .must(matchQuery("field1", "b"))
+                                .must(matchQuery("field1", "c"))
+                ).endObject())
+                .execute().actionGet();
+        client().prepareIndex("test", PercolatorService.TYPE_NAME, "4")
+                .setRouting("a")
+                .setSource(jsonBuilder().startObject().field("query", matchAllQuery()).endObject())
+                .execute().actionGet();
+
+        MultiPercolateResponse response = client().prepareMultiPercolate()
+                .add(client().preparePercolate()
+                        .setIndices("test").setDocumentType("type")
+                        .setRouting("a")
+                        .setPercolateDoc(docBuilder().setDoc(jsonBuilder().startObject().field("field1", "b").endObject())))
+                .add(client().preparePercolate()
+                        .setIndices("test").setDocumentType("type")
+                        .setRouting("a")
+                        .setPercolateDoc(docBuilder().setDoc(yamlBuilder().startObject().field("field1", "c").endObject())))
+                .add(client().preparePercolate()
+                        .setIndices("test").setDocumentType("type")
+                        .setRouting("a")
+                        .setPercolateDoc(docBuilder().setDoc(smileBuilder().startObject().field("field1", "b c").endObject())))
+                .add(client().preparePercolate()
+                        .setIndices("test").setDocumentType("type")
+                        .setRouting("a")
+                        .setPercolateDoc(docBuilder().setDoc(jsonBuilder().startObject().field("field1", "d").endObject())))
+                .add(client().preparePercolate() // non existing doc, so error element
+                        .setIndices("test").setDocumentType("type")
+                        .setRouting("a")
+                        .setGetRequest(Requests.getRequest("test").type("type").id("5")))
+                .execute().actionGet();
+
+        MultiPercolateResponse.Item item = response.getItems()[0];
+        assertMatchCount(item.response(), 2l);
+        assertThat(item.getResponse().getMatches(), arrayWithSize(2));
+        assertThat(item.errorMessage(), nullValue());
+        assertThat(convertFromTextArray(item.getResponse().getMatches(), "test"), arrayContainingInAnyOrder("1", "4"));
+
+        item = response.getItems()[1];
+        assertThat(item.errorMessage(), nullValue());
+
+        assertMatchCount(item.response(), 2l);
+        assertThat(item.getResponse().getMatches(), arrayWithSize(2));
+        assertThat(convertFromTextArray(item.getResponse().getMatches(), "test"), arrayContainingInAnyOrder("2", "4"));
+
+        item = response.getItems()[2];
+        assertThat(item.errorMessage(), nullValue());
+        assertMatchCount(item.response(), 4l);
+        assertThat(convertFromTextArray(item.getResponse().getMatches(), "test"), arrayContainingInAnyOrder("1", "2", "3", "4"));
+
+        item = response.getItems()[3];
+        assertThat(item.errorMessage(), nullValue());
+        assertMatchCount(item.response(), 1l);
+        assertThat(item.getResponse().getMatches(), arrayWithSize(1));
+        assertThat(convertFromTextArray(item.getResponse().getMatches(), "test"), arrayContaining("4"));
+
+        item = response.getItems()[4];
+        assertThat(item.getResponse(), nullValue());
+        assertThat(item.errorMessage(), notNullValue());
+        assertThat(item.errorMessage(), containsString("document missing"));
+    }
+
     @Test
     public void testExistingDocsOnly() throws Exception {
         createIndex("test");