Stop returning cluster state size by default (#40016)

Computing the compressed size of the cluster state on every invocation
of cluster:monitor/state action is expensive, and the value of this
field is dubious anyway. Therefore we want to remove computing this
field. As a first step, we stop computing and return this field by
default. To avoid breaking users, we will give them a system property to
use to tide them over until the next major release when we will actually
remove this field. This comes with a deprecation warning too, and the
backport to the appropriate minor will also include a note in the
migration guide. There will be a follow-up to remove this field in the
next major version.

Author: jasontedor

qa/cluster-state-size/build.gradle

@@ -0,0 +1,33 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.elasticsearch.gradle.test.RestIntegTestTask
+
+apply plugin: 'elasticsearch.standalone-test'
+
+task integTest(type: RestIntegTestTask) {
+    mustRunAfter(precommit)
+}
+
+integTestCluster {
+    systemProperty 'es.cluster_state.size', 'true'
+}
+
+unitTest.enabled = false
+check.dependsOn integTest

qa/cluster-state-size/src/test/java/org/elasticsearch/cluster_state_size/ClusterStateSizeRestIT.java

@@ -0,0 +1,62 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.elasticsearch.cluster_state_size;
+
+import com.carrotsearch.randomizedtesting.annotations.ParametersFactory;
+import org.elasticsearch.Version;
+import org.elasticsearch.client.RequestOptions;
+import org.elasticsearch.test.rest.yaml.ClientYamlTestCandidate;
+import org.elasticsearch.test.rest.yaml.ESClientYamlSuiteTestCase;
+
+import java.util.Collections;
+
+public class ClusterStateSizeRestIT extends ESClientYamlSuiteTestCase {
+
+    public ClusterStateSizeRestIT(final ClientYamlTestCandidate candidate) {
+        super(candidate);
+    }
+
+    @ParametersFactory
+    public static Iterable<Object[]> parameters() throws Exception {
+        return ESClientYamlSuiteTestCase.createParameters();
+    }
+
+    @Override
+    protected RequestOptions getCatNodesVersionMasterRequestOptions() {
+        final RequestOptions.Builder builder = RequestOptions.DEFAULT.toBuilder();
+        final VersionSensitiveWarningsHandler handler = new VersionSensitiveWarningsHandler(Collections.singleton(Version.CURRENT));
+        handler.current("es.cluster_state.size is deprecated and will be removed in 7.0.0");
+        builder.setWarningsHandler(handler);
+        return builder.build();
+    }
+
+    @Override
+    protected boolean preserveClusterSettings() {
+        // we did not add any cluster settings, we are the only test using this cluster, and trying to clean these will trigger a warning
+        return true;
+    }
+
+    @Override
+    protected boolean preserveTemplatesUponCompletion() {
+        // we did not add any templates, we are the only test using this cluster, and trying to clean these will trigger a warning
+        return true;
+    }
+
+}

qa/cluster-state-size/src/test/resources/rest-api-spec/test/10_basic.yml

@@ -0,0 +1,15 @@
+---
+"get cluster state returns cluster state size with human readable format":
+  - skip:
+      reason: deprecation was added in 6.7.0
+      features: "warnings"
+
+  - do:
+      warnings:
+        - 'es.cluster_state.size is deprecated and will be removed in 7.0.0'
+      cluster.state:
+        human: true
+
+  - is_true: master_node
+  - gte: { compressed_size_in_bytes: 50 }
+  - is_true: compressed_size

rest-api-spec/src/main/resources/rest-api-spec/test/cluster.state/10_basic.yml

@@ -6,14 +6,17 @@
   - is_true: master_node
 
 ---
-"get cluster state returns cluster state size with human readable format":
+"get cluster state does not return cluster state size by default":
+  - skip:
+      version: " - 7.99.99"
+      reason: "backwards compatibility break in 6.7.0 removes compressed_size* from the response"
   - do:
       cluster.state:
         human: true
 
   - is_true: master_node
-  - gte: { compressed_size_in_bytes: 50 }
-  - is_true: compressed_size
+  - is_false: compressed_size_in_bytes
+  - is_false: compressed_size
 
 ---
 "get cluster state returns cluster_uuid at the top level":
@@ -27,5 +30,3 @@
 
   - is_true: cluster_uuid
   - is_true: master_node
-  - gte: { compressed_size_in_bytes: 50 }
-  - is_true: compressed_size

server/src/main/java/org/elasticsearch/action/admin/cluster/state/ClusterStateResponse.java

@@ -73,6 +73,7 @@ public ClusterName getClusterName() {
      * of the cluster state as it would be transmitted over the network during
      * intra-node communication.
      */
+    @Deprecated
     public ByteSizeValue getTotalCompressedSize() {
         return totalCompressedSize;
     }

server/src/main/java/org/elasticsearch/action/admin/cluster/state/TransportClusterStateAction.java

@@ -20,6 +20,8 @@
 package org.elasticsearch.action.admin.cluster.state;
 
 import com.carrotsearch.hppc.cursors.ObjectObjectCursor;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 import org.elasticsearch.Version;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.support.ActionFilters;
@@ -35,6 +37,7 @@
 import org.elasticsearch.cluster.routing.RoutingTable;
 import org.elasticsearch.cluster.service.ClusterService;
 import org.elasticsearch.common.inject.Inject;
+import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.unit.TimeValue;
 import org.elasticsearch.node.NodeClosedException;
 import org.elasticsearch.threadpool.ThreadPool;
@@ -45,6 +48,24 @@
 
 public class TransportClusterStateAction extends TransportMasterNodeReadAction<ClusterStateRequest, ClusterStateResponse> {
 
+    private final Logger logger = LogManager.getLogger(getClass());
+    private final DeprecationLogger deprecationLogger = new DeprecationLogger(logger);
+
+    public static final boolean CLUSTER_STATE_SIZE;
+
+    static {
+        final String property = System.getProperty("es.cluster_state.size");
+        if (property == null) {
+            CLUSTER_STATE_SIZE = false;
+        } else {
+            final boolean clusterStateSize = Boolean.parseBoolean(property);
+            if (clusterStateSize) {
+                CLUSTER_STATE_SIZE = true;
+            } else {
+                throw new IllegalArgumentException("es.cluster_state.size can only be unset or [true] but was [" + property + "]");
+            }
+        }
+    }
 
     @Inject
     public TransportClusterStateAction(TransportService transportService, ClusterService clusterService,
@@ -182,8 +203,16 @@ private void buildResponse(final ClusterStateRequest request,
                 }
             }
         }
-        listener.onResponse(new ClusterStateResponse(currentState.getClusterName(), builder.build(),
-            PublicationTransportHandler.serializeFullClusterState(currentState, Version.CURRENT).length(), false));
+
+        final long sizeInBytes;
+        if (CLUSTER_STATE_SIZE) {
+            deprecationLogger.deprecated("es.cluster_state.size is deprecated and will be removed in 7.0.0");
+            sizeInBytes = PublicationTransportHandler.serializeFullClusterState(currentState, Version.CURRENT).length();
+        } else {
+            sizeInBytes = 0;
+        }
+
+        listener.onResponse(new ClusterStateResponse(currentState.getClusterName(), builder.build(), sizeInBytes, false));
     }
 
 

server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestClusterStateAction.java

@@ -21,6 +21,7 @@
 
 import org.elasticsearch.action.admin.cluster.state.ClusterStateRequest;
 import org.elasticsearch.action.admin.cluster.state.ClusterStateResponse;
+import org.elasticsearch.action.admin.cluster.state.TransportClusterStateAction;
 import org.elasticsearch.action.support.IndicesOptions;
 import org.elasticsearch.client.Requests;
 import org.elasticsearch.client.node.NodeClient;
@@ -102,8 +103,12 @@ public RestResponse buildResponse(ClusterStateResponse response, XContentBuilder
                     builder.field(Fields.WAIT_FOR_TIMED_OUT, response.isWaitForTimedOut());
                 }
                 builder.field(Fields.CLUSTER_NAME, response.getClusterName().value());
-                builder.humanReadableField(Fields.CLUSTER_STATE_SIZE_IN_BYTES, Fields.CLUSTER_STATE_SIZE,
-                        response.getTotalCompressedSize());
+                if (TransportClusterStateAction.CLUSTER_STATE_SIZE) {
+                    builder.humanReadableField(
+                            Fields.CLUSTER_STATE_SIZE_IN_BYTES,
+                            Fields.CLUSTER_STATE_SIZE,
+                            response.getTotalCompressedSize());
+                }
                 response.getState().toXContent(builder, request);
                 builder.endObject();
                 return new BytesRestResponse(RestStatus.OK, builder);

server/src/test/java/org/elasticsearch/cluster/GetClusterStateTests.java

@@ -22,7 +22,7 @@
 import org.elasticsearch.action.admin.cluster.state.ClusterStateResponse;
 import org.elasticsearch.test.ESSingleNodeTestCase;
 
-import static org.hamcrest.Matchers.greaterThanOrEqualTo;
+import static org.hamcrest.Matchers.equalTo;
 
 /**
  * Tests for the get cluster state API.
@@ -36,8 +36,8 @@ public void testGetClusterState() {
         ClusterStateResponse response = client().admin().cluster().prepareState().get();
         assertNotNull(response.getState());
         assertNotNull(response.getClusterName());
-        // assume the cluster state size is 50 bytes or more, just so we aren't testing against size of 0
-        assertThat(response.getTotalCompressedSize().getBytes(), greaterThanOrEqualTo(50L));
+        // the cluster state size is no longer computed by default
+        assertThat(response.getTotalCompressedSize().getBytes(), equalTo(0L));
     }
 
     public void testSizeDerivedFromFullClusterState() {

test/framework/src/main/java/org/elasticsearch/test/rest/yaml/ESClientYamlSuiteTestCase.java

@@ -24,6 +24,7 @@
 import org.elasticsearch.Version;
 import org.elasticsearch.client.Node;
 import org.elasticsearch.client.Request;
+import org.elasticsearch.client.RequestOptions;
 import org.elasticsearch.client.Response;
 import org.elasticsearch.client.RestClient;
 import org.elasticsearch.client.RestClientBuilder;
@@ -291,10 +292,11 @@ private static void validateSpec(ClientYamlSuiteRestSpec restSpec) {
         }
     }
 
-    private static Tuple<Version, Version> readVersionsFromCatNodes(RestClient restClient) throws IOException {
+    private Tuple<Version, Version> readVersionsFromCatNodes(RestClient restClient) throws IOException {
         // we simply go to the _cat/nodes API and parse all versions in the cluster
-        Request request = new Request("GET", "/_cat/nodes");
+        final Request request = new Request("GET", "/_cat/nodes");
         request.addParameter("h", "version,master");
+        request.setOptions(getCatNodesVersionMasterRequestOptions());
         Response response = restClient.performRequest(request);
         ClientYamlTestResponse restTestResponse = new ClientYamlTestResponse(response);
         String nodesCatResponse = restTestResponse.getBodyAsString();
@@ -319,6 +321,10 @@ private static Tuple<Version, Version> readVersionsFromCatNodes(RestClient restC
         return new Tuple<>(version, masterVersion);
     }
 
+    protected RequestOptions getCatNodesVersionMasterRequestOptions() {
+        return RequestOptions.DEFAULT;
+    }
+
     public void test() throws IOException {
         //skip test if it matches one of the blacklist globs
         for (BlacklistedPathPatternMatcher blacklistedPathMatcher : blacklistPathMatchers) {