Skip to content

Commit 7bfe441

Browse files
arteamarteam
committed
Use query param instead of a system property for opting in for new cluster health response code
1 parent 04b56f2 commit 7bfe441

File tree

12 files changed

+78
-55
lines changed

12 files changed

+78
-55
lines changed

‎client/rest-high-level/src/test/java/org/elasticsearch/client/ClusterClientIT.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -37,13 +37,13 @@
3737
import org.elasticsearch.common.compress.CompressedXContent;
3838
import org.elasticsearch.common.settings.Settings;
3939
import org.elasticsearch.common.unit.ByteSizeUnit;
40-
import org.elasticsearch.core.TimeValue;
41-
import org.elasticsearch.xcontent.XContentType;
4240
import org.elasticsearch.common.xcontent.support.XContentMapValues;
41+
import org.elasticsearch.core.TimeValue;
4342
import org.elasticsearch.indices.recovery.RecoverySettings;
4443
import org.elasticsearch.rest.RestStatus;
4544
import org.elasticsearch.transport.RemoteClusterService;
4645
import org.elasticsearch.transport.SniffConnectionStrategy;
46+
import org.elasticsearch.xcontent.XContentType;
4747

4848
import java.io.IOException;
4949
import java.util.HashMap;
@@ -316,7 +316,7 @@ public void testClusterHealthNotFoundIndex() throws IOException {
316316
assertThat(response.getStatus(), equalTo(ClusterHealthStatus.RED));
317317
assertNoIndices(response);
318318
assertWarnings("The HTTP status code for a cluster health timeout will be changed from 408 to 200 in a " +
319-
"future version. Set the [es.cluster_health.request_timeout_200] system property to [true] to suppress this message and " +
319+
"future version. Set the [return_200_for_cluster_health_timeout] query parameter to [true] to suppress this message and " +
320320
"opt in to the future behaviour now.");
321321
}
322322

‎rest-api-spec/src/main/resources/rest-api-spec/api/cluster.health.json‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,10 @@
102102
"red"
103103
],
104104
"description":"Wait until cluster is in a specific state"
105+
},
106+
"return_200_for_cluster_health_timeout":{
107+
"type":"boolean",
108+
"description":"Whether to return HTTP 200 instead of 408 in case of a cluster health timeout from the server side"
105109
}
106110
}
107111
}

‎server/src/main/java/org/elasticsearch/action/admin/cluster/health/ClusterHealthRequest.java‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ public class ClusterHealthRequest extends MasterNodeReadRequest<ClusterHealthReq
3535
private ActiveShardCount waitForActiveShards = ActiveShardCount.NONE;
3636
private String waitForNodes = "";
3737
private Priority waitForEvents = null;
38+
private boolean return200ForClusterHealthTimeout;
39+
3840
/**
3941
* Only used by the high-level REST Client. Controls the details level of the health information returned.
4042
* The default value is 'cluster'.
@@ -67,6 +69,9 @@ public ClusterHealthRequest(StreamInput in) throws IOException {
6769
} else {
6870
indicesOptions = IndicesOptions.lenientExpandOpen();
6971
}
72+
if (in.getVersion().onOrAfter(Version.V_7_16_0)) {
73+
return200ForClusterHealthTimeout = in.readBoolean();
74+
}
7075
}
7176

7277
@Override
@@ -97,6 +102,9 @@ public void writeTo(StreamOutput out) throws IOException {
97102
if (out.getVersion().onOrAfter(Version.V_7_2_0)) {
98103
indicesOptions.writeIndicesOptions(out);
99104
}
105+
if (out.getVersion().onOrAfter(Version.V_7_16_0)) {
106+
out.writeBoolean(return200ForClusterHealthTimeout);
107+
}
100108
}
101109

102110
@Override
@@ -236,6 +244,14 @@ public ClusterHealthRequest waitForEvents(Priority waitForEvents) {
236244
return this;
237245
}
238246

247+
public boolean isReturn200ForClusterHealthTimeout() {
248+
return return200ForClusterHealthTimeout;
249+
}
250+
251+
public void setReturn200ForClusterHealthTimeout(boolean return200ForClusterHealthTimeout) {
252+
this.return200ForClusterHealthTimeout = return200ForClusterHealthTimeout;
253+
}
254+
239255
public Priority waitForEvents() {
240256
return this.waitForEvents;
241257
}

‎server/src/main/java/org/elasticsearch/action/admin/cluster/health/ClusterHealthResponse.java‎

Lines changed: 25 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -8,24 +8,25 @@
88

99
package org.elasticsearch.action.admin.cluster.health;
1010

11+
import org.elasticsearch.Version;
1112
import org.elasticsearch.action.ActionResponse;
1213
import org.elasticsearch.cluster.ClusterState;
1314
import org.elasticsearch.cluster.health.ClusterHealthStatus;
1415
import org.elasticsearch.cluster.health.ClusterIndexHealth;
1516
import org.elasticsearch.cluster.health.ClusterStateHealth;
16-
import org.elasticsearch.common.logging.DeprecationLogger;
17-
import org.elasticsearch.xcontent.ParseField;
1817
import org.elasticsearch.common.Strings;
1918
import org.elasticsearch.common.io.stream.StreamInput;
2019
import org.elasticsearch.common.io.stream.StreamOutput;
20+
import org.elasticsearch.common.logging.DeprecationLogger;
21+
import org.elasticsearch.common.xcontent.StatusToXContentObject;
2122
import org.elasticsearch.core.TimeValue;
23+
import org.elasticsearch.rest.RestStatus;
24+
import org.elasticsearch.rest.action.search.RestSearchAction;
2225
import org.elasticsearch.xcontent.ConstructingObjectParser;
2326
import org.elasticsearch.xcontent.ObjectParser;
24-
import org.elasticsearch.common.xcontent.StatusToXContentObject;
27+
import org.elasticsearch.xcontent.ParseField;
2528
import org.elasticsearch.xcontent.XContentBuilder;
2629
import org.elasticsearch.xcontent.XContentParser;
27-
import org.elasticsearch.rest.RestStatus;
28-
import org.elasticsearch.rest.action.search.RestSearchAction;
2930

3031
import java.io.IOException;
3132
import java.util.HashMap;
@@ -101,10 +102,10 @@ public class ClusterHealthResponse extends ActionResponse implements StatusToXCo
101102

102103
private static final ObjectParser.NamedObjectParser<ClusterIndexHealth, Void> INDEX_PARSER =
103104
(XContentParser parser, Void context, String index) -> ClusterIndexHealth.innerFromXContent(parser, index);
104-
private static final String ES_CLUSTER_HEALTH_REQUEST_TIMEOUT_200_KEY = "es.cluster_health.request_timeout_200";
105+
static final String ES_CLUSTER_HEALTH_REQUEST_TIMEOUT_200_KEY = "return_200_for_cluster_health_timeout";
105106
static final String CLUSTER_HEALTH_REQUEST_TIMEOUT_DEPRECATION_MSG = "The HTTP status code for a cluster health timeout " +
106107
"will be changed from 408 to 200 in a future version. Set the [" + ES_CLUSTER_HEALTH_REQUEST_TIMEOUT_200_KEY + "] " +
107-
"system property to [true] to suppress this message and opt in to the future behaviour now.";
108+
"query parameter to [true] to suppress this message and opt in to the future behaviour now.";
108109

109110
static {
110111
// ClusterStateHealth fields
@@ -137,14 +138,10 @@ public class ClusterHealthResponse extends ActionResponse implements StatusToXCo
137138
private boolean timedOut = false;
138139
private ClusterStateHealth clusterStateHealth;
139140
private ClusterHealthStatus clusterHealthStatus;
140-
private boolean esClusterHealthRequestTimeout200 = readEsClusterHealthRequestTimeout200FromProperty();
141+
private boolean return200ForClusterHealthTimeout;
141142

142-
public ClusterHealthResponse() {
143-
}
144-
145-
/** For the testing of opting in for the 200 status code without setting a system property */
146-
ClusterHealthResponse(boolean esClusterHealthRequestTimeout200) {
147-
this.esClusterHealthRequestTimeout200 = esClusterHealthRequestTimeout200;
143+
public ClusterHealthResponse(boolean return200ForServerTimeout) {
144+
this.return200ForClusterHealthTimeout = return200ForServerTimeout;
148145
}
149146

150147
public ClusterHealthResponse(StreamInput in) throws IOException {
@@ -157,22 +154,29 @@ public ClusterHealthResponse(StreamInput in) throws IOException {
157154
numberOfInFlightFetch = in.readInt();
158155
delayedUnassignedShards= in.readInt();
159156
taskMaxWaitingTime = in.readTimeValue();
157+
if (in.getVersion().onOrAfter(Version.V_8_0_0)) {
158+
return200ForClusterHealthTimeout = in.readBoolean();
159+
}
160160
}
161161

162162
/** needed for plugins BWC */
163-
public ClusterHealthResponse(String clusterName, String[] concreteIndices, ClusterState clusterState) {
164-
this(clusterName, concreteIndices, clusterState, -1, -1, -1, TimeValue.timeValueHours(0));
163+
public ClusterHealthResponse(String clusterName, String[] concreteIndices, ClusterState clusterState,
164+
boolean return200ForServerTimeout) {
165+
this(clusterName, concreteIndices, clusterState, -1, -1, -1, TimeValue.timeValueHours(0),
166+
return200ForServerTimeout);
165167
}
166168

167169
public ClusterHealthResponse(String clusterName, String[] concreteIndices, ClusterState clusterState, int numberOfPendingTasks,
168-
int numberOfInFlightFetch, int delayedUnassignedShards, TimeValue taskMaxWaitingTime) {
170+
int numberOfInFlightFetch, int delayedUnassignedShards, TimeValue taskMaxWaitingTime,
171+
boolean return200ForServerTimeout) {
169172
this.clusterName = clusterName;
170173
this.numberOfPendingTasks = numberOfPendingTasks;
171174
this.numberOfInFlightFetch = numberOfInFlightFetch;
172175
this.delayedUnassignedShards = delayedUnassignedShards;
173176
this.taskMaxWaitingTime = taskMaxWaitingTime;
174177
this.clusterStateHealth = new ClusterStateHealth(clusterState, concreteIndices);
175178
this.clusterHealthStatus = clusterStateHealth.getStatus();
179+
this.return200ForClusterHealthTimeout = return200ForServerTimeout;
176180
}
177181

178182
/**
@@ -304,6 +308,9 @@ public void writeTo(StreamOutput out) throws IOException {
304308
out.writeInt(numberOfInFlightFetch);
305309
out.writeInt(delayedUnassignedShards);
306310
out.writeTimeValue(taskMaxWaitingTime);
311+
if (out.getVersion().onOrAfter(Version.V_8_0_0)) {
312+
out.writeBoolean(return200ForClusterHealthTimeout);
313+
}
307314
}
308315

309316
@Override
@@ -316,7 +323,7 @@ public RestStatus status() {
316323
if (isTimedOut() == false) {
317324
return RestStatus.OK;
318325
}
319-
if (esClusterHealthRequestTimeout200) {
326+
if (return200ForClusterHealthTimeout) {
320327
return RestStatus.OK;
321328
} else {
322329
deprecationLogger.compatibleCritical("cluster_health_request_timeout", CLUSTER_HEALTH_REQUEST_TIMEOUT_DEPRECATION_MSG);
@@ -381,17 +388,4 @@ public int hashCode() {
381388
return Objects.hash(clusterName, numberOfPendingTasks, numberOfInFlightFetch, delayedUnassignedShards, taskMaxWaitingTime,
382389
timedOut, clusterStateHealth, clusterHealthStatus);
383390
}
384-
385-
private static boolean readEsClusterHealthRequestTimeout200FromProperty() {
386-
String property = System.getProperty(ES_CLUSTER_HEALTH_REQUEST_TIMEOUT_200_KEY);
387-
if (property == null) {
388-
return false;
389-
}
390-
if (Boolean.parseBoolean(property)) {
391-
return true;
392-
} else {
393-
throw new IllegalArgumentException(ES_CLUSTER_HEALTH_REQUEST_TIMEOUT_200_KEY + " can only be unset or [true] but was ["
394-
+ property + "]");
395-
}
396-
}
397391
}

‎server/src/main/java/org/elasticsearch/action/admin/cluster/health/TransportClusterHealthAction.java‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@
3030
import org.elasticsearch.cluster.service.ClusterService;
3131
import org.elasticsearch.common.Strings;
3232
import org.elasticsearch.common.inject.Inject;
33-
import org.elasticsearch.core.TimeValue;
3433
import org.elasticsearch.common.util.CollectionUtils;
34+
import org.elasticsearch.core.TimeValue;
3535
import org.elasticsearch.index.IndexNotFoundException;
3636
import org.elasticsearch.node.NodeClosedException;
3737
import org.elasticsearch.tasks.Task;
@@ -225,7 +225,8 @@ private enum TimeoutState {
225225

226226
private ClusterHealthResponse getResponse(final ClusterHealthRequest request, ClusterState clusterState,
227227
final int waitFor, TimeoutState timeoutState) {
228-
ClusterHealthResponse response = clusterHealth(request, clusterState, clusterService.getMasterService().numberOfPendingTasks(),
228+
ClusterHealthResponse response = clusterHealth(request, clusterState,
229+
clusterService.getMasterService().numberOfPendingTasks(),
229230
allocationService.getNumberOfInFlightFetches(), clusterService.getMasterService().getMaxTaskWaitTime());
230231
int readyCounter = prepareResponse(request, response, clusterState, indexNameExpressionResolver);
231232
boolean valid = (readyCounter == waitFor);
@@ -324,8 +325,8 @@ static int prepareResponse(final ClusterHealthRequest request, final ClusterHeal
324325
}
325326

326327

327-
private ClusterHealthResponse clusterHealth(ClusterHealthRequest request, ClusterState clusterState, int numberOfPendingTasks,
328-
int numberOfInFlightFetch, TimeValue pendingTaskTimeInQueue) {
328+
private ClusterHealthResponse clusterHealth(ClusterHealthRequest request, ClusterState clusterState,
329+
int numberOfPendingTasks, int numberOfInFlightFetch, TimeValue pendingTaskTimeInQueue) {
329330
if (logger.isTraceEnabled()) {
330331
logger.trace("Calculating health based on state version [{}]", clusterState.version());
331332
}
@@ -337,12 +338,13 @@ private ClusterHealthResponse clusterHealth(ClusterHealthRequest request, Cluste
337338
// one of the specified indices is not there - treat it as RED.
338339
ClusterHealthResponse response = new ClusterHealthResponse(clusterState.getClusterName().value(), Strings.EMPTY_ARRAY,
339340
clusterState, numberOfPendingTasks, numberOfInFlightFetch, UnassignedInfo.getNumberOfDelayedUnassigned(clusterState),
340-
pendingTaskTimeInQueue);
341+
pendingTaskTimeInQueue, request.isReturn200ForClusterHealthTimeout());
341342
response.setStatus(ClusterHealthStatus.RED);
342343
return response;
343344
}
344345

345-
return new ClusterHealthResponse(clusterState.getClusterName().value(), concreteIndices, clusterState, numberOfPendingTasks,
346-
numberOfInFlightFetch, UnassignedInfo.getNumberOfDelayedUnassigned(clusterState), pendingTaskTimeInQueue);
346+
return new ClusterHealthResponse(clusterState.getClusterName().value(), concreteIndices, clusterState,
347+
numberOfPendingTasks, numberOfInFlightFetch, UnassignedInfo.getNumberOfDelayedUnassigned(clusterState), pendingTaskTimeInQueue,
348+
request.isReturn200ForClusterHealthTimeout());
347349
}
348350
}

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,10 @@ public static ClusterHealthRequest fromRequest(final RestRequest request) {
8181
if (request.param("wait_for_events") != null) {
8282
clusterHealthRequest.waitForEvents(Priority.valueOf(request.param("wait_for_events").toUpperCase(Locale.ROOT)));
8383
}
84+
if (request.param("return_200_for_cluster_health_timeout") != null) {
85+
clusterHealthRequest.setReturn200ForClusterHealthTimeout(
86+
Boolean.parseBoolean(request.param("return_200_for_cluster_health_timeout")));
87+
}
8488
return clusterHealthRequest;
8589
}
8690

‎server/src/test/java/org/elasticsearch/action/admin/cluster/health/ClusterHealthRequestTests.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ public void testSerialize() throws Exception {
4343
assertThat(cloneRequest.waitForEvents(), equalTo(originalRequest.waitForEvents()));
4444
assertIndicesEquals(cloneRequest.indices(), originalRequest.indices());
4545
assertThat(cloneRequest.indicesOptions(), equalTo(originalRequest.indicesOptions()));
46+
assertThat(cloneRequest.isReturn200ForClusterHealthTimeout(), equalTo(originalRequest.isReturn200ForClusterHealthTimeout()));
4647
}
4748

4849
public void testRequestReturnsHiddenIndicesByDefault() {

‎server/src/test/java/org/elasticsearch/action/admin/cluster/health/ClusterHealthResponsesTests.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,10 @@
2020
import org.elasticsearch.common.io.stream.Writeable;
2121
import org.elasticsearch.common.settings.Settings;
2222
import org.elasticsearch.core.TimeValue;
23-
import org.elasticsearch.xcontent.ToXContent;
24-
import org.elasticsearch.xcontent.XContentParser;
2523
import org.elasticsearch.rest.RestStatus;
2624
import org.elasticsearch.test.AbstractSerializingTestCase;
25+
import org.elasticsearch.xcontent.ToXContent;
26+
import org.elasticsearch.xcontent.XContentParser;
2727
import org.hamcrest.Matchers;
2828

2929
import java.io.IOException;
@@ -43,7 +43,7 @@ public class ClusterHealthResponsesTests extends AbstractSerializingTestCase<Clu
4343
private final ClusterHealthRequest.Level level = randomFrom(ClusterHealthRequest.Level.values());
4444

4545
public void testIsTimeout() {
46-
ClusterHealthResponse res = new ClusterHealthResponse();
46+
ClusterHealthResponse res = new ClusterHealthResponse(false);
4747
for (int i = 0; i < 5; i++) {
4848
res.setTimedOut(randomBoolean());
4949
if (res.isTimedOut()) {
@@ -70,7 +70,7 @@ public void testClusterHealth() throws IOException {
7070
int delayedUnassigned = randomIntBetween(0, 200);
7171
TimeValue pendingTaskInQueueTime = TimeValue.timeValueMillis(randomIntBetween(1000, 100000));
7272
ClusterHealthResponse clusterHealth = new ClusterHealthResponse("bla", new String[] {Metadata.ALL},
73-
clusterState, pendingTasks, inFlight, delayedUnassigned, pendingTaskInQueueTime);
73+
clusterState, pendingTasks, inFlight, delayedUnassigned, pendingTaskInQueueTime, false);
7474
clusterHealth = maybeSerialize(clusterHealth);
7575
assertClusterHealth(clusterHealth);
7676
assertThat(clusterHealth.getNumberOfPendingTasks(), Matchers.equalTo(pendingTasks));

‎server/src/test/java/org/elasticsearch/action/admin/cluster/health/TransportClusterHealthActionTests.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,17 +37,17 @@ public void testWaitForInitializingShards() throws Exception {
3737
final ClusterHealthRequest request = new ClusterHealthRequest();
3838
request.waitForNoInitializingShards(true);
3939
ClusterState clusterState = randomClusterStateWithInitializingShards("test", 0);
40-
ClusterHealthResponse response = new ClusterHealthResponse("", indices, clusterState);
40+
ClusterHealthResponse response = new ClusterHealthResponse("", indices, clusterState, false);
4141
assertThat(TransportClusterHealthAction.prepareResponse(request, response, clusterState, null), equalTo(1));
4242

4343
request.waitForNoInitializingShards(true);
4444
clusterState = randomClusterStateWithInitializingShards("test", between(1, 10));
45-
response = new ClusterHealthResponse("", indices, clusterState);
45+
response = new ClusterHealthResponse("", indices, clusterState, false);
4646
assertThat(TransportClusterHealthAction.prepareResponse(request, response, clusterState, null), equalTo(0));
4747

4848
request.waitForNoInitializingShards(false);
4949
clusterState = randomClusterStateWithInitializingShards("test", randomInt(20));
50-
response = new ClusterHealthResponse("", indices, clusterState);
50+
response = new ClusterHealthResponse("", indices, clusterState, false);
5151
assertThat(TransportClusterHealthAction.prepareResponse(request, response, clusterState, null), equalTo(0));
5252
}
5353

@@ -57,11 +57,11 @@ public void testWaitForAllShards() {
5757
request.waitForActiveShards(ActiveShardCount.ALL);
5858

5959
ClusterState clusterState = randomClusterStateWithInitializingShards("test", 1);
60-
ClusterHealthResponse response = new ClusterHealthResponse("", indices, clusterState);
60+
ClusterHealthResponse response = new ClusterHealthResponse( "", indices, clusterState, false);
6161
assertThat(TransportClusterHealthAction.prepareResponse(request, response, clusterState, null), equalTo(0));
6262

6363
clusterState = ClusterState.builder(ClusterName.CLUSTER_NAME_SETTING.getDefault(Settings.EMPTY)).build();
64-
response = new ClusterHealthResponse("", indices, clusterState);
64+
response = new ClusterHealthResponse("", indices, clusterState, false);
6565
assertThat(TransportClusterHealthAction.prepareResponse(request, response, clusterState, null), equalTo(1));
6666
}
6767

‎server/src/test/java/org/elasticsearch/rest/action/admin/cluster/RestClusterHealthActionTests.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ public void testFromRequest() {
5151
params.put("wait_for_active_shards", String.valueOf(waitForActiveShards));
5252
params.put("wait_for_nodes", waitForNodes);
5353
params.put("wait_for_events", waitForEvents.name());
54+
boolean requestTimeout200 = randomBoolean();
55+
params.put("return_200_for_cluster_health_timeout", String.valueOf(requestTimeout200));
5456

5557
FakeRestRequest restRequest = buildRestRequest(params);
5658
ClusterHealthRequest clusterHealthRequest = RestClusterHealthAction.fromRequest(restRequest);
@@ -65,7 +67,7 @@ public void testFromRequest() {
6567
assertThat(clusterHealthRequest.waitForActiveShards(), equalTo(ActiveShardCount.parseString(String.valueOf(waitForActiveShards))));
6668
assertThat(clusterHealthRequest.waitForNodes(), equalTo(waitForNodes));
6769
assertThat(clusterHealthRequest.waitForEvents(), equalTo(waitForEvents));
68-
70+
assertThat(clusterHealthRequest.isReturn200ForClusterHealthTimeout(), equalTo(requestTimeout200));
6971
}
7072

7173
private FakeRestRequest buildRestRequest(Map<String, String> params) {

0 commit comments

Comments
 (0)