Only output sub failures if there are some

Author: thecoop

server/src/main/java/org/elasticsearch/action/search/SearchResponse.java

@@ -19,6 +19,7 @@
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.io.stream.Writeable;
 import org.elasticsearch.common.lucene.Lucene;
+import org.elasticsearch.common.util.CollectionUtils;
 import org.elasticsearch.common.util.concurrent.ConcurrentCollections;
 import org.elasticsearch.common.xcontent.ChunkedToXContentHelper;
 import org.elasticsearch.common.xcontent.ChunkedToXContentObject;
@@ -408,7 +409,9 @@ public Iterator<? extends ToXContent> toXContentChunked(ToXContent.Params params
     public Iterator<? extends ToXContent> innerToXContentChunked(ToXContent.Params params) {
         return Iterators.concat(
             ChunkedToXContentHelper.chunk(SearchResponse.this::headerToXContent),
-            ChunkedToXContentHelper.array(SUBSIDIARY_FAILURES.getPreferredName(), Iterators.forArray(subsidiaryFailures)),
+            CollectionUtils.isEmpty(subsidiaryFailures)
+                ? Collections.emptyIterator()
+                : ChunkedToXContentHelper.array(SUBSIDIARY_FAILURES.getPreferredName(), Iterators.forArray(subsidiaryFailures)),
             Iterators.single(clusters),
             Iterators.concat(
                 hits.toXContentChunked(params),

server/src/test/java/org/elasticsearch/action/search/SearchResponseMergerTests.java

@@ -60,6 +60,7 @@
 import static java.util.Collections.emptyMap;
 import static java.util.Collections.singletonList;
 import static org.elasticsearch.test.InternalAggregationTestCase.emptyReduceContextBuilder;
+import static org.hamcrest.Matchers.arrayContainingInAnyOrder;
 import static org.hamcrest.Matchers.containsInAnyOrder;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.greaterThan;
@@ -135,6 +136,60 @@ public void testMergeTookInMillis() throws InterruptedException {
         }
     }
 
+    public void testMergeSubsidiaryFailures() throws InterruptedException {
+        SearchTimeProvider searchTimeProvider = new SearchTimeProvider(0, 0, () -> 0);
+        try (
+            SearchResponseMerger merger = new SearchResponseMerger(
+                0,
+                0,
+                SearchContext.TRACK_TOTAL_HITS_ACCURATE,
+                searchTimeProvider,
+                emptyReduceContextBuilder()
+            )
+        ) {
+            List<SubsidiaryFailure> allFailures = new ArrayList<>();
+
+            for (int i = 0; i < numResponses; i++) {
+                int numFailures = randomIntBetween(0, 3);
+                SubsidiaryFailure[] subsidiaryFailures = new SubsidiaryFailure[numFailures];
+                for (int j = 0; j < numFailures; j++) {
+                    subsidiaryFailures[j] = new SubsidiaryFailure(randomAlphaOfLength(5), new IllegalArgumentException());
+                }
+                Collections.addAll(allFailures, subsidiaryFailures);
+                SearchResponse searchResponse = new SearchResponse(
+                    SearchHits.EMPTY_WITH_TOTAL_HITS,
+                    null,
+                    null,
+                    false,
+                    null,
+                    null,
+                    1,
+                    null,
+                    1,
+                    1,
+                    0,
+                    100L,
+                    ShardSearchFailure.EMPTY_ARRAY,
+                    subsidiaryFailures,
+                    SearchResponse.Clusters.EMPTY
+                );
+                try {
+                    addResponse(merger, searchResponse);
+                } finally {
+                    searchResponse.decRef();
+                }
+            }
+            awaitResponsesAdded();
+            assertEquals(numResponses, merger.numResponses());
+            SearchResponse mergedResponse = merger.getMergedResponse(SearchResponse.Clusters.EMPTY);
+            try {
+                assertThat(mergedResponse.getSubsidiaryFailures(), arrayContainingInAnyOrder(allFailures.toArray()));
+            } finally {
+                mergedResponse.decRef();
+            }
+        }
+    }
+
     public void testMergeShardFailures() throws InterruptedException {
         SearchTimeProvider searchTimeProvider = new SearchTimeProvider(0, 0, () -> 0);
         try (

server/src/test/java/org/elasticsearch/action/search/SearchResponseTests.java

@@ -413,6 +413,52 @@ public void testToXContent() throws IOException {
                 response.decRef();
             }
         }
+        {
+            SearchResponse response = new SearchResponse(
+                sHits,
+                null,
+                null,
+                false,
+                null,
+                null,
+                1,
+                null,
+                0,
+                0,
+                0,
+                0,
+                ShardSearchFailure.EMPTY_ARRAY,
+                new SubsidiaryFailure[] { new SubsidiaryFailure("rescore", new IOException("io")) },
+                SearchResponse.Clusters.EMPTY
+            );
+            try {
+                String expectedString = XContentHelper.stripWhitespace("""
+                    {
+                      "took": 0,
+                      "timed_out": false,
+                      "_shards": {
+                        "total": 0,
+                        "successful": 0,
+                        "skipped": 0,
+                        "failed": 0
+                      },
+                      "subsidiaryFailures": [ { "phase": "rescore", "failure":
+                        {"type":"i_o_exception", "reason":"io"}
+                      } ],
+                      "hits": {
+                        "total": {
+                          "value": 100,
+                          "relation": "eq"
+                        },
+                        "max_score": 1.5,
+                        "hits": [ { "_id": "id1", "_score": 2.0 } ]
+                      }
+                    }""");
+                assertEquals(expectedString, Strings.toString(response));
+            } finally {
+                response.decRef();
+            }
+        }
         {
             SearchResponse response = new SearchResponse(
                 sHits,