ShardSearchFailure#readFrom to set index and shardId (#33161)

As part of recent changes made to `ShardOperationFailedException` we introduced `index` and `shardId` members to the base class, but the subclasses are entirely responsible for the serialization of such fields. In the case of `ShardSearchFailure`, we have an additional `SearchShardTarget` instance member which also holds the index and the shardId, hence they get serialized as part of `SearchShardTarget` itself. When de-serializing a `ShardSearchFailure` though, we need to remember to also set the parent class `index` and `shardId` fields otherwise they get lost Relates to #32640
elastic · Sep 18, 2018 · 923b33c · 923b33c
1 parent 00d07ca
commit 923b33c
Show file tree

Hide file tree

Showing 6 changed files with 37 additions and 9 deletions.
diff --git a/server/src/main/java/org/elasticsearch/action/ShardOperationFailedException.java b/server/src/main/java/org/elasticsearch/action/ShardOperationFailedException.java
@@ -33,7 +33,7 @@
 public abstract class ShardOperationFailedException implements Streamable, ToXContent {
 
     protected String index;
-    protected int shardId;
+    protected int shardId = -1;
     protected String reason;
     protected RestStatus status;
     protected Throwable cause;

diff --git a/server/src/main/java/org/elasticsearch/action/search/ShardSearchFailure.java b/server/src/main/java/org/elasticsearch/action/search/ShardSearchFailure.java
@@ -54,8 +54,7 @@ public class ShardSearchFailure extends ShardOperationFailedException {
 
     private SearchShardTarget shardTarget;
 
-    private ShardSearchFailure() {
-
+    ShardSearchFailure() {
     }
 
     public ShardSearchFailure(Exception e) {
@@ -101,6 +100,8 @@ public static ShardSearchFailure readShardSearchFailure(StreamInput in) throws I
     public void readFrom(StreamInput in) throws IOException {
         if (in.readBoolean()) {
             shardTarget = new SearchShardTarget(in);
+            index = shardTarget.getFullyQualifiedIndexName();
+            shardId = shardTarget.getShardId().getId();
         }
         reason = in.readString();
         status = RestStatus.readFrom(in);

diff --git a/server/src/main/java/org/elasticsearch/action/support/replication/ReplicationResponse.java b/server/src/main/java/org/elasticsearch/action/support/replication/ReplicationResponse.java
@@ -271,7 +271,7 @@ public boolean primary() {
             public void readFrom(StreamInput in) throws IOException {
                 shardId = ShardId.readShardId(in);
                 super.shardId = shardId.getId();
-                super.index = shardId.getIndexName();
+                index = shardId.getIndexName();
                 nodeId = in.readOptionalString();
                 cause = in.readException();
                 status = RestStatus.readFrom(in);

diff --git a/server/src/main/java/org/elasticsearch/snapshots/SnapshotShardFailure.java b/server/src/main/java/org/elasticsearch/snapshots/SnapshotShardFailure.java
@@ -102,7 +102,7 @@ public void readFrom(StreamInput in) throws IOException {
         nodeId = in.readOptionalString();
         shardId = ShardId.readShardId(in);
         super.shardId = shardId.getId();
-        super.index = shardId.getIndexName();
+        index = shardId.getIndexName();
         reason = in.readString();
         status = RestStatus.readFrom(in);
     }

diff --git a/server/src/test/java/org/elasticsearch/action/search/SearchResponseTests.java b/server/src/test/java/org/elasticsearch/action/search/SearchResponseTests.java
@@ -20,6 +20,7 @@
 package org.elasticsearch.action.search;
 
 import org.elasticsearch.Version;
+import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.io.stream.BytesStreamOutput;
@@ -183,7 +184,7 @@ public void testFromXContentWithFailures() throws IOException {
         int numFailures = randomIntBetween(1, 5);
         ShardSearchFailure[] failures = new ShardSearchFailure[numFailures];
         for (int i = 0; i < failures.length; i++) {
-            failures[i] = ShardSearchFailureTests.createTestItem();
+            failures[i] = ShardSearchFailureTests.createTestItem(IndexMetaData.INDEX_UUID_NA_VALUE);
         }
         SearchResponse response = createTestItem(failures);
         XContentType xcontentType = randomFrom(XContentType.values());

diff --git a/server/src/test/java/org/elasticsearch/action/search/ShardSearchFailureTests.java b/server/src/test/java/org/elasticsearch/action/search/ShardSearchFailureTests.java
@@ -19,6 +19,7 @@
 
 package org.elasticsearch.action.search;
 
+import org.elasticsearch.Version;
 import org.elasticsearch.action.OriginalIndices;
 import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.common.ParsingException;
@@ -30,6 +31,7 @@
 import org.elasticsearch.index.shard.ShardId;
 import org.elasticsearch.search.SearchShardTarget;
 import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.test.VersionUtils;
 
 import java.io.IOException;
 
@@ -38,7 +40,7 @@
 
 public class ShardSearchFailureTests extends ESTestCase {
 
-    public static ShardSearchFailure createTestItem() {
+    public static ShardSearchFailure createTestItem(String indexUuid) {
         String randomMessage = randomAlphaOfLengthBetween(3, 20);
         Exception ex = new ParsingException(0, 0, randomMessage , new IllegalArgumentException("some bad argument"));
         SearchShardTarget searchShardTarget = null;
@@ -47,7 +49,7 @@ public static ShardSearchFailure createTestItem() {
             String indexName = randomAlphaOfLengthBetween(5, 10);
             String clusterAlias = randomBoolean() ? randomAlphaOfLengthBetween(5, 10) : null;
             searchShardTarget = new SearchShardTarget(nodeId,
-                    new ShardId(new Index(indexName, IndexMetaData.INDEX_UUID_NA_VALUE), randomInt()), clusterAlias, OriginalIndices.NONE);
+                    new ShardId(new Index(indexName, indexUuid), randomInt()), clusterAlias, OriginalIndices.NONE);
         }
         return new ShardSearchFailure(ex, searchShardTarget);
     }
@@ -66,7 +68,7 @@ public void testFromXContentWithRandomFields() throws IOException {
     }
 
     private void doFromXContentTestWithRandomFields(boolean addRandomFields) throws IOException {
-        ShardSearchFailure response = createTestItem();
+        ShardSearchFailure response = createTestItem(IndexMetaData.INDEX_UUID_NA_VALUE);
         XContentType xContentType = randomFrom(XContentType.values());
         boolean humanReadable = randomBoolean();
         BytesReference originalBytes = toShuffledXContent(response, xContentType, ToXContent.EMPTY_PARAMS, humanReadable);
@@ -134,4 +136,28 @@ public void testToXContentWithClusterAlias() throws IOException {
                 + "}",
             xContent.utf8ToString());
     }
+
+    public void testSerialization() throws IOException {
+        ShardSearchFailure testItem = createTestItem(randomAlphaOfLength(12));
+        Version version = VersionUtils.randomVersion(random());
+        ShardSearchFailure deserializedInstance = copyStreamable(testItem, writableRegistry(),
+            ShardSearchFailure::new, version);
+        if (version.onOrAfter(Version.V_5_6_0)) {
+            assertEquals(testItem.index(), deserializedInstance.index());
+            assertEquals(testItem.shard(), deserializedInstance.shard());
+        } else {
+            //clusterAlias does not get serialized prior to 5.6
+            if (testItem.shard() != null && testItem.shard().getClusterAlias() != null) {
+                SearchShardTarget shard = testItem.shard();
+                assertEquals(shard.getIndex(), deserializedInstance.index());
+                SearchShardTarget newShard = new SearchShardTarget(shard.getNodeId(), shard.getShardId(), null, shard.getOriginalIndices());
+                assertEquals(newShard, deserializedInstance.shard());
+            } else {
+                assertEquals(testItem.shard(), deserializedInstance.shard());
+            }
+        }
+        assertEquals(testItem.shardId(), deserializedInstance.shardId());
+        assertEquals(testItem.reason(), deserializedInstance.reason());
+        assertEquals(testItem.status(), deserializedInstance.status());
+    }
 }