Skip to content

Commit 74aa7b0

Browse files
authored
Enhance Parent circuit breaker error message (#32056)
* 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
1 parent ac960bf commit 74aa7b0

File tree

2 files changed

+49
-10
lines changed

2 files changed

+49
-10
lines changed

server/src/main/java/org/elasticsearch/indices/breaker/HierarchyCircuitBreakerService.java

Lines changed: 44 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import java.util.concurrent.ConcurrentHashMap;
3838
import java.util.concurrent.ConcurrentMap;
3939
import java.util.concurrent.atomic.AtomicLong;
40+
import java.util.stream.Collectors;
4041

4142
/**
4243
* CircuitBreakerService that attempts to redistribute space between breakers
@@ -215,7 +216,7 @@ public AllCircuitBreakerStats stats() {
215216
}
216217
// Manually add the parent breaker settings since they aren't part of the breaker map
217218
allStats.add(new CircuitBreakerStats(CircuitBreaker.PARENT, parentSettings.getLimit(),
218-
parentUsed(0L), 1.0, parentTripCount.get()));
219+
parentUsed(0L).totalUsage, 1.0, parentTripCount.get()));
219220
return new AllCircuitBreakerStats(allStats.toArray(new CircuitBreakerStats[allStats.size()]));
220221
}
221222

@@ -225,15 +226,26 @@ public CircuitBreakerStats stats(String name) {
225226
return new CircuitBreakerStats(breaker.getName(), breaker.getLimit(), breaker.getUsed(), breaker.getOverhead(), breaker.getTrippedCount());
226227
}
227228

228-
private long parentUsed(long newBytesReserved) {
229+
private static class ParentMemoryUsage {
230+
final long baseUsage;
231+
final long totalUsage;
232+
233+
ParentMemoryUsage(final long baseUsage, final long totalUsage) {
234+
this.baseUsage = baseUsage;
235+
this.totalUsage = totalUsage;
236+
}
237+
}
238+
239+
private ParentMemoryUsage parentUsed(long newBytesReserved) {
229240
if (this.trackRealMemoryUsage) {
230-
return currentMemoryUsage() + newBytesReserved;
241+
final long current = currentMemoryUsage();
242+
return new ParentMemoryUsage(current, current + newBytesReserved);
231243
} else {
232244
long parentEstimated = 0;
233245
for (CircuitBreaker breaker : this.breakers.values()) {
234246
parentEstimated += breaker.getUsed() * breaker.getOverhead();
235247
}
236-
return parentEstimated;
248+
return new ParentMemoryUsage(parentEstimated, parentEstimated);
237249
}
238250
}
239251

@@ -246,15 +258,37 @@ long currentMemoryUsage() {
246258
* Checks whether the parent breaker has been tripped
247259
*/
248260
public void checkParentLimit(long newBytesReserved, String label) throws CircuitBreakingException {
249-
long totalUsed = parentUsed(newBytesReserved);
261+
final ParentMemoryUsage parentUsed = parentUsed(newBytesReserved);
250262
long parentLimit = this.parentSettings.getLimit();
251-
if (totalUsed > parentLimit) {
263+
if (parentUsed.totalUsage > parentLimit) {
252264
this.parentTripCount.incrementAndGet();
253-
final String message = "[parent] Data too large, data for [" + label + "]" +
254-
" would be [" + totalUsed + "/" + new ByteSizeValue(totalUsed) + "]" +
265+
final StringBuilder message = new StringBuilder("[parent] Data too large, data for [" + label + "]" +
266+
" would be [" + parentUsed.totalUsage + "/" + new ByteSizeValue(parentUsed.totalUsage) + "]" +
255267
", which is larger than the limit of [" +
256-
parentLimit + "/" + new ByteSizeValue(parentLimit) + "]";
257-
throw new CircuitBreakingException(message, totalUsed, parentLimit);
268+
parentLimit + "/" + new ByteSizeValue(parentLimit) + "]");
269+
if (this.trackRealMemoryUsage) {
270+
final long realUsage = parentUsed.baseUsage;
271+
message.append(", real usage: [");
272+
message.append(realUsage);
273+
message.append("/");
274+
message.append(new ByteSizeValue(realUsage));
275+
message.append("], new bytes reserved: [");
276+
message.append(newBytesReserved);
277+
message.append("/");
278+
message.append(new ByteSizeValue(newBytesReserved));
279+
message.append("]");
280+
} else {
281+
message.append(", usages [");
282+
message.append(String.join(", ",
283+
this.breakers.entrySet().stream().map(e -> {
284+
final CircuitBreaker breaker = e.getValue();
285+
final long breakerUsed = (long)(breaker.getUsed() * breaker.getOverhead());
286+
return e.getKey() + "=" + breakerUsed + "/" + new ByteSizeValue(breakerUsed);
287+
})
288+
.collect(Collectors.toList())));
289+
message.append("]");
290+
}
291+
throw new CircuitBreakingException(message.toString(), parentUsed.totalUsage, parentLimit);
258292
}
259293
}
260294

server/src/test/java/org/elasticsearch/indices/breaker/HierarchyCircuitBreakerServiceTests.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,8 @@ public void testBorrowingSiblingBreakerMemory() throws Exception {
199199
.addEstimateBytesAndMaybeBreak(new ByteSizeValue(50, ByteSizeUnit.MB).getBytes(), "should break"));
200200
assertThat(exception.getMessage(), containsString("[parent] Data too large, data for [should break] would be"));
201201
assertThat(exception.getMessage(), containsString("which is larger than the limit of [209715200/200mb]"));
202+
assertThat(exception.getMessage(),
203+
containsString("usages [request=157286400/150mb, fielddata=54001664/51.5mb, in_flight_requests=0/0b, accounting=0/0b]"));
202204
}
203205
}
204206

@@ -239,6 +241,9 @@ long currentMemoryUsage() {
239241
// it was the parent that rejected the reservation
240242
assertThat(exception.getMessage(), containsString("[parent] Data too large, data for [request] would be"));
241243
assertThat(exception.getMessage(), containsString("which is larger than the limit of [200/200b]"));
244+
assertThat(exception.getMessage(),
245+
containsString("real usage: [181/181b], new bytes reserved: [" + (reservationInBytes * 2) +
246+
"/" + new ByteSizeValue(reservationInBytes * 2) + "]"));
242247
assertEquals(0, requestBreaker.getTrippedCount());
243248
assertEquals(1, service.stats().getStats(CircuitBreaker.PARENT).getTrippedCount());
244249

0 commit comments

Comments
 (0)