From 41abf6e81da0e3d9c3d0aa1dea5cd4896019d1e1 Mon Sep 17 00:00:00 2001 From: Alex Bumbu Date: Thu, 19 Jan 2017 15:38:18 +0000 Subject: [PATCH] Add used memory amount to CircuitBreakingException message (#22521) --- .../breaker/ChildMemoryCircuitBreaker.java | 8 ++-- .../common/breaker/MemoryCircuitBreaker.java | 4 +- .../HierarchyCircuitBreakerService.java | 9 ++-- .../breaker/MemoryCircuitBreakerTests.java | 3 ++ .../breaker/CircuitBreakerServiceIT.java | 47 ++++++++++++------- 5 files changed, 45 insertions(+), 26 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/common/breaker/ChildMemoryCircuitBreaker.java b/core/src/main/java/org/elasticsearch/common/breaker/ChildMemoryCircuitBreaker.java index 68bf52e9e0d..e4019f9f665 100644 --- a/core/src/main/java/org/elasticsearch/common/breaker/ChildMemoryCircuitBreaker.java +++ b/core/src/main/java/org/elasticsearch/common/breaker/ChildMemoryCircuitBreaker.java @@ -90,12 +90,12 @@ public class ChildMemoryCircuitBreaker implements CircuitBreaker { @Override public void circuitBreak(String fieldName, long bytesNeeded) { this.trippedCount.incrementAndGet(); - final String message = "[" + this.name + "] Data too large, data for [" + - fieldName + "] would be larger than limit of [" + + final String message = "[" + this.name + "] Data too large, data for [" + fieldName + "]" + + " would be [" + bytesNeeded + "/" + new ByteSizeValue(bytesNeeded) + "]" + + ", which is larger than the limit of [" + memoryBytesLimit + "/" + new ByteSizeValue(memoryBytesLimit) + "]"; logger.debug("{}", message); - throw new CircuitBreakingException(message, - bytesNeeded, this.memoryBytesLimit); + throw new CircuitBreakingException(message, bytesNeeded, memoryBytesLimit); } /** diff --git a/core/src/main/java/org/elasticsearch/common/breaker/MemoryCircuitBreaker.java b/core/src/main/java/org/elasticsearch/common/breaker/MemoryCircuitBreaker.java index 23e76a9fd35..dbd1fe92ffe 100644 --- a/core/src/main/java/org/elasticsearch/common/breaker/MemoryCircuitBreaker.java +++ b/core/src/main/java/org/elasticsearch/common/breaker/MemoryCircuitBreaker.java @@ -79,7 +79,9 @@ public class MemoryCircuitBreaker implements CircuitBreaker { @Override public void circuitBreak(String fieldName, long bytesNeeded) throws CircuitBreakingException { this.trippedCount.incrementAndGet(); - final String message = "Data too large, data for field [" + fieldName + "] would be larger than limit of [" + + final String message = "[" + getName() + "] Data too large, data for field [" + fieldName + "]" + + " would be [" + bytesNeeded + "/" + new ByteSizeValue(bytesNeeded) + "]" + + ", which is larger than the limit of [" + memoryBytesLimit + "/" + new ByteSizeValue(memoryBytesLimit) + "]"; logger.debug("{}", message); throw new CircuitBreakingException(message, bytesNeeded, memoryBytesLimit); diff --git a/core/src/main/java/org/elasticsearch/indices/breaker/HierarchyCircuitBreakerService.java b/core/src/main/java/org/elasticsearch/indices/breaker/HierarchyCircuitBreakerService.java index de31a57283b..88cff7f559a 100644 --- a/core/src/main/java/org/elasticsearch/indices/breaker/HierarchyCircuitBreakerService.java +++ b/core/src/main/java/org/elasticsearch/indices/breaker/HierarchyCircuitBreakerService.java @@ -208,10 +208,11 @@ public class HierarchyCircuitBreakerService extends CircuitBreakerService { long parentLimit = this.parentSettings.getLimit(); if (totalUsed > parentLimit) { this.parentTripCount.incrementAndGet(); - throw new CircuitBreakingException("[parent] Data too large, data for [" + - label + "] would be larger than limit of [" + - parentLimit + "/" + new ByteSizeValue(parentLimit) + "]", - totalUsed, parentLimit); + final String message = "[parent] Data too large, data for [" + label + "]" + + " would be [" + totalUsed + "/" + new ByteSizeValue(totalUsed) + "]" + + ", which is larger than the limit of [" + + parentLimit + "/" + new ByteSizeValue(parentLimit) + "]"; + throw new CircuitBreakingException(message, totalUsed, parentLimit); } } diff --git a/core/src/test/java/org/elasticsearch/common/breaker/MemoryCircuitBreakerTests.java b/core/src/test/java/org/elasticsearch/common/breaker/MemoryCircuitBreakerTests.java index f10a0da3029..ff93e04bf92 100644 --- a/core/src/test/java/org/elasticsearch/common/breaker/MemoryCircuitBreakerTests.java +++ b/core/src/test/java/org/elasticsearch/common/breaker/MemoryCircuitBreakerTests.java @@ -235,6 +235,9 @@ public class MemoryCircuitBreakerTests extends ESTestCase { fail("should never reach this"); } catch (CircuitBreakingException cbe) { assertThat("breaker was tripped exactly twice", breaker.getTrippedCount(), equalTo(2L)); + + long newUsed = (long)(breaker.getUsed() * breaker.getOverhead()); + assertThat(cbe.getMessage().contains("would be [" + newUsed + "/"), equalTo(true)); assertThat(cbe.getMessage().contains("field [" + field + "]"), equalTo(true)); } } diff --git a/core/src/test/java/org/elasticsearch/indices/memory/breaker/CircuitBreakerServiceIT.java b/core/src/test/java/org/elasticsearch/indices/memory/breaker/CircuitBreakerServiceIT.java index 63517dbdc97..0ef915b8276 100644 --- a/core/src/test/java/org/elasticsearch/indices/memory/breaker/CircuitBreakerServiceIT.java +++ b/core/src/test/java/org/elasticsearch/indices/memory/breaker/CircuitBreakerServiceIT.java @@ -153,8 +153,11 @@ public class CircuitBreakerServiceIT extends ESIntegTestCase { // execute a search that loads field data (sorting on the "test" field) // again, this time it should trip the breaker SearchRequestBuilder searchRequest = client.prepareSearch("cb-test").setQuery(matchAllQuery()).addSort("test", SortOrder.DESC); - assertFailures(searchRequest, RestStatus.INTERNAL_SERVER_ERROR, - containsString("Data too large, data for [test] would be larger than limit of [100/100b]")); + + String errMsg = "Data too large, data for [test] would be"; + assertFailures(searchRequest, RestStatus.INTERNAL_SERVER_ERROR, containsString(errMsg)); + errMsg = "which is larger than the limit of [100/100b]"; + assertFailures(searchRequest, RestStatus.INTERNAL_SERVER_ERROR, containsString(errMsg)); NodesStatsResponse stats = client.admin().cluster().prepareNodesStats().setBreaker(true).get(); int breaks = 0; @@ -201,9 +204,12 @@ public class CircuitBreakerServiceIT extends ESIntegTestCase { // execute a search that loads field data (sorting on the "test" field) // again, this time it should trip the breaker - assertFailures(client.prepareSearch("ramtest").setQuery(matchAllQuery()).addSort("test", SortOrder.DESC), - RestStatus.INTERNAL_SERVER_ERROR, - containsString("Data too large, data for [test] would be larger than limit of [100/100b]")); + SearchRequestBuilder searchRequest = client.prepareSearch("ramtest").setQuery(matchAllQuery()).addSort("test", SortOrder.DESC); + + String errMsg = "Data too large, data for [test] would be"; + assertFailures(searchRequest, RestStatus.INTERNAL_SERVER_ERROR, containsString(errMsg)); + errMsg = "which is larger than the limit of [100/100b]"; + assertFailures(searchRequest, RestStatus.INTERNAL_SERVER_ERROR, containsString(errMsg)); NodesStatsResponse stats = client.admin().cluster().prepareNodesStats().setBreaker(true).get(); int breaks = 0; @@ -247,14 +253,20 @@ public class CircuitBreakerServiceIT extends ESIntegTestCase { client.prepareSearch("cb-test").setQuery(matchAllQuery()).addSort("test", SortOrder.DESC).get(); fail("should have thrown an exception"); } catch (Exception e) { - String errMsg = "[fielddata] Data too large, data for [test] would be larger than limit of [10/10b]"; - assertThat("Exception: [" + e.toString() + "] should contain a CircuitBreakingException", - e.toString(), containsString(errMsg)); + String errMsg = "CircuitBreakingException[[fielddata] Data too large, data for [test] would be"; + assertThat("Exception: [" + e.toString() + "] should contain a CircuitBreakingException", e.toString(), containsString(errMsg)); + errMsg = "which is larger than the limit of [10/10b]]"; + assertThat("Exception: [" + e.toString() + "] should contain a CircuitBreakingException", e.toString(), containsString(errMsg)); } - assertFailures(client.prepareSearch("cb-test").setQuery(matchAllQuery()).addSort("test", SortOrder.DESC), - RestStatus.INTERNAL_SERVER_ERROR, - containsString("Data too large, data for [test] would be larger than limit of [10/10b]")); + // execute a search that loads field data (sorting on the "test" field) + // again, this time it should trip the breaker + SearchRequestBuilder searchRequest = client.prepareSearch("cb-test").setQuery(matchAllQuery()).addSort("test", SortOrder.DESC); + + String errMsg = "Data too large, data for [test] would be"; + assertFailures(searchRequest, RestStatus.INTERNAL_SERVER_ERROR, containsString(errMsg)); + errMsg = "which is larger than the limit of [10/10b]"; + assertFailures(searchRequest, RestStatus.INTERNAL_SERVER_ERROR, containsString(errMsg)); reset(); @@ -318,9 +330,8 @@ public class CircuitBreakerServiceIT extends ESIntegTestCase { fail("aggregation should have tripped the breaker"); } catch (Exception e) { String errMsg = "CircuitBreakingException[[request] Data too large"; - assertThat("Exception: [" + e.toString() + "] should contain a CircuitBreakingException", - e.toString(), containsString(errMsg)); - errMsg = "would be larger than limit of [10/10b]]"; + assertThat("Exception: [" + e.toString() + "] should contain a CircuitBreakingException", e.toString(), containsString(errMsg)); + errMsg = "which is larger than the limit of [10/10b]]"; assertThat("Exception: [" + e.toString() + "] should contain a CircuitBreakingException", e.toString(), containsString(errMsg)); } } @@ -356,9 +367,11 @@ public class CircuitBreakerServiceIT extends ESIntegTestCase { assertTrue("there should be shard failures", resp.getFailedShards() > 0); fail("aggregation should have tripped the breaker"); } catch (Exception e) { - String errMsg = "CircuitBreakingException[[request] " + - "Data too large, data for [] would be larger than limit of [100/100b]]"; - assertThat("Exception: " + e.toString() + " should contain a CircuitBreakingException", + String errMsg = "CircuitBreakingException[[request] Data too large, data for [] would be"; + assertThat("Exception: [" + e.toString() + "] should contain a CircuitBreakingException", + e.toString(), containsString(errMsg)); + errMsg = "which is larger than the limit of [100/100b]]"; + assertThat("Exception: [" + e.toString() + "] should contain a CircuitBreakingException", e.toString(), containsString(errMsg)); } }