[GRPC] Handle ShardSearchFailure properly

CHANGELOG.md

@@ -29,6 +29,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Service does not start on Windows with OpenJDK ([#20615](https://github.com/opensearch-project/OpenSearch/pull/20615))
 - Update RemoteClusterStateCleanupManager to performed batched deletions of stale ClusterMetadataManifests and address deletion timeout issues ([#20566](https://github.com/opensearch-project/OpenSearch/pull/20566))
 - Fix the regression of terms agg optimization at high cardinality ([#20623](https://github.com/opensearch-project/OpenSearch/pull/20623))
+- Fix ShardSearchFailure in transport-grpc ([#20641](https://github.com/opensearch-project/OpenSearch/pull/20641))
 
 ### Dependencies
 - Bump `ch.qos.logback:logback-core` and `ch.qos.logback:logback-classic` from 1.5.24 to 1.5.27 ([#20525](https://github.com/opensearch-project/OpenSearch/pull/20525))

gradle/libs.versions.toml

@@ -24,7 +24,7 @@ kotlin                  = "1.7.10"
 antlr4                  = "4.13.1"
 guava                   = "33.2.1-jre"
 gson                    = "2.13.2"
-opensearchprotobufs     = "1.2.0"
+opensearchprotobufs     = "1.3.0"
 protobuf                = "3.25.8"
 jakarta_annotation      = "1.3.5"
 google_http_client      = "1.44.1"

modules/transport-grpc/licenses/protobufs-1.3.0.jar.sha1

@@ -0,0 +1 @@
+a141d00a9de80085436c648502d1b015fd89b9f6

modules/transport-grpc/spi/licenses/protobufs-1.3.0.jar.sha1

@@ -0,0 +1 @@
+a141d00a9de80085436c648502d1b015fd89b9f6

modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/exceptions/shardoperationfailedexception/ShardOperationFailedExceptionProtoUtils.java

@@ -31,12 +31,15 @@ private ShardOperationFailedExceptionProtoUtils() {
      * This method is similar to {@link org.opensearch.core.action.ShardOperationFailedException#toXContent(XContentBuilder, ToXContent.Params)}
      * This method is overridden by various exception classes, which are hardcoded here.
      *
+     * This method converts to the legacy ShardFailure proto type for backward compatibility.
+     * For ShardSearchFailure, use the new failures_2 field with ShardSearchFailure proto type.
+     *
      * @param exception The ShardOperationFailedException to convert metadata from
-     * @return ShardFailure
+     * @return ShardFailure proto object
      */
     public static ShardFailure toProto(ShardOperationFailedException exception) throws IOException {
         return switch (exception) {
-            case ShardSearchFailure ssf -> ShardSearchFailureProtoUtils.toProto(ssf);
+            case ShardSearchFailure ssf -> ShardSearchFailureProtoUtils.toLegacyProto(ssf);
             case SnapshotShardFailure ssf -> SnapshotShardFailureProtoUtils.toProto(ssf);
             case DefaultShardOperationFailedException dsofe -> DefaultShardOperationFailedExceptionProtoUtils.toProto(dsofe);
             case ReplicationResponse.ShardInfo.Failure sf -> ReplicationResponseShardInfoFailureProtoUtils.toProto(sf);

modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/exceptions/shardoperationfailedexception/ShardSearchFailureProtoUtils.java

@@ -25,20 +25,45 @@ private ShardSearchFailureProtoUtils() {
     }
 
     /**
-     * Converts the metadata from a ShardSearchFailure to a Protocol Buffer Struct.
-     * Similar to {@link ShardSearchFailure#toXContent(XContentBuilder, ToXContent.Params)}     *
+     * Converts the metadata from a ShardSearchFailure to a Protocol Buffer ShardSearchFailure.
+     * Similar to {@link ShardSearchFailure#toXContent(XContentBuilder, ToXContent.Params)}
      *
      * @param exception The ShardSearchFailure to convert
-     * @return A Protocol Buffer Struct containing the exception metadata
+     * @return A Protocol Buffer ShardSearchFailure containing the exception metadata
      */
-    public static ShardFailure toProto(ShardSearchFailure exception) throws IOException {
+    public static org.opensearch.protobufs.ShardSearchFailure toProto(ShardSearchFailure exception) throws IOException {
+        org.opensearch.protobufs.ShardSearchFailure.Builder shardSearchFailure = org.opensearch.protobufs.ShardSearchFailure.newBuilder();
+        shardSearchFailure.setShard(exception.shardId());
+        if (exception.index() != null) {
+            shardSearchFailure.setIndex(exception.index());
+        }
+        if (exception.shard() != null && exception.shard().getNodeId() != null) {
+            shardSearchFailure.setNode(exception.shard().getNodeId());
+        }
+        shardSearchFailure.setReason(OpenSearchExceptionProtoUtils.generateThrowableProto(exception.getCause()));
+        return shardSearchFailure.build();
+    }
+
+    /**
+     * Converts the metadata from a ShardSearchFailure to the legacy ShardFailure proto type.
+     * This is for backward compatibility with older clients.
+     * @deprecated Use {@link #toProto(ShardSearchFailure)} which returns ShardSearchFailure proto type
+     *
+     * @param exception The ShardSearchFailure to convert
+     * @return A Protocol Buffer ShardFailure containing the exception metadata (without primary field)
+     */
+    @Deprecated
+    public static ShardFailure toLegacyProto(ShardSearchFailure exception) throws IOException {
         ShardFailure.Builder shardFailure = ShardFailure.newBuilder();
         shardFailure.setShard(exception.shardId());
-        shardFailure.setIndex(exception.index());
+        if (exception.index() != null) {
+            shardFailure.setIndex(exception.index());
+        }
         if (exception.shard() != null && exception.shard().getNodeId() != null) {
             shardFailure.setNode(exception.shard().getNodeId());
         }
         shardFailure.setReason(OpenSearchExceptionProtoUtils.generateThrowableProto(exception.getCause()));
+        // Note: primary field is not set for search failures as it's not applicable
         return shardFailure.build();
     }
 }

modules/transport-grpc/src/main/java/org/opensearch/transport/grpc/proto/response/search/ShardStatisticsProtoUtils.java

@@ -9,12 +9,14 @@
 
 import org.opensearch.ExceptionsHelper;
 import org.opensearch.action.admin.indices.stats.ShardStats;
+import org.opensearch.action.search.ShardSearchFailure;
 import org.opensearch.core.action.ShardOperationFailedException;
 import org.opensearch.core.common.util.CollectionUtils;
 import org.opensearch.core.xcontent.ToXContent;
 import org.opensearch.core.xcontent.XContentBuilder;
 import org.opensearch.protobufs.ShardStatistics;
 import org.opensearch.transport.grpc.proto.response.exceptions.shardoperationfailedexception.ShardOperationFailedExceptionProtoUtils;
+import org.opensearch.transport.grpc.proto.response.exceptions.shardoperationfailedexception.ShardSearchFailureProtoUtils;
 
 import java.io.IOException;
 
@@ -57,6 +59,12 @@ protected static ShardStatistics getShardStats(
         shardStats.setFailed(failed);
         if (CollectionUtils.isEmpty(shardFailures) == false) {
             for (ShardOperationFailedException shardFailure : ExceptionsHelper.groupBy(shardFailures)) {
+                // Populate the new failures_2 field with ShardSearchFailure proto type
+                if (shardFailure instanceof ShardSearchFailure) {
+                    shardStats.addFailures2(ShardSearchFailureProtoUtils.toProto((ShardSearchFailure) shardFailure));
+                }
+
+                // Also populate the deprecated failures field for backward compatibility
                 shardStats.addFailures(ShardOperationFailedExceptionProtoUtils.toProto(shardFailure));
             }
         }

modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/exceptions/shardoperationfailedexception/ShardOperationFailedExceptionProtoUtilsTests.java

@@ -34,14 +34,18 @@ public void testToProtoWithShardSearchFailure() throws IOException {
         // Create a ShardSearchFailure
         ShardSearchFailure shardSearchFailure = new ShardSearchFailure(new Exception("fake exception"), searchShardTarget);
 
-        // Call the method under test
+        // Call the method under test - should call toLegacyProto for backward compatibility
         ShardFailure protoFailure = ShardOperationFailedExceptionProtoUtils.toProto(shardSearchFailure);
 
         // Verify the result
         assertNotNull("Proto failure should not be null", protoFailure);
         assertEquals("Index should match", "test_index", protoFailure.getIndex());
         assertEquals("Shard ID should match", 1, protoFailure.getShard());
         assertEquals("Node ID should match", "test_node", protoFailure.getNode());
+
+        // Verify primary field is false (default) for search failures
+        // ShardSearchFailure doesn't set primary, so it defaults to false in the legacy proto
+        assertFalse("Primary should be false for search failures", protoFailure.getPrimary());
     }
 
     public void testToProtoWithSnapshotShardFailure() throws IOException {

modules/transport-grpc/src/test/java/org/opensearch/transport/grpc/proto/response/exceptions/shardoperationfailedexception/ShardSearchFailureProtoUtilsTests.java

@@ -0,0 +1,113 @@
+/*
+ * 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.transport.grpc.proto.response.exceptions.shardoperationfailedexception;
+
+import org.opensearch.action.search.ShardSearchFailure;
+import org.opensearch.core.index.shard.ShardId;
+import org.opensearch.protobufs.ShardFailure;
+import org.opensearch.search.SearchShardTarget;
+import org.opensearch.test.OpenSearchTestCase;
+
+import java.io.IOException;
+
+public class ShardSearchFailureProtoUtilsTests extends OpenSearchTestCase {
+
+    public void testToProtoWithNodeId() throws IOException {
+        // Create a SearchShardTarget with a nodeId
+        ShardId shardId = new ShardId("test_index", "_na_", 1);
+        SearchShardTarget searchShardTarget = new SearchShardTarget("test_node", shardId, null, null);
+
+        // Create a ShardSearchFailure
+        ShardSearchFailure shardSearchFailure = new ShardSearchFailure(new Exception("fake exception"), searchShardTarget);
+
+        // Call the method under test
+        org.opensearch.protobufs.ShardSearchFailure protoFailure = ShardSearchFailureProtoUtils.toProto(shardSearchFailure);
+
+        // Verify the result
+        assertNotNull("Proto failure should not be null", protoFailure);
+        assertEquals("Index should match", "test_index", protoFailure.getIndex());
+        assertEquals("Shard ID should match", 1, protoFailure.getShard());
+        assertEquals("Node ID should match", "test_node", protoFailure.getNode());
+        assertTrue("Reason should be set", protoFailure.hasReason());
+
+        // Verify primary field is NOT present (ShardSearchFailure proto doesn't have it)
+        // This is verified by the proto type itself - ShardSearchFailure doesn't define primary field
+    }
+
+    public void testToProtoWithoutNodeId() throws IOException {
+        // Create a ShardSearchFailure without nodeId (shard target is null)
+        ShardSearchFailure shardSearchFailure = new ShardSearchFailure(new Exception("fake exception"));
+
+        // Call the method under test
+        org.opensearch.protobufs.ShardSearchFailure protoFailure = ShardSearchFailureProtoUtils.toProto(shardSearchFailure);
+
+        // Verify the result
+        assertNotNull("Proto failure should not be null", protoFailure);
+        assertTrue("Reason should be set", protoFailure.hasReason());
+        // Node should not be set when shard target is null
+        assertFalse("Node should not be set", protoFailure.hasNode());
+    }
+
+    public void testToLegacyProtoWithNodeId() throws IOException {
+        // Create a SearchShardTarget with a nodeId
+        ShardId shardId = new ShardId("legacy_index", "_na_", 2);
+        SearchShardTarget searchShardTarget = new SearchShardTarget("legacy_node", shardId, null, null);
+
+        // Create a ShardSearchFailure
+        ShardSearchFailure shardSearchFailure = new ShardSearchFailure(new Exception("legacy exception"), searchShardTarget);
+
+        // Call the legacy method
+        ShardFailure protoFailure = ShardSearchFailureProtoUtils.toLegacyProto(shardSearchFailure);
+
+        // Verify the result
+        assertNotNull("Proto failure should not be null", protoFailure);
+        assertEquals("Index should match", "legacy_index", protoFailure.getIndex());
+        assertEquals("Shard ID should match", 2, protoFailure.getShard());
+        assertEquals("Node ID should match", "legacy_node", protoFailure.getNode());
+        assertTrue("Reason should be set", protoFailure.hasReason());
+
+        // Note: primary field exists in ShardFailure proto but is not set for search failures
+        // It will default to false
+        assertFalse("Primary should be false (default) for search failures", protoFailure.getPrimary());
+    }
+
+    public void testToLegacyProtoWithoutNodeId() throws IOException {
+        // Create a ShardSearchFailure without nodeId
+        ShardSearchFailure shardSearchFailure = new ShardSearchFailure(new Exception("legacy exception without node"));
+
+        // Call the legacy method
+        ShardFailure protoFailure = ShardSearchFailureProtoUtils.toLegacyProto(shardSearchFailure);
+
+        // Verify the result
+        assertNotNull("Proto failure should not be null", protoFailure);
+        assertTrue("Reason should be set", protoFailure.hasReason());
+        // Node should not be set when shard target is null
+        assertFalse("Node should not be set", protoFailure.hasNode());
+    }
+
+    public void testToProtoVsLegacyProtoHaveSameData() throws IOException {
+        // Create test data
+        ShardId shardId = new ShardId("compare_index", "_na_", 3);
+        SearchShardTarget searchShardTarget = new SearchShardTarget("compare_node", shardId, null, null);
+        ShardSearchFailure shardSearchFailure = new ShardSearchFailure(new Exception("compare exception"), searchShardTarget);
+
+        // Get both proto types
+        org.opensearch.protobufs.ShardSearchFailure newProto = ShardSearchFailureProtoUtils.toProto(shardSearchFailure);
+        ShardFailure legacyProto = ShardSearchFailureProtoUtils.toLegacyProto(shardSearchFailure);
+
+        // Verify common fields have same values
+        assertEquals("Index should match between new and legacy", legacyProto.getIndex(), newProto.getIndex());
+        assertEquals("Shard should match between new and legacy", legacyProto.getShard(), newProto.getShard());
+        assertEquals("Node should match between new and legacy", legacyProto.getNode(), newProto.getNode());
+
+        // Both should have reason set
+        assertTrue("New proto should have reason", newProto.hasReason());
+        assertTrue("Legacy proto should have reason", legacyProto.hasReason());
+    }
+}