Skip to content

Commit

Permalink
Always call the over limit strategy
Browse files Browse the repository at this point in the history 
Now determine strategy based on whether real memory usage is
tracked.
  • Loading branch information
@henningandersen
henningandersen committed Jul 7, 2020
1 parent 845dc10 commit 3f61f93
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 24 deletions.
21 changes: 7 additions & 14 deletions server/src/main/java/org/elasticsearch/indices/breaker/HierarchyCircuitBreakerService.java
View file
Expand Up @@ -45,6 +45,7 @@
import java.util.List;
import java.util.Map;
import java.util.concurrent.atomic.AtomicLong;
import java.util.function.Function;
import java.util.function.LongSupplier;
import java.util.stream.Collectors;

Expand Down Expand Up @@ -113,11 +114,11 @@ public class HierarchyCircuitBreakerService extends CircuitBreakerService {
private final OverLimitStrategy overLimitStrategy;

public HierarchyCircuitBreakerService(Settings settings, List<BreakerSettings> customBreakers, ClusterSettings clusterSettings) {
this(settings, customBreakers, clusterSettings, createOverLimitStrategy());
this(settings, customBreakers, clusterSettings, HierarchyCircuitBreakerService::createOverLimitStrategy);
}

HierarchyCircuitBreakerService(Settings settings, List<BreakerSettings> customBreakers, ClusterSettings clusterSettings,
OverLimitStrategy overLimitStrategy) {
Function<Boolean, OverLimitStrategy> overLimitStrategyFactory) {
super();
HashMap<String, CircuitBreaker> childCircuitBreakers = new HashMap<>();
childCircuitBreakers.put(CircuitBreaker.FIELDDATA, validateAndCreateBreaker(
Expand Down Expand Up @@ -182,7 +183,7 @@ public HierarchyCircuitBreakerService(Settings settings, List<BreakerSettings> c
(name, updatedValues) -> updateCircuitBreakerSettings(name, updatedValues.v1(), updatedValues.v2()),
(s, t) -> {});

this.overLimitStrategy = overLimitStrategy;
this.overLimitStrategy = overLimitStrategyFactory.apply(this.trackRealMemoryUsage);
}

private void updateCircuitBreakerSettings(String name, ByteSizeValue newLimit, Double newOverhead) {
Expand Down Expand Up @@ -309,7 +310,7 @@ public long getParentLimit() {
public void checkParentLimit(long newBytesReserved, String label) throws CircuitBreakingException {
final MemoryUsage memoryUsed = memoryUsed(newBytesReserved);
long parentLimit = this.parentSettings.getLimit();
if (memoryUsed.totalUsage > parentLimit && doubleCheckMemoryUsed(memoryUsed).totalUsage > parentLimit) {
if (memoryUsed.totalUsage > parentLimit && overLimitStrategy.overLimit(memoryUsed).totalUsage > parentLimit) {
this.parentTripCount.incrementAndGet();
final StringBuilder message = new StringBuilder("[parent] Data too large, data for [" + label + "]" +
" would be [" + memoryUsed.totalUsage + "/" + new ByteSizeValue(memoryUsed.totalUsage) + "]" +
Expand Down Expand Up @@ -343,14 +344,6 @@ public void checkParentLimit(long newBytesReserved, String label) throws Circuit
}
}

MemoryUsage doubleCheckMemoryUsed(MemoryUsage memoryUsed) {
if (this.trackRealMemoryUsage) {
return overLimitStrategy.overLimit(memoryUsed);
} else {
return memoryUsed;
}
}

private CircuitBreaker validateAndCreateBreaker(BreakerSettings breakerSettings) {
// Validate the settings
validateSettings(new BreakerSettings[] {breakerSettings});
Expand All @@ -362,9 +355,9 @@ private CircuitBreaker validateAndCreateBreaker(BreakerSettings breakerSettings)
breakerSettings.getName());
}

private static OverLimitStrategy createOverLimitStrategy() {
static OverLimitStrategy createOverLimitStrategy(boolean trackRealMemoryUsage) {
JvmInfo jvmInfo = JvmInfo.jvmInfo();
if (jvmInfo.useG1GC().equals("true")
if (trackRealMemoryUsage && jvmInfo.useG1GC().equals("true")
// messing with GC is "dangerous" so we apply an escape hatch. Not intended to be used.
&& Booleans.parseBoolean(System.getProperty("es.real_memory_circuit_breaker.g1.double_check.enabled"), true)) {
// hardcode interval, do not want any tuning of it outside code changes.
Expand Down
36 changes: 26 additions & 10 deletions .../src/test/java/org/elasticsearch/indices/breaker/HierarchyCircuitBreakerServiceTests.java
View file
Expand Up @@ -53,6 +53,7 @@
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.lessThanOrEqualTo;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.nullValue;
Expand Down Expand Up @@ -305,7 +306,8 @@ public void testParentTriggersG1GCBeforeBreaking() throws InterruptedException,
final HierarchyCircuitBreakerService service = new HierarchyCircuitBreakerService(clusterSettings,
Collections.emptyList(),
new ClusterSettings(clusterSettings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS),
new HierarchyCircuitBreakerService.G1OverLimitStrategy(JvmInfo.jvmInfo(), HierarchyCircuitBreakerService::realMemoryUsage,
trackRealMemoryUsage -> new HierarchyCircuitBreakerService.G1OverLimitStrategy(JvmInfo.jvmInfo(),
HierarchyCircuitBreakerService::realMemoryUsage,
HierarchyCircuitBreakerService.createYoungGcCountSupplier(), time::get, interval) {

@Override
Expand Down Expand Up @@ -383,16 +385,18 @@ public void testParentDoesOverLimitCheck() {
final HierarchyCircuitBreakerService service = new HierarchyCircuitBreakerService(clusterSettings,
Collections.emptyList(),
new ClusterSettings(clusterSettings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS),
memoryUsed -> {
assertTrue(overLimitTriggered.compareAndSet(false, true));
if (saveTheDay) {
return new HierarchyCircuitBreakerService.MemoryUsage(memoryUsed.baseUsage / 2,
memoryUsed.totalUsage - (memoryUsed.baseUsage / 2), memoryUsed.transientChildUsage,
memoryUsed.permanentChildUsage);
} else {
return memoryUsed;
trackRealMemoryUsage ->
memoryUsed -> {
assertTrue(overLimitTriggered.compareAndSet(false, true));
if (saveTheDay) {
return new HierarchyCircuitBreakerService.MemoryUsage(memoryUsed.baseUsage / 2,
memoryUsed.totalUsage - (memoryUsed.baseUsage / 2), memoryUsed.transientChildUsage,
memoryUsed.permanentChildUsage);
} else {
return memoryUsed;
}
}
});
);

int allocationSize = g1RegionSize > 0 ? (int) (g1RegionSize / 2) : 1024 * 1024;
int allocationCount = (int) (JvmInfo.jvmInfo().getConfiguredMaxHeapSize() / allocationSize) + 1;
Expand Down Expand Up @@ -558,6 +562,18 @@ void overLimitTriggered(boolean leader) {
threads.forEach(thread -> assertFalse(thread.isAlive()));
}

public void testCreateOverLimitStrategy() {
assertThat(HierarchyCircuitBreakerService.createOverLimitStrategy(false),
not(instanceOf(HierarchyCircuitBreakerService.G1OverLimitStrategy.class)));
if (JvmInfo.jvmInfo().useG1GC().equals("true")) {
assertThat(HierarchyCircuitBreakerService.createOverLimitStrategy(true),
instanceOf(HierarchyCircuitBreakerService.G1OverLimitStrategy.class));
} else {
assertThat(HierarchyCircuitBreakerService.createOverLimitStrategy(true),
not(instanceOf(HierarchyCircuitBreakerService.G1OverLimitStrategy.class)));
}
}

public void testTrippedCircuitBreakerDurability() {
Settings clusterSettings = Settings.builder()
.put(HierarchyCircuitBreakerService.USE_REAL_MEMORY_USAGE_SETTING.getKey(), Boolean.FALSE)
Expand Down

0 comments on commit 3f61f93

Please sign in to comment.