Repository: aurora Updated Branches: refs/heads/master 29e9d9181 -> 5b46fd158
Add jittering to TruncatedBinaryBackoff. Reviewed at https://reviews.apache.org/r/41717/ Project: http://git-wip-us.apache.org/repos/asf/aurora/repo Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/5b46fd15 Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/5b46fd15 Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/5b46fd15 Branch: refs/heads/master Commit: 5b46fd158bd02eee767edb377ca6a667e5d8b3e6 Parents: 29e9d91 Author: Tony Dong <[email protected]> Authored: Sun Jan 3 09:57:09 2016 -0800 Committer: Bill Farner <[email protected]> Committed: Sun Jan 3 09:57:09 2016 -0800 ---------------------------------------------------------------------- .../aurora/common/args/parsers/EnumParser.java | 1 + .../common/util/TruncatedBinaryBackoff.java | 46 ++++++++++++++++---- .../common/util/TruncatedBinaryBackoffTest.java | 32 +++++++++----- .../aurora/scheduler/offers/OffersModule.java | 2 +- .../scheduling/RescheduleCalculator.java | 2 +- 5 files changed, 61 insertions(+), 22 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/aurora/blob/5b46fd15/commons/src/main/java/org/apache/aurora/common/args/parsers/EnumParser.java ---------------------------------------------------------------------- diff --git a/commons/src/main/java/org/apache/aurora/common/args/parsers/EnumParser.java b/commons/src/main/java/org/apache/aurora/common/args/parsers/EnumParser.java index 9f6a3ff..1c1e9bf 100644 --- a/commons/src/main/java/org/apache/aurora/common/args/parsers/EnumParser.java +++ b/commons/src/main/java/org/apache/aurora/common/args/parsers/EnumParser.java @@ -25,6 +25,7 @@ import org.apache.aurora.common.args.ParserOracle; * @author John Sirois */ @ArgParser +@SuppressWarnings("unused") public class EnumParser<T extends Enum<T>> implements Parser<T> { @Override http://git-wip-us.apache.org/repos/asf/aurora/blob/5b46fd15/commons/src/main/java/org/apache/aurora/common/util/TruncatedBinaryBackoff.java ---------------------------------------------------------------------- diff --git a/commons/src/main/java/org/apache/aurora/common/util/TruncatedBinaryBackoff.java b/commons/src/main/java/org/apache/aurora/common/util/TruncatedBinaryBackoff.java index fd74b9f..e580a19 100644 --- a/commons/src/main/java/org/apache/aurora/common/util/TruncatedBinaryBackoff.java +++ b/commons/src/main/java/org/apache/aurora/common/util/TruncatedBinaryBackoff.java @@ -24,6 +24,7 @@ public class TruncatedBinaryBackoff implements BackoffStrategy { private final long initialBackoffMs; private final long maxBackoffIntervalMs; private final boolean stopAtMax; + private final Random random; /** * Creates a new TruncatedBinaryBackoff that will start by backing off for {@code initialBackoff} @@ -31,25 +32,52 @@ public class TruncatedBinaryBackoff implements BackoffStrategy { * which point shouldContinue() will return false and any future backoffs will always wait for * that amount of time. * - * @param initialBackoff the intial amount of time to backoff + * @param initialBackoff the initial amount of time to backoff * @param maxBackoff the maximum amount of time to backoff * @param stopAtMax whether shouldContinue() returns false when the max is reached + * @param random An instance of the random util which can be used for applying jitter to backoff. */ - public TruncatedBinaryBackoff(Amount<Long, Time> initialBackoff, - Amount<Long, Time> maxBackoff, boolean stopAtMax) { + public TruncatedBinaryBackoff( + Amount<Long, Time> initialBackoff, + Amount<Long, Time> maxBackoff, + boolean stopAtMax, + Random random) { + Preconditions.checkNotNull(initialBackoff); Preconditions.checkNotNull(maxBackoff); + Preconditions.checkNotNull(random); Preconditions.checkArgument(initialBackoff.getValue() > 0); Preconditions.checkArgument(maxBackoff.compareTo(initialBackoff) >= 0); initialBackoffMs = initialBackoff.as(Time.MILLISECONDS); maxBackoffIntervalMs = maxBackoff.as(Time.MILLISECONDS); this.stopAtMax = stopAtMax; + this.random = random; + } + + /** + * Same as the main constructor + * {@link TruncatedBinaryBackoff#TruncatedBinaryBackoff(Amount, Amount, boolean, Random)}, + * but this will use a default Random implementation returned by + * {@link Random.Util#newDefaultRandom()}. + * + * @param initialBackoff the initial amount of time to backoff + * @param maxBackoff the maximum amount of time to backoff + * @param stopAtMax whether shouldContinue() returns false when the max is reached + */ + public TruncatedBinaryBackoff( + Amount<Long, Time> initialBackoff, + Amount<Long, Time> maxBackoff, + boolean stopAtMax) { + + this(initialBackoff, maxBackoff, stopAtMax, Random.Util.newDefaultRandom()); } /** - * Same as main constructor, but this will always return true from shouldContinue(). + * Same as the constructor + * {@link TruncatedBinaryBackoff#TruncatedBinaryBackoff(Amount, Amount, boolean)}, + * but this will always return true from shouldContinue(). * - * @param initialBackoff the intial amount of time to backoff + * @param initialBackoff the initial amount of time to backoff * @param maxBackoff the maximum amount of time to backoff */ public TruncatedBinaryBackoff(Amount<Long, Time> initialBackoff, Amount<Long, Time> maxBackoff) { @@ -59,9 +87,11 @@ public class TruncatedBinaryBackoff implements BackoffStrategy { @Override public long calculateBackoffMs(long lastBackoffMs) { Preconditions.checkArgument(lastBackoffMs >= 0); - long backoff = (lastBackoffMs == 0) ? initialBackoffMs - : Math.min(maxBackoffIntervalMs, lastBackoffMs * 2); - return backoff; + long halfBackoff = (lastBackoffMs == 0) ? initialBackoffMs : lastBackoffMs; + + return Math.min( + maxBackoffIntervalMs, + halfBackoff + Math.round(random.nextDouble() * halfBackoff)); } @Override http://git-wip-us.apache.org/repos/asf/aurora/blob/5b46fd15/commons/src/test/java/org/apache/aurora/common/util/TruncatedBinaryBackoffTest.java ---------------------------------------------------------------------- diff --git a/commons/src/test/java/org/apache/aurora/common/util/TruncatedBinaryBackoffTest.java b/commons/src/test/java/org/apache/aurora/common/util/TruncatedBinaryBackoffTest.java index 127a603..7eb3780 100644 --- a/commons/src/test/java/org/apache/aurora/common/util/TruncatedBinaryBackoffTest.java +++ b/commons/src/test/java/org/apache/aurora/common/util/TruncatedBinaryBackoffTest.java @@ -20,6 +20,7 @@ import org.junit.Test; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; /** * @author John Sirois @@ -54,37 +55,44 @@ public class TruncatedBinaryBackoffTest { public void testCalculateBackoffMs() { TruncatedBinaryBackoff backoff = new TruncatedBinaryBackoff(Amount.of(1L, Time.MILLISECONDS), - Amount.of(6L, Time.MILLISECONDS)); + Amount.of(12L, Time.MILLISECONDS)); try { backoff.calculateBackoffMs(-1L); + fail("calculateBackoffMs should throw an exception when give a negative value."); } catch (IllegalArgumentException e) { // expected } - assertEquals(1, backoff.calculateBackoffMs(0)); - assertEquals(2, backoff.calculateBackoffMs(1)); - assertEquals(4, backoff.calculateBackoffMs(2)); - assertEquals(6, backoff.calculateBackoffMs(4)); - assertEquals(6, backoff.calculateBackoffMs(8)); + long calculateBackoffMs0 = backoff.calculateBackoffMs(0); + assertTrue(1 <= calculateBackoffMs0 && calculateBackoffMs0 <= 2); + + long calculateBackoffMs1 = backoff.calculateBackoffMs(1); + assertTrue(1 <= calculateBackoffMs1 && calculateBackoffMs1 <= 2); + + long calculateBackoffMs2 = backoff.calculateBackoffMs(2); + assertTrue(2 <= calculateBackoffMs2 && calculateBackoffMs2 <= 4); + + long calculateBackoffMs4 = backoff.calculateBackoffMs(4); + assertTrue(4 <= calculateBackoffMs4 && calculateBackoffMs4 <= 8); + + long calculateBackoffMs8 = backoff.calculateBackoffMs(8); + assertTrue(8 <= calculateBackoffMs8 && calculateBackoffMs8 <= 12); + + assertEquals(12, backoff.calculateBackoffMs(16)); } @Test - public void testCalculateBackoffStop() { + public void testShouldContinue() { TruncatedBinaryBackoff backoff = new TruncatedBinaryBackoff(Amount.of(1L, Time.MILLISECONDS), Amount.of(6L, Time.MILLISECONDS), true); assertTrue(backoff.shouldContinue(0)); - assertEquals(1, backoff.calculateBackoffMs(0)); assertTrue(backoff.shouldContinue(1)); - assertEquals(2, backoff.calculateBackoffMs(1)); assertTrue(backoff.shouldContinue(2)); - assertEquals(4, backoff.calculateBackoffMs(2)); assertTrue(backoff.shouldContinue(4)); - assertEquals(6, backoff.calculateBackoffMs(4)); assertFalse(backoff.shouldContinue(6)); - assertEquals(6, backoff.calculateBackoffMs(8)); assertFalse(backoff.shouldContinue(8)); } } http://git-wip-us.apache.org/repos/asf/aurora/blob/5b46fd15/src/main/java/org/apache/aurora/scheduler/offers/OffersModule.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/offers/OffersModule.java b/src/main/java/org/apache/aurora/scheduler/offers/OffersModule.java index fbc589e..0f228b1 100644 --- a/src/main/java/org/apache/aurora/scheduler/offers/OffersModule.java +++ b/src/main/java/org/apache/aurora/scheduler/offers/OffersModule.java @@ -53,7 +53,7 @@ public class OffersModule extends AbstractModule { new RandomJitterReturnDelay( MIN_OFFER_HOLD_TIME.get().as(Time.MILLISECONDS), OFFER_HOLD_JITTER_WINDOW.get().as(Time.MILLISECONDS), - new Random.SystemRandom(new java.util.Random()))); + Random.Util.newDefaultRandom())); bind(OfferManager.class).to(OfferManager.OfferManagerImpl.class); bind(OfferManager.OfferManagerImpl.class).in(Singleton.class); expose(OfferManager.class); http://git-wip-us.apache.org/repos/asf/aurora/blob/5b46fd15/src/main/java/org/apache/aurora/scheduler/scheduling/RescheduleCalculator.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/scheduling/RescheduleCalculator.java b/src/main/java/org/apache/aurora/scheduler/scheduling/RescheduleCalculator.java index 291bf5f..e5b7bfe 100644 --- a/src/main/java/org/apache/aurora/scheduler/scheduling/RescheduleCalculator.java +++ b/src/main/java/org/apache/aurora/scheduler/scheduling/RescheduleCalculator.java @@ -74,7 +74,7 @@ public interface RescheduleCalculator { private final Storage storage; private final RescheduleCalculatorSettings settings; // TODO(wfarner): Inject 'random' in the constructor for better test coverage. - private final Random random = new Random.SystemRandom(new java.util.Random()); + private final Random random = Random.Util.newDefaultRandom(); private static final Predicate<ScheduleStatus> IS_ACTIVE_STATUS = Predicates.in(Tasks.ACTIVE_STATES);
