MultiGet should not fail entirely if alias resolves to many indices (#20858)

MultiGet should not fail entirely when one of the items of a multi get request refers to an alias that points to multiple indices.

closes #20845

buildSrc/src/main/resources/checkstyle_suppressions.xml

@@ -145,7 +145,6 @@
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]action[/\\]get[/\\]GetRequest.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]action[/\\]get[/\\]MultiGetRequest.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]action[/\\]get[/\\]TransportGetAction.java" checks="LineLength" />
-  <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]action[/\\]get[/\\]TransportMultiGetAction.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]action[/\\]get[/\\]TransportShardMultiGetAction.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]action[/\\]index[/\\]IndexRequest.java" checks="LineLength" />
   <suppress files="core[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]action[/\\]index[/\\]IndexRequestBuilder.java" checks="LineLength" />

core/src/main/java/org/elasticsearch/action/get/TransportMultiGetAction.java

@@ -47,8 +47,8 @@ public class TransportMultiGetAction extends HandledTransportAction<MultiGetRequ
     @Inject
     public TransportMultiGetAction(Settings settings, ThreadPool threadPool, TransportService transportService,
                                    ClusterService clusterService, TransportShardMultiGetAction shardAction,
-                                   ActionFilters actionFilters, IndexNameExpressionResolver indexNameExpressionResolver) {
-        super(settings, MultiGetAction.NAME, threadPool, transportService, actionFilters, indexNameExpressionResolver, MultiGetRequest::new);
+                                   ActionFilters actionFilters, IndexNameExpressionResolver resolver) {
+        super(settings, MultiGetAction.NAME, threadPool, transportService, actionFilters, resolver, MultiGetRequest::new);
         this.clusterService = clusterService;
         this.shardAction = shardAction;
     }
@@ -56,36 +56,47 @@ public class TransportMultiGetAction extends HandledTransportAction<MultiGetRequ
     @Override
     protected void doExecute(final MultiGetRequest request, final ActionListener<MultiGetResponse> listener) {
         ClusterState clusterState = clusterService.state();
-
         clusterState.blocks().globalBlockedRaiseException(ClusterBlockLevel.READ);
 
         final AtomicArray<MultiGetItemResponse> responses = new AtomicArray<>(request.items.size());
+        final Map<ShardId, MultiGetShardRequest> shardRequests = new HashMap<>();
 
-        Map<ShardId, MultiGetShardRequest> shardRequests = new HashMap<>();
         for (int i = 0; i < request.items.size(); i++) {
             MultiGetRequest.Item item = request.items.get(i);
+
             if (!clusterState.metaData().hasConcreteIndex(item.index())) {
-                responses.set(i, new MultiGetItemResponse(null, new MultiGetResponse.Failure(item.index(), item.type(), item.id(), new IndexNotFoundException(item.index()))));
+                responses.set(i, newItemFailure(item.index(), item.type(), item.id(), new IndexNotFoundException(item.index())));
                 continue;
             }
-            item.routing(clusterState.metaData().resolveIndexRouting(item.parent(), item.routing(), item.index()));
-            String concreteSingleIndex = indexNameExpressionResolver.concreteSingleIndex(clusterState, item).getName();
-            if (item.routing() == null && clusterState.getMetaData().routingRequired(concreteSingleIndex, item.type())) {
-                responses.set(i, new MultiGetItemResponse(null, new MultiGetResponse.Failure(concreteSingleIndex, item.type(), item.id(),
-                        new IllegalArgumentException("routing is required for [" + concreteSingleIndex + "]/[" + item.type() + "]/[" + item.id() + "]"))));
+
+            String concreteSingleIndex;
+            try {
+                item.routing(clusterState.metaData().resolveIndexRouting(item.parent(), item.routing(), item.index()));
+                concreteSingleIndex = indexNameExpressionResolver.concreteSingleIndex(clusterState, item).getName();
+
+                if ((item.routing() == null) && (clusterState.getMetaData().routingRequired(concreteSingleIndex, item.type()))) {
+                    String message = "routing is required for [" + concreteSingleIndex + "]/[" + item.type() + "]/[" + item.id() + "]";
+                    responses.set(i, newItemFailure(concreteSingleIndex, item.type(), item.id(), new IllegalArgumentException(message)));
+                    continue;
+                }
+            } catch (Exception e) {
+                responses.set(i, newItemFailure(item.index(), item.type(), item.id(), e));
                 continue;
             }
+
             ShardId shardId = clusterService.operationRouting()
-                    .getShards(clusterState, concreteSingleIndex, item.id(), item.routing(), null).shardId();
+                    .getShards(clusterState, concreteSingleIndex, item.id(), item.routing(), null)
+                    .shardId();
+
             MultiGetShardRequest shardRequest = shardRequests.get(shardId);
             if (shardRequest == null) {
-                shardRequest = new MultiGetShardRequest(request, shardId.getIndexName(), shardId.id());
+                shardRequest = new MultiGetShardRequest(request, shardId.getIndexName(), shardId.getId());
                 shardRequests.put(shardId, shardRequest);
             }
             shardRequest.add(i, item);
         }
 
-        if (shardRequests.size() == 0) {
+        if (shardRequests.isEmpty()) {
             // only failures..
             listener.onResponse(new MultiGetResponse(responses.toArray(new MultiGetItemResponse[responses.length()])));
         }
@@ -97,7 +108,8 @@ public class TransportMultiGetAction extends HandledTransportAction<MultiGetRequ
                 @Override
                 public void onResponse(MultiGetShardResponse response) {
                     for (int i = 0; i < response.locations.size(); i++) {
-                        responses.set(response.locations.get(i), new MultiGetItemResponse(response.responses.get(i), response.failures.get(i)));
+                        MultiGetItemResponse itemResponse = new MultiGetItemResponse(response.responses.get(i), response.failures.get(i));
+                        responses.set(response.locations.get(i), itemResponse);
                     }
                     if (counter.decrementAndGet() == 0) {
                         finishHim();
@@ -109,8 +121,7 @@ public class TransportMultiGetAction extends HandledTransportAction<MultiGetRequ
                     // create failures for all relevant requests
                     for (int i = 0; i < shardRequest.locations.size(); i++) {
                         MultiGetRequest.Item item = shardRequest.items.get(i);
-                        responses.set(shardRequest.locations.get(i), new MultiGetItemResponse(null,
-                                new MultiGetResponse.Failure(shardRequest.index(), item.type(), item.id(), e)));
+                        responses.set(shardRequest.locations.get(i), newItemFailure(shardRequest.index(), item.type(), item.id(), e));
                     }
                     if (counter.decrementAndGet() == 0) {
                         finishHim();
@@ -123,4 +134,8 @@ public class TransportMultiGetAction extends HandledTransportAction<MultiGetRequ
             });
         }
     }
+
+    private static MultiGetItemResponse newItemFailure(String index, String type, String id, Exception exception) {
+        return new MultiGetItemResponse(null, new MultiGetResponse.Failure(index, type, id, exception));
+    }
 }

rest-api-spec/src/main/resources/rest-api-spec/test/mget/14_alias_to_multiple_indices.yaml

@@ -0,0 +1,42 @@
+---
+"Multi Get with alias that resolves to multiple indices":
+
+  - do:
+      bulk:
+        refresh: true
+        body: |
+          {"index": {"_index": "test_1", "_type": "test", "_id": 1}}
+          { "foo": "bar" }
+          {"index": {"_index": "test_2", "_type": "test", "_id": 2}}
+          { "foo": "bar" }
+          {"index": {"_index": "test_3", "_type": "test", "_id": 3}}
+          { "foo": "bar" }
+
+  - do:
+      indices.put_alias:
+        index: test_2
+        name:  test_two_and_three
+
+  - do:
+      indices.put_alias:
+        index: test_3
+        name:  test_two_and_three
+
+  - do:
+      mget:
+        body:
+          docs:
+            - { _index: test_1, _type: test, _id: 1}
+            - { _index: test_two_and_three, _type: test, _id: 2}
+
+  - is_true: docs.0.found
+  - match: { docs.0._index:     test_1      }
+  - match: { docs.0._type:      test        }
+  - match: { docs.0._id:        "1"         }
+
+  - is_false: docs.1.found
+  - match: { docs.1._index:     test_two_and_three      }
+  - match: { docs.1._type:      test                    }
+  - match: { docs.1._id:        "2"                     }
+  - match: { docs.1.error.root_cause.0.type: "illegal_argument_exception" }
+  - match: { docs.1.error.root_cause.0.reason: "/Alias.\\[test_two_and_three\\].has.more.than.one.index.associated.with.it.\\[\\[test_[23]{1},.test_[23]{1}\\]\\],.can't.execute.a.single.index.op/" }