From 90add0c5d3100f1fa65a38689a3c45fe9a905bbe Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Fri, 13 Jul 2018 14:18:15 -0600 Subject: [PATCH 1/2] Enhance Parent circuit breaker error message This adds information about either the current real usage (if tracking "real" memory usage) or the child breaker usages to the exception message when the parent circuit breaker trips. The messages now look like: ``` [parent] Data too large, data for [my_request] would be [211288064/201.5mb], which is larger than the limit of [209715200/200mb], usages [request=157286400/150mb, fielddata=54001664/51.5mb, in_flight_requests=0/0b, accounting=0/0b] ``` Or when tracking real memory usage: ``` [parent] Data too large, data for [request] would be [251/251b], which is larger than the limit of [200/200b], real usage: [181/181b], new bytes reserved: [70/70b] ``` --- .../HierarchyCircuitBreakerService.java | 29 +++++++++++++++++-- .../HierarchyCircuitBreakerServiceTests.java | 5 ++++ 2 files changed, 31 insertions(+), 3 deletions(-) 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 ebebcfd253c3b..44f0ccf33b929 100644 --- a/server/src/main/java/org/elasticsearch/indices/breaker/HierarchyCircuitBreakerService.java +++ b/server/src/main/java/org/elasticsearch/indices/breaker/HierarchyCircuitBreakerService.java @@ -37,6 +37,7 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.atomic.AtomicLong; +import java.util.stream.Collectors; /** * CircuitBreakerService that attempts to redistribute space between breakers @@ -250,11 +251,33 @@ public void checkParentLimit(long newBytesReserved, String label) throws Circuit long parentLimit = this.parentSettings.getLimit(); if (totalUsed > parentLimit) { this.parentTripCount.incrementAndGet(); - final String message = "[parent] Data too large, data for [" + label + "]" + + final StringBuilder message = new StringBuilder("[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); + parentLimit + "/" + new ByteSizeValue(parentLimit) + "]"); + if (this.trackRealMemoryUsage) { + final long realUsage = currentMemoryUsage(); + message.append(", real usage: ["); + message.append(realUsage); + message.append("/"); + message.append(new ByteSizeValue(realUsage)); + message.append("], new bytes reserved: ["); + message.append(newBytesReserved); + message.append("/"); + message.append(new ByteSizeValue(newBytesReserved)); + message.append("]"); + } else { + message.append(", usages ["); + message.append(String.join(", ", + this.breakers.entrySet().stream().map(e -> { + final CircuitBreaker breaker = e.getValue(); + final long breakerUsed = (long)(breaker.getUsed() * breaker.getOverhead()); + return e.getKey() + "=" + breakerUsed + "/" + new ByteSizeValue(breakerUsed); + }) + .collect(Collectors.toList()))); + message.append("]"); + } + throw new CircuitBreakingException(message.toString(), totalUsed, parentLimit); } } diff --git a/server/src/test/java/org/elasticsearch/indices/breaker/HierarchyCircuitBreakerServiceTests.java b/server/src/test/java/org/elasticsearch/indices/breaker/HierarchyCircuitBreakerServiceTests.java index 00bd15d244fde..4918584ee2be7 100644 --- a/server/src/test/java/org/elasticsearch/indices/breaker/HierarchyCircuitBreakerServiceTests.java +++ b/server/src/test/java/org/elasticsearch/indices/breaker/HierarchyCircuitBreakerServiceTests.java @@ -199,6 +199,8 @@ public void testBorrowingSiblingBreakerMemory() throws Exception { .addEstimateBytesAndMaybeBreak(new ByteSizeValue(50, ByteSizeUnit.MB).getBytes(), "should break")); assertThat(exception.getMessage(), containsString("[parent] Data too large, data for [should break] would be")); assertThat(exception.getMessage(), containsString("which is larger than the limit of [209715200/200mb]")); + assertThat(exception.getMessage(), + containsString("usages [request=157286400/150mb, fielddata=54001664/51.5mb, in_flight_requests=0/0b, accounting=0/0b]")); } } @@ -239,6 +241,9 @@ long currentMemoryUsage() { // it was the parent that rejected the reservation assertThat(exception.getMessage(), containsString("[parent] Data too large, data for [request] would be")); assertThat(exception.getMessage(), containsString("which is larger than the limit of [200/200b]")); + assertThat(exception.getMessage(), + containsString("real usage: [181/181b], new bytes reserved: [" + (reservationInBytes * 2) + + "/" + new ByteSizeValue(reservationInBytes * 2) + "]")); assertEquals(0, requestBreaker.getTrippedCount()); assertEquals(1, service.stats().getStats(CircuitBreaker.PARENT).getTrippedCount()); From ceec2ea8d168d2b8ba68aebcdd9988b8619b3459 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Thu, 19 Jul 2018 15:19:19 -0600 Subject: [PATCH 2/2] Only call currentMemoryUsage once by returning structured object --- .../HierarchyCircuitBreakerService.java | 29 +++++++++++++------ 1 file changed, 20 insertions(+), 9 deletions(-) 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 44f0ccf33b929..7e6a9c29a8344 100644 --- a/server/src/main/java/org/elasticsearch/indices/breaker/HierarchyCircuitBreakerService.java +++ b/server/src/main/java/org/elasticsearch/indices/breaker/HierarchyCircuitBreakerService.java @@ -216,7 +216,7 @@ public AllCircuitBreakerStats stats() { } // Manually add the parent breaker settings since they aren't part of the breaker map allStats.add(new CircuitBreakerStats(CircuitBreaker.PARENT, parentSettings.getLimit(), - parentUsed(0L), 1.0, parentTripCount.get())); + parentUsed(0L).totalUsage, 1.0, parentTripCount.get())); return new AllCircuitBreakerStats(allStats.toArray(new CircuitBreakerStats[allStats.size()])); } @@ -226,15 +226,26 @@ public CircuitBreakerStats stats(String name) { return new CircuitBreakerStats(breaker.getName(), breaker.getLimit(), breaker.getUsed(), breaker.getOverhead(), breaker.getTrippedCount()); } - private long parentUsed(long newBytesReserved) { + private static class ParentMemoryUsage { + final long baseUsage; + final long totalUsage; + + ParentMemoryUsage(final long baseUsage, final long totalUsage) { + this.baseUsage = baseUsage; + this.totalUsage = totalUsage; + } + } + + private ParentMemoryUsage parentUsed(long newBytesReserved) { if (this.trackRealMemoryUsage) { - return currentMemoryUsage() + newBytesReserved; + final long current = currentMemoryUsage(); + return new ParentMemoryUsage(current, current + newBytesReserved); } else { long parentEstimated = 0; for (CircuitBreaker breaker : this.breakers.values()) { parentEstimated += breaker.getUsed() * breaker.getOverhead(); } - return parentEstimated; + return new ParentMemoryUsage(parentEstimated, parentEstimated); } } @@ -247,16 +258,16 @@ long currentMemoryUsage() { * Checks whether the parent breaker has been tripped */ public void checkParentLimit(long newBytesReserved, String label) throws CircuitBreakingException { - long totalUsed = parentUsed(newBytesReserved); + final ParentMemoryUsage parentUsed = parentUsed(newBytesReserved); long parentLimit = this.parentSettings.getLimit(); - if (totalUsed > parentLimit) { + if (parentUsed.totalUsage > parentLimit) { this.parentTripCount.incrementAndGet(); final StringBuilder message = new StringBuilder("[parent] Data too large, data for [" + label + "]" + - " would be [" + totalUsed + "/" + new ByteSizeValue(totalUsed) + "]" + + " would be [" + parentUsed.totalUsage + "/" + new ByteSizeValue(parentUsed.totalUsage) + "]" + ", which is larger than the limit of [" + parentLimit + "/" + new ByteSizeValue(parentLimit) + "]"); if (this.trackRealMemoryUsage) { - final long realUsage = currentMemoryUsage(); + final long realUsage = parentUsed.baseUsage; message.append(", real usage: ["); message.append(realUsage); message.append("/"); @@ -277,7 +288,7 @@ public void checkParentLimit(long newBytesReserved, String label) throws Circuit .collect(Collectors.toList()))); message.append("]"); } - throw new CircuitBreakingException(message.toString(), totalUsed, parentLimit); + throw new CircuitBreakingException(message.toString(), parentUsed.totalUsage, parentLimit); } }