aurora-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject git commit: Adding initial GC task delay on scheduler restart.
Date Sat, 06 Sep 2014 01:19:21 GMT
Repository: incubator-aurora
Updated Branches:
  refs/heads/master 019ca28f3 -> f7292bb54


Adding initial GC task delay on scheduler restart.

Bugs closed: AURORA-608

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


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

Branch: refs/heads/master
Commit: f7292bb5468e2735d1c1080c3f3f33cf280e159e
Parents: 019ca28
Author: Maxim Khutornenko <maxim@apache.org>
Authored: Fri Sep 5 18:18:54 2014 -0700
Committer: Maxim Khutornenko <maxim@apache.org>
Committed: Fri Sep 5 18:18:54 2014 -0700

----------------------------------------------------------------------
 .../scheduler/async/GcExecutorLauncher.java     | 36 ++++++++++----------
 .../scheduler/async/GcExecutorLauncherTest.java | 26 +++++++++++---
 2 files changed, 39 insertions(+), 23 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/f7292bb5/src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java b/src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java
index 65f4049..cfab578 100644
--- a/src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java
+++ b/src/main/java/org/apache/aurora/scheduler/async/GcExecutorLauncher.java
@@ -13,11 +13,11 @@
  */
 package org.apache.aurora.scheduler.async;
 
+import java.util.Collections;
 import java.util.Map;
 import java.util.Set;
 import java.util.UUID;
 import java.util.concurrent.Executor;
-import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.logging.Logger;
 
@@ -28,8 +28,6 @@ import com.google.common.base.Optional;
 import com.google.common.base.Predicates;
 import com.google.common.base.Supplier;
 import com.google.common.base.Throwables;
-import com.google.common.cache.Cache;
-import com.google.common.cache.CacheBuilder;
 import com.google.common.collect.Maps;
 import com.google.protobuf.ByteString;
 import com.twitter.common.quantity.Amount;
@@ -99,7 +97,7 @@ public class GcExecutorLauncher implements TaskLauncher {
   private final Executor executor;
   private final Driver driver;
   private final Supplier<String> uuidGenerator;
-  private final Cache<String, Long> pulses;
+  private final Map<String, Long> pulses;
   private final AtomicLong lostTasks;
 
   @Inject
@@ -142,9 +140,7 @@ public class GcExecutorLauncher implements TaskLauncher {
     this.executor = requireNonNull(executor);
     this.driver = requireNonNull(driver);
     this.uuidGenerator = requireNonNull(uuidGenerator);
-    this.pulses = CacheBuilder.newBuilder()
-        .expireAfterWrite(settings.getMaxGcInterval(), TimeUnit.MILLISECONDS)
-        .build();
+    this.pulses = Collections.synchronizedMap(Maps.<String, Long>newHashMap());
     this.lostTasks = statsProvider.makeCounter(LOST_TASKS_STAT_NAME);
   }
 
@@ -207,13 +203,12 @@ public class GcExecutorLauncher implements TaskLauncher {
   @Override
   public boolean willUse(final Offer offer) {
     if (!settings.getGcExecutorPath().isPresent()
-        || isAlive(offer.getHostname())
-        || !sufficientResources(offer)) {
+        || !sufficientResources(offer)
+        || !isTimeToCollect(offer.getHostname())) {
 
       return false;
     }
 
-    pulses.put(offer.getHostname(), clock.nowMillis() + settings.getDelayMs());
     executor.execute(new Runnable() {
       @Override
       public void run() {
@@ -242,9 +237,19 @@ public class GcExecutorLauncher implements TaskLauncher {
     // No-op.
   }
 
-  private boolean isAlive(String hostname) {
-    Optional<Long> timestamp = Optional.fromNullable(pulses.getIfPresent(hostname));
-    return timestamp.isPresent() && clock.nowMillis() < timestamp.get();
+  private boolean isTimeToCollect(String hostname) {
+    boolean result = false;
+    Optional<Long> timestamp = Optional.fromNullable(pulses.get(hostname));
+    if (timestamp.isPresent()) {
+      if (clock.nowMillis() >= timestamp.get()) {
+        pulses.put(hostname, clock.nowMillis() + settings.getDelayMs());
+        result = true;
+      }
+    } else {
+      pulses.put(hostname, clock.nowMillis() + settings.getDelayMs());
+    }
+
+    return result;
   }
 
   public static class GcExecutorSettings {
@@ -258,11 +263,6 @@ public class GcExecutorLauncher implements TaskLauncher {
     }
 
     @VisibleForTesting
-    long getMaxGcInterval() {
-      return gcInterval.as(Time.MILLISECONDS);
-    }
-
-    @VisibleForTesting
     int getDelayMs() {
       return gcInterval.as(Time.MILLISECONDS).intValue();
     }

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/f7292bb5/src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java b/src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java
index 0413707..f2d153f 100644
--- a/src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/async/GcExecutorLauncherTest.java
@@ -119,25 +119,37 @@ public class GcExecutorLauncherTest extends EasyMockTest {
     IScheduledTask b = makeTask(JOB_A, FAILED);
     IScheduledTask c = makeTask(JOB_A, FAILED);
 
-    // First call - no tasks to be collected.
+    // Third call - no tasks to be collected.
     expectGetTasksByHost(HOST, a, b, c);
     expectAdjustRetainedTasks(a, b, c);
 
-    // Third call - two tasks collected.
+    // Fourth call - two tasks collected.
     expectGetTasksByHost(HOST, a);
     expectAdjustRetainedTasks(a);
 
+    // Fifth call - the last task collected.
+    expectGetTasksByHost(HOST);
+    expectAdjustRetainedTasks();
+
     replayAndConstruct();
 
     // First call - no items in the cache, no tasks collected.
-    assertTrue(gcExecutorLauncher.willUse(OFFER));
+    assertFalse(gcExecutorLauncher.willUse(OFFER));
 
     // Second call - host item alive, no tasks collected.
     clock.advance(Amount.of((long) SETTINGS.getDelayMs() - 1, Time.MILLISECONDS));
     assertFalse(gcExecutorLauncher.willUse(OFFER));
 
-    // Third call - two tasks collected.
-    clock.advance(Amount.of(15L, Time.MINUTES));
+    // Third call - host item expires (initial delay), no tasks collected
+    clock.advance(Amount.of(1L, Time.HOURS));
+    assertTrue(gcExecutorLauncher.willUse(OFFER));
+
+    // Fourth call - host item expires (regular delay), two tasks collected
+    clock.advance(Amount.of(1L, Time.HOURS));
+    assertTrue(gcExecutorLauncher.willUse(OFFER));
+
+    // Fifth call - host item expires (regular delay), one task collected
+    clock.advance(Amount.of(1L, Time.HOURS));
     assertTrue(gcExecutorLauncher.willUse(OFFER));
   }
 
@@ -205,6 +217,10 @@ public class GcExecutorLauncherTest extends EasyMockTest {
     replayAndConstruct();
 
     // First call - no items in the cache, no tasks collected.
+    assertFalse(gcExecutorLauncher.willUse(OFFER));
+
+    // Second call - host item expires initial delay, one task collected.
+    clock.advance(Amount.of(1L, Time.HOURS));
     assertTrue(gcExecutorLauncher.willUse(OFFER));
   }
 


Mime
View raw message