Handle index not found properly and return null in response

Signed-off-by: Aman Khare <[email protected]>

server/src/main/java/org/opensearch/gateway/BaseShardResponse.java

@@ -0,0 +1,51 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
+package org.opensearch.gateway;
+
+import org.opensearch.core.common.io.stream.StreamInput;
+import org.opensearch.core.common.io.stream.StreamOutput;
+
+import java.io.IOException;
+
+/**
+ * Base response class for shard response.
+ *
+ * @opensearch.internal
+ */
+public abstract class BaseShardResponse {
+
+    private final Exception storeException;
+
+    public BaseShardResponse(Exception storeException) {
+        this.storeException = storeException;
+    }
+
+    public abstract boolean isEmpty();
+
+    public Exception getException() {
+        return storeException;
+    }
+
+    public BaseShardResponse(StreamInput in) throws IOException {
+        if (in.readBoolean()) {
+            storeException = in.readException();
+        } else {
+            storeException = null;
+        }
+    }
+
+    public void writeTo(StreamOutput out) throws IOException {
+        if (storeException != null) {
+            out.writeBoolean(true);
+            out.writeException(storeException);
+        } else {
+            out.writeBoolean(false);
+        }
+    }
+}

server/src/main/java/org/opensearch/indices/store/TransportNodesListShardStoreMetadata.java

@@ -52,15 +52,13 @@
 import org.opensearch.core.index.shard.ShardId;
 import org.opensearch.env.NodeEnvironment;
 import org.opensearch.gateway.AsyncShardFetch;
-import org.opensearch.index.store.Store;
 import org.opensearch.indices.IndicesService;
 import org.opensearch.indices.store.TransportNodesListShardStoreMetadataHelper.StoreFilesMetadata;
 import org.opensearch.threadpool.ThreadPool;
 import org.opensearch.transport.TransportRequest;
 import org.opensearch.transport.TransportService;
 
 import java.io.IOException;
-import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -157,19 +155,7 @@ protected NodeStoreFilesMetadata nodeOperation(NodeRequest request) {
 
     private StoreFilesMetadata listStoreMetadata(NodeRequest request) throws IOException {
         final ShardId shardId = request.getShardId();
-        try {
-            return listShardMetadataInternal(
-                logger,
-                shardId,
-                nodeEnv,
-                indicesService,
-                request.getCustomDataPath(),
-                settings,
-                clusterService
-            );
-        } catch (IOException e) {
-            return new StoreFilesMetadata(shardId, Store.MetadataSnapshot.EMPTY, Collections.emptyList());
-        }
+        return listShardMetadataInternal(logger, shardId, nodeEnv, indicesService, request.getCustomDataPath(), settings, clusterService);
     }
 
     /**

server/src/main/java/org/opensearch/indices/store/TransportNodesListShardStoreMetadataBatch.java

@@ -8,7 +8,6 @@
 
 package org.opensearch.indices.store;
 
-import org.apache.logging.log4j.message.ParameterizedMessage;
 import org.opensearch.OpenSearchException;
 import org.opensearch.action.ActionType;
 import org.opensearch.action.FailedNodeException;
@@ -28,6 +27,7 @@
 import org.opensearch.core.index.shard.ShardId;
 import org.opensearch.env.NodeEnvironment;
 import org.opensearch.gateway.AsyncShardFetch;
+import org.opensearch.gateway.BaseShardResponse;
 import org.opensearch.index.store.Store;
 import org.opensearch.indices.IndicesService;
 import org.opensearch.indices.store.TransportNodesListShardStoreMetadataHelper.StoreFilesMetadata;
@@ -42,6 +42,8 @@
 import java.util.Map;
 import java.util.Objects;
 
+import static org.opensearch.indices.store.TransportNodesListShardStoreMetadataHelper.INDEX_NOT_FOUND;
+
 /**
  * Transport action for fetching the batch of shard stores Metadata from a list of transport nodes
  *
@@ -139,28 +141,33 @@ protected NodeStoreFilesMetadataBatch nodeOperation(NodeRequest request) {
      */
     private Map<ShardId, NodeStoreFilesMetadata> listStoreMetadata(NodeRequest request) throws IOException {
         Map<ShardId, NodeStoreFilesMetadata> shardStoreMetadataMap = new HashMap<ShardId, NodeStoreFilesMetadata>();
-        for (ShardAttributes shardAttributes : request.getShardAttributes().values()) {
-            final ShardId shardId = shardAttributes.getShardId();
+        for (Map.Entry<ShardId, ShardAttributes> shardAttributes : request.getShardAttributes().entrySet()) {
+            final ShardId shardId = shardAttributes.getKey();
             try {
                 StoreFilesMetadata storeFilesMetadata = TransportNodesListShardStoreMetadataHelper.listShardMetadataInternal(
                     logger,
                     shardId,
                     nodeEnv,
                     indicesService,
-                    shardAttributes.getCustomDataPath(),
+                    shardAttributes.getValue().getCustomDataPath(),
                     settings,
                     clusterService
                 );
                 shardStoreMetadataMap.put(shardId, new NodeStoreFilesMetadata(storeFilesMetadata, null));
-            } catch (Exception e) {
-                logger.debug(
-                    new ParameterizedMessage("Faced following exception while trying to get Shard Metadata for {}: ", shardId.toString()),
-                    e
-                );
-                shardStoreMetadataMap.put(
-                    shardId,
-                    new NodeStoreFilesMetadata(new StoreFilesMetadata(shardId, Store.MetadataSnapshot.EMPTY, Collections.emptyList()), e)
-                );
+            } catch (IOException e) {
+                // should return null in case of known exceptions being returned from listShardMetadataInternal method.
+                if (e.getMessage().contains(INDEX_NOT_FOUND)) {
+                    shardStoreMetadataMap.put(shardId, null);
+                } else {
+                    // return actual exception as it is for unknown exceptions
+                    shardStoreMetadataMap.put(
+                        shardId,
+                        new NodeStoreFilesMetadata(
+                            new StoreFilesMetadata(shardId, Store.MetadataSnapshot.EMPTY, Collections.emptyList()),
+                            e
+                        )
+                    );
+                }
             }
         }
         return shardStoreMetadataMap;
@@ -232,52 +239,43 @@ protected void writeNodesTo(StreamOutput out, List<NodeStoreFilesMetadataBatch>
      *
      * @opensearch.internal
      */
-    public static class NodeStoreFilesMetadata {
+    public static class NodeStoreFilesMetadata extends BaseShardResponse {
 
         private StoreFilesMetadata storeFilesMetadata;
-        private Exception storeFileFetchException;
 
-        public NodeStoreFilesMetadata(StoreFilesMetadata storeFilesMetadata) {
-            this.storeFilesMetadata = storeFilesMetadata;
-            this.storeFileFetchException = null;
+        @Override
+        public boolean isEmpty() {
+            return storeFilesMetadata == null || storeFilesMetadata().isEmpty() && getException() == null;
         }
 
         public NodeStoreFilesMetadata(StreamInput in) throws IOException {
-            storeFilesMetadata = new StoreFilesMetadata(in);
+            super(in);
             if (in.readBoolean()) {
-                this.storeFileFetchException = in.readException();
+                this.storeFilesMetadata = new StoreFilesMetadata(in);
             } else {
-                this.storeFileFetchException = null;
+                this.storeFilesMetadata = null;
             }
         }
 
         public NodeStoreFilesMetadata(StoreFilesMetadata storeFilesMetadata, Exception storeFileFetchException) {
+            super(storeFileFetchException);
             this.storeFilesMetadata = storeFilesMetadata;
-            this.storeFileFetchException = storeFileFetchException;
         }
 
         public StoreFilesMetadata storeFilesMetadata() {
             return storeFilesMetadata;
         }
 
-        public static NodeStoreFilesMetadata readListShardStoreNodeOperationResponse(StreamInput in) throws IOException {
-            return new NodeStoreFilesMetadata(in);
-        }
-
         public void writeTo(StreamOutput out) throws IOException {
-            storeFilesMetadata.writeTo(out);
-            if (storeFileFetchException != null) {
+            super.writeTo(out);
+            if (storeFilesMetadata != null) {
                 out.writeBoolean(true);
-                out.writeException(storeFileFetchException);
+                storeFilesMetadata.writeTo(out);
             } else {
                 out.writeBoolean(false);
             }
         }
 
-        public Exception getStoreFileFetchException() {
-            return storeFileFetchException;
-        }
-
         @Override
         public String toString() {
             return "[[" + storeFilesMetadata + "]]";

server/src/main/java/org/opensearch/indices/store/TransportNodesListShardStoreMetadataHelper.java

@@ -48,6 +48,9 @@
  * @opensearch.internal
  */
 public class TransportNodesListShardStoreMetadataHelper {
+
+    public static final String INDEX_NOT_FOUND = "node doesn't have meta data for index ";
+
     public static StoreFilesMetadata listShardMetadataInternal(
         Logger logger,
         final ShardId shardId,
@@ -75,10 +78,10 @@ public static StoreFilesMetadata listShardMetadataInternal(
                         return storeFilesMetadata;
                     } catch (org.apache.lucene.index.IndexNotFoundException e) {
                         logger.trace(new ParameterizedMessage("[{}] node is missing index, responding with empty", shardId), e);
-                        throw e;
+                        return new StoreFilesMetadata(shardId, Store.MetadataSnapshot.EMPTY, Collections.emptyList());
                     } catch (IOException e) {
                         logger.warn(new ParameterizedMessage("[{}] can't read metadata from store, responding with empty", shardId), e);
-                        throw e;
+                        return new StoreFilesMetadata(shardId, Store.MetadataSnapshot.EMPTY, Collections.emptyList());
                     }
                 }
             }
@@ -93,7 +96,7 @@ public static StoreFilesMetadata listShardMetadataInternal(
                         customDataPath = new IndexSettings(metadata, settings).customDataPath();
                     } else {
                         logger.trace("{} node doesn't have meta data for the requests index", shardId);
-                        throw new OpenSearchException("node doesn't have meta data for index " + shardId.getIndex());
+                        throw new OpenSearchException(INDEX_NOT_FOUND + shardId.getIndex());
                     }
                 }
             }