You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@aurora.apache.org by wf...@apache.org on 2014/07/01 22:42:01 UTC

git commit: Export zero for racks that have no lost tasks.

Repository: incubator-aurora
Updated Branches:
  refs/heads/master 5bd683594 -> 5665b43a0


Export zero for racks that have no lost tasks.

Bugs closed: AURORA-548

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


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

Branch: refs/heads/master
Commit: 5665b43a0a30fb1b0d447b8996afae8fabfc491a
Parents: 5bd6835
Author: Bill Farner <wf...@apache.org>
Authored: Tue Jul 1 13:39:33 2014 -0700
Committer: Bill Farner <wf...@apache.org>
Committed: Tue Jul 1 13:39:33 2014 -0700

----------------------------------------------------------------------
 .../org/apache/aurora/scheduler/TaskVars.java   | 30 +++++++++-----
 .../apache/aurora/scheduler/TaskVarsTest.java   | 41 ++++++++++++++------
 2 files changed, 50 insertions(+), 21 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/5665b43a/src/main/java/org/apache/aurora/scheduler/TaskVars.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/TaskVars.java b/src/main/java/org/apache/aurora/scheduler/TaskVars.java
index ed63d5a..6654c16 100644
--- a/src/main/java/org/apache/aurora/scheduler/TaskVars.java
+++ b/src/main/java/org/apache/aurora/scheduler/TaskVars.java
@@ -23,6 +23,7 @@ import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Function;
 import com.google.common.base.Optional;
 import com.google.common.base.Predicate;
+import com.google.common.base.Strings;
 import com.google.common.base.Supplier;
 import com.google.common.cache.CacheBuilder;
 import com.google.common.cache.CacheLoader;
@@ -113,15 +114,12 @@ class TaskVars implements EventSubscriber {
   public void taskChangedState(TaskStateChange stateChange) {
     IScheduledTask task = stateChange.getTask();
     Optional<ScheduleStatus> previousState = stateChange.getOldState();
-    if (stateChange.isTransition() && !previousState.equals(Optional.of(ScheduleStatus.INIT))) {
-      decrementCount(previousState.get());
-    }
-
-    incrementCount(task.getStatus());
-
-    if (stateChange.getNewState() == ScheduleStatus.LOST) {
-      final String host = stateChange.getTask().getAssignedTask().getSlaveHost();
-      Optional<String> rack = storage.consistentRead(new Work.Quiet<Optional<String>>() {
+    final String host = stateChange.getTask().getAssignedTask().getSlaveHost();
+    Optional<String> rack;
+    if (Strings.isNullOrEmpty(stateChange.getTask().getAssignedTask().getSlaveHost())) {
+      rack = Optional.absent();
+    } else {
+      rack = storage.consistentRead(new Work.Quiet<Optional<String>>() {
         @Override
         public Optional<String> apply(StoreProvider storeProvider) {
           Optional<IAttribute> rack = FluentIterable
@@ -130,7 +128,21 @@ class TaskVars implements EventSubscriber {
           return rack.transform(ATTR_VALUE);
         }
       });
+    }
 
+    // Always dummy-read the lost-tasks-per-rack stat. This ensures that there is at least a zero
+    // exported for all racks.
+    if (rack.isPresent()) {
+      counters.getUnchecked(rackStatName(rack.get()));
+    }
+
+    if (stateChange.isTransition() && !previousState.equals(Optional.of(ScheduleStatus.INIT))) {
+      decrementCount(previousState.get());
+    }
+
+    incrementCount(task.getStatus());
+
+    if (stateChange.getNewState() == ScheduleStatus.LOST) {
       if (rack.isPresent()) {
         counters.getUnchecked(rackStatName(rack.get())).increment();
       } else {

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/5665b43a/src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java b/src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java
index 2e128f4..d02714c 100644
--- a/src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/TaskVarsTest.java
@@ -30,6 +30,7 @@ import org.apache.aurora.gen.Identity;
 import org.apache.aurora.gen.ScheduleStatus;
 import org.apache.aurora.gen.ScheduledTask;
 import org.apache.aurora.gen.TaskConfig;
+import org.apache.aurora.scheduler.base.Tasks;
 import org.apache.aurora.scheduler.events.PubsubEvent.SchedulerActive;
 import org.apache.aurora.scheduler.events.PubsubEvent.TaskStateChange;
 import org.apache.aurora.scheduler.events.PubsubEvent.TasksDeleted;
@@ -49,6 +50,7 @@ import static org.apache.aurora.gen.ScheduleStatus.INIT;
 import static org.apache.aurora.gen.ScheduleStatus.LOST;
 import static org.apache.aurora.gen.ScheduleStatus.PENDING;
 import static org.apache.aurora.gen.ScheduleStatus.RUNNING;
+import static org.apache.aurora.scheduler.TaskVars.rackStatName;
 import static org.easymock.EasyMock.expect;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
@@ -108,14 +110,18 @@ public class TaskVarsTest extends EasyMockTest {
   }
 
   private IScheduledTask makeTask(String job, ScheduleStatus status, String host) {
-    return IScheduledTask.build(new ScheduledTask()
+    ScheduledTask task = new ScheduledTask()
         .setStatus(status)
         .setAssignedTask(new AssignedTask()
             .setTaskId(TASK_ID)
-            .setSlaveHost(host)
             .setTask(new TaskConfig()
                 .setJobName(job)
-                .setOwner(new Identity(ROLE_A, ROLE_A + "-user")))));
+                .setOwner(new Identity(ROLE_A, ROLE_A + "-user"))));
+    if (Tasks.SLAVE_ASSIGNED_STATES.contains(status) || Tasks.isTerminated(status)) {
+      task.getAssignedTask().setSlaveHost(host);
+    }
+
+    return IScheduledTask.build(task);
   }
 
   private IScheduledTask makeTask(String job, ScheduleStatus status) {
@@ -161,21 +167,26 @@ public class TaskVarsTest extends EasyMockTest {
   public void testTaskLifeCycle() {
     expectStatusCountersInitialized();
 
+    IScheduledTask taskA = makeTask(JOB_A, INIT);
+    expectGetHostRack("hostA", "rackA").atLeastOnce();
+    expectStatExport(rackStatName("rackA"));
+
     control.replay();
     schedulerActivated();
 
-    IScheduledTask taskA = makeTask(JOB_A, INIT);
-    changeState(taskA, PENDING);
+    changeState(makeTask(JOB_A, INIT), PENDING);
     assertEquals(1, getValue(PENDING));
     changeState(IScheduledTask.build(taskA.newBuilder().setStatus(PENDING)), ASSIGNED);
     assertEquals(0, getValue(PENDING));
     assertEquals(1, getValue(ASSIGNED));
+    taskA = makeTask(JOB_A, ASSIGNED, "hostA");
     changeState(IScheduledTask.build(taskA.newBuilder().setStatus(ASSIGNED)), RUNNING);
     assertEquals(0, getValue(ASSIGNED));
     assertEquals(1, getValue(RUNNING));
     changeState(IScheduledTask.build(taskA.newBuilder().setStatus(RUNNING)), FINISHED);
     assertEquals(0, getValue(RUNNING));
     assertEquals(1, getValue(FINISHED));
+    assertEquals(0, getValue(rackStatName("rackA")));
     vars.tasksDeleted(new TasksDeleted(ImmutableSet.of(
         IScheduledTask.build(taskA.newBuilder().setStatus(FINISHED)))));
     assertAllZero();
@@ -184,19 +195,25 @@ public class TaskVarsTest extends EasyMockTest {
   @Test
   public void testLoadsFromStorage() {
     expectStatusCountersInitialized();
+    expectGetHostRack("hostA", "rackA").atLeastOnce();
+    expectGetHostRack("hostB", "rackB").atLeastOnce();
+    expectStatExport(rackStatName("rackA"));
+    expectStatExport(rackStatName("rackB"));
 
     control.replay();
     schedulerActivated(
         makeTask(JOB_A, PENDING),
-        makeTask(JOB_A, RUNNING),
-        makeTask(JOB_A, FINISHED),
+        makeTask(JOB_A, RUNNING, "hostA"),
+        makeTask(JOB_A, FINISHED, "hostA"),
         makeTask(JOB_B, PENDING),
-        makeTask(JOB_B, FAILED));
+        makeTask(JOB_B, FAILED, "hostB"));
 
     assertEquals(2, getValue(PENDING));
     assertEquals(1, getValue(RUNNING));
     assertEquals(1, getValue(FINISHED));
     assertEquals(1, getValue(FAILED));
+    assertEquals(0, getValue(rackStatName("rackA")));
+    assertEquals(0, getValue(rackStatName("rackB")));
   }
 
   private IExpectationSetters<?> expectGetHostRack(String host, String rackToReturn) {
@@ -215,8 +232,8 @@ public class TaskVarsTest extends EasyMockTest {
     expectGetHostRack("host2", "rackB").atLeastOnce();
     expectGetHostRack("host3", "rackB").atLeastOnce();
 
-    expectStatExport(TaskVars.rackStatName("rackA"));
-    expectStatExport(TaskVars.rackStatName("rackB"));
+    expectStatExport(rackStatName("rackA"));
+    expectStatExport(rackStatName("rackB"));
 
     control.replay();
     schedulerActivated();
@@ -231,8 +248,8 @@ public class TaskVarsTest extends EasyMockTest {
     changeState(c, LOST);
     changeState(d, LOST);
 
-    assertEquals(2, getValue(TaskVars.rackStatName("rackA")));
-    assertEquals(2, getValue(TaskVars.rackStatName("rackB")));
+    assertEquals(2, getValue(rackStatName("rackA")));
+    assertEquals(2, getValue(rackStatName("rackB")));
   }
 
   @Test