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.
This commit is contained in:
Jason Tedor 2019-03-14 08:56:19 -04:00
parent d02bca1314
commit 9181668edf
No known key found for this signature in database
GPG Key ID: FA89F05560F16BC5
9 changed files with 166 additions and 14 deletions

View File

@ -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

View File

@ -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;
}
}

View File

@ -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

View File

@ -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.0.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

View File

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

View File

@ -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.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;
@ -45,6 +48,24 @@ import java.util.function.Predicate;
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,
@ -183,8 +204,16 @@ public class TransportClusterStateAction extends TransportMasterNodeReadAction<C
}
}
}
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));
}

View File

@ -21,6 +21,7 @@ package org.elasticsearch.rest.action.admin.cluster;
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 class RestClusterStateAction extends BaseRestHandler {
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,
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);

View File

@ -22,7 +22,7 @@ package org.elasticsearch.cluster;
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 class GetClusterStateTests extends ESSingleNodeTestCase {
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() {

View File

@ -24,6 +24,7 @@ import org.apache.http.HttpHost;
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 @@ public abstract class ESClientYamlSuiteTestCase extends ESRestTestCase {
}
}
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 @@ public abstract class ESClientYamlSuiteTestCase extends ESRestTestCase {
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) {