From 3d53daeb2f58c5b6986f80a238f8b28d90447f88 Mon Sep 17 00:00:00 2001 From: Daniel Mitterdorfer Date: Tue, 3 Jul 2018 09:17:16 +0200 Subject: [PATCH] Account for XContent overhead in in-flight breaker So far the in-flight request circuit breaker has only accounted for the on-the-wire representation of a request. However, we convert the raw request into XContent internally which increases the overhead. Therefore, we increase the value of the corresponding setting `network.breaker.inflight_requests.overhead` from one to two. While this value is still rather conservative (we assume that the representation as structured objects has no overhead compared to the byte[]), it is closer to reality than the current value. Relates #31613 --- .../migration/migrate_7_0/indices.asciidoc | 6 ++++++ .../modules/indices/circuit_breaker.asciidoc | 6 ++++-- .../breaker/HierarchyCircuitBreakerService.java | 2 +- .../elasticsearch/rest/RestControllerTests.java | 16 ++++++++-------- 4 files changed, 19 insertions(+), 11 deletions(-) diff --git a/docs/reference/migration/migrate_7_0/indices.asciidoc b/docs/reference/migration/migrate_7_0/indices.asciidoc index db0c0ede466d0..b03a6014d5bc5 100644 --- a/docs/reference/migration/migrate_7_0/indices.asciidoc +++ b/docs/reference/migration/migrate_7_0/indices.asciidoc @@ -64,3 +64,9 @@ The following previously deprecated url parameter have been removed: * `filter_cache` - use `query` instead * `request_cache` - use `request` instead * `field_data` - use `fielddata` instead + +==== `network.breaker.inflight_requests.overhead` increased to 2 + +Previously the in flight requests circuit breaker considered only the raw byte representation. +By bumping the value of `network.breaker.inflight_requests.overhead` from 1 to 2, this circuit +breaker considers now also the memory overhead of representing the request as a structured object. \ No newline at end of file diff --git a/docs/reference/modules/indices/circuit_breaker.asciidoc b/docs/reference/modules/indices/circuit_breaker.asciidoc index 03cdb307b9f1e..559137a821036 100644 --- a/docs/reference/modules/indices/circuit_breaker.asciidoc +++ b/docs/reference/modules/indices/circuit_breaker.asciidoc @@ -60,7 +60,9 @@ request) from exceeding a certain amount of memory. The in flight requests circuit breaker allows Elasticsearch to limit the memory usage of all currently active incoming requests on transport or HTTP level from exceeding a certain amount of -memory on a node. The memory usage is based on the content length of the request itself. +memory on a node. The memory usage is based on the content length of the request itself. This +circuit breaker also considers that memory is not only needed for representing the raw request but +also as a structured object which is reflected by default overhead. `network.breaker.inflight_requests.limit`:: @@ -70,7 +72,7 @@ memory on a node. The memory usage is based on the content length of the request `network.breaker.inflight_requests.overhead`:: A constant that all in flight requests estimations are multiplied with to determine a - final estimation. Defaults to 1 + final estimation. Defaults to 2. [[accounting-circuit-breaker]] [float] diff --git a/server/src/main/java/org/elasticsearch/indices/breaker/HierarchyCircuitBreakerService.java b/server/src/main/java/org/elasticsearch/indices/breaker/HierarchyCircuitBreakerService.java index 9ea8a3df29492..e3a914c730ec8 100644 --- a/server/src/main/java/org/elasticsearch/indices/breaker/HierarchyCircuitBreakerService.java +++ b/server/src/main/java/org/elasticsearch/indices/breaker/HierarchyCircuitBreakerService.java @@ -73,7 +73,7 @@ public class HierarchyCircuitBreakerService extends CircuitBreakerService { public static final Setting IN_FLIGHT_REQUESTS_CIRCUIT_BREAKER_LIMIT_SETTING = Setting.memorySizeSetting("network.breaker.inflight_requests.limit", "100%", Property.Dynamic, Property.NodeScope); public static final Setting IN_FLIGHT_REQUESTS_CIRCUIT_BREAKER_OVERHEAD_SETTING = - Setting.doubleSetting("network.breaker.inflight_requests.overhead", 1.0d, 0.0d, Property.Dynamic, Property.NodeScope); + Setting.doubleSetting("network.breaker.inflight_requests.overhead", 2.0d, 0.0d, Property.Dynamic, Property.NodeScope); public static final Setting IN_FLIGHT_REQUESTS_CIRCUIT_BREAKER_TYPE_SETTING = new Setting<>("network.breaker.inflight_requests.type", "memory", CircuitBreaker.Type::parseValue, Property.NodeScope); diff --git a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java index a090cc40b6857..348b85a8ba4a1 100644 --- a/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java +++ b/server/src/test/java/org/elasticsearch/rest/RestControllerTests.java @@ -239,7 +239,7 @@ public boolean canTripCircuitBreaker() { public void testDispatchRequestAddsAndFreesBytesOnSuccess() { int contentLength = BREAKER_LIMIT.bytesAsInt(); - String content = randomAlphaOfLength(contentLength); + String content = randomAlphaOfLength((int) Math.round(contentLength / inFlightRequestsBreaker.getOverhead())); RestRequest request = testRestRequest("/", content, XContentType.JSON); AssertingChannel channel = new AssertingChannel(request, true, RestStatus.OK); @@ -251,7 +251,7 @@ public void testDispatchRequestAddsAndFreesBytesOnSuccess() { public void testDispatchRequestAddsAndFreesBytesOnError() { int contentLength = BREAKER_LIMIT.bytesAsInt(); - String content = randomAlphaOfLength(contentLength); + String content = randomAlphaOfLength((int) Math.round(contentLength / inFlightRequestsBreaker.getOverhead())); RestRequest request = testRestRequest("/error", content, XContentType.JSON); AssertingChannel channel = new AssertingChannel(request, true, RestStatus.BAD_REQUEST); @@ -263,7 +263,7 @@ public void testDispatchRequestAddsAndFreesBytesOnError() { public void testDispatchRequestAddsAndFreesBytesOnlyOnceOnError() { int contentLength = BREAKER_LIMIT.bytesAsInt(); - String content = randomAlphaOfLength(contentLength); + String content = randomAlphaOfLength((int) Math.round(contentLength / inFlightRequestsBreaker.getOverhead())); // we will produce an error in the rest handler and one more when sending the error response RestRequest request = testRestRequest("/error", content, XContentType.JSON); ExceptionThrowingChannel channel = new ExceptionThrowingChannel(request, true); @@ -276,7 +276,7 @@ public void testDispatchRequestAddsAndFreesBytesOnlyOnceOnError() { public void testDispatchRequestLimitsBytes() { int contentLength = BREAKER_LIMIT.bytesAsInt() + 1; - String content = randomAlphaOfLength(contentLength); + String content = randomAlphaOfLength((int) Math.round(contentLength / inFlightRequestsBreaker.getOverhead())); RestRequest request = testRestRequest("/", content, XContentType.JSON); AssertingChannel channel = new AssertingChannel(request, true, RestStatus.SERVICE_UNAVAILABLE); @@ -287,7 +287,7 @@ public void testDispatchRequestLimitsBytes() { } public void testDispatchRequiresContentTypeForRequestsWithContent() { - String content = randomAlphaOfLengthBetween(1, BREAKER_LIMIT.bytesAsInt()); + String content = randomAlphaOfLength((int) Math.round(BREAKER_LIMIT.getBytes() / inFlightRequestsBreaker.getOverhead())); RestRequest request = testRestRequest("/", content, null); AssertingChannel channel = new AssertingChannel(request, true, RestStatus.NOT_ACCEPTABLE); restController = new RestController( @@ -312,7 +312,7 @@ public void testDispatchDoesNotRequireContentTypeForRequestsWithoutContent() { } public void testDispatchFailsWithPlainText() { - String content = randomAlphaOfLengthBetween(1, BREAKER_LIMIT.bytesAsInt()); + String content = randomAlphaOfLength((int) Math.round(BREAKER_LIMIT.getBytes() / inFlightRequestsBreaker.getOverhead())); FakeRestRequest fakeRestRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY) .withContent(new BytesArray(content), null).withPath("/foo") .withHeaders(Collections.singletonMap("Content-Type", Collections.singletonList("text/plain"))).build(); @@ -342,7 +342,7 @@ public void testDispatchUnsupportedContentType() { public void testDispatchWorksWithNewlineDelimitedJson() { final String mimeType = "application/x-ndjson"; - String content = randomAlphaOfLengthBetween(1, BREAKER_LIMIT.bytesAsInt()); + String content = randomAlphaOfLength((int) Math.round(BREAKER_LIMIT.getBytes() / inFlightRequestsBreaker.getOverhead())); FakeRestRequest fakeRestRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY) .withContent(new BytesArray(content), null).withPath("/foo") .withHeaders(Collections.singletonMap("Content-Type", Collections.singletonList(mimeType))).build(); @@ -366,7 +366,7 @@ public boolean supportsContentStream() { public void testDispatchWithContentStream() { final String mimeType = randomFrom("application/json", "application/smile"); - String content = randomAlphaOfLengthBetween(1, BREAKER_LIMIT.bytesAsInt()); + String content = randomAlphaOfLength((int) Math.round(BREAKER_LIMIT.getBytes() / inFlightRequestsBreaker.getOverhead())); final List contentTypeHeader = Collections.singletonList(mimeType); FakeRestRequest fakeRestRequest = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY) .withContent(new BytesArray(content), RestRequest.parseContentType(contentTypeHeader)).withPath("/foo")