Skip to content

Commit

Permalink
Rename to "over limit" rather than "double check"
Browse files Browse the repository at this point in the history
  • Loading branch information
@henningandersen
henningandersen committed Jul 1, 2020
1 parent c8ac1af commit 1d007ca
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 40 deletions.
30 changes: 15 additions & 15 deletions server/src/main/java/org/elasticsearch/indices/breaker/HierarchyCircuitBreakerService.java
View file
Expand Up @@ -107,7 +107,7 @@ public class HierarchyCircuitBreakerService extends CircuitBreakerService {
// Tripped count for when redistribution was attempted but wasn't successful
private final AtomicLong parentTripCount = new AtomicLong(0);

private final DoubleCheckStrategy doubleCheckStrategy;
private final OverLimitStrategy overLimitStrategy;

public HierarchyCircuitBreakerService(Settings settings, List<BreakerSettings> customBreakers, ClusterSettings clusterSettings) {
this(settings, customBreakers, clusterSettings,
Expand All @@ -116,7 +116,7 @@ public HierarchyCircuitBreakerService(Settings settings, List<BreakerSettings> c
}

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

this.doubleCheckStrategy = doubleCheckStrategy;
this.overLimitStrategy = overLimitStrategy;
}

private void updateCircuitBreakerSettings(String name, ByteSizeValue newLimit, Double newOverhead) {
Expand Down Expand Up @@ -344,7 +344,7 @@ public void checkParentLimit(long newBytesReserved, String label) throws Circuit

MemoryUsage doubleCheckMemoryUsed(MemoryUsage memoryUsed) {
if (this.trackRealMemoryUsage) {
return doubleCheckStrategy.doubleCheck(memoryUsed);
return overLimitStrategy.overLimit(memoryUsed);
} else {
return memoryUsed;
}
Expand All @@ -361,22 +361,22 @@ private CircuitBreaker validateAndCreateBreaker(BreakerSettings breakerSettings)
breakerSettings.getName());
}

private static DoubleCheckStrategy createDoubleCheckStrategy(JvmInfo jvmInfo, LongSupplier currentMemoryUsageSupplier,
LongSupplier timeSupplier, long minimumInterval) {
private static OverLimitStrategy createDoubleCheckStrategy(JvmInfo jvmInfo, LongSupplier currentMemoryUsageSupplier,
LongSupplier timeSupplier, long minimumInterval) {
if (jvmInfo.useG1GC().equals("true")
// messing with GC is "dangerous" so we apply an escape hatch. Not intended to be used.
&& Boolean.parseBoolean(System.getProperty("es.real_memory_circuit_breaker.g1.double_check.enabled", "true"))) {
return new G1DoubleCheckStrategy(jvmInfo, currentMemoryUsageSupplier, timeSupplier, minimumInterval);
return new G1OverLimitStrategy(jvmInfo, currentMemoryUsageSupplier, timeSupplier, minimumInterval);
} else {
return memoryUsed -> memoryUsed;
}
}

interface DoubleCheckStrategy {
MemoryUsage doubleCheck(MemoryUsage memoryUsed);
interface OverLimitStrategy {
MemoryUsage overLimit(MemoryUsage memoryUsed);
}

static class G1DoubleCheckStrategy implements DoubleCheckStrategy {
static class G1OverLimitStrategy implements OverLimitStrategy {
private final long g1RegionSize;
private final LongSupplier currentMemoryUsageSupplier;
private final LongSupplier timeSupplier;
Expand All @@ -387,8 +387,8 @@ static class G1DoubleCheckStrategy implements DoubleCheckStrategy {
private long blackHole;
private final Object lock = new Object();

G1DoubleCheckStrategy(JvmInfo jvmInfo, LongSupplier currentMemoryUsageSupplier,
LongSupplier timeSupplier, long minimumInterval) {
G1OverLimitStrategy(JvmInfo jvmInfo, LongSupplier currentMemoryUsageSupplier,
LongSupplier timeSupplier, long minimumInterval) {
assert minimumInterval > 0;
this.currentMemoryUsageSupplier = currentMemoryUsageSupplier;
this.timeSupplier = timeSupplier;
Expand Down Expand Up @@ -421,12 +421,12 @@ static long fallbackRegionSize(JvmInfo jvmInfo) {
}

@Override
public MemoryUsage doubleCheck(MemoryUsage memoryUsed) {
public MemoryUsage overLimit(MemoryUsage memoryUsed) {
long maxHeap = JvmInfo.jvmInfo().getMem().getHeapMax().getBytes();
boolean leader;
synchronized (lock) {
leader = timeSupplier.getAsLong() >= lastCheckTime + minimumInterval;
doubleCheckingRealMemoryUsed(leader);
overLimitTriggered(leader);
if (leader) {
logger.info("attempting to trigger G1GC due to high heap usage [{}]", memoryUsed.baseUsage);
long localBlackHole = 0;
Expand Down Expand Up @@ -468,7 +468,7 @@ public MemoryUsage doubleCheck(MemoryUsage memoryUsed) {
}
}

void doubleCheckingRealMemoryUsed(boolean leader) {
void overLimitTriggered(boolean leader) {
// for tests to override.
}
}
Expand Down
50 changes: 25 additions & 25 deletions .../src/test/java/org/elasticsearch/indices/breaker/HierarchyCircuitBreakerServiceTests.java
View file
Expand Up @@ -284,12 +284,12 @@ long currentMemoryUsage() {
}

/**
* "Integration test" checking that we ask the G1 double check before parent breaking.
* "Integration test" checking that we ask the G1 over limit check before parent breaking.
* Given that it depends on GC, the main assertion that we do not get a circuit breaking exception in the threads towards
* the end of the test is not enabled. The following tests checks this in more unit test style.
*/
public void testParentTriggersG1GCBeforeBreaking() throws InterruptedException, TimeoutException, BrokenBarrierException {
assumeTrue("Only G1GC can utilize the double check hack", JvmInfo.jvmInfo().useG1GC().equals("true"));
assumeTrue("Only G1GC can utilize the over limit check", JvmInfo.jvmInfo().useG1GC().equals("true"));
long g1RegionSize = JvmInfo.jvmInfo().getG1RegionSize();
assumeTrue("Must have region size", g1RegionSize > 0);

Expand All @@ -298,21 +298,21 @@ public void testParentTriggersG1GCBeforeBreaking() throws InterruptedException,
.build();

AtomicInteger leaderTriggerCount = new AtomicInteger();
AtomicReference<Consumer<Boolean>> onDoubleCheck = new AtomicReference<>(leader -> {});
AtomicReference<Consumer<Boolean>> onOverLimit = new AtomicReference<>(leader -> {});
AtomicLong time = new AtomicLong(randomLongBetween(Long.MIN_VALUE/2, Long.MAX_VALUE/2));
long interval = randomLongBetween(1, 1000);
final HierarchyCircuitBreakerService service = new HierarchyCircuitBreakerService(clusterSettings,
Collections.emptyList(),
new ClusterSettings(clusterSettings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS),
new HierarchyCircuitBreakerService.G1DoubleCheckStrategy(JvmInfo.jvmInfo(), HierarchyCircuitBreakerService::realMemoryUsage,
new HierarchyCircuitBreakerService.G1OverLimitStrategy(JvmInfo.jvmInfo(), HierarchyCircuitBreakerService::realMemoryUsage,
time::get, interval) {

@Override
void doubleCheckingRealMemoryUsed(boolean leader) {
void overLimitTriggered(boolean leader) {
if (leader) {
leaderTriggerCount.incrementAndGet();
}
onDoubleCheck.get().accept(leader);
onOverLimit.get().accept(leader);
}
});

Expand All @@ -333,7 +333,7 @@ void doubleCheckingRealMemoryUsed(boolean leader) {
}

time.addAndGet(randomLongBetween(interval, interval + 10));
onDoubleCheck.set(leader -> {
onOverLimit.set(leader -> {
if (leader) {
data.clear();
}
Expand Down Expand Up @@ -370,20 +370,20 @@ void doubleCheckingRealMemoryUsed(boolean leader) {
assertThat(leaderTriggerCount.get(), equalTo(2));
}

public void testParentDoesDoubleCheck() {
public void testParentDoesOverLimitCheck() {
long g1RegionSize = JvmInfo.jvmInfo().getG1RegionSize();

Settings clusterSettings = Settings.builder()
.put(HierarchyCircuitBreakerService.USE_REAL_MEMORY_USAGE_SETTING.getKey(), Boolean.TRUE)
.put(HierarchyCircuitBreakerService.TOTAL_CIRCUIT_BREAKER_LIMIT_SETTING.getKey(), "50%")
.build();
boolean saveTheDay = randomBoolean();
AtomicBoolean doubleChecked = new AtomicBoolean();
AtomicBoolean overLimitTriggered = new AtomicBoolean();
final HierarchyCircuitBreakerService service = new HierarchyCircuitBreakerService(clusterSettings,
Collections.emptyList(),
new ClusterSettings(clusterSettings, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS),
memoryUsed -> {
assertTrue(doubleChecked.compareAndSet(false, true));
assertTrue(overLimitTriggered.compareAndSet(false, true));
if (saveTheDay) {
return new HierarchyCircuitBreakerService.MemoryUsage(memoryUsed.baseUsage / 2,
memoryUsed.totalUsage - (memoryUsed.baseUsage / 2), memoryUsed.transientChildUsage,
Expand All @@ -397,7 +397,7 @@ public void testParentDoesDoubleCheck() {
int allocationCount = (int) (JvmInfo.jvmInfo().getConfiguredMaxHeapSize() / allocationSize) + 1;
List<byte[]> data = new ArrayList<>();
try {
for (int i = 0 ; i < allocationCount && doubleChecked.get() == false; ++i) {
for (int i = 0 ; i < allocationCount && overLimitTriggered.get() == false; ++i) {
data.add(new byte[allocationSize]);
service.checkParentLimit(0, "test");
}
Expand All @@ -410,25 +410,25 @@ public void testParentDoesDoubleCheck() {
}

public void testFallbackG1RegionSize() {
assumeTrue("Only G1GC can utilize the double check hack", JvmInfo.jvmInfo().useG1GC().equals("true"));
assumeTrue("Only G1GC can utilize the over limit check", JvmInfo.jvmInfo().useG1GC().equals("true"));
assumeTrue("Must have region size", JvmInfo.jvmInfo().getG1RegionSize() > 0);

assertThat(HierarchyCircuitBreakerService.G1DoubleCheckStrategy.fallbackRegionSize(JvmInfo.jvmInfo()),
assertThat(HierarchyCircuitBreakerService.G1OverLimitStrategy.fallbackRegionSize(JvmInfo.jvmInfo()),
equalTo(JvmInfo.jvmInfo().getG1RegionSize()));
}

public void testG1DoubleCheckStrategy() {
public void testG1OverLimitStrategy() {
AtomicLong time = new AtomicLong(randomLongBetween(Long.MIN_VALUE/2, Long.MAX_VALUE/2));
AtomicInteger leaderTriggerCount = new AtomicInteger();
AtomicInteger nonLeaderTriggerCount = new AtomicInteger();
long interval = randomLongBetween(1, 1000);
AtomicLong memoryUsage = new AtomicLong();
HierarchyCircuitBreakerService.G1DoubleCheckStrategy strategy =
new HierarchyCircuitBreakerService.G1DoubleCheckStrategy(JvmInfo.jvmInfo(), memoryUsage::get,
HierarchyCircuitBreakerService.G1OverLimitStrategy strategy =
new HierarchyCircuitBreakerService.G1OverLimitStrategy(JvmInfo.jvmInfo(), memoryUsage::get,
time::get, interval) {

@Override
void doubleCheckingRealMemoryUsed(boolean leader) {
void overLimitTriggered(boolean leader) {
if (leader) {
leaderTriggerCount.incrementAndGet();
} else {
Expand All @@ -441,11 +441,11 @@ void doubleCheckingRealMemoryUsed(boolean leader) {
randomLongBetween(0, 50),
randomLongBetween(0, 50));

assertThat(strategy.doubleCheck(input), sameInstance(input));
assertThat(strategy.overLimit(input), sameInstance(input));
assertThat(leaderTriggerCount.get(), equalTo(1));

memoryUsage.set(99);
HierarchyCircuitBreakerService.MemoryUsage output = strategy.doubleCheck(input);
HierarchyCircuitBreakerService.MemoryUsage output = strategy.overLimit(input);
assertThat(output, not(sameInstance(input)));
assertThat(output.baseUsage, equalTo(memoryUsage.get()));
assertThat(output.totalUsage, equalTo(99 + input.totalUsage - 100));
Expand All @@ -454,7 +454,7 @@ void doubleCheckingRealMemoryUsed(boolean leader) {
assertThat(nonLeaderTriggerCount.get(), equalTo(1));

time.addAndGet(randomLongBetween(interval, interval * 2));
output = strategy.doubleCheck(input);
output = strategy.overLimit(input);
assertThat(output, not(sameInstance(input)));
assertThat(output.baseUsage, equalTo(memoryUsage.get()));
assertThat(output.totalUsage, equalTo(99 + input.totalUsage - 100));
Expand All @@ -463,17 +463,17 @@ void doubleCheckingRealMemoryUsed(boolean leader) {
assertThat(leaderTriggerCount.get(), equalTo(2));
}

public void testG1DoubleCheckStrategyThrottling() throws InterruptedException, BrokenBarrierException, TimeoutException {
public void testG1OverLimitStrategyThrottling() throws InterruptedException, BrokenBarrierException, TimeoutException {
AtomicLong time = new AtomicLong(randomLongBetween(Long.MIN_VALUE/2, Long.MAX_VALUE/2));
AtomicInteger leaderTriggerCount = new AtomicInteger();
long interval = randomLongBetween(1, 1000);
AtomicLong memoryUsage = new AtomicLong();
HierarchyCircuitBreakerService.G1DoubleCheckStrategy strategy =
new HierarchyCircuitBreakerService.G1DoubleCheckStrategy(JvmInfo.jvmInfo(), memoryUsage::get,
HierarchyCircuitBreakerService.G1OverLimitStrategy strategy =
new HierarchyCircuitBreakerService.G1OverLimitStrategy(JvmInfo.jvmInfo(), memoryUsage::get,
time::get, interval) {

@Override
void doubleCheckingRealMemoryUsed(boolean leader) {
void overLimitTriggered(boolean leader) {
if (leader) {
leaderTriggerCount.incrementAndGet();
}
Expand All @@ -494,7 +494,7 @@ void doubleCheckingRealMemoryUsed(boolean leader) {
HierarchyCircuitBreakerService.MemoryUsage input =
new HierarchyCircuitBreakerService.MemoryUsage(randomLongBetween(0, 100), randomLongBetween(0, 100),
randomLongBetween(0, 100), randomLongBetween(0, 100));
HierarchyCircuitBreakerService.MemoryUsage output = strategy.doubleCheck(input);
HierarchyCircuitBreakerService.MemoryUsage output = strategy.overLimit(input);
assertThat(output.totalUsage, equalTo(output.baseUsage + input.totalUsage - input.baseUsage));
assertThat(output.transientChildUsage, equalTo(input.transientChildUsage));
assertThat(output.permanentChildUsage, equalTo(input.permanentChildUsage));
Expand Down

0 comments on commit 1d007ca

Please sign in to comment.