Skip to content

Commit

Permalink
Enhance Parent circuit breaker error message (#32056)
Browse files Browse the repository at this point in the history
* 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]
```

* Only call currentMemoryUsage once by returning structured object
  • Loading branch information
dakrone authored Jul 20, 2018
1 parent ac960bf commit 74aa7b0
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -215,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()]));
}

Expand All @@ -225,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);
}
}

Expand All @@ -246,15 +258,37 @@ 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 String message = "[parent] Data too large, data for [" + label + "]" +
" would be [" + totalUsed + "/" + new ByteSizeValue(totalUsed) + "]" +
final StringBuilder message = new StringBuilder("[parent] Data too large, data for [" + label + "]" +
" would be [" + parentUsed.totalUsage + "/" + new ByteSizeValue(parentUsed.totalUsage) + "]" +
", 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 = parentUsed.baseUsage;
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(), parentUsed.totalUsage, parentLimit);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]"));
}
}

Expand Down Expand Up @@ -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());

Expand Down

0 comments on commit 74aa7b0

Please sign in to comment.