From fe3179d9cccb569784434b2135ca9ae13d5158d3 Mon Sep 17 00:00:00 2001 From: Simon Willnauer Date: Mon, 10 Aug 2015 21:47:33 +0200 Subject: [PATCH] Return `408 REQUEST_TIMEOUT` if `_cluster/health` times out Today we return `200 OK` which is misleading since we really didn't produce a valid response / didn't wait long enough. --- .../cluster/health/ClusterHealthResponse.java | 12 +++++++----- .../health/RestClusterHealthAction.java | 5 ++--- .../health}/ClusterHealthResponsesTests.java | 17 ++++++++++++++++- .../test/rest/section/DoSection.java | 3 ++- .../test/cat.allocation/10_basic.yaml | 3 +-- .../cluster.health/20_request_timeout.yaml | 18 ++++++++++++++++++ 6 files changed, 46 insertions(+), 12 deletions(-) rename core/src/test/java/org/elasticsearch/{cluster => action/admin/cluster/health}/ClusterHealthResponsesTests.java (95%) create mode 100644 rest-api-spec/src/main/resources/rest-api-spec/test/cluster.health/20_request_timeout.yaml diff --git a/core/src/main/java/org/elasticsearch/action/admin/cluster/health/ClusterHealthResponse.java b/core/src/main/java/org/elasticsearch/action/admin/cluster/health/ClusterHealthResponse.java index cb94778de51..7172c254dd8 100644 --- a/core/src/main/java/org/elasticsearch/action/admin/cluster/health/ClusterHealthResponse.java +++ b/core/src/main/java/org/elasticsearch/action/admin/cluster/health/ClusterHealthResponse.java @@ -31,10 +31,7 @@ import org.elasticsearch.cluster.routing.ShardRouting; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.unit.TimeValue; -import org.elasticsearch.common.xcontent.ToXContent; -import org.elasticsearch.common.xcontent.XContentBuilder; -import org.elasticsearch.common.xcontent.XContentBuilderString; -import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.*; import org.elasticsearch.rest.RestStatus; import java.io.IOException; @@ -49,7 +46,7 @@ import static org.elasticsearch.action.admin.cluster.health.ClusterIndexHealth.r /** * */ -public class ClusterHealthResponse extends ActionResponse implements Iterable, ToXContent { +public class ClusterHealthResponse extends ActionResponse implements Iterable, StatusToXContent { private String clusterName; int numberOfNodes = 0; @@ -332,6 +329,11 @@ public class ClusterHealthResponse extends ActionResponse implements Iterable(channel)); + client.admin().cluster().health(clusterHealthRequest, new RestStatusToXContentListener(channel)); } } diff --git a/core/src/test/java/org/elasticsearch/cluster/ClusterHealthResponsesTests.java b/core/src/test/java/org/elasticsearch/action/admin/cluster/health/ClusterHealthResponsesTests.java similarity index 95% rename from core/src/test/java/org/elasticsearch/cluster/ClusterHealthResponsesTests.java rename to core/src/test/java/org/elasticsearch/action/admin/cluster/health/ClusterHealthResponsesTests.java index 50d7bc99102..d66c1bc54ec 100644 --- a/core/src/test/java/org/elasticsearch/cluster/ClusterHealthResponsesTests.java +++ b/core/src/test/java/org/elasticsearch/action/admin/cluster/health/ClusterHealthResponsesTests.java @@ -17,7 +17,7 @@ * under the License. */ -package org.elasticsearch.cluster; +package org.elasticsearch.action.admin.cluster.health; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.Version; @@ -26,6 +26,8 @@ import org.elasticsearch.action.admin.cluster.health.ClusterHealthStatus; import org.elasticsearch.action.admin.cluster.health.ClusterIndexHealth; import org.elasticsearch.action.admin.cluster.health.ClusterShardHealth; import org.elasticsearch.action.support.IndicesOptions; +import org.elasticsearch.cluster.ClusterName; +import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver; import org.elasticsearch.cluster.metadata.MetaData; @@ -35,6 +37,7 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.index.shard.ShardId; +import org.elasticsearch.rest.RestStatus; import org.elasticsearch.test.ESTestCase; import org.hamcrest.Matchers; import org.junit.Test; @@ -181,6 +184,18 @@ public class ClusterHealthResponsesTests extends ESTestCase { assertThat(clusterHealth.getValidationFailures(), empty()); } + public void testIsTimeout() throws IOException { + ClusterHealthResponse res = new ClusterHealthResponse(); + for (int i = 0; i < 5; i++) { + res.timedOut = randomBoolean(); + if (res.isTimedOut()) { + assertEquals(RestStatus.REQUEST_TIMEOUT, res.status()); + } else { + assertEquals(RestStatus.OK, res.status()); + } + } + } + @Test public void testClusterHealth() throws IOException { ShardCounter counter = new ShardCounter(); diff --git a/core/src/test/java/org/elasticsearch/test/rest/section/DoSection.java b/core/src/test/java/org/elasticsearch/test/rest/section/DoSection.java index cf15029a9df..f36a6307c61 100644 --- a/core/src/test/java/org/elasticsearch/test/rest/section/DoSection.java +++ b/core/src/test/java/org/elasticsearch/test/rest/section/DoSection.java @@ -131,6 +131,7 @@ public class DoSection implements ExecutableSection { catches.put("missing", tuple("404", equalTo(404))); catches.put("conflict", tuple("409", equalTo(409))); catches.put("forbidden", tuple("403", equalTo(403))); - catches.put("request", tuple("4xx|5xx", allOf(greaterThanOrEqualTo(400), not(equalTo(404)), not(equalTo(409)), not(equalTo(403))))); + catches.put("request_timeout", tuple("408", equalTo(408))); + catches.put("request", tuple("4xx|5xx", allOf(greaterThanOrEqualTo(400), not(equalTo(404)), not(equalTo(408)), not(equalTo(409)), not(equalTo(403))))); } } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/cat.allocation/10_basic.yaml b/rest-api-spec/src/main/resources/rest-api-spec/test/cat.allocation/10_basic.yaml index e1a25c97feb..ea508e15919 100755 --- a/rest-api-spec/src/main/resources/rest-api-spec/test/cat.allocation/10_basic.yaml +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/cat.allocation/10_basic.yaml @@ -48,8 +48,7 @@ - do: cluster.health: - wait_for_status: green - timeout: 1s + wait_for_status: yellow - do: cat.allocation: diff --git a/rest-api-spec/src/main/resources/rest-api-spec/test/cluster.health/20_request_timeout.yaml b/rest-api-spec/src/main/resources/rest-api-spec/test/cluster.health/20_request_timeout.yaml new file mode 100644 index 00000000000..295eea3edeb --- /dev/null +++ b/rest-api-spec/src/main/resources/rest-api-spec/test/cluster.health/20_request_timeout.yaml @@ -0,0 +1,18 @@ +--- +"cluster health request timeout": + - do: + catch: request_timeout + cluster.health: + wait_for_nodes: 10 + timeout: 1s + + - 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 }