From 3a46812a335a56c56106aabd98029854ee2b54b3 Mon Sep 17 00:00:00 2001 From: Daniel Mitterdorfer Date: Mon, 9 May 2016 11:16:49 +0200 Subject: [PATCH] Free bytes reserved on request breaker With this commit we free all bytes reserved on the request circuit breaker. Closes #18144 --- .../org/elasticsearch/http/HttpServer.java | 5 +- .../elasticsearch/rest/BytesRestResponse.java | 4 -- .../elasticsearch/rest/RestController.java | 6 ++- .../RestClusterAllocationExplainAction.java | 6 ++- .../alias/head/RestAliasesExistAction.java | 8 +-- .../indices/RestIndicesExistsAction.java | 5 +- .../exists/types/RestTypesExistsAction.java | 5 +- .../head/RestHeadIndexTemplateAction.java | 5 +- .../rest/action/get/RestHeadAction.java | 5 +- .../rest/action/main/RestMainAction.java | 3 +- .../elasticsearch/http/HttpServerTests.java | 4 +- .../rest/action/main/RestMainActionIT.java | 52 +++++++++++++++++++ 12 files changed, 84 insertions(+), 24 deletions(-) create mode 100644 core/src/test/java/org/elasticsearch/rest/action/main/RestMainActionIT.java diff --git a/core/src/main/java/org/elasticsearch/http/HttpServer.java b/core/src/main/java/org/elasticsearch/http/HttpServer.java index 45abad0fb81..e96b0a85beb 100644 --- a/core/src/main/java/org/elasticsearch/http/HttpServer.java +++ b/core/src/main/java/org/elasticsearch/http/HttpServer.java @@ -21,6 +21,7 @@ package org.elasticsearch.http; import org.elasticsearch.common.Nullable; import org.elasticsearch.common.breaker.CircuitBreaker; +import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.component.AbstractLifecycleComponent; import org.elasticsearch.common.inject.Inject; @@ -125,10 +126,10 @@ public class HttpServer extends AbstractLifecycleComponent implement channel.sendResponse(restResponse); } } catch (IOException e) { - channel.sendResponse(new BytesRestResponse(INTERNAL_SERVER_ERROR)); + channel.sendResponse(new BytesRestResponse(INTERNAL_SERVER_ERROR, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY)); } } else { - channel.sendResponse(new BytesRestResponse(FORBIDDEN)); + channel.sendResponse(new BytesRestResponse(FORBIDDEN, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY)); } } diff --git a/core/src/main/java/org/elasticsearch/rest/BytesRestResponse.java b/core/src/main/java/org/elasticsearch/rest/BytesRestResponse.java index 52f624849fc..2194732445d 100644 --- a/core/src/main/java/org/elasticsearch/rest/BytesRestResponse.java +++ b/core/src/main/java/org/elasticsearch/rest/BytesRestResponse.java @@ -40,10 +40,6 @@ public class BytesRestResponse extends RestResponse { private final BytesReference content; private final String contentType; - public BytesRestResponse(RestStatus status) { - this(status, TEXT_CONTENT_TYPE, BytesArray.EMPTY); - } - /** * Creates a new response based on {@link XContentBuilder}. */ diff --git a/core/src/main/java/org/elasticsearch/rest/RestController.java b/core/src/main/java/org/elasticsearch/rest/RestController.java index 6da1a929f7c..aa567af09f7 100644 --- a/core/src/main/java/org/elasticsearch/rest/RestController.java +++ b/core/src/main/java/org/elasticsearch/rest/RestController.java @@ -20,6 +20,7 @@ package org.elasticsearch.rest; import org.elasticsearch.common.Nullable; +import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.component.AbstractLifecycleComponent; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.path.PathTrie; @@ -215,9 +216,10 @@ public class RestController extends AbstractLifecycleComponent { } else { if (request.method() == RestRequest.Method.OPTIONS) { // when we have OPTIONS request, simply send OK by default (with the Access Control Origin header which gets automatically added) - channel.sendResponse(new BytesRestResponse(OK)); + channel.sendResponse(new BytesRestResponse(OK, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY)); } else { - channel.sendResponse(new BytesRestResponse(BAD_REQUEST, "No handler found for uri [" + request.uri() + "] and method [" + request.method() + "]")); + final String msg = "No handler found for uri [" + request.uri() + "] and method [" + request.method() + "]"; + channel.sendResponse(new BytesRestResponse(BAD_REQUEST, msg)); } } } diff --git a/core/src/main/java/org/elasticsearch/rest/action/admin/cluster/allocation/RestClusterAllocationExplainAction.java b/core/src/main/java/org/elasticsearch/rest/action/admin/cluster/allocation/RestClusterAllocationExplainAction.java index 06ba2a9be87..0785151e8d7 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/admin/cluster/allocation/RestClusterAllocationExplainAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/admin/cluster/allocation/RestClusterAllocationExplainAction.java @@ -24,6 +24,7 @@ import org.elasticsearch.action.admin.cluster.allocation.ClusterAllocationExplai import org.elasticsearch.action.admin.cluster.allocation.ClusterAllocationExplainResponse; import org.elasticsearch.client.Client; import org.elasticsearch.client.Requests; +import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.Settings; @@ -67,7 +68,8 @@ public class RestClusterAllocationExplainAction extends BaseRestHandler { req = ClusterAllocationExplainRequest.parse(parser); } catch (IOException e) { logger.debug("failed to parse allocation explain request", e); - channel.sendResponse(new BytesRestResponse(ExceptionsHelper.status(e))); + channel.sendResponse( + new BytesRestResponse(ExceptionsHelper.status(e), BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY)); return; } } @@ -83,7 +85,7 @@ public class RestClusterAllocationExplainAction extends BaseRestHandler { }); } catch (Exception e) { logger.error("failed to explain allocation", e); - channel.sendResponse(new BytesRestResponse(ExceptionsHelper.status(e))); + channel.sendResponse(new BytesRestResponse(ExceptionsHelper.status(e), BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY)); } } } diff --git a/core/src/main/java/org/elasticsearch/rest/action/admin/indices/alias/head/RestAliasesExistAction.java b/core/src/main/java/org/elasticsearch/rest/action/admin/indices/alias/head/RestAliasesExistAction.java index 15ea664245d..65f8fd363ea 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/admin/indices/alias/head/RestAliasesExistAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/admin/indices/alias/head/RestAliasesExistAction.java @@ -26,6 +26,7 @@ import org.elasticsearch.action.admin.indices.alias.get.GetAliasesRequest; import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.client.Client; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.rest.BaseRestHandler; @@ -65,9 +66,9 @@ public class RestAliasesExistAction extends BaseRestHandler { public void onResponse(AliasesExistResponse response) { try { if (response.isExists()) { - channel.sendResponse(new BytesRestResponse(OK)); + channel.sendResponse(new BytesRestResponse(OK, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY)); } else { - channel.sendResponse(new BytesRestResponse(NOT_FOUND)); + channel.sendResponse(new BytesRestResponse(NOT_FOUND, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY)); } } catch (Throwable e) { onFailure(e); @@ -77,7 +78,8 @@ public class RestAliasesExistAction extends BaseRestHandler { @Override public void onFailure(Throwable e) { try { - channel.sendResponse(new BytesRestResponse(ExceptionsHelper.status(e))); + channel.sendResponse( + new BytesRestResponse(ExceptionsHelper.status(e), BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY)); } catch (Exception e1) { logger.error("Failed to send failure response", e1); } diff --git a/core/src/main/java/org/elasticsearch/rest/action/admin/indices/exists/indices/RestIndicesExistsAction.java b/core/src/main/java/org/elasticsearch/rest/action/admin/indices/exists/indices/RestIndicesExistsAction.java index 72dea18abd9..0e240352a73 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/admin/indices/exists/indices/RestIndicesExistsAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/admin/indices/exists/indices/RestIndicesExistsAction.java @@ -24,6 +24,7 @@ import org.elasticsearch.action.admin.indices.exists.indices.IndicesExistsRespon import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.client.Client; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.rest.BaseRestHandler; @@ -58,9 +59,9 @@ public class RestIndicesExistsAction extends BaseRestHandler { @Override public RestResponse buildResponse(IndicesExistsResponse response) { if (response.isExists()) { - return new BytesRestResponse(OK); + return new BytesRestResponse(OK, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY); } else { - return new BytesRestResponse(NOT_FOUND); + return new BytesRestResponse(NOT_FOUND, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY); } } diff --git a/core/src/main/java/org/elasticsearch/rest/action/admin/indices/exists/types/RestTypesExistsAction.java b/core/src/main/java/org/elasticsearch/rest/action/admin/indices/exists/types/RestTypesExistsAction.java index dd206dcb63a..7a55be7e3af 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/admin/indices/exists/types/RestTypesExistsAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/admin/indices/exists/types/RestTypesExistsAction.java @@ -23,6 +23,7 @@ import org.elasticsearch.action.admin.indices.exists.types.TypesExistsResponse; import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.client.Client; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.rest.BaseRestHandler; @@ -59,9 +60,9 @@ public class RestTypesExistsAction extends BaseRestHandler { @Override public RestResponse buildResponse(TypesExistsResponse response) throws Exception { if (response.isExists()) { - return new BytesRestResponse(OK); + return new BytesRestResponse(OK, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY); } else { - return new BytesRestResponse(NOT_FOUND); + return new BytesRestResponse(NOT_FOUND, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY); } } }); diff --git a/core/src/main/java/org/elasticsearch/rest/action/admin/indices/template/head/RestHeadIndexTemplateAction.java b/core/src/main/java/org/elasticsearch/rest/action/admin/indices/template/head/RestHeadIndexTemplateAction.java index 648d083e763..2a40de984b9 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/admin/indices/template/head/RestHeadIndexTemplateAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/admin/indices/template/head/RestHeadIndexTemplateAction.java @@ -21,6 +21,7 @@ package org.elasticsearch.rest.action.admin.indices.template.head; import org.elasticsearch.action.admin.indices.template.get.GetIndexTemplatesRequest; import org.elasticsearch.action.admin.indices.template.get.GetIndexTemplatesResponse; import org.elasticsearch.client.Client; +import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.rest.BaseRestHandler; @@ -57,9 +58,9 @@ public class RestHeadIndexTemplateAction extends BaseRestHandler { public RestResponse buildResponse(GetIndexTemplatesResponse getIndexTemplatesResponse) { boolean templateExists = getIndexTemplatesResponse.getIndexTemplates().size() > 0; if (templateExists) { - return new BytesRestResponse(OK); + return new BytesRestResponse(OK, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY); } else { - return new BytesRestResponse(NOT_FOUND); + return new BytesRestResponse(NOT_FOUND, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY); } } }); diff --git a/core/src/main/java/org/elasticsearch/rest/action/get/RestHeadAction.java b/core/src/main/java/org/elasticsearch/rest/action/get/RestHeadAction.java index 9c98c98012c..747e62ea381 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/get/RestHeadAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/get/RestHeadAction.java @@ -23,6 +23,7 @@ import org.elasticsearch.action.get.GetRequest; import org.elasticsearch.action.get.GetResponse; import org.elasticsearch.client.Client; import org.elasticsearch.common.Strings; +import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.rest.BaseRestHandler; @@ -66,9 +67,9 @@ public class RestHeadAction extends BaseRestHandler { @Override public RestResponse buildResponse(GetResponse response) { if (!response.isExists()) { - return new BytesRestResponse(NOT_FOUND); + return new BytesRestResponse(NOT_FOUND, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY); } else { - return new BytesRestResponse(OK); + return new BytesRestResponse(OK, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY); } } }); diff --git a/core/src/main/java/org/elasticsearch/rest/action/main/RestMainAction.java b/core/src/main/java/org/elasticsearch/rest/action/main/RestMainAction.java index b9e57b4965b..e57bb790e5e 100644 --- a/core/src/main/java/org/elasticsearch/rest/action/main/RestMainAction.java +++ b/core/src/main/java/org/elasticsearch/rest/action/main/RestMainAction.java @@ -25,7 +25,6 @@ import org.elasticsearch.action.main.MainResponse; import org.elasticsearch.client.Client; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.settings.Settings; -import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.BytesRestResponse; @@ -66,7 +65,7 @@ public class RestMainAction extends BaseRestHandler { static BytesRestResponse convertMainResponse(MainResponse response, RestRequest request, XContentBuilder builder) throws IOException { RestStatus status = response.isAvailable() ? RestStatus.OK : RestStatus.SERVICE_UNAVAILABLE; if (request.method() == RestRequest.Method.HEAD) { - return new BytesRestResponse(status); + return new BytesRestResponse(status, builder); } // Default to pretty printing, but allow ?pretty=false to disable diff --git a/core/src/test/java/org/elasticsearch/http/HttpServerTests.java b/core/src/test/java/org/elasticsearch/http/HttpServerTests.java index 28fc315d3e1..9f002afd2dc 100644 --- a/core/src/test/java/org/elasticsearch/http/HttpServerTests.java +++ b/core/src/test/java/org/elasticsearch/http/HttpServerTests.java @@ -22,6 +22,7 @@ import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.breaker.CircuitBreaker; import org.elasticsearch.common.bytes.ByteBufferBytesReference; +import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.component.AbstractLifecycleComponent; import org.elasticsearch.common.settings.ClusterSettings; @@ -66,7 +67,8 @@ public class HttpServerTests extends ESTestCase { HttpServerTransport httpServerTransport = new TestHttpServerTransport(); RestController restController = new RestController(settings); restController.registerHandler(RestRequest.Method.GET, "/", - (request, channel) -> channel.sendResponse(new BytesRestResponse(RestStatus.OK))); + (request, channel) -> channel.sendResponse( + new BytesRestResponse(RestStatus.OK, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY))); restController.registerHandler(RestRequest.Method.GET, "/error", (request, channel) -> { throw new IllegalArgumentException("test error"); }); diff --git a/core/src/test/java/org/elasticsearch/rest/action/main/RestMainActionIT.java b/core/src/test/java/org/elasticsearch/rest/action/main/RestMainActionIT.java new file mode 100644 index 00000000000..0ad40f84cf7 --- /dev/null +++ b/core/src/test/java/org/elasticsearch/rest/action/main/RestMainActionIT.java @@ -0,0 +1,52 @@ +/* + * 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.rest.action.main; + +import org.elasticsearch.common.network.NetworkModule; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.test.ESIntegTestCase; +import org.elasticsearch.test.rest.client.http.HttpResponse; + +import java.io.IOException; + +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.nullValue; + +public class RestMainActionIT extends ESIntegTestCase { + @Override + protected Settings nodeSettings(int nodeOrdinal) { + return Settings.builder() + .put(super.nodeSettings(nodeOrdinal)) + .put(NetworkModule.HTTP_ENABLED.getKey(), true) + .build(); + } + + public void testHeadRequest() throws IOException { + final HttpResponse response = httpClient().method("HEAD").path("/").execute(); + assertThat(response.getStatusCode(), equalTo(200)); + assertThat(response.getBody(), nullValue()); + } + + public void testGetRequest() throws IOException { + final HttpResponse response = httpClient().path("/").execute(); + assertThat(response.getStatusCode(), equalTo(200)); + assertThat(response.getBody(), containsString("cluster_name")); + } +}