Remove unsupported group_shard_failures parameter (#33208)

We have had support for the `group_shard_failures` parameter in our code for a while, since we introduced failures grouping. When we introduced validation of parameters at REST, we seem to have forgotten to expose such parameter. Given that the parameter is effectively not supported for many months now, that no user has complained about that and that grouping is the expected behaviour, this commit removes support for the parameter.

Author: javanna

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

@@ -134,11 +134,10 @@ private static String buildMessage(String phaseName, String msg, ShardSearchFail
     @Override
     protected void metadataToXContent(XContentBuilder builder, Params params) throws IOException {
         builder.field("phase", phaseName);
-        final boolean group = params.paramAsBoolean("group_shard_failures", true); // we group by default
-        builder.field("grouped", group); // notify that it's grouped
+        builder.field("grouped", true); // notify that it's grouped
         builder.field("failed_shards");
         builder.startArray();
-        ShardOperationFailedException[] failures = group ? ExceptionsHelper.groupBy(shardFailures) : shardFailures;
+        ShardOperationFailedException[] failures = ExceptionsHelper.groupBy(shardFailures);
         for (ShardOperationFailedException failure : failures) {
             builder.startObject();
             failure.toXContent(builder, params);

server/src/main/java/org/elasticsearch/rest/action/RestActions.java

@@ -90,8 +90,7 @@ public static void buildBroadcastShardsHeader(XContentBuilder builder, Params pa
         builder.field(FAILED_FIELD.getPreferredName(), failed);
         if (shardFailures != null && shardFailures.length > 0) {
             builder.startArray(FAILURES_FIELD.getPreferredName());
-            final boolean group = params.paramAsBoolean("group_shard_failures", true); // we group by default
-            for (ShardOperationFailedException shardFailure : group ? ExceptionsHelper.groupBy(shardFailures) : shardFailures) {
+            for (ShardOperationFailedException shardFailure : ExceptionsHelper.groupBy(shardFailures)) {
                 builder.startObject();
                 shardFailure.toXContent(builder, params);
                 builder.endObject();

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

@@ -26,7 +26,6 @@
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.xcontent.ToXContent;
 import org.elasticsearch.common.xcontent.XContent;
-import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
 import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.index.Index;
@@ -38,8 +37,6 @@
 
 import java.io.IOException;
 
-import static java.util.Collections.singletonMap;
-import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
 import static org.hamcrest.CoreMatchers.hasItem;
 import static org.hamcrest.Matchers.hasSize;
 
@@ -87,56 +84,6 @@ public void testToXContent() throws IOException {
                             "}" +
                         "}" +
                 "]}", Strings.toString(exception));
-
-        // Failures are NOT grouped
-        ToXContent.MapParams params = new ToXContent.MapParams(singletonMap("group_shard_failures", "false"));
-        try (XContentBuilder builder = jsonBuilder()) {
-            builder.startObject();
-            exception.toXContent(builder, params);
-            builder.endObject();
-
-            assertEquals("{" +
-                    "\"type\":\"search_phase_execution_exception\"," +
-                    "\"reason\":\"all shards failed\"," +
-                    "\"phase\":\"test\"," +
-                    "\"grouped\":false," +
-                    "\"failed_shards\":[" +
-                            "{" +
-                                "\"shard\":0," +
-                                "\"index\":\"foo\"," +
-                                "\"node\":\"node_1\"," +
-                                "\"reason\":{" +
-                                            "\"type\":\"parsing_exception\"," +
-                                            "\"reason\":\"foobar\"," +
-                                            "\"line\":1," +
-                                            "\"col\":2" +
-                                "}" +
-                            "}," +
-                            "{" +
-                                "\"shard\":1," +
-                                "\"index\":\"foo\"," +
-                                "\"node\":\"node_2\"," +
-                                "\"reason\":{" +
-                                            "\"type\":\"index_shard_closed_exception\"," +
-                                            "\"reason\":\"CurrentState[CLOSED] Closed\"," +
-                                            "\"index_uuid\":\"_na_\"," +
-                                            "\"shard\":\"1\"," +
-                                            "\"index\":\"foo\"" +
-                                "}" +
-                            "}," +
-                            "{" +
-                                "\"shard\":2," +
-                                "\"index\":\"foo\"," +
-                                "\"node\":\"node_3\"," +
-                                "\"reason\":{" +
-                                            "\"type\":\"parsing_exception\"," +
-                                            "\"reason\":\"foobar\"," +
-                                            "\"line\":5," +
-                                            "\"col\":7" +
-                                "}" +
-                            "}" +
-                    "]}", Strings.toString(builder));
-        }
     }
 
     public void testToAndFromXContent() throws IOException {

server/src/test/java/org/elasticsearch/action/support/broadcast/AbstractBroadcastResponseTestCase.java

@@ -33,7 +33,6 @@
 
 import java.io.IOException;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.List;
 
 import static org.hamcrest.CoreMatchers.anyOf;
@@ -130,29 +129,6 @@ public void testFailuresDeduplication() throws IOException {
         assertThat(parsedFailures[1].shardId(), equalTo(2));
         assertThat(parsedFailures[1].status(), equalTo(RestStatus.INTERNAL_SERVER_ERROR));
         assertThat(parsedFailures[1].getCause().getMessage(), containsString("fizz"));
-
-        ToXContent.Params params = new ToXContent.MapParams(Collections.singletonMap("group_shard_failures", "false"));
-        BytesReference bytesReferenceWithoutDedup = toShuffledXContent(response, xContentType, params, humanReadable);
-        try(XContentParser parser = createParser(xContentType.xContent(), bytesReferenceWithoutDedup)) {
-            parsedResponse = doParseInstance(parser);
-            assertNull(parser.nextToken());
-        }
-
-        assertThat(parsedResponse.getShardFailures().length, equalTo(3));
-        parsedFailures = parsedResponse.getShardFailures();
-        for (int i = 0; i < 3; i++) {
-            if (i < 2) {
-                assertThat(parsedFailures[i].index(), equalTo("test"));
-                assertThat(parsedFailures[i].shardId(), equalTo(i));
-                assertThat(parsedFailures[i].status(), equalTo(RestStatus.INTERNAL_SERVER_ERROR));
-                assertThat(parsedFailures[i].getCause().getMessage(), containsString("foo"));
-            } else {
-                assertThat(parsedFailures[i].index(), equalTo("test"));
-                assertThat(parsedFailures[i].shardId(), equalTo(i));
-                assertThat(parsedFailures[i].status(), equalTo(RestStatus.INTERNAL_SERVER_ERROR));
-                assertThat(parsedFailures[i].getCause().getMessage(), containsString("fizz"));
-            }
-        }
     }
 
     public void testToXContent() {