Revert "Chunked encoding for RestGetIndicesAction (#92016)" (#92033)

This reverts commit 75de8f8 due to #92032 and elastic/kibana#146758
elastic · Dec 1, 2022 · 91d69d9 · 91d69d9
1 parent 2b8fde6
commit 91d69d9
Show file tree

Hide file tree

Showing 6 changed files with 66 additions and 77 deletions.
diff --git a/...ransport-netty4/src/javaRestTest/java/org/elasticsearch/rest/Netty4HeadBodyIsEmptyIT.java b/...ransport-netty4/src/javaRestTest/java/org/elasticsearch/rest/Netty4HeadBodyIsEmptyIT.java
@@ -25,7 +25,6 @@
 import static org.elasticsearch.rest.RestStatus.OK;
 import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder;
 import static org.hamcrest.Matchers.greaterThan;
-import static org.hamcrest.Matchers.nullValue;
 
 public class Netty4HeadBodyIsEmptyIT extends ESRestTestCase {
     public void testHeadRoot() throws IOException {
@@ -60,8 +59,8 @@ public void testDocumentExists() throws IOException {
 
     public void testIndexExists() throws IOException {
         createTestDoc();
-        headTestCase("/test", emptyMap(), nullValue(Integer.class));
-        headTestCase("/test", singletonMap("pretty", "true"), nullValue(Integer.class));
+        headTestCase("/test", emptyMap(), greaterThan(0));
+        headTestCase("/test", singletonMap("pretty", "true"), greaterThan(0));
     }
 
     public void testAliasExists() throws IOException {
@@ -178,8 +177,7 @@ private void headTestCase(
         request.setOptions(expectWarnings(expectedWarnings));
         Response response = client().performRequest(request);
         assertEquals(expectedStatusCode, response.getStatusLine().getStatusCode());
-        final var contentLength = response.getHeader("Content-Length");
-        assertThat(contentLength == null ? null : Integer.valueOf(contentLength), matcher);
+        assertThat(Integer.valueOf(response.getHeader("Content-Length")), matcher);
         assertNull("HEAD requests shouldn't have a response body but " + url + " did", response.getEntity());
     }
 

diff --git a/server/src/main/java/org/elasticsearch/action/ActionModule.java b/server/src/main/java/org/elasticsearch/action/ActionModule.java
@@ -768,7 +768,7 @@ public void initRestHandlers(Supplier<DiscoveryNodes> nodesInCluster) {
         registerHandler.accept(new RestResetFeatureStateAction());
         registerHandler.accept(new RestGetFeatureUpgradeStatusAction());
         registerHandler.accept(new RestPostFeatureUpgradeAction());
-        registerHandler.accept(new RestGetIndicesAction());
+        registerHandler.accept(new RestGetIndicesAction(threadPool));
         registerHandler.accept(new RestIndicesStatsAction());
         registerHandler.accept(new RestIndicesSegmentsAction());
         registerHandler.accept(new RestIndicesShardStoresAction());

diff --git a/server/src/main/java/org/elasticsearch/action/admin/indices/get/GetIndexResponse.java b/server/src/main/java/org/elasticsearch/action/admin/indices/get/GetIndexResponse.java
@@ -13,18 +13,16 @@
 import org.elasticsearch.cluster.metadata.AliasMetadata;
 import org.elasticsearch.cluster.metadata.MappingMetadata;
 import org.elasticsearch.common.Strings;
-import org.elasticsearch.common.collect.Iterators;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.common.xcontent.ChunkedToXContent;
 import org.elasticsearch.core.RestApiVersion;
 import org.elasticsearch.index.mapper.MapperService;
-import org.elasticsearch.xcontent.ToXContent;
+import org.elasticsearch.xcontent.ToXContentObject;
+import org.elasticsearch.xcontent.XContentBuilder;
 
 import java.io.IOException;
 import java.util.Arrays;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
@@ -35,7 +33,7 @@
 /**
  * A response for a get index action.
  */
-public class GetIndexResponse extends ActionResponse implements ChunkedToXContent {
+public class GetIndexResponse extends ActionResponse implements ToXContentObject {
 
     private Map<String, MappingMetadata> mappings = Map.of();
     private Map<String, List<AliasMetadata>> aliases = Map.of();
@@ -180,58 +178,59 @@ public void writeTo(StreamOutput out) throws IOException {
     }
 
     @Override
-    public Iterator<? extends ToXContent> toXContentChunked(ToXContent.Params ignored) {
-        return Iterators.concat(
-            Iterators.single((builder, params) -> builder.startObject()),
-            Arrays.stream(indices).<ToXContent>map(index -> (builder, params) -> {
+    public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {
+        builder.startObject();
+        {
+            for (final String index : indices) {
                 builder.startObject(index);
-
-                builder.startObject("aliases");
-                List<AliasMetadata> indexAliases = aliases.get(index);
-                if (indexAliases != null) {
-                    for (final AliasMetadata alias : indexAliases) {
-                        AliasMetadata.Builder.toXContent(alias, builder, params);
+                {
+                    builder.startObject("aliases");
+                    List<AliasMetadata> indexAliases = aliases.get(index);
+                    if (indexAliases != null) {
+                        for (final AliasMetadata alias : indexAliases) {
+                            AliasMetadata.Builder.toXContent(alias, builder, params);
+                        }
                     }
-                }
-                builder.endObject();
+                    builder.endObject();
 
-                MappingMetadata indexMappings = mappings.get(index);
-                if (indexMappings == null) {
-                    builder.startObject("mappings").endObject();
-                } else {
-                    if (builder.getRestApiVersion() == RestApiVersion.V_7
-                        && params.paramAsBoolean(INCLUDE_TYPE_NAME_PARAMETER, DEFAULT_INCLUDE_TYPE_NAME_POLICY)) {
-                        builder.startObject("mappings");
-                        builder.field(MapperService.SINGLE_MAPPING_NAME, indexMappings.sourceAsMap());
-                        builder.endObject();
+                    MappingMetadata indexMappings = mappings.get(index);
+                    if (indexMappings == null) {
+                        builder.startObject("mappings").endObject();
                     } else {
-                        builder.field("mappings", indexMappings.sourceAsMap());
+                        if (builder.getRestApiVersion() == RestApiVersion.V_7
+                            && params.paramAsBoolean(INCLUDE_TYPE_NAME_PARAMETER, DEFAULT_INCLUDE_TYPE_NAME_POLICY)) {
+                            builder.startObject("mappings");
+                            builder.field(MapperService.SINGLE_MAPPING_NAME, indexMappings.sourceAsMap());
+                            builder.endObject();
+                        } else {
+                            builder.field("mappings", indexMappings.sourceAsMap());
+                        }
                     }
-                }
 
-                builder.startObject("settings");
-                Settings indexSettings = settings.get(index);
-                if (indexSettings != null) {
-                    indexSettings.toXContent(builder, params);
-                }
-                builder.endObject();
-
-                Settings defaultIndexSettings = defaultSettings.get(index);
-                if (defaultIndexSettings != null && defaultIndexSettings.isEmpty() == false) {
-                    builder.startObject("defaults");
-                    defaultIndexSettings.toXContent(builder, params);
+                    builder.startObject("settings");
+                    Settings indexSettings = settings.get(index);
+                    if (indexSettings != null) {
+                        indexSettings.toXContent(builder, params);
+                    }
                     builder.endObject();
-                }
 
-                String dataStream = dataStreams.get(index);
-                if (dataStream != null) {
-                    builder.field("data_stream", dataStream);
-                }
+                    Settings defaultIndexSettings = defaultSettings.get(index);
+                    if (defaultIndexSettings != null && defaultIndexSettings.isEmpty() == false) {
+                        builder.startObject("defaults");
+                        defaultIndexSettings.toXContent(builder, params);
+                        builder.endObject();
+                    }
 
-                return builder.endObject();
-            }).iterator(),
-            Iterators.single((builder, params) -> builder.endObject())
-        );
+                    String dataStream = dataStreams.get(index);
+                    if (dataStream != null) {
+                        builder.field("data_stream", dataStream);
+                    }
+                }
+                builder.endObject();
+            }
+        }
+        builder.endObject();
+        return builder;
     }
 
     @Override

diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetIndicesAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/indices/RestGetIndicesAction.java
@@ -17,8 +17,9 @@
 import org.elasticsearch.core.RestApiVersion;
 import org.elasticsearch.rest.BaseRestHandler;
 import org.elasticsearch.rest.RestRequest;
+import org.elasticsearch.rest.action.DispatchingRestToXContentListener;
 import org.elasticsearch.rest.action.RestCancellableNodeClient;
-import org.elasticsearch.rest.action.RestChunkedToXContentListener;
+import org.elasticsearch.threadpool.ThreadPool;
 
 import java.io.IOException;
 import java.util.List;
@@ -38,6 +39,12 @@ public class RestGetIndicesAction extends BaseRestHandler {
 
     private static final Set<String> COMPATIBLE_RESPONSE_PARAMS = addToCopy(Settings.FORMAT_PARAMS, INCLUDE_TYPE_NAME_PARAMETER);
 
+    private final ThreadPool threadPool;
+
+    public RestGetIndicesAction(ThreadPool threadPool) {
+        this.threadPool = threadPool;
+    }
+
     @Override
     public List<Route> routes() {
         return List.of(new Route(GET, "/{index}"), new Route(HEAD, "/{index}"));
@@ -69,7 +76,10 @@ public RestChannelConsumer prepareRequest(final RestRequest request, final NodeC
         final var httpChannel = request.getHttpChannel();
         return channel -> new RestCancellableNodeClient(client, httpChannel).admin()
             .indices()
-            .getIndex(getIndexRequest, new RestChunkedToXContentListener<>(channel));
+            .getIndex(
+                getIndexRequest,
+                new DispatchingRestToXContentListener<>(threadPool.executor(ThreadPool.Names.MANAGEMENT), channel, request)
+            );
     }
 
     /**

diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/get/GetIndexResponseTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/get/GetIndexResponseTests.java
@@ -18,9 +18,7 @@
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.index.RandomCreateIndexGenerator;
 import org.elasticsearch.test.AbstractWireSerializingTestCase;
-import org.elasticsearch.xcontent.ToXContent;
 
-import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.Comparator;
@@ -29,9 +27,6 @@
 import java.util.Locale;
 import java.util.Map;
 
-import static org.elasticsearch.xcontent.ToXContent.EMPTY_PARAMS;
-import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder;
-
 public class GetIndexResponseTests extends AbstractWireSerializingTestCase<GetIndexResponse> {
 
     @Override
@@ -78,18 +73,4 @@ protected GetIndexResponse createTestInstance() {
         }
         return new GetIndexResponse(indices, mappings, aliases, settings, defaultSettings, dataStreams);
     }
-
-    public void testChunking() throws IOException {
-        final var response = createTestInstance();
-
-        try (var builder = jsonBuilder()) {
-            int chunkCount = 0;
-            final var iterator = response.toXContentChunked(EMPTY_PARAMS);
-            while (iterator.hasNext()) {
-                iterator.next().toXContent(builder, ToXContent.EMPTY_PARAMS);
-                chunkCount += 1;
-            }
-            assertEquals(response.getIndices().length + 2, chunkCount);
-        } // closing the builder verifies that the XContent is well-formed
-    }
 }
diff --git a/.../src/test/java/org/elasticsearch/rest/action/admin/indices/RestGetIndicesActionTests.java b/.../src/test/java/org/elasticsearch/rest/action/admin/indices/RestGetIndicesActionTests.java
@@ -9,6 +9,7 @@
 package org.elasticsearch.rest.action.admin.indices;
 
 import org.elasticsearch.client.internal.node.NodeClient;
+import org.elasticsearch.common.util.concurrent.DeterministicTaskQueue;
 import org.elasticsearch.core.RestApiVersion;
 import org.elasticsearch.rest.RestRequest;
 import org.elasticsearch.test.ESTestCase;
@@ -36,7 +37,7 @@ public void testIncludeTypeNamesWarning() throws IOException {
             Map.of("Content-Type", contentTypeHeader, "Accept", contentTypeHeader)
         ).withMethod(RestRequest.Method.GET).withPath("/some_index").withParams(params).build();
 
-        RestGetIndicesAction handler = new RestGetIndicesAction();
+        RestGetIndicesAction handler = new RestGetIndicesAction(new DeterministicTaskQueue().getThreadPool());
         handler.prepareRequest(request, mock(NodeClient.class));
         assertCriticalWarnings(RestGetIndicesAction.TYPES_DEPRECATION_MESSAGE);
 
@@ -57,7 +58,7 @@ public void testIncludeTypeNamesWarningExists() throws IOException {
             Map.of("Content-Type", contentTypeHeader, "Accept", contentTypeHeader)
         ).withMethod(RestRequest.Method.HEAD).withPath("/some_index").withParams(params).build();
 
-        RestGetIndicesAction handler = new RestGetIndicesAction();
+        RestGetIndicesAction handler = new RestGetIndicesAction(new DeterministicTaskQueue().getThreadPool());
         handler.prepareRequest(request, mock(NodeClient.class));
     }
 }