[7.16] Revert "Deprecate returning 408 for a server timeout on _cluster/health (#78180)" (#80831)

* Revert "Allow deprecation warning for the return_200_for_cluster_health_timeout parameter (#80178) (#80445)"

This reverts commit 5a5300753d.

* Revert "[7.x] Use query param instead of a system property for opting in for new cluster health response code (#79397) (#79435)"

This reverts commit d8c14281

* Revert "[7.x] Deprecate returning 408 for a server timeout on `_cluster/health` (#78180) (#78940)"

This reverts commit 03bd55d1
This commit is contained in:
Artem Prigoda 2021-11-18 16:57:39 +01:00 committed by GitHub
parent 19fb52f98c
commit 47da0dca20
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
16 changed files with 27 additions and 169 deletions

View file

@ -338,11 +338,6 @@ public class ClusterClientIT extends ESRestHighLevelClientTestCase {
assertThat(response.status(), equalTo(RestStatus.REQUEST_TIMEOUT));
assertThat(response.getStatus(), equalTo(ClusterHealthStatus.RED));
assertNoIndices(response);
assertWarnings(
"The HTTP status code for a cluster health timeout will be changed from 408 to 200 in a "
+ "future version. Set the [return_200_for_cluster_health_timeout] query parameter to [true] to suppress this message and "
+ "opt in to the future behaviour now."
);
}
public void testRemoteInfo() throws Exception {

View file

@ -97,11 +97,6 @@ include::{es-repo-dir}/rest-api/common-parms.asciidoc[tag=timeoutparms]
provided or better, i.e. `green` > `yellow` > `red`. By default, will not
wait for any status.
`return_200_for_cluster_health_timeout`::
(Optional, Boolean) A boolean value which controls whether to return HTTP 200
status code instead of HTTP 408 in case of a cluster health timeout from
the server side. Defaults to false.
[[cluster-health-api-response-body]]
==== {api-response-body-title}

View file

@ -340,27 +340,6 @@ by Elastic products and should not be accessed directly.
[[breaking_716_cluster_deprecations]]
==== Cluster deprecations
[[deprecate-cluster-health-408]]
.Distinguishing cluster health timeout status by HTTP response code is now deprecated.
[%collapsible]
====
*Details* +
The {ref}/cluster-health.html[cluster health API] includes options for waiting
for certain health conditions to be satisfied. If the requested conditions are
not satisfied within a timeout then {es} will send back a normal response
including the field `"timed_out": true`. In earlier versions it would also use
the HTTP response code `408 Request timeout` if the request timed out, and `200
OK` otherwise. The `408 Request timeout` response code is not appropriate for
this situation and its use is deprecated. Future versions will use the response
code `200 OK` for both cases.
*Impact* +
Update your application to read the `"timed_out"` field of the response instead
of the HTTP response code to determine whether the request timed out or not. To
avoid deprecation warnings and opt into the future behaviour, include the query
parameter `?return_200_for_cluster_health_timeout` in your request.
====
[[script-context-cache-deprecated]]
.The script context cache is deprecated.
[%collapsible]

View file

@ -102,10 +102,6 @@
"red"
],
"description":"Wait until cluster is in a specific state"
},
"return_200_for_cluster_health_timeout":{
"type":"boolean",
"description":"Whether to return HTTP 200 instead of 408 in case of a cluster health timeout from the server side"
}
}
}

View file

@ -35,28 +35,3 @@
- match: { initializing_shards: 0 }
- match: { unassigned_shards: 0 }
- gte: { number_of_pending_tasks: 0 }
---
"cluster health request timeout with 200 response code":
- skip:
version: " - 7.15.99"
reason: "return_200_for_cluster_health_timeout was added in 7.16"
features: [ "allowed_warnings" ]
- do:
allowed_warnings:
- 'the [return_200_for_cluster_health_timeout] parameter is deprecated and will be removed in a future release.'
cluster.health:
timeout: 1ms
wait_for_active_shards: 5
return_200_for_cluster_health_timeout: true
- is_true: cluster_name
- is_true: timed_out
- gte: { number_of_nodes: 1 }
- gte: { number_of_data_nodes: 1 }
- match: { active_primary_shards: 0 }
- match: { active_shards: 0 }
- match: { relocating_shards: 0 }
- match: { initializing_shards: 0 }
- match: { unassigned_shards: 0 }
- gte: { number_of_pending_tasks: 0 }

View file

@ -35,8 +35,6 @@ public class ClusterHealthRequest extends MasterNodeReadRequest<ClusterHealthReq
private ActiveShardCount waitForActiveShards = ActiveShardCount.NONE;
private String waitForNodes = "";
private Priority waitForEvents = null;
private boolean return200ForClusterHealthTimeout;
/**
* Only used by the high-level REST Client. Controls the details level of the health information returned.
* The default value is 'cluster'.
@ -70,9 +68,6 @@ public class ClusterHealthRequest extends MasterNodeReadRequest<ClusterHealthReq
} else {
indicesOptions = IndicesOptions.lenientExpandOpen();
}
if (in.getVersion().onOrAfter(Version.V_7_16_0)) {
return200ForClusterHealthTimeout = in.readBoolean();
}
}
@Override
@ -105,11 +100,6 @@ public class ClusterHealthRequest extends MasterNodeReadRequest<ClusterHealthReq
if (out.getVersion().onOrAfter(Version.V_7_2_0)) {
indicesOptions.writeIndicesOptions(out);
}
if (out.getVersion().onOrAfter(Version.V_7_16_0)) {
out.writeBoolean(return200ForClusterHealthTimeout);
} else if (return200ForClusterHealthTimeout) {
throw new IllegalArgumentException("Can't fix response code in a cluster involving nodes with version " + out.getVersion());
}
}
@Override
@ -253,18 +243,6 @@ public class ClusterHealthRequest extends MasterNodeReadRequest<ClusterHealthReq
return this.waitForEvents;
}
public boolean doesReturn200ForClusterHealthTimeout() {
return return200ForClusterHealthTimeout;
}
/**
* Sets whether to return HTTP 200 status code instead of HTTP 408 in case of a
* cluster health timeout from the server side.
*/
public void return200ForClusterHealthTimeout(boolean return200ForClusterHealthTimeout) {
this.return200ForClusterHealthTimeout = return200ForClusterHealthTimeout;
}
/**
* Set the level of detail for the health information to be returned.
* Only used by the high-level REST Client.

View file

@ -8,7 +8,6 @@
package org.elasticsearch.action.admin.cluster.health;
import org.elasticsearch.Version;
import org.elasticsearch.action.ActionResponse;
import org.elasticsearch.cluster.ClusterState;
import org.elasticsearch.cluster.health.ClusterHealthStatus;
@ -17,12 +16,9 @@ import org.elasticsearch.cluster.health.ClusterStateHealth;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
import org.elasticsearch.common.logging.DeprecationCategory;
import org.elasticsearch.common.logging.DeprecationLogger;
import org.elasticsearch.common.xcontent.StatusToXContentObject;
import org.elasticsearch.core.TimeValue;
import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.rest.action.search.RestSearchAction;
import org.elasticsearch.xcontent.ConstructingObjectParser;
import org.elasticsearch.xcontent.ObjectParser;
import org.elasticsearch.xcontent.ParseField;
@ -59,7 +55,6 @@ public class ClusterHealthResponse extends ActionResponse implements StatusToXCo
private static final String INITIALIZING_SHARDS = "initializing_shards";
private static final String UNASSIGNED_SHARDS = "unassigned_shards";
private static final String INDICES = "indices";
private static final DeprecationLogger deprecationLogger = DeprecationLogger.getLogger(RestSearchAction.class);
private static final ConstructingObjectParser<ClusterHealthResponse, Void> PARSER = new ConstructingObjectParser<>(
"cluster_health_response",
@ -124,12 +119,6 @@ public class ClusterHealthResponse extends ActionResponse implements StatusToXCo
XContentParser parser,
Void context,
String index) -> ClusterIndexHealth.innerFromXContent(parser, index);
static final String ES_CLUSTER_HEALTH_REQUEST_TIMEOUT_200_KEY = "return_200_for_cluster_health_timeout";
static final String CLUSTER_HEALTH_REQUEST_TIMEOUT_DEPRECATION_MSG = "The HTTP status code for a cluster health timeout "
+ "will be changed from 408 to 200 in a future version. Set the ["
+ ES_CLUSTER_HEALTH_REQUEST_TIMEOUT_200_KEY
+ "] "
+ "query parameter to [true] to suppress this message and opt in to the future behaviour now.";
static {
// ClusterStateHealth fields
@ -162,7 +151,8 @@ public class ClusterHealthResponse extends ActionResponse implements StatusToXCo
private boolean timedOut = false;
private ClusterStateHealth clusterStateHealth;
private ClusterHealthStatus clusterHealthStatus;
private boolean return200ForClusterHealthTimeout;
public ClusterHealthResponse() {}
public ClusterHealthResponse(StreamInput in) throws IOException {
super(in);
@ -174,19 +164,11 @@ public class ClusterHealthResponse extends ActionResponse implements StatusToXCo
numberOfInFlightFetch = in.readInt();
delayedUnassignedShards = in.readInt();
taskMaxWaitingTime = in.readTimeValue();
if (in.getVersion().onOrAfter(Version.V_7_16_0)) {
return200ForClusterHealthTimeout = in.readBoolean();
}
}
/** needed for plugins BWC */
public ClusterHealthResponse(
String clusterName,
String[] concreteIndices,
ClusterState clusterState,
boolean return200ForServerTimeout
) {
this(clusterName, concreteIndices, clusterState, -1, -1, -1, TimeValue.timeValueHours(0), return200ForServerTimeout);
public ClusterHealthResponse(String clusterName, String[] concreteIndices, ClusterState clusterState) {
this(clusterName, concreteIndices, clusterState, -1, -1, -1, TimeValue.timeValueHours(0));
}
public ClusterHealthResponse(
@ -196,8 +178,7 @@ public class ClusterHealthResponse extends ActionResponse implements StatusToXCo
int numberOfPendingTasks,
int numberOfInFlightFetch,
int delayedUnassignedShards,
TimeValue taskMaxWaitingTime,
boolean return200ForServerTimeout
TimeValue taskMaxWaitingTime
) {
this.clusterName = clusterName;
this.numberOfPendingTasks = numberOfPendingTasks;
@ -206,7 +187,6 @@ public class ClusterHealthResponse extends ActionResponse implements StatusToXCo
this.taskMaxWaitingTime = taskMaxWaitingTime;
this.clusterStateHealth = new ClusterStateHealth(clusterState, concreteIndices);
this.clusterHealthStatus = clusterStateHealth.getStatus();
this.return200ForClusterHealthTimeout = return200ForServerTimeout;
}
/**
@ -345,11 +325,6 @@ public class ClusterHealthResponse extends ActionResponse implements StatusToXCo
out.writeInt(numberOfInFlightFetch);
out.writeInt(delayedUnassignedShards);
out.writeTimeValue(taskMaxWaitingTime);
if (out.getVersion().onOrAfter(Version.V_7_16_0)) {
out.writeBoolean(return200ForClusterHealthTimeout);
} else if (return200ForClusterHealthTimeout) {
throw new IllegalArgumentException("Can't fix response code in a cluster involving nodes with version " + out.getVersion());
}
}
@Override
@ -359,19 +334,7 @@ public class ClusterHealthResponse extends ActionResponse implements StatusToXCo
@Override
public RestStatus status() {
if (isTimedOut() == false) {
return RestStatus.OK;
}
if (return200ForClusterHealthTimeout) {
return RestStatus.OK;
} else {
deprecationLogger.critical(
DeprecationCategory.API,
"cluster_health_request_timeout",
CLUSTER_HEALTH_REQUEST_TIMEOUT_DEPRECATION_MSG
);
return RestStatus.REQUEST_TIMEOUT;
}
return isTimedOut() ? RestStatus.REQUEST_TIMEOUT : RestStatus.OK;
}
@Override

View file

@ -418,8 +418,7 @@ public class TransportClusterHealthAction extends TransportMasterNodeReadAction<
numberOfPendingTasks,
numberOfInFlightFetch,
UnassignedInfo.getNumberOfDelayedUnassigned(clusterState),
pendingTaskTimeInQueue,
request.doesReturn200ForClusterHealthTimeout()
pendingTaskTimeInQueue
);
response.setStatus(ClusterHealthStatus.RED);
return response;
@ -432,8 +431,7 @@ public class TransportClusterHealthAction extends TransportMasterNodeReadAction<
numberOfPendingTasks,
numberOfInFlightFetch,
UnassignedInfo.getNumberOfDelayedUnassigned(clusterState),
pendingTaskTimeInQueue,
request.doesReturn200ForClusterHealthTimeout()
pendingTaskTimeInQueue
);
}
}

View file

@ -83,9 +83,6 @@ public class RestClusterHealthAction extends BaseRestHandler {
if (request.param("wait_for_events") != null) {
clusterHealthRequest.waitForEvents(Priority.valueOf(request.param("wait_for_events").toUpperCase(Locale.ROOT)));
}
clusterHealthRequest.return200ForClusterHealthTimeout(
request.paramAsBoolean("return_200_for_cluster_health_timeout", clusterHealthRequest.doesReturn200ForClusterHealthTimeout())
);
return clusterHealthRequest;
}

View file

@ -42,7 +42,6 @@ public class ClusterHealthRequestTests extends ESTestCase {
assertThat(cloneRequest.waitForEvents(), equalTo(originalRequest.waitForEvents()));
assertIndicesEquals(cloneRequest.indices(), originalRequest.indices());
assertThat(cloneRequest.indicesOptions(), equalTo(originalRequest.indicesOptions()));
assertThat(cloneRequest.doesReturn200ForClusterHealthTimeout(), equalTo(originalRequest.doesReturn200ForClusterHealthTimeout()));
}
public void testRequestReturnsHiddenIndicesByDefault() {

View file

@ -15,7 +15,6 @@ import org.elasticsearch.cluster.health.ClusterIndexHealth;
import org.elasticsearch.cluster.health.ClusterIndexHealthTests;
import org.elasticsearch.cluster.health.ClusterStateHealth;
import org.elasticsearch.cluster.metadata.Metadata;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.stream.BytesStreamOutput;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.Writeable;
@ -44,26 +43,17 @@ public class ClusterHealthResponsesTests extends AbstractSerializingTestCase<Clu
private final ClusterHealthRequest.Level level = randomFrom(ClusterHealthRequest.Level.values());
public void testIsTimeout() {
ClusterHealthResponse res = new ClusterHealthResponse("", Strings.EMPTY_ARRAY, ClusterState.EMPTY_STATE, false);
ClusterHealthResponse res = new ClusterHealthResponse();
for (int i = 0; i < 5; i++) {
res.setTimedOut(randomBoolean());
if (res.isTimedOut()) {
assertEquals(RestStatus.REQUEST_TIMEOUT, res.status());
assertWarnings(ClusterHealthResponse.CLUSTER_HEALTH_REQUEST_TIMEOUT_DEPRECATION_MSG);
} else {
assertEquals(RestStatus.OK, res.status());
}
}
}
public void testTimeoutReturns200IfOptedIn() {
ClusterHealthResponse res = new ClusterHealthResponse("", Strings.EMPTY_ARRAY, ClusterState.EMPTY_STATE, true);
for (int i = 0; i < 5; i++) {
res.setTimedOut(randomBoolean());
assertEquals(RestStatus.OK, res.status());
}
}
public void testClusterHealth() throws IOException {
ClusterState clusterState = ClusterState.builder(ClusterName.CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)).build();
int pendingTasks = randomIntBetween(0, 200);
@ -77,8 +67,7 @@ public class ClusterHealthResponsesTests extends AbstractSerializingTestCase<Clu
pendingTasks,
inFlight,
delayedUnassigned,
pendingTaskInQueueTime,
false
pendingTaskInQueueTime
);
clusterHealth = maybeSerialize(clusterHealth);
assertClusterHealth(clusterHealth);

View file

@ -37,17 +37,17 @@ public class TransportClusterHealthActionTests extends ESTestCase {
final ClusterHealthRequest request = new ClusterHealthRequest();
request.waitForNoInitializingShards(true);
ClusterState clusterState = randomClusterStateWithInitializingShards("test", 0);
ClusterHealthResponse response = new ClusterHealthResponse("", indices, clusterState, false);
ClusterHealthResponse response = new ClusterHealthResponse("", indices, clusterState);
assertThat(TransportClusterHealthAction.prepareResponse(request, response, clusterState, null), equalTo(1));
request.waitForNoInitializingShards(true);
clusterState = randomClusterStateWithInitializingShards("test", between(1, 10));
response = new ClusterHealthResponse("", indices, clusterState, false);
response = new ClusterHealthResponse("", indices, clusterState);
assertThat(TransportClusterHealthAction.prepareResponse(request, response, clusterState, null), equalTo(0));
request.waitForNoInitializingShards(false);
clusterState = randomClusterStateWithInitializingShards("test", randomInt(20));
response = new ClusterHealthResponse("", indices, clusterState, false);
response = new ClusterHealthResponse("", indices, clusterState);
assertThat(TransportClusterHealthAction.prepareResponse(request, response, clusterState, null), equalTo(0));
}
@ -57,11 +57,11 @@ public class TransportClusterHealthActionTests extends ESTestCase {
request.waitForActiveShards(ActiveShardCount.ALL);
ClusterState clusterState = randomClusterStateWithInitializingShards("test", 1);
ClusterHealthResponse response = new ClusterHealthResponse("", indices, clusterState, false);
ClusterHealthResponse response = new ClusterHealthResponse("", indices, clusterState);
assertThat(TransportClusterHealthAction.prepareResponse(request, response, clusterState, null), equalTo(0));
clusterState = ClusterState.builder(ClusterName.CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)).build();
response = new ClusterHealthResponse("", indices, clusterState, false);
response = new ClusterHealthResponse("", indices, clusterState);
assertThat(TransportClusterHealthAction.prepareResponse(request, response, clusterState, null), equalTo(1));
}

View file

@ -51,8 +51,6 @@ public class RestClusterHealthActionTests extends ESTestCase {
params.put("wait_for_active_shards", String.valueOf(waitForActiveShards));
params.put("wait_for_nodes", waitForNodes);
params.put("wait_for_events", waitForEvents.name());
boolean requestTimeout200 = randomBoolean();
params.put("return_200_for_cluster_health_timeout", String.valueOf(requestTimeout200));
FakeRestRequest restRequest = buildRestRequest(params);
ClusterHealthRequest clusterHealthRequest = RestClusterHealthAction.fromRequest(restRequest);
@ -67,7 +65,7 @@ public class RestClusterHealthActionTests extends ESTestCase {
assertThat(clusterHealthRequest.waitForActiveShards(), equalTo(ActiveShardCount.parseString(String.valueOf(waitForActiveShards))));
assertThat(clusterHealthRequest.waitForNodes(), equalTo(waitForNodes));
assertThat(clusterHealthRequest.waitForEvents(), equalTo(waitForEvents));
assertThat(clusterHealthRequest.doesReturn200ForClusterHealthTimeout(), equalTo(requestTimeout200));
}
private FakeRestRequest buildRestRequest(Map<String, String> params) {

View file

@ -35,7 +35,6 @@ import org.elasticsearch.cluster.metadata.IndexTemplateMetadata;
import org.elasticsearch.cluster.metadata.Metadata;
import org.elasticsearch.cluster.node.DiscoveryNode;
import org.elasticsearch.cluster.node.DiscoveryNodes;
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.collect.ImmutableOpenMap;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.common.transport.TransportAddress;
@ -111,7 +110,7 @@ public class MlIndexAndAliasTests extends ESTestCase {
clusterAdminClient = mock(ClusterAdminClient.class);
doAnswer(invocationOnMock -> {
ActionListener<ClusterHealthResponse> listener = (ActionListener<ClusterHealthResponse>) invocationOnMock.getArguments()[1];
listener.onResponse(new ClusterHealthResponse("", Strings.EMPTY_ARRAY, ClusterState.EMPTY_STATE, false));
listener.onResponse(new ClusterHealthResponse());
return null;
}).when(clusterAdminClient).health(any(ClusterHealthRequest.class), any(ActionListener.class));

View file

@ -26,7 +26,6 @@ import org.elasticsearch.common.util.concurrent.EsExecutors;
import org.elasticsearch.common.util.concurrent.EsRejectedExecutionException;
import org.elasticsearch.common.util.concurrent.ThreadContext;
import org.elasticsearch.core.CheckedConsumer;
import org.elasticsearch.core.TimeValue;
import org.elasticsearch.env.Environment;
import org.elasticsearch.env.TestEnvironment;
import org.elasticsearch.index.analysis.AnalysisRegistry;
@ -172,9 +171,7 @@ public class AutodetectProcessManagerTests extends ESTestCase {
}
if (t.getTypeName().contains("ClusterHealthResponse")) {
ActionListener<ClusterHealthResponse> listener = (ActionListener<ClusterHealthResponse>) l;
listener.onResponse(
new ClusterHealthResponse("test", new String[0], ClusterState.EMPTY_STATE, 0, 0, 0, TimeValue.ZERO, false)
);
listener.onResponse(new ClusterHealthResponse());
return null;
}
fail("Mock not configured to handle generic type " + t.getTypeName());

View file

@ -192,7 +192,7 @@ public class TransformInternalIndexTests extends ESTestCase {
doAnswer(invocationOnMock -> {
@SuppressWarnings("unchecked")
ActionListener<ClusterHealthResponse> listener = (ActionListener<ClusterHealthResponse>) invocationOnMock.getArguments()[1];
listener.onResponse(new ClusterHealthResponse("", Strings.EMPTY_ARRAY, ClusterState.EMPTY_STATE, false));
listener.onResponse(new ClusterHealthResponse());
return null;
}).when(clusterClient).health(any(), any());
@ -276,7 +276,7 @@ public class TransformInternalIndexTests extends ESTestCase {
doAnswer(invocationOnMock -> {
@SuppressWarnings("unchecked")
ActionListener<ClusterHealthResponse> listener = (ActionListener<ClusterHealthResponse>) invocationOnMock.getArguments()[1];
listener.onResponse(new ClusterHealthResponse("", Strings.EMPTY_ARRAY, ClusterState.EMPTY_STATE, false));
listener.onResponse(new ClusterHealthResponse());
return null;
}).when(clusterClient).health(any(), any());