Repository: aurora
Updated Branches:
  refs/heads/master 13ac06ab8 -> b8ffc62b7


Fixing default TaskReconciler initial delay.

Bugs closed: AURORA-1339

Reviewed at https://reviews.apache.org/r/34733/


Project: http://git-wip-us.apache.org/repos/asf/aurora/repo
Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/b8ffc62b
Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/b8ffc62b
Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/b8ffc62b

Branch: refs/heads/master
Commit: b8ffc62b79fb41a149a3328177c599844a2cafd7
Parents: 13ac06a
Author: Maxim Khutornenko <[email protected]>
Authored: Thu May 28 09:17:45 2015 -0700
Committer: Maxim Khutornenko <[email protected]>
Committed: Thu May 28 09:17:45 2015 -0700

----------------------------------------------------------------------
 .../aurora/scheduler/async/AsyncModule.java     |  2 +-
 .../aurora/scheduler/async/TaskReconciler.java  | 19 ++++--
 .../scheduler/async/TaskReconcilerTest.java     | 62 ++++++++++++++------
 3 files changed, 59 insertions(+), 24 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/aurora/blob/b8ffc62b/src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java 
b/src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java
index 5f24668..8bcac6c 100644
--- a/src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java
+++ b/src/main/java/org/apache/aurora/scheduler/async/AsyncModule.java
@@ -186,7 +186,7 @@ public class AsyncModule extends AbstractModule {
   @CmdLine(name = "reconciliation_initial_delay",
       help = "Initial amount of time to delay task reconciliation after 
scheduler start up.")
   private static final Arg<Amount<Long, Time>> RECONCILIATION_INITIAL_DELAY =
-      Arg.create(Amount.of(Long.MAX_VALUE, Time.MINUTES));
+      Arg.create(Amount.of((long) Integer.MAX_VALUE, Time.MINUTES));
 
   @Positive
   @CmdLine(name = "reconciliation_explicit_interval",

http://git-wip-us.apache.org/repos/asf/aurora/blob/b8ffc62b/src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java
----------------------------------------------------------------------
diff --git 
a/src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java 
b/src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java
index 23f5f64..ccdbc02 100644
--- a/src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java
+++ b/src/main/java/org/apache/aurora/scheduler/async/TaskReconciler.java
@@ -35,6 +35,7 @@ import org.apache.mesos.Protos;
 
 import static java.util.Objects.requireNonNull;
 
+import static com.google.common.base.Preconditions.checkArgument;
 import static com.twitter.common.quantity.Time.MINUTES;
 
 /**
@@ -58,10 +59,10 @@ public class TaskReconciler extends AbstractIdleService {
   private final AtomicLong implicitRuns;
 
   static class TaskReconcilerSettings {
-    private final Amount<Long, Time> initialDelay;
     private final Amount<Long, Time> explicitInterval;
     private final Amount<Long, Time> implicitInterval;
-    private final Amount<Long, Time> scheduleSpread;
+    private final long explicitDelayMinutes;
+    private final long implicitDelayMinutes;
 
     @VisibleForTesting
     TaskReconcilerSettings(
@@ -70,10 +71,16 @@ public class TaskReconciler extends AbstractIdleService {
         Amount<Long, Time> implicitInterval,
         Amount<Long, Time> scheduleSpread) {
 
-      this.initialDelay = requireNonNull(initialDelay);
       this.explicitInterval = requireNonNull(explicitInterval);
       this.implicitInterval = requireNonNull(implicitInterval);
-      this.scheduleSpread = requireNonNull(scheduleSpread);
+      explicitDelayMinutes = requireNonNull(initialDelay).as(MINUTES);
+      implicitDelayMinutes = initialDelay.as(MINUTES) + 
scheduleSpread.as(MINUTES);
+      checkArgument(
+          explicitDelayMinutes >= 0,
+          "Invalid explicit reconciliation delay: " + explicitDelayMinutes);
+      checkArgument(
+          implicitDelayMinutes >= 0L,
+          "Invalid implicit reconciliation delay: " + implicitDelayMinutes);
     }
   }
 
@@ -109,7 +116,7 @@ public class TaskReconciler extends AbstractIdleService {
             explicitRuns.incrementAndGet();
           }
         },
-        settings.initialDelay.as(MINUTES),
+        settings.explicitDelayMinutes,
         settings.explicitInterval.as(MINUTES),
         MINUTES.getTimeUnit());
 
@@ -122,7 +129,7 @@ public class TaskReconciler extends AbstractIdleService {
             implicitRuns.incrementAndGet();
           }
         },
-        settings.initialDelay.as(MINUTES) + 
settings.scheduleSpread.as(MINUTES),
+        settings.implicitDelayMinutes,
         settings.implicitInterval.as(MINUTES),
         MINUTES.getTimeUnit());
   }

http://git-wip-us.apache.org/repos/asf/aurora/blob/b8ffc62b/src/test/java/org/apache/aurora/scheduler/async/TaskReconcilerTest.java
----------------------------------------------------------------------
diff --git 
a/src/test/java/org/apache/aurora/scheduler/async/TaskReconcilerTest.java 
b/src/test/java/org/apache/aurora/scheduler/async/TaskReconcilerTest.java
index f56ffd2..9ed1dba 100644
--- a/src/test/java/org/apache/aurora/scheduler/async/TaskReconcilerTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/async/TaskReconcilerTest.java
@@ -31,19 +31,34 @@ import 
org.apache.aurora.scheduler.testing.FakeScheduledExecutor;
 import org.junit.Before;
 import org.junit.Test;
 
+import static com.twitter.common.quantity.Time.MINUTES;
+
 import static 
org.apache.aurora.scheduler.async.TaskReconciler.EXPLICIT_STAT_NAME;
 import static 
org.apache.aurora.scheduler.async.TaskReconciler.IMPLICIT_STAT_NAME;
 import static org.apache.aurora.scheduler.async.TaskReconciler.TASK_TO_PROTO;
+import static 
org.apache.aurora.scheduler.async.TaskReconciler.TaskReconcilerSettings;
 import static org.easymock.EasyMock.expect;
 import static org.easymock.EasyMock.expectLastCall;
 import static org.junit.Assert.assertEquals;
 
 public class TaskReconcilerTest extends EasyMockTest {
+  private static final Amount<Long, Time> INITIAL_DELAY = Amount.of(10L, 
MINUTES);
+  private static final Amount<Long, Time> EXPLICIT_SCHEDULE = Amount.of(60L, 
MINUTES);
+  private static final Amount<Long, Time> IMPLICT_SCHEDULE = Amount.of(180L, 
MINUTES);
+  private static final Amount<Long, Time> SPREAD = Amount.of(30L, MINUTES);
+  private static final TaskReconcilerSettings SETTINGS = new 
TaskReconcilerSettings(
+      INITIAL_DELAY,
+      EXPLICIT_SCHEDULE,
+      IMPLICT_SCHEDULE,
+      SPREAD);
+
   private StorageTestUtil storageUtil;
   private StatsProvider statsProvider;
   private Driver driver;
   private ScheduledExecutorService executorService;
   private FakeScheduledExecutor clock;
+  private AtomicLong explicitRuns;
+  private AtomicLong implicitRuns;
 
   @Before
   public void setUp() {
@@ -51,15 +66,15 @@ public class TaskReconcilerTest extends EasyMockTest {
     statsProvider = createMock(StatsProvider.class);
     driver = createMock(Driver.class);
     executorService = createMock(ScheduledExecutorService.class);
-    clock = FakeScheduledExecutor.scheduleAtFixedRateExecutor(executorService, 
2, 5);
+    explicitRuns = new AtomicLong();
+    implicitRuns = new AtomicLong();
   }
 
   @Test
   public void testExecution() {
-    AtomicLong explicitRuns = new AtomicLong();
-    AtomicLong implicitRuns = new AtomicLong();
     
expect(statsProvider.makeCounter(EXPLICIT_STAT_NAME)).andReturn(explicitRuns);
     
expect(statsProvider.makeCounter(IMPLICIT_STAT_NAME)).andReturn(implicitRuns);
+    clock = FakeScheduledExecutor.scheduleAtFixedRateExecutor(executorService, 
2, 5);
 
     IScheduledTask task = TaskTestUtil.makeTask("id1", TaskTestUtil.JOB);
     storageUtil.expectOperations();
@@ -73,17 +88,8 @@ public class TaskReconcilerTest extends EasyMockTest {
 
     control.replay();
 
-    Amount<Long, Time> initialDelay = Amount.of(10L, Time.MINUTES);
-    Amount<Long, Time> explicitSchedule = Amount.of(60L, Time.MINUTES);
-    Amount<Long, Time> implicitSchedule = Amount.of(180L, Time.MINUTES);
-    Amount<Long, Time> spread = Amount.of(30L, Time.MINUTES);
-
     TaskReconciler reconciler = new TaskReconciler(
-        new TaskReconciler.TaskReconcilerSettings(
-            initialDelay,
-            explicitSchedule,
-            implicitSchedule,
-            spread),
+        SETTINGS,
         storageUtil.storage,
         driver,
         executorService,
@@ -91,20 +97,42 @@ public class TaskReconcilerTest extends EasyMockTest {
 
     reconciler.startAsync().awaitRunning();
 
-    clock.advance(initialDelay);
+    clock.advance(INITIAL_DELAY);
     assertEquals(1L, explicitRuns.get());
     assertEquals(0L, implicitRuns.get());
 
-    clock.advance(spread);
+    clock.advance(SPREAD);
     assertEquals(1L, explicitRuns.get());
     assertEquals(1L, implicitRuns.get());
 
-    clock.advance(explicitSchedule);
+    clock.advance(EXPLICIT_SCHEDULE);
     assertEquals(2L, explicitRuns.get());
     assertEquals(1L, implicitRuns.get());
 
-    clock.advance(implicitSchedule);
+    clock.advance(IMPLICT_SCHEDULE);
     assertEquals(5L, explicitRuns.get());
     assertEquals(2L, implicitRuns.get());
   }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void testInvalidImplicitDelay() throws Exception {
+    control.replay();
+
+    new TaskReconcilerSettings(
+        INITIAL_DELAY,
+        EXPLICIT_SCHEDULE,
+        IMPLICT_SCHEDULE,
+        Amount.of(Long.MAX_VALUE, MINUTES));
+  }
+
+  @Test(expected = IllegalArgumentException.class)
+  public void testInvalidExplicitDelay() throws Exception {
+    control.replay();
+
+    new TaskReconcilerSettings(
+        Amount.of(Long.MAX_VALUE, MINUTES),
+        EXPLICIT_SCHEDULE,
+        IMPLICT_SCHEDULE,
+        SPREAD);
+  }
 }

Reply via email to