Better handling of item errors in _mtermvectors API (elastic#65324)

Currently an error in a `_mtermvectors`, for example because querying through an alias that has several indices assigned to it, fails the whole request. Instead we should only fail the problematic item in the multi item request, like we e.g. do in same situations in _mget.
jaymode · Nov 23, 2020 · ae3880e · ae3880e
1 parent 7b8637e
commit ae3880e
Show file tree

Hide file tree

Showing 2 changed files with 71 additions and 14 deletions.
diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/mtermvectors/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/mtermvectors/10_basic.yml
@@ -53,3 +53,59 @@ setup:
 
   - match: {docs.0.term_vectors.text.terms.brown.term_freq: 2}
   - match: {docs.0.term_vectors.text.terms.brown.ttf: 2}
+
+---
+"Tests index not found error in item":
+
+  - do:
+        mtermvectors:
+          "term_statistics" : true
+          "body" :
+            "docs":
+              -
+                 "_id" : "testing_document"
+                 "_index" : "testidx"
+              -
+                 "_id" : "testing_document"
+                 "_index" : "wrong_idx"
+
+  - match: {docs.0.term_vectors.text.terms.brown.term_freq: 2}
+  - match: {docs.0.term_vectors.text.terms.brown.ttf: 2}
+  - match: {docs.1.error.type: "index_not_found_exception"}
+  - match: {docs.1.error.reason: "no such index [wrong_idx]"}
+
+---
+"Tests catching other exceptions per item":
+
+  - do:
+      indices.create:
+          index: testidx2
+
+  - do:
+      indices.put_alias:
+        index: testidx
+        name: test_alias
+
+  - do:
+      indices.put_alias:
+        index: testidx2
+        name: test_alias
+
+  - do:
+        mtermvectors:
+          "term_statistics" : true
+          "body" :
+            "docs":
+              -
+                 "_id" : "testing_document"
+                 "_index" : "testidx"
+              -
+                 "_id" : "testing_document"
+                 "_index" : "test_alias"
+
+  - match: {docs.0.term_vectors.text.terms.brown.term_freq: 2}
+  - match: {docs.0.term_vectors.text.terms.brown.ttf: 2}
+  - match: {docs.1.error.type: "illegal_argument_exception"}
+  - match: {docs.1.error.reason: "/Alias.\\[test_alias\\].has.more.than.one.index.associated.with.it.\\[\\[testidx2?,.testidx2?\\]\\].*/"}
+
+
diff --git a/...r/src/main/java/org/elasticsearch/action/termvectors/TransportMultiTermVectorsAction.java b/...r/src/main/java/org/elasticsearch/action/termvectors/TransportMultiTermVectorsAction.java
@@ -30,7 +30,6 @@
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.util.concurrent.AtomicArray;
-import org.elasticsearch.index.IndexNotFoundException;
 import org.elasticsearch.index.shard.ShardId;
 import org.elasticsearch.tasks.Task;
 import org.elasticsearch.transport.TransportService;
@@ -66,22 +65,24 @@ protected void doExecute(Task task, final MultiTermVectorsRequest request, final
         Map<ShardId, MultiTermVectorsShardRequest> shardRequests = new HashMap<>();
         for (int i = 0; i < request.requests.size(); i++) {
             TermVectorsRequest termVectorsRequest = request.requests.get(i);
-            termVectorsRequest.routing(clusterState.metadata().resolveIndexRouting(termVectorsRequest.routing(),
-                termVectorsRequest.index()));
-            if (!clusterState.metadata().hasConcreteIndex(termVectorsRequest.index())) {
-                responses.set(i, new MultiTermVectorsItemResponse(null,
-                        new MultiTermVectorsResponse.Failure(termVectorsRequest.index(), termVectorsRequest.id(),
-                        new IndexNotFoundException(termVectorsRequest.index()))));
-                continue;
-            }
-            String concreteSingleIndex = indexNameExpressionResolver.concreteSingleIndex(clusterState, termVectorsRequest).getName();
-            if (termVectorsRequest.routing() == null &&
-                clusterState.getMetadata().routingRequired(concreteSingleIndex)) {
+            String concreteSingleIndex;
+            try {
+                termVectorsRequest.routing(clusterState.metadata().resolveIndexRouting(termVectorsRequest.routing(),
+                    termVectorsRequest.index()));
+                concreteSingleIndex = indexNameExpressionResolver.concreteSingleIndex(clusterState, termVectorsRequest).getName();
+                if (termVectorsRequest.routing() == null &&
+                    clusterState.getMetadata().routingRequired(concreteSingleIndex)) {
+                    responses.set(i, new MultiTermVectorsItemResponse(null,
+                            new MultiTermVectorsResponse.Failure(concreteSingleIndex, termVectorsRequest.id(),
+                                    new RoutingMissingException(concreteSingleIndex, termVectorsRequest.id()))));
+                    continue;
+                }
+            } catch (Exception e) {
                 responses.set(i, new MultiTermVectorsItemResponse(null,
-                        new MultiTermVectorsResponse.Failure(concreteSingleIndex, termVectorsRequest.id(),
-                                new RoutingMissingException(concreteSingleIndex, termVectorsRequest.id()))));
+                    new MultiTermVectorsResponse.Failure(termVectorsRequest.index(), termVectorsRequest.id(), e)));
                 continue;
             }
+
             ShardId shardId = clusterService.operationRouting().shardId(clusterState, concreteSingleIndex,
                     termVectorsRequest.id(), termVectorsRequest.routing());
             MultiTermVectorsShardRequest shardRequest = shardRequests.get(shardId);