Skip to content

Commit

Permalink
Revert "Deprecate returning 408 for a server timeout on `_cluster/hea…
Browse files Browse the repository at this point in the history
…lth` (elastic#78180)"

This reverts commit f266eb3
  • Loading branch information
arteam committed Nov 18, 2021
1 parent a544e23 commit af31969
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 78 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.function.BiConsumer;

import static java.util.Collections.emptyMap;
import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked;
Expand All @@ -74,13 +73,11 @@ public void testClusterPutSettings() throws IOException {
ClusterUpdateSettingsRequest setRequest = new ClusterUpdateSettingsRequest();
setRequest.transientSettings(transientSettings);
setRequest.persistentSettings(map);
RequestOptions options = RequestOptions.DEFAULT.toBuilder().setWarningsHandler(WarningsHandler.PERMISSIVE).build();

ClusterUpdateSettingsResponse setResponse = execute(
setRequest,
highLevelClient().cluster()::putSettings,
highLevelClient().cluster()::putSettingsAsync,
options
highLevelClient().cluster()::putSettingsAsync
);

assertAcked(setResponse);
Expand All @@ -107,8 +104,7 @@ public void testClusterPutSettings() throws IOException {
ClusterUpdateSettingsResponse resetResponse = execute(
resetRequest,
highLevelClient().cluster()::putSettings,
highLevelClient().cluster()::putSettingsAsync,
options
highLevelClient().cluster()::putSettingsAsync
);

assertThat(resetResponse.getTransientSettings().get(transientSettingKey), equalTo(null));
Expand All @@ -123,22 +119,11 @@ public void testClusterPutSettings() throws IOException {
assertThat(persistentResetValue, equalTo(null));
}

public void testClusterUpdateTransientSettingNonExistent() {
testClusterUpdateSettingNonExistent((settings, request) -> request.transientSettings(settings), "transient");
}

public void testClusterUpdatePersistentSettingNonExistent() {
testClusterUpdateSettingNonExistent((settings, request) -> request.persistentSettings(settings), "persistent");
}

private void testClusterUpdateSettingNonExistent(
final BiConsumer<Settings.Builder, ClusterUpdateSettingsRequest> consumer,
String label
) {
public void testClusterUpdateSettingNonExistent() {
String setting = "no_idea_what_you_are_talking_about";
int value = 10;
ClusterUpdateSettingsRequest clusterUpdateSettingsRequest = new ClusterUpdateSettingsRequest();
consumer.accept(Settings.builder().put(setting, value), clusterUpdateSettingsRequest);
clusterUpdateSettingsRequest.transientSettings(Settings.builder().put(setting, value).build());

ElasticsearchException exception = expectThrows(
ElasticsearchException.class,
Expand All @@ -151,9 +136,7 @@ private void testClusterUpdateSettingNonExistent(
assertThat(exception.status(), equalTo(RestStatus.BAD_REQUEST));
assertThat(
exception.getMessage(),
equalTo(
"Elasticsearch exception [type=illegal_argument_exception, reason=" + label + " setting [" + setting + "], not recognized]"
)
equalTo("Elasticsearch exception [type=illegal_argument_exception, reason=transient setting [" + setting + "], not recognized]")
);
}

Expand Down Expand Up @@ -337,11 +320,6 @@ public void testClusterHealthNotFoundIndex() throws IOException {
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 [es.cluster_health.request_timeout_200] system property to [true] to suppress this message and "
+ "opt in to the future behaviour now."
);
}

public void testRemoteInfo() throws Exception {
Expand All @@ -353,13 +331,13 @@ public void testRemoteInfo() throws Exception {
ClusterGetSettingsResponse settingsResponse = highLevelClient().cluster().getSettings(settingsRequest, RequestOptions.DEFAULT);

List<String> seeds = SniffConnectionStrategy.REMOTE_CLUSTER_SEEDS.getConcreteSettingForNamespace(clusterAlias)
.get(settingsResponse.getPersistentSettings());
int connectionsPerCluster = SniffConnectionStrategy.REMOTE_CONNECTIONS_PER_CLUSTER.get(settingsResponse.getPersistentSettings());
.get(settingsResponse.getTransientSettings());
int connectionsPerCluster = SniffConnectionStrategy.REMOTE_CONNECTIONS_PER_CLUSTER.get(settingsResponse.getTransientSettings());
TimeValue initialConnectionTimeout = RemoteClusterService.REMOTE_INITIAL_CONNECTION_TIMEOUT_SETTING.get(
settingsResponse.getPersistentSettings()
settingsResponse.getTransientSettings()
);
boolean skipUnavailable = RemoteClusterService.REMOTE_CLUSTER_SKIP_UNAVAILABLE.getConcreteSettingForNamespace(clusterAlias)
.get(settingsResponse.getPersistentSettings());
.get(settingsResponse.getTransientSettings());

RemoteInfoRequest request = new RemoteInfoRequest();
RemoteInfoResponse response = execute(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,9 @@
import org.elasticsearch.common.Strings;
import org.elasticsearch.common.io.stream.StreamInput;
import org.elasticsearch.common.io.stream.StreamOutput;
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;
Expand Down Expand Up @@ -57,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",
Expand Down Expand Up @@ -122,12 +119,6 @@ public class ClusterHealthResponse extends ActionResponse implements StatusToXCo
XContentParser parser,
Void context,
String index) -> ClusterIndexHealth.innerFromXContent(parser, index);
private static final String ES_CLUSTER_HEALTH_REQUEST_TIMEOUT_200_KEY = "es.cluster_health.request_timeout_200";
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
+ "] "
+ "system property to [true] to suppress this message and opt in to the future behaviour now.";

static {
// ClusterStateHealth fields
Expand Down Expand Up @@ -160,15 +151,9 @@ public class ClusterHealthResponse extends ActionResponse implements StatusToXCo
private boolean timedOut = false;
private ClusterStateHealth clusterStateHealth;
private ClusterHealthStatus clusterHealthStatus;
private boolean esClusterHealthRequestTimeout200 = readEsClusterHealthRequestTimeout200FromProperty();

public ClusterHealthResponse() {}

/** For the testing of opting in for the 200 status code without setting a system property */
ClusterHealthResponse(boolean esClusterHealthRequestTimeout200) {
this.esClusterHealthRequestTimeout200 = esClusterHealthRequestTimeout200;
}

public ClusterHealthResponse(StreamInput in) throws IOException {
super(in);
clusterName = in.readString();
Expand Down Expand Up @@ -349,15 +334,7 @@ public String toString() {

@Override
public RestStatus status() {
if (isTimedOut() == false) {
return RestStatus.OK;
}
if (esClusterHealthRequestTimeout200) {
return RestStatus.OK;
} else {
deprecationLogger.compatibleCritical("cluster_health_request_timeout", CLUSTER_HEALTH_REQUEST_TIMEOUT_DEPRECATION_MSG);
return RestStatus.REQUEST_TIMEOUT;
}
return isTimedOut() ? RestStatus.REQUEST_TIMEOUT : RestStatus.OK;
}

@Override
Expand Down Expand Up @@ -425,18 +402,4 @@ public int hashCode() {
clusterHealthStatus
);
}

private static boolean readEsClusterHealthRequestTimeout200FromProperty() {
String property = System.getProperty(ES_CLUSTER_HEALTH_REQUEST_TIMEOUT_200_KEY);
if (property == null) {
return false;
}
if (Boolean.parseBoolean(property)) {
return true;
} else {
throw new IllegalArgumentException(
ES_CLUSTER_HEALTH_REQUEST_TIMEOUT_200_KEY + " can only be unset or [true] but was [" + property + "]"
);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,21 +48,12 @@ public void testIsTimeout() {
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(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);
Expand Down

0 comments on commit af31969

Please sign in to comment.