From 6749b2c3069ee2688bb9da4e2bcf86cde9a8982b Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Tue, 25 Nov 2014 11:53:29 +0100 Subject: [PATCH] [TEST] Reduce stringency of breaker assertions While in a perfect world we should only ever have 2 circuit breaker trips, it's possible to get a race condition between the child and the parent breaker with many threads. Since multiple breaking exceptions are not actually a bad thing, it's okay to relax the constraints in the test. The race conditions are due to no locking inside the breaker logic, to ensure that it is as low overhead as possible. Even though no locking is used, we use atomic counters internally to ensure that the "estimated" numbers for the breakers are never out of sync (which this test still checks with no leeway). --- .../common/breaker/MemoryCircuitBreakerTests.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/test/java/org/elasticsearch/common/breaker/MemoryCircuitBreakerTests.java b/src/test/java/org/elasticsearch/common/breaker/MemoryCircuitBreakerTests.java index 24b51c64dbb..698858e70f8 100644 --- a/src/test/java/org/elasticsearch/common/breaker/MemoryCircuitBreakerTests.java +++ b/src/test/java/org/elasticsearch/common/breaker/MemoryCircuitBreakerTests.java @@ -33,6 +33,7 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; /** * Tests for the Memory Aggregating Circuit Breaker @@ -182,9 +183,6 @@ public class MemoryCircuitBreakerTests extends ElasticsearchTestCase { breaker.addEstimateBytesAndMaybeBreak(1L, "test"); } catch (CircuitBreakingException e) { tripped.incrementAndGet(); - if (tripped.get() > 2) { - assertThat("tripped too many times: " + tripped.get(), true, equalTo(false)); - } } catch (Throwable e2) { lastException.set(e2); } @@ -210,8 +208,8 @@ public class MemoryCircuitBreakerTests extends ElasticsearchTestCase { assertThat("no other exceptions were thrown", lastException.get(), equalTo(null)); assertThat("breaker should be reset back to the parent limit after parent breaker trips", breaker.getUsed(), equalTo((long)parentLimit)); - assertThat("parent breaker was tripped exactly twice", parentTripped.get(), equalTo(2)); - assertThat("total breaker was tripped exactly twice", tripped.get(), equalTo(2)); + assertThat("parent breaker was tripped at least twice", parentTripped.get(), greaterThanOrEqualTo(2)); + assertThat("total breaker was tripped at least twice", tripped.get(), greaterThanOrEqualTo(2)); assertThat("breaker total is expected value: " + parentLimit, breaker.getUsed(), equalTo((long) parentLimit)); }