Remove cluster state size (#40061)

This commit removes the cluster state size field from the cluster state response, and drops the backwards compatibility layer added in 6.7.0 to continue to support this field. As calculation of this field was expensive and had dubious value, we have elected to remove this field.
elastic · Mar 15, 2019 · 0195626 · 0195626
1 parent c079729
commit 0195626
Show file tree

Hide file tree

Showing 17 changed files with 30 additions and 248 deletions.
diff --git a/build.gradle b/build.gradle
@@ -162,8 +162,8 @@ task verifyVersions {
  * after the backport of the backcompat code is complete.
  */
 
-boolean bwc_tests_enabled = true
-final String bwc_tests_disabled_issue = "" /* place a PR link here when committing bwc changes */
+boolean bwc_tests_enabled = false
+final String bwc_tests_disabled_issue = "https://github.com/elastic/elasticsearch/pull/40061" /* place a PR link here when committing bwc changes */
 if (bwc_tests_enabled == false) {
   if (bwc_tests_disabled_issue.isEmpty()) {
     throw new GradleException("bwc_tests_disabled_issue must be set when bwc_tests_enabled == false")

diff --git a/...usters/src/test/java/org/elasticsearch/search/CrossClusterSearchUnavailableClusterIT.java b/...usters/src/test/java/org/elasticsearch/search/CrossClusterSearchUnavailableClusterIT.java
@@ -127,7 +127,7 @@ private static MockTransportService startTransport(
                             builder.add(node);
                         }
                         ClusterState build = ClusterState.builder(clusterName).nodes(builder.build()).build();
-                        channel.sendResponse(new ClusterStateResponse(clusterName, build, 0L, false));
+                        channel.sendResponse(new ClusterStateResponse(clusterName, build, false));
                     });
             newService.start();
             newService.acceptIncomingRequests();

diff --git a/qa/cluster-state-size/build.gradle b/qa/cluster-state-size/build.gradle
diff --git a/...state-size/src/test/java/org/elasticsearch/cluster_state_size/ClusterStateSizeRestIT.java b/...state-size/src/test/java/org/elasticsearch/cluster_state_size/ClusterStateSizeRestIT.java
diff --git a/qa/cluster-state-size/src/test/resources/rest-api-spec/test/10_basic.yml b/qa/cluster-state-size/src/test/resources/rest-api-spec/test/10_basic.yml
diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/cluster.state/10_basic.yml b/rest-api-spec/src/main/resources/rest-api-spec/test/cluster.state/10_basic.yml
@@ -5,19 +5,6 @@
 
   - is_true: master_node
 
----
-"get cluster state does not return cluster state size by default":
-  - skip:
-      version: " - 6.6.99"
-      reason: "backwards compatibility break in 6.7.0 removes compressed_size* from the response"
-  - do:
-      cluster.state:
-        human: true
-
-  - is_true: master_node
-  - is_false: compressed_size_in_bytes
-  - is_false: compressed_size
-
 ---
 "get cluster state returns cluster_uuid at the top level":
   - skip:

diff --git a/server/src/main/java/org/elasticsearch/action/admin/cluster/state/ClusterStateResponse.java b/server/src/main/java/org/elasticsearch/action/admin/cluster/state/ClusterStateResponse.java
@@ -19,6 +19,7 @@
 
 package org.elasticsearch.action.admin.cluster.state;
 
+import org.elasticsearch.Version;
 import org.elasticsearch.action.ActionResponse;
 import org.elasticsearch.cluster.ClusterName;
 import org.elasticsearch.cluster.ClusterState;
@@ -37,18 +38,14 @@ public class ClusterStateResponse extends ActionResponse {
 
     private ClusterName clusterName;
     private ClusterState clusterState;
-    // the total compressed size of the full cluster state, not just
-    // the parts included in this response
-    private ByteSizeValue totalCompressedSize;
     private boolean waitForTimedOut = false;
 
     public ClusterStateResponse() {
     }
 
-    public ClusterStateResponse(ClusterName clusterName, ClusterState clusterState, long sizeInBytes, boolean waitForTimedOut) {
+    public ClusterStateResponse(ClusterName clusterName, ClusterState clusterState, boolean waitForTimedOut) {
         this.clusterName = clusterName;
         this.clusterState = clusterState;
-        this.totalCompressedSize = new ByteSizeValue(sizeInBytes);
         this.waitForTimedOut = waitForTimedOut;
     }
 
@@ -67,17 +64,6 @@ public ClusterName getClusterName() {
         return this.clusterName;
     }
 
-    /**
-     * The total compressed size of the full cluster state, not just the parts
-     * returned by {@link #getState()}.  The total compressed size is the size
-     * of the cluster state as it would be transmitted over the network during
-     * intra-node communication.
-     */
-    @Deprecated
-    public ByteSizeValue getTotalCompressedSize() {
-        return totalCompressedSize;
-    }
-
     /**
      * Returns whether the request timed out waiting for a cluster state with a metadata version equal or
      * higher than the specified metadata.
@@ -91,7 +77,9 @@ public void readFrom(StreamInput in) throws IOException {
         super.readFrom(in);
         clusterName = new ClusterName(in);
         clusterState = in.readOptionalWriteable(innerIn -> ClusterState.readFrom(innerIn, null));
-        totalCompressedSize = new ByteSizeValue(in);
+        if (in.getVersion().before(Version.V_7_0_0)) {
+            new ByteSizeValue(in);
+        }
         waitForTimedOut = in.readBoolean();
     }
 
@@ -100,7 +88,9 @@ public void writeTo(StreamOutput out) throws IOException {
         super.writeTo(out);
         clusterName.writeTo(out);
         out.writeOptionalWriteable(clusterState);
-        totalCompressedSize.writeTo(out);
+        if (out.getVersion().before(Version.V_7_0_0)) {
+            ByteSizeValue.ZERO.writeTo(out);
+        }
         out.writeBoolean(waitForTimedOut);
     }
 
@@ -114,8 +104,7 @@ public boolean equals(Object o) {
             // Best effort. Only compare cluster state version and master node id,
             // because cluster state doesn't implement equals()
             Objects.equals(getVersion(clusterState), getVersion(response.clusterState)) &&
-            Objects.equals(getMasterNodeId(clusterState), getMasterNodeId(response.clusterState)) &&
-            Objects.equals(totalCompressedSize, response.totalCompressedSize);
+            Objects.equals(getMasterNodeId(clusterState), getMasterNodeId(response.clusterState));
     }
 
     @Override
@@ -126,7 +115,6 @@ public int hashCode() {
             clusterName,
             getVersion(clusterState),
             getMasterNodeId(clusterState),
-            totalCompressedSize,
             waitForTimedOut
         );
     }

diff --git a/...c/main/java/org/elasticsearch/action/admin/cluster/state/TransportClusterStateAction.java b/...c/main/java/org/elasticsearch/action/admin/cluster/state/TransportClusterStateAction.java
@@ -22,22 +22,19 @@
 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;
 import org.elasticsearch.action.support.master.TransportMasterNodeReadAction;
 import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.ClusterStateObserver;
 import org.elasticsearch.cluster.block.ClusterBlockException;
-import org.elasticsearch.cluster.coordination.PublicationTransportHandler;
 import org.elasticsearch.cluster.metadata.IndexMetaData;
 import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
 import org.elasticsearch.cluster.metadata.MetaData;
 import org.elasticsearch.cluster.metadata.MetaData.Custom;
 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;
@@ -49,21 +46,11 @@
 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 + "]");
-            }
+        if (property != null) {
+                throw new IllegalArgumentException("es.cluster_state.size is no longer respected but was [" + property + "]");
         }
     }
 
@@ -127,7 +114,7 @@ public void onClusterServiceClose() {
                     @Override
                     public void onTimeout(TimeValue timeout) {
                         try {
-                            listener.onResponse(new ClusterStateResponse(clusterState.getClusterName(), null, 0L, true));
+                            listener.onResponse(new ClusterStateResponse(clusterState.getClusterName(), null, true));
                         } catch (Exception e) {
                             listener.onFailure(e);
                         }
@@ -204,15 +191,7 @@ private void buildResponse(final ClusterStateRequest request,
             }
         }
 
-        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));
+        listener.onResponse(new ClusterStateResponse(currentState.getClusterName(), builder.build(), false));
     }
 
 

diff --git a/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestClusterStateAction.java b/server/src/main/java/org/elasticsearch/rest/action/admin/cluster/RestClusterStateAction.java
@@ -21,7 +21,6 @@
 
 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;
@@ -103,12 +102,6 @@ public RestResponse buildResponse(ClusterStateResponse response, XContentBuilder
                     builder.field(Fields.WAIT_FOR_TIMED_OUT, response.isWaitForTimedOut());
                 }
                 builder.field(Fields.CLUSTER_NAME, response.getClusterName().value());
-                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);
@@ -138,7 +131,6 @@ public boolean canTripCircuitBreaker() {
     static final class Fields {
         static final String WAIT_FOR_TIMED_OUT = "wait_for_timed_out";
         static final String CLUSTER_NAME = "cluster_name";
-        static final String CLUSTER_STATE_SIZE = "compressed_size";
-        static final String CLUSTER_STATE_SIZE_IN_BYTES = "compressed_size_in_bytes";
     }
+
 }
diff --git a/...src/test/java/org/elasticsearch/action/admin/cluster/state/ClusterStateResponseTests.java b/...src/test/java/org/elasticsearch/action/admin/cluster/state/ClusterStateResponseTests.java
@@ -45,7 +45,7 @@ protected ClusterStateResponse createTestInstance() {
             }
             clusterState = clusterStateBuilder.build();
         }
-        return new ClusterStateResponse(clusterName, clusterState, randomNonNegativeLong(), randomBoolean());
+        return new ClusterStateResponse(clusterName, clusterState, randomBoolean());
     }
 
     @Override

diff --git a/server/src/test/java/org/elasticsearch/client/transport/FailAndRetryMockTransport.java b/server/src/test/java/org/elasticsearch/client/transport/FailAndRetryMockTransport.java
@@ -101,7 +101,7 @@ public void sendRequest(long requestId, String action, TransportRequest request,
                     } else if (ClusterStateAction.NAME.equals(action)) {
                         TransportResponseHandler transportResponseHandler = responseHandlers.onResponseReceived(requestId, listener);
                         ClusterState clusterState = getMockClusterState(node);
-                        transportResponseHandler.handleResponse(new ClusterStateResponse(clusterName, clusterState, 0L, false));
+                        transportResponseHandler.handleResponse(new ClusterStateResponse(clusterName, clusterState, false));
                     } else if (TransportService.HANDSHAKE_ACTION_NAME.equals(action)) {
                         TransportResponseHandler transportResponseHandler = responseHandlers.onResponseReceived(requestId, listener);
                         Version version = node.getVersion();

diff --git a/server/src/test/java/org/elasticsearch/client/transport/TransportClientHeadersTests.java b/server/src/test/java/org/elasticsearch/client/transport/TransportClientHeadersTests.java
@@ -175,7 +175,7 @@ public <T extends TransportResponse> void sendRequest(Transport.Connection conne
                             address.address().getHostString(), address.getAddress(), address, Collections.emptyMap(),
                                 Collections.singleton(DiscoveryNode.Role.DATA), Version.CURRENT)));
                         ((TransportResponseHandler<ClusterStateResponse>) handler)
-                                .handleResponse(new ClusterStateResponse(cluster1, builder.build(), 0L, false));
+                                .handleResponse(new ClusterStateResponse(cluster1, builder.build(), false));
                         clusterStateLatch.countDown();
                     } else if (TransportService.HANDSHAKE_ACTION_NAME .equals(action)) {
                         ((TransportResponseHandler<TransportService.HandshakeResponse>) handler).handleResponse(

diff --git a/...er/src/test/java/org/elasticsearch/client/transport/TransportClientNodesServiceTests.java b/...er/src/test/java/org/elasticsearch/client/transport/TransportClientNodesServiceTests.java
@@ -427,7 +427,7 @@ public void messageReceived(ClusterStateRequest request, TransportChannel channe
 
             DiscoveryNodes discoveryNodes = DiscoveryNodes.builder().add(transportService.getLocalDiscoNode()).build();
             ClusterState build = ClusterState.builder(ClusterName.DEFAULT).nodes(discoveryNodes).build();
-            channel.sendResponse(new ClusterStateResponse(ClusterName.DEFAULT, build, 0L, false));
+            channel.sendResponse(new ClusterStateResponse(ClusterName.DEFAULT, build, false));
         }
 
         void failToRespond() {