You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@aurora.apache.org by zm...@apache.org on 2015/01/15 23:52:23 UTC

incubator-aurora git commit: Ensure TaskStateChange event emitted by StateManagerImpl has valid data.

Repository: incubator-aurora
Updated Branches:
  refs/heads/master 1acb428a1 -> b75ed0f8f


Ensure TaskStateChange event emitted by StateManagerImpl has valid data.

The `StateManagerImpl` class emits a pubsub event when a task transitions to the
ASSIGNED state but before it attaches the associated slave id and host. This
patch ensures that the pubsub event has the associated slave id and host when
emitted.

Testing Done:
./gradlew build -Pq

Bugs closed: AURORA-1016

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


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

Branch: refs/heads/master
Commit: b75ed0f8fec22d58d906a1b94f51c7d6f33b4c20
Parents: 1acb428
Author: Zameer Manji <zm...@apache.org>
Authored: Thu Jan 15 14:52:16 2015 -0800
Committer: Zameer Manji <zm...@twitter.com>
Committed: Thu Jan 15 14:52:16 2015 -0800

----------------------------------------------------------------------
 .../scheduler/state/StateManagerImpl.java       | 22 +++++++++--------
 .../scheduler/state/StateManagerImplTest.java   | 26 ++++++++++++++++++++
 2 files changed, 38 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/b75ed0f8/src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java b/src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java
index bd6a05f..8721466 100644
--- a/src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java
+++ b/src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java
@@ -178,17 +178,8 @@ public class StateManagerImpl implements StateManager {
     requireNonNull(slaveId);
     requireNonNull(assignedPorts);
 
-    boolean success = updateTaskAndExternalState(
-        storeProvider.getUnsafeTaskStore(),
-        Optional.<ScheduleStatus>absent(),
-        taskId,
-        ASSIGNED,
-        Optional.<String>absent());
-
-    Preconditions.checkState(
-        success,
-        "Attempt to assign task " + taskId + " to " + slaveHost + " failed");
     Query.Builder query = Query.taskScoped(taskId);
+
     storeProvider.getUnsafeTaskStore().mutateTasks(query,
         new Function<IScheduledTask, IScheduledTask>() {
           @Override
@@ -203,6 +194,17 @@ public class StateManagerImpl implements StateManager {
           }
         });
 
+    boolean success = updateTaskAndExternalState(
+        storeProvider.getUnsafeTaskStore(),
+        Optional.<ScheduleStatus>absent(),
+        taskId,
+        ASSIGNED,
+        Optional.<String>absent());
+
+    Preconditions.checkState(
+        success,
+        "Attempt to assign task " + taskId + " to " + slaveHost + " failed");
+
     return Iterables.getOnlyElement(
         Iterables.transform(
             storeProvider.getTaskStore().fetchTasks(query),

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/b75ed0f8/src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java b/src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java
index 03b1c9b..17b1702 100644
--- a/src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java
@@ -48,6 +48,7 @@ import org.apache.aurora.scheduler.storage.entities.IScheduledTask;
 import org.apache.aurora.scheduler.storage.entities.ITaskConfig;
 import org.apache.aurora.scheduler.storage.mem.MemStorage;
 import org.apache.mesos.Protos.SlaveID;
+import org.easymock.Capture;
 import org.easymock.EasyMock;
 import org.easymock.IAnswer;
 import org.easymock.IArgumentMatcher;
@@ -64,6 +65,7 @@ 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.gen.ScheduleStatus.THROTTLED;
+import static org.easymock.EasyMock.capture;
 import static org.easymock.EasyMock.expect;
 import static org.easymock.EasyMock.expectLastCall;
 import static org.junit.Assert.assertEquals;
@@ -476,6 +478,30 @@ public class StateManagerImplTest extends EasyMockTest {
     insertTask(task, 0);
   }
 
+  @Test
+  public void testAssignTaskPubsub() {
+    // This test ensures the pubsub events emitted by assigning tasks have slave id and host set.
+    ITaskConfig task = makeTask(JIM, MY_JOB);
+    String taskId = "a";
+
+    expect(taskIdGenerator.generate(task, 0)).andReturn(taskId);
+
+    expectStateTransitions(taskId, INIT, PENDING);
+
+    Capture<TaskStateChange> taskStateChangeCapture = createCapture();
+    eventSink.post(capture(taskStateChangeCapture));
+
+    control.replay();
+
+    insertTask(task, 0);
+    assignTask(taskId, HOST_A);
+
+    TaskStateChange change = taskStateChangeCapture.getValue();
+    assertEquals(ASSIGNED, change.getNewState());
+    assertEquals(HOST_A, change.getTask().getAssignedTask().getSlaveHost());
+    assertEquals(HOST_A, change.getTask().getAssignedTask().getSlaveId());
+  }
+
   private void expectStateTransitions(
       String taskId,
       ScheduleStatus initial,