From 7a8cdf0ee367b59f2496c375d1230f42499f7ad9 Mon Sep 17 00:00:00 2001 From: anhefti Date: Mon, 2 Sep 2019 12:56:32 +0200 Subject: [PATCH] SEBSERV-60 implemented --- .../seb/sebserver/gbl/async/AsyncService.java | 20 +++++---- .../sebserver/gbl/async/CircuitBreaker.java | 20 +++++++++ .../gbl/async/MemoizingCircuitBreaker.java | 42 ++++++++++++++++--- .../lms/impl/OpenEdxLmsAPITemplate.java | 9 +++- .../async/MemoizingCircuitBreakerTest.java | 32 ++++++++++++-- src/test/resources/logback-test.xml | 14 +++++++ src/test/resources/logback.xml | 26 ------------ 7 files changed, 119 insertions(+), 44 deletions(-) create mode 100644 src/test/resources/logback-test.xml delete mode 100644 src/test/resources/logback.xml diff --git a/src/main/java/ch/ethz/seb/sebserver/gbl/async/AsyncService.java b/src/main/java/ch/ethz/seb/sebserver/gbl/async/AsyncService.java index 87632603..6c154027 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gbl/async/AsyncService.java +++ b/src/main/java/ch/ethz/seb/sebserver/gbl/async/AsyncService.java @@ -40,18 +40,23 @@ public class AsyncService { timeToRecover); } - public MemoizingCircuitBreaker createMemoizingCircuitBreaker( - final Supplier blockingSupplier) { - - return new MemoizingCircuitBreaker<>(this.asyncRunner, blockingSupplier, true); - } +// public MemoizingCircuitBreaker createMemoizingCircuitBreaker( +// final Supplier blockingSupplier) { +// +// return new MemoizingCircuitBreaker<>( +// this.asyncRunner, +// blockingSupplier, +// true, +// Constants.HOUR_IN_MILLIS); +// } public MemoizingCircuitBreaker createMemoizingCircuitBreaker( final Supplier blockingSupplier, final int maxFailingAttempts, final long maxBlockingTime, final long timeToRecover, - final boolean momoized) { + final boolean momoized, + final long maxMemoizingTime) { return new MemoizingCircuitBreaker<>( this.asyncRunner, @@ -59,7 +64,8 @@ public class AsyncService { maxFailingAttempts, maxBlockingTime, timeToRecover, - momoized); + momoized, + maxMemoizingTime); } } diff --git a/src/main/java/ch/ethz/seb/sebserver/gbl/async/CircuitBreaker.java b/src/main/java/ch/ethz/seb/sebserver/gbl/async/CircuitBreaker.java index c2dfc6f1..9fcce361 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gbl/async/CircuitBreaker.java +++ b/src/main/java/ch/ethz/seb/sebserver/gbl/async/CircuitBreaker.java @@ -102,6 +102,26 @@ public final class CircuitBreaker { this.timeToRecover = timeToRecover; } + public int getMaxFailingAttempts() { + return this.maxFailingAttempts; + } + + public long getMaxBlockingTime() { + return this.maxBlockingTime; + } + + public long getTimeToRecover() { + return this.timeToRecover; + } + + public AtomicInteger getFailingCount() { + return this.failingCount; + } + + public long getLastSuccessTime() { + return this.lastSuccessTime; + } + public Result protectedRun(final Supplier supplier) { final long currentTime = System.currentTimeMillis(); diff --git a/src/main/java/ch/ethz/seb/sebserver/gbl/async/MemoizingCircuitBreaker.java b/src/main/java/ch/ethz/seb/sebserver/gbl/async/MemoizingCircuitBreaker.java index c7be11a6..31fca167 100644 --- a/src/main/java/ch/ethz/seb/sebserver/gbl/async/MemoizingCircuitBreaker.java +++ b/src/main/java/ch/ethz/seb/sebserver/gbl/async/MemoizingCircuitBreaker.java @@ -54,21 +54,42 @@ public final class MemoizingCircuitBreaker implements Supplier> { private final Supplier supplier; private final boolean memoizing; + private final long maxMemoizingTime; + private long lastMemoizingTime = 0; private Result cached = null; /** Create new CircuitBreakerSupplier. * * @param asyncRunner the AsyncRunner used to create asynchronous calls on the given supplier function * @param supplier The Supplier function that can fail or block for a long time - * @param memoizing whether the memoizing functionality is on or off */ + * @param memoizing whether the memoizing functionality is on or off + * @param maxMemoizingTime the maximal time memorized data is valid */ MemoizingCircuitBreaker( final AsyncRunner asyncRunner, final Supplier supplier, - final boolean memoizing) { + final boolean memoizing, + final long maxMemoizingTime) { this.delegate = new CircuitBreaker<>(asyncRunner); this.supplier = supplier; this.memoizing = memoizing; + this.maxMemoizingTime = maxMemoizingTime; + } + + public CircuitBreaker getDelegate() { + return this.delegate; + } + + public boolean isMemoizing() { + return this.memoizing; + } + + public long getMaxMemoizingTime() { + return this.maxMemoizingTime; + } + + public long getLastMemoizingTime() { + return this.lastMemoizingTime; } /** Create new CircuitBreakerSupplier. @@ -79,14 +100,16 @@ public final class MemoizingCircuitBreaker implements Supplier> { * @param maxBlockingTime the maximal time that an call attempt can block until an error is responded * @param timeToRecover the time the circuit breaker needs to cool-down on OPEN-STATE before going back to HALF_OPEN * state - * @param memoizing whether the memoizing functionality is on or off */ + * @param memoizing whether the memoizing functionality is on or off + * @param maxMemoizingTime the maximal time memorized data is valid */ MemoizingCircuitBreaker( final AsyncRunner asyncRunner, final Supplier supplier, final int maxFailingAttempts, final long maxBlockingTime, final long timeToRecover, - final boolean memoizing) { + final boolean memoizing, + final long maxMemoizingTime) { this.delegate = new CircuitBreaker<>( asyncRunner, @@ -95,6 +118,7 @@ public final class MemoizingCircuitBreaker implements Supplier> { timeToRecover); this.supplier = supplier; this.memoizing = memoizing; + this.maxMemoizingTime = maxMemoizingTime; } @Override @@ -102,10 +126,15 @@ public final class MemoizingCircuitBreaker implements Supplier> { final Result result = this.delegate.protectedRun(this.supplier); if (result.hasError()) { if (this.memoizing && this.cached != null) { - if (log.isDebugEnabled()) { - log.debug("Return cached at: {}", System.currentTimeMillis()); + final long currentTimeMillis = System.currentTimeMillis(); + if (currentTimeMillis - this.lastMemoizingTime > this.maxMemoizingTime) { + if (log.isDebugEnabled()) { + log.debug("Max memoizing time reached. Return original error"); + } + return result; } + log.warn("Return cached at: {}", System.currentTimeMillis()); return this.cached; } @@ -117,6 +146,7 @@ public final class MemoizingCircuitBreaker implements Supplier> { } this.cached = result; + this.lastMemoizingTime = System.currentTimeMillis(); } return result; } diff --git a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/OpenEdxLmsAPITemplate.java b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/OpenEdxLmsAPITemplate.java index b2bd4045..1be10784 100644 --- a/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/OpenEdxLmsAPITemplate.java +++ b/src/main/java/ch/ethz/seb/sebserver/webservice/servicelayer/lms/impl/OpenEdxLmsAPITemplate.java @@ -46,6 +46,7 @@ import org.springframework.security.oauth2.common.OAuth2AccessToken; import org.springframework.util.LinkedMultiValueMap; import org.springframework.util.MultiValueMap; +import ch.ethz.seb.sebserver.gbl.Constants; import ch.ethz.seb.sebserver.gbl.api.APIMessage; import ch.ethz.seb.sebserver.gbl.async.AsyncService; import ch.ethz.seb.sebserver.gbl.async.MemoizingCircuitBreaker; @@ -97,7 +98,13 @@ final class OpenEdxLmsAPITemplate implements LmsAPITemplate { this.knownTokenAccessPaths.addAll(Arrays.asList(alternativeTokenRequestPaths)); } - this.allQuizzesSupplier = asyncService.createMemoizingCircuitBreaker(allQuizzesSupplier()); + this.allQuizzesSupplier = asyncService.createMemoizingCircuitBreaker( + allQuizzesSupplier(), + 3, + Constants.MINUTE_IN_MILLIS, + Constants.MINUTE_IN_MILLIS, + true, + Constants.HOUR_IN_MILLIS); } @Override diff --git a/src/test/java/ch/ethz/seb/sebserver/gbl/async/MemoizingCircuitBreakerTest.java b/src/test/java/ch/ethz/seb/sebserver/gbl/async/MemoizingCircuitBreakerTest.java index 39e4ae2f..cba247f6 100644 --- a/src/test/java/ch/ethz/seb/sebserver/gbl/async/MemoizingCircuitBreakerTest.java +++ b/src/test/java/ch/ethz/seb/sebserver/gbl/async/MemoizingCircuitBreakerTest.java @@ -42,7 +42,7 @@ public class MemoizingCircuitBreakerTest { @Test public void roundtrip1() throws InterruptedException { final MemoizingCircuitBreaker circuitBreaker = this.asyncService.createMemoizingCircuitBreaker( - tester(100, 5, 10), 3, 500, 1000, true); + tester(100, 5, 10), 3, 500, 1000, true, 1000); assertNull(circuitBreaker.getChached()); @@ -84,13 +84,39 @@ public class MemoizingCircuitBreakerTest { } + @Test + public void failing() throws InterruptedException { + final MemoizingCircuitBreaker circuitBreaker = this.asyncService.createMemoizingCircuitBreaker( + tester(50, 1, 10), 3, 100, 100, true, 1000); + + assertNull(circuitBreaker.getChached()); + + // fist call okay.. for memoizing + Result result = circuitBreaker.get(); // 1. call... + assertFalse(result.hasError()); + assertEquals("Hello", result.get()); + assertTrue(circuitBreaker.getLastMemoizingTime() > 0); + + // second call is okay from memoizing + Thread.sleep(100); + result = circuitBreaker.get(); // 2. call... + assertFalse(result.hasError()); + assertEquals("Hello", result.get()); + assertTrue(circuitBreaker.getLastMemoizingTime() > 0); + + // third call is failing because of memoizing timeout + Thread.sleep(1000); + result = circuitBreaker.get(); // 3. call... + assertTrue(result.hasError()); + } + private Supplier tester(final long delay, final int unavailableAfter, final int unavailableUntil) { final AtomicInteger count = new AtomicInteger(0); final AtomicBoolean wasUnavailable = new AtomicBoolean(false); return () -> { final int attempts = count.getAndIncrement(); - log.info("tester answers {} {}", attempts, Thread.currentThread()); + log.debug("tester answers {} {}", attempts, Thread.currentThread()); try { Thread.sleep(delay); @@ -107,6 +133,4 @@ public class MemoizingCircuitBreakerTest { }; } - // TODO timeout test: test also the behavior on timeout, is the thread being interrupted and released or not (should!) - } diff --git a/src/test/resources/logback-test.xml b/src/test/resources/logback-test.xml new file mode 100644 index 00000000..75c3949c --- /dev/null +++ b/src/test/resources/logback-test.xml @@ -0,0 +1,14 @@ + + + + + %d{HH:mm:ss.SSS} %-5level [%thread]:[%logger] %msg%n + + + + + + + + + \ No newline at end of file diff --git a/src/test/resources/logback.xml b/src/test/resources/logback.xml deleted file mode 100644 index cd46df9a..00000000 --- a/src/test/resources/logback.xml +++ /dev/null @@ -1,26 +0,0 @@ - - - - - %d{HH:mm:ss.SSS} %-5level [%thread]:[%logger] %msg%n - - - - - - - - - - - - - - - - - - - - - \ No newline at end of file