You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@aurora.apache.org by ma...@apache.org on 2015/05/13 20:09:59 UTC
aurora git commit: Enhancing the StateManager.changeState result.
Repository: aurora
Updated Branches:
refs/heads/master 7eba711c3 -> 2dc1d59f1
Enhancing the StateManager.changeState result.
Reviewed at https://reviews.apache.org/r/34148/
Project: http://git-wip-us.apache.org/repos/asf/aurora/repo
Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/2dc1d59f
Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/2dc1d59f
Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/2dc1d59f
Branch: refs/heads/master
Commit: 2dc1d59f1e772e220b3bfb26480c3b90c688800f
Parents: 7eba711
Author: Maxim Khutornenko <ma...@apache.org>
Authored: Wed May 13 11:08:42 2015 -0700
Committer: Maxim Khutornenko <ma...@apache.org>
Committed: Wed May 13 11:08:42 2015 -0700
----------------------------------------------------------------------
.../aurora/scheduler/async/TaskTimeout.java | 7 +--
.../scheduler/state/StateChangeResult.java | 45 ++++++++++++++++++++
.../aurora/scheduler/state/StateManager.java | 6 +--
.../scheduler/state/StateManagerImpl.java | 16 ++++---
.../scheduler/state/TaskStateMachine.java | 13 +++++-
.../scheduler/state/TransitionResult.java | 27 ++++++++----
.../thrift/SchedulerThriftInterface.java | 3 +-
.../aurora/scheduler/UserTaskLauncherTest.java | 5 ++-
.../scheduler/async/TaskSchedulerTest.java | 3 +-
.../scheduler/async/TaskThrottlerTest.java | 3 +-
.../aurora/scheduler/async/TaskTimeoutTest.java | 7 +--
.../async/preemptor/PreemptorImplTest.java | 3 +-
.../cron/quartz/AuroraCronJobTest.java | 3 +-
.../state/MaintenanceControllerImplTest.java | 2 +-
.../scheduler/state/StateManagerImplTest.java | 32 +++++++-------
.../scheduler/state/TaskStateMachineTest.java | 41 +++++++++++-------
.../thrift/SchedulerThriftInterfaceTest.java | 7 +--
.../aurora/scheduler/updater/JobUpdaterIT.java | 4 +-
18 files changed, 156 insertions(+), 71 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/aurora/blob/2dc1d59f/src/main/java/org/apache/aurora/scheduler/async/TaskTimeout.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/async/TaskTimeout.java b/src/main/java/org/apache/aurora/scheduler/async/TaskTimeout.java
index 90e6149..e250f33 100644
--- a/src/main/java/org/apache/aurora/scheduler/async/TaskTimeout.java
+++ b/src/main/java/org/apache/aurora/scheduler/async/TaskTimeout.java
@@ -32,6 +32,7 @@ import com.twitter.common.stats.StatsProvider;
import org.apache.aurora.gen.ScheduleStatus;
import org.apache.aurora.scheduler.events.PubsubEvent.EventSubscriber;
import org.apache.aurora.scheduler.events.PubsubEvent.TaskStateChange;
+import org.apache.aurora.scheduler.state.StateChangeResult;
import org.apache.aurora.scheduler.state.StateManager;
import org.apache.aurora.scheduler.storage.Storage;
@@ -116,9 +117,9 @@ class TaskTimeout extends AbstractIdleService implements EventSubscriber {
// canceled, but in the event of a state transition race, including transientState
// prevents an unintended task timeout.
// Note: This requires LOST transitions trigger Driver.killTask.
- boolean movedToLost = storage.write(new MutateWork.Quiet<Boolean>() {
+ StateChangeResult result = storage.write(new MutateWork.Quiet<StateChangeResult>() {
@Override
- public Boolean apply(Storage.MutableStoreProvider storeProvider) {
+ public StateChangeResult apply(Storage.MutableStoreProvider storeProvider) {
return stateManager.changeState(
storeProvider,
taskId,
@@ -128,7 +129,7 @@ class TaskTimeout extends AbstractIdleService implements EventSubscriber {
}
});
- if (movedToLost) {
+ if (result == StateChangeResult.SUCCESS) {
LOG.info("Timeout reached for task " + taskId + ":" + taskId);
timedOutTasks.incrementAndGet();
}
http://git-wip-us.apache.org/repos/asf/aurora/blob/2dc1d59f/src/main/java/org/apache/aurora/scheduler/state/StateChangeResult.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/state/StateChangeResult.java b/src/main/java/org/apache/aurora/scheduler/state/StateChangeResult.java
new file mode 100644
index 0000000..d64a776
--- /dev/null
+++ b/src/main/java/org/apache/aurora/scheduler/state/StateChangeResult.java
@@ -0,0 +1,45 @@
+/**
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.aurora.scheduler.state;
+
+/**
+ * The result of a task state change request.
+ */
+public enum StateChangeResult {
+ /**
+ * Task is already in target state.
+ */
+ NOOP,
+
+ /**
+ * Task state has been changed successfully.
+ */
+ SUCCESS,
+
+ /**
+ * Change is illegal given the current task state.
+ */
+ ILLEGAL,
+
+ /**
+ * Same as {@link #ILLEGAL} but processing state change request generated some task side effects
+ * (e.g.: a driver.kill() request has been placed).
+ */
+ ILLEGAL_WITH_SIDE_EFFECTS,
+
+ /**
+ * Change is not allowed due to the task not in CAS state.
+ */
+ INVALID_CAS_STATE
+}
http://git-wip-us.apache.org/repos/asf/aurora/blob/2dc1d59f/src/main/java/org/apache/aurora/scheduler/state/StateManager.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/state/StateManager.java b/src/main/java/org/apache/aurora/scheduler/state/StateManager.java
index 71bfefb..5d34fe3 100644
--- a/src/main/java/org/apache/aurora/scheduler/state/StateManager.java
+++ b/src/main/java/org/apache/aurora/scheduler/state/StateManager.java
@@ -44,12 +44,10 @@ public interface StateManager {
* decision to defer the action was mde.
* @param newState State to move the task to.
* @param auditMessage Message to include with the transition.
- * @return {@code true} if the transition was performed and the task was moved to
- * {@code newState}, {@code false} if the transition was not allowed, or the task was not
- * in {@code casState}.
+ * @return {@link StateChangeResult}.
*
*/
- boolean changeState(
+ StateChangeResult changeState(
MutableStoreProvider storeProvider,
String taskId,
Optional<ScheduleStatus> casState,
http://git-wip-us.apache.org/repos/asf/aurora/blob/2dc1d59f/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 d87bb38..1b8733b 100644
--- a/src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java
+++ b/src/main/java/org/apache/aurora/scheduler/state/StateManagerImpl.java
@@ -68,6 +68,8 @@ import static org.apache.aurora.gen.ScheduleStatus.ASSIGNED;
import static org.apache.aurora.gen.ScheduleStatus.INIT;
import static org.apache.aurora.gen.ScheduleStatus.PENDING;
import static org.apache.aurora.gen.ScheduleStatus.THROTTLED;
+import static org.apache.aurora.scheduler.state.StateChangeResult.INVALID_CAS_STATE;
+import static org.apache.aurora.scheduler.state.StateChangeResult.SUCCESS;
/**
* Manager of all persistence-related operations for the scheduler. Acts as a controller for
@@ -149,7 +151,7 @@ public class StateManagerImpl implements StateManager {
}
@Override
- public boolean changeState(
+ public StateChangeResult changeState(
MutableStoreProvider storeProvider,
String taskId,
Optional<ScheduleStatus> casState,
@@ -192,7 +194,7 @@ public class StateManagerImpl implements StateManager {
}
});
- boolean success = updateTaskAndExternalState(
+ StateChangeResult changeResult = updateTaskAndExternalState(
storeProvider.getUnsafeTaskStore(),
Optional.<ScheduleStatus>absent(),
taskId,
@@ -200,7 +202,7 @@ public class StateManagerImpl implements StateManager {
Optional.<String>absent());
Preconditions.checkState(
- success,
+ changeResult == SUCCESS,
"Attempt to assign task " + taskId + " to " + slaveHost + " failed");
return Iterables.getOnlyElement(
@@ -223,7 +225,7 @@ public class StateManagerImpl implements StateManager {
}
});
- private boolean updateTaskAndExternalState(
+ private StateChangeResult updateTaskAndExternalState(
TaskStore.Mutable taskStore,
Optional<ScheduleStatus> casState,
String taskId,
@@ -238,7 +240,7 @@ public class StateManagerImpl implements StateManager {
if (casState.isPresent()
&& (!task.isPresent() || casState.get() != task.get().getStatus())) {
- return false;
+ return INVALID_CAS_STATE;
}
return updateTaskAndExternalState(
@@ -277,7 +279,7 @@ public class StateManagerImpl implements StateManager {
private static final Ordering<SideEffect> ACTION_ORDER =
Ordering.explicit(ACTIONS_IN_ORDER).onResultOf(GET_ACTION);
- private boolean updateTaskAndExternalState(
+ private StateChangeResult updateTaskAndExternalState(
TaskStore.Mutable taskStore,
String taskId,
// Note: This argument is deliberately non-final, and should not be made final.
@@ -399,7 +401,7 @@ public class StateManagerImpl implements StateManager {
eventSink.post(event);
}
- return result.isSuccess();
+ return result.getResult();
}
@Override
http://git-wip-us.apache.org/repos/asf/aurora/blob/2dc1d59f/src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java b/src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java
index 4a7ca62..48d0ff6 100644
--- a/src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java
+++ b/src/main/java/org/apache/aurora/scheduler/state/TaskStateMachine.java
@@ -48,6 +48,10 @@ import static org.apache.aurora.scheduler.state.SideEffect.Action.INCREMENT_FAIL
import static org.apache.aurora.scheduler.state.SideEffect.Action.KILL;
import static org.apache.aurora.scheduler.state.SideEffect.Action.RESCHEDULE;
import static org.apache.aurora.scheduler.state.SideEffect.Action.SAVE_STATE;
+import static org.apache.aurora.scheduler.state.StateChangeResult.ILLEGAL;
+import static org.apache.aurora.scheduler.state.StateChangeResult.ILLEGAL_WITH_SIDE_EFFECTS;
+import static org.apache.aurora.scheduler.state.StateChangeResult.NOOP;
+import static org.apache.aurora.scheduler.state.StateChangeResult.SUCCESS;
import static org.apache.aurora.scheduler.state.TaskStateMachine.TaskState.ASSIGNED;
import static org.apache.aurora.scheduler.state.TaskStateMachine.TaskState.DELETED;
import static org.apache.aurora.scheduler.state.TaskStateMachine.TaskState.DRAINING;
@@ -523,13 +527,18 @@ class TaskStateMachine {
*/
TaskState taskState = status.transform(STATUS_TO_TASK_STATE).or(TaskState.DELETED);
if (stateMachine.getState() == taskState) {
- return new TransitionResult(false, ImmutableSet.<SideEffect>of());
+ return new TransitionResult(NOOP, ImmutableSet.<SideEffect>of());
}
boolean success = stateMachine.transition(taskState);
ImmutableSet<SideEffect> transitionEffects = ImmutableSet.copyOf(sideEffects);
sideEffects.clear();
- return new TransitionResult(success, transitionEffects);
+ if (success) {
+ return new TransitionResult(SUCCESS, transitionEffects);
+ }
+ return new TransitionResult(
+ transitionEffects.isEmpty() ? ILLEGAL : ILLEGAL_WITH_SIDE_EFFECTS,
+ transitionEffects);
}
/**
http://git-wip-us.apache.org/repos/asf/aurora/blob/2dc1d59f/src/main/java/org/apache/aurora/scheduler/state/TransitionResult.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/state/TransitionResult.java b/src/main/java/org/apache/aurora/scheduler/state/TransitionResult.java
index 874c554..6928c66 100644
--- a/src/main/java/org/apache/aurora/scheduler/state/TransitionResult.java
+++ b/src/main/java/org/apache/aurora/scheduler/state/TransitionResult.java
@@ -15,30 +15,39 @@ package org.apache.aurora.scheduler.state;
import java.util.Objects;
+import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableSet;
+import static org.apache.aurora.scheduler.state.StateChangeResult.ILLEGAL_WITH_SIDE_EFFECTS;
+import static org.apache.aurora.scheduler.state.StateChangeResult.SUCCESS;
+
/**
* The actions that should be performed in response to a state transition attempt.
*
* {@see TaskStateMachine}
*/
public class TransitionResult {
- private final boolean success;
+ private final StateChangeResult result;
private final ImmutableSet<SideEffect> sideEffects;
/**
* Creates a transition result with the given side effects.
*
- * @param success Whether the transition attempt relevant to this result was successful.
+ * @param result Transition attempt result.
* @param sideEffects Actions that must be performed in response to the state transition.
*/
- public TransitionResult(boolean success, ImmutableSet<SideEffect> sideEffects) {
- this.success = success;
+ public TransitionResult(StateChangeResult result, ImmutableSet<SideEffect> sideEffects) {
+ this.result = result;
this.sideEffects = Objects.requireNonNull(sideEffects);
+ if (!this.sideEffects.isEmpty()) {
+ Preconditions.checkArgument(
+ result == SUCCESS || result == ILLEGAL_WITH_SIDE_EFFECTS,
+ "Invalid transition result for a non-empty set of side effects");
+ }
}
- public boolean isSuccess() {
- return success;
+ public StateChangeResult getResult() {
+ return result;
}
public ImmutableSet<SideEffect> getSideEffects() {
@@ -52,19 +61,19 @@ public class TransitionResult {
}
TransitionResult other = (TransitionResult) o;
- return Objects.equals(success, other.success)
+ return Objects.equals(result, other.result)
&& Objects.equals(sideEffects, other.sideEffects);
}
@Override
public int hashCode() {
- return Objects.hash(success, sideEffects);
+ return Objects.hash(result, sideEffects);
}
@Override
public String toString() {
return com.google.common.base.Objects.toStringHelper(this)
- .add("success", success)
+ .add("result", result)
.add("sideEffects", sideEffects)
.toString();
}
http://git-wip-us.apache.org/repos/asf/aurora/blob/2dc1d59f/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java b/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
index 160db12..1ca3a9b 100644
--- a/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
+++ b/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java
@@ -98,6 +98,7 @@ import org.apache.aurora.scheduler.quota.QuotaManager.QuotaException;
import org.apache.aurora.scheduler.state.LockManager;
import org.apache.aurora.scheduler.state.LockManager.LockException;
import org.apache.aurora.scheduler.state.MaintenanceController;
+import org.apache.aurora.scheduler.state.StateChangeResult;
import org.apache.aurora.scheduler.state.StateManager;
import org.apache.aurora.scheduler.state.UUIDGenerator;
import org.apache.aurora.scheduler.storage.CronJobStore;
@@ -570,7 +571,7 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin {
boolean tasksKilled = false;
for (String taskId : Tasks.ids(tasks)) {
- tasksKilled |= stateManager.changeState(
+ tasksKilled |= StateChangeResult.SUCCESS == stateManager.changeState(
storeProvider,
taskId,
Optional.<ScheduleStatus>absent(),
http://git-wip-us.apache.org/repos/asf/aurora/blob/2dc1d59f/src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java b/src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java
index 3243232..8da488d 100644
--- a/src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/UserTaskLauncherTest.java
@@ -21,6 +21,7 @@ import org.apache.aurora.gen.HostAttributes;
import org.apache.aurora.gen.ScheduleStatus;
import org.apache.aurora.scheduler.async.OfferManager;
import org.apache.aurora.scheduler.mesos.Offers;
+import org.apache.aurora.scheduler.state.StateChangeResult;
import org.apache.aurora.scheduler.state.StateManager;
import org.apache.aurora.scheduler.storage.Storage.StorageException;
import org.apache.aurora.scheduler.storage.entities.IHostAttributes;
@@ -78,7 +79,7 @@ public class UserTaskLauncherTest extends EasyMockTest {
Optional.<ScheduleStatus>absent(),
RUNNING,
Optional.of("fake message")))
- .andReturn(true);
+ .andReturn(StateChangeResult.SUCCESS);
control.replay();
@@ -127,7 +128,7 @@ public class UserTaskLauncherTest extends EasyMockTest {
Optional.<ScheduleStatus>absent(),
FAILED,
Optional.of(UserTaskLauncher.MEMORY_LIMIT_DISPLAY)))
- .andReturn(false);
+ .andReturn(StateChangeResult.ILLEGAL);
control.replay();
http://git-wip-us.apache.org/repos/asf/aurora/blob/2dc1d59f/src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java b/src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java
index f17c434..f348541 100644
--- a/src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/async/TaskSchedulerTest.java
@@ -54,6 +54,7 @@ import org.apache.aurora.scheduler.filter.AttributeAggregate;
import org.apache.aurora.scheduler.filter.SchedulingFilter.ResourceRequest;
import org.apache.aurora.scheduler.mesos.Driver;
import org.apache.aurora.scheduler.state.MaintenanceController;
+import org.apache.aurora.scheduler.state.StateChangeResult;
import org.apache.aurora.scheduler.state.StateManager;
import org.apache.aurora.scheduler.state.TaskAssigner;
import org.apache.aurora.scheduler.state.TaskAssigner.Assignment;
@@ -358,7 +359,7 @@ public class TaskSchedulerTest extends EasyMockTest {
eq(Optional.of(PENDING)),
eq(LOST),
eq(TaskSchedulerImpl.LAUNCH_FAILED_MSG)))
- .andReturn(true);
+ .andReturn(StateChangeResult.SUCCESS);
replayAndCreateScheduler();
http://git-wip-us.apache.org/repos/asf/aurora/blob/2dc1d59f/src/test/java/org/apache/aurora/scheduler/async/TaskThrottlerTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/async/TaskThrottlerTest.java b/src/test/java/org/apache/aurora/scheduler/async/TaskThrottlerTest.java
index a637101..5772e15 100644
--- a/src/test/java/org/apache/aurora/scheduler/async/TaskThrottlerTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/async/TaskThrottlerTest.java
@@ -29,6 +29,7 @@ import org.apache.aurora.gen.ScheduledTask;
import org.apache.aurora.gen.TaskEvent;
import org.apache.aurora.scheduler.base.Tasks;
import org.apache.aurora.scheduler.events.PubsubEvent.TaskStateChange;
+import org.apache.aurora.scheduler.state.StateChangeResult;
import org.apache.aurora.scheduler.state.StateManager;
import org.apache.aurora.scheduler.storage.entities.IScheduledTask;
import org.apache.aurora.scheduler.storage.testing.StorageTestUtil;
@@ -130,7 +131,7 @@ public class TaskThrottlerTest extends EasyMockTest {
Optional.of(THROTTLED),
PENDING,
Optional.<String>absent()))
- .andReturn(true);
+ .andReturn(StateChangeResult.SUCCESS);
}
private IScheduledTask makeTask(String id, ScheduleStatus status) {
http://git-wip-us.apache.org/repos/asf/aurora/blob/2dc1d59f/src/test/java/org/apache/aurora/scheduler/async/TaskTimeoutTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/async/TaskTimeoutTest.java b/src/test/java/org/apache/aurora/scheduler/async/TaskTimeoutTest.java
index 88fc172..b98a8d7 100644
--- a/src/test/java/org/apache/aurora/scheduler/async/TaskTimeoutTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/async/TaskTimeoutTest.java
@@ -31,6 +31,7 @@ import org.apache.aurora.gen.ScheduledTask;
import org.apache.aurora.gen.TaskConfig;
import org.apache.aurora.gen.TaskEvent;
import org.apache.aurora.scheduler.events.PubsubEvent.TaskStateChange;
+import org.apache.aurora.scheduler.state.StateChangeResult;
import org.apache.aurora.scheduler.state.StateManager;
import org.apache.aurora.scheduler.storage.entities.IScheduledTask;
import org.apache.aurora.scheduler.storage.testing.StorageTestUtil;
@@ -143,7 +144,7 @@ public class TaskTimeoutTest extends EasyMockTest {
Optional.of(KILLING),
LOST,
TaskTimeout.TIMEOUT_MESSAGE))
- .andReturn(true);
+ .andReturn(StateChangeResult.SUCCESS);
replayAndCreate();
@@ -161,7 +162,7 @@ public class TaskTimeoutTest extends EasyMockTest {
Optional.of(ASSIGNED),
LOST,
TaskTimeout.TIMEOUT_MESSAGE))
- .andReturn(true);
+ .andReturn(StateChangeResult.SUCCESS);
replayAndCreate();
@@ -180,7 +181,7 @@ public class TaskTimeoutTest extends EasyMockTest {
Optional.of(KILLING),
LOST,
TaskTimeout.TIMEOUT_MESSAGE))
- .andReturn(false);
+ .andReturn(StateChangeResult.ILLEGAL);
replayAndCreate();
http://git-wip-us.apache.org/repos/asf/aurora/blob/2dc1d59f/src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java b/src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java
index 32d18a9..6ecdbd1 100644
--- a/src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/async/preemptor/PreemptorImplTest.java
@@ -31,6 +31,7 @@ import org.apache.aurora.scheduler.async.OfferManager;
import org.apache.aurora.scheduler.async.preemptor.Preemptor.PreemptorImpl;
import org.apache.aurora.scheduler.base.TaskGroupKey;
import org.apache.aurora.scheduler.base.Tasks;
+import org.apache.aurora.scheduler.state.StateChangeResult;
import org.apache.aurora.scheduler.state.StateManager;
import org.apache.aurora.scheduler.stats.CachedCounters;
import org.apache.aurora.scheduler.storage.Storage;
@@ -155,7 +156,7 @@ public class PreemptorImplTest extends EasyMockTest {
eq(Optional.<ScheduleStatus>absent()),
eq(ScheduleStatus.PREEMPTING),
EasyMock.<Optional<String>>anyObject()))
- .andReturn(true);
+ .andReturn(StateChangeResult.SUCCESS);
}
private static PreemptionProposal createPreemptionProposal(IScheduledTask task) {
http://git-wip-us.apache.org/repos/asf/aurora/blob/2dc1d59f/src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java b/src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java
index 831803f..b9e1657 100644
--- a/src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJobTest.java
@@ -25,6 +25,7 @@ import org.apache.aurora.gen.AssignedTask;
import org.apache.aurora.gen.CronCollisionPolicy;
import org.apache.aurora.gen.ScheduleStatus;
import org.apache.aurora.gen.ScheduledTask;
+import org.apache.aurora.scheduler.state.StateChangeResult;
import org.apache.aurora.scheduler.state.StateManager;
import org.apache.aurora.scheduler.storage.Storage;
import org.apache.aurora.scheduler.storage.db.DbUtil;
@@ -123,7 +124,7 @@ public class AuroraCronJobTest extends EasyMockTest {
eq(Optional.<ScheduleStatus>absent()),
eq(ScheduleStatus.KILLING),
eq(AuroraCronJob.KILL_AUDIT_MESSAGE)))
- .andReturn(true);
+ .andReturn(StateChangeResult.SUCCESS);
backoffHelper.doUntilSuccess(EasyMock.capture(capture));
stateManager.insertPendingTasks(
EasyMock.<MutableStoreProvider>anyObject(),
http://git-wip-us.apache.org/repos/asf/aurora/blob/2dc1d59f/src/test/java/org/apache/aurora/scheduler/state/MaintenanceControllerImplTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/state/MaintenanceControllerImplTest.java b/src/test/java/org/apache/aurora/scheduler/state/MaintenanceControllerImplTest.java
index 7b101bc..0d54049 100644
--- a/src/test/java/org/apache/aurora/scheduler/state/MaintenanceControllerImplTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/state/MaintenanceControllerImplTest.java
@@ -193,7 +193,7 @@ public class MaintenanceControllerImplTest extends EasyMockTest {
Optional.<ScheduleStatus>absent(),
ScheduleStatus.DRAINING,
MaintenanceControllerImpl.DRAINING_MESSAGE))
- .andReturn(true);
+ .andReturn(StateChangeResult.SUCCESS);
}
private void expectFetchTasksByHost(String hostName, ImmutableSet<ScheduledTask> tasks) {
http://git-wip-us.apache.org/repos/asf/aurora/blob/2dc1d59f/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 15e4d38..ff0ef02 100644
--- a/src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/state/StateManagerImplTest.java
@@ -69,12 +69,14 @@ 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.apache.aurora.scheduler.state.StateChangeResult.ILLEGAL;
+import static org.apache.aurora.scheduler.state.StateChangeResult.INVALID_CAS_STATE;
+import static org.apache.aurora.scheduler.state.StateChangeResult.NOOP;
+import static org.apache.aurora.scheduler.state.StateChangeResult.SUCCESS;
import static org.easymock.EasyMock.capture;
import static org.easymock.EasyMock.expect;
import static org.easymock.EasyMock.expectLastCall;
import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
public class StateManagerImplTest extends EasyMockTest {
@@ -216,8 +218,8 @@ public class StateManagerImplTest extends EasyMockTest {
control.replay();
insertTask(NON_SERVICE_CONFIG, 0);
- assertEquals(true, changeState(taskId, KILLING));
- assertEquals(false, changeState(taskId, KILLING));
+ assertEquals(SUCCESS, changeState(taskId, KILLING));
+ assertEquals(ILLEGAL, changeState(taskId, KILLING));
}
@Test
@@ -231,10 +233,10 @@ public class StateManagerImplTest extends EasyMockTest {
insertTask(NON_SERVICE_CONFIG, 0);
assignTask(taskId, HOST_A);
- assertEquals(true, changeState(taskId, RUNNING));
- assertEquals(true, changeState(taskId, KILLING));
- assertEquals(true, changeState(taskId, KILLED));
- assertEquals(false, changeState(taskId, KILLED));
+ assertEquals(SUCCESS, changeState(taskId, RUNNING));
+ assertEquals(SUCCESS, changeState(taskId, KILLING));
+ assertEquals(SUCCESS, changeState(taskId, KILLED));
+ assertEquals(NOOP, changeState(taskId, KILLED));
}
@Test
@@ -370,12 +372,12 @@ public class StateManagerImplTest extends EasyMockTest {
insertTask(config, 0);
assignTask(taskId, HOST_A);
- assertFalse(changeState(
+ assertEquals(INVALID_CAS_STATE, changeState(
taskId,
Optional.of(PENDING),
RUNNING,
Optional.<String>absent()));
- assertTrue(changeState(
+ assertEquals(SUCCESS, changeState(
taskId,
Optional.of(ASSIGNED),
FAILED,
@@ -386,7 +388,7 @@ public class StateManagerImplTest extends EasyMockTest {
public void testCasTaskNotFound() {
control.replay();
- assertFalse(changeState(
+ assertEquals(INVALID_CAS_STATE, changeState(
"a",
Optional.of(PENDING),
ASSIGNED,
@@ -551,15 +553,15 @@ public class StateManagerImplTest extends EasyMockTest {
});
}
- private boolean changeState(
+ private StateChangeResult changeState(
final String taskId,
final Optional<ScheduleStatus> casState,
final ScheduleStatus newState,
final Optional<String> auditMessage) {
- return storage.write(new Storage.MutateWork.Quiet<Boolean>() {
+ return storage.write(new Storage.MutateWork.Quiet<StateChangeResult>() {
@Override
- public Boolean apply(Storage.MutableStoreProvider storeProvider) {
+ public StateChangeResult apply(Storage.MutableStoreProvider storeProvider) {
return stateManager.changeState(
storeProvider,
taskId,
@@ -570,7 +572,7 @@ public class StateManagerImplTest extends EasyMockTest {
});
}
- private boolean changeState(final String taskId, final ScheduleStatus status) {
+ private StateChangeResult changeState(final String taskId, final ScheduleStatus status) {
return changeState(
taskId,
Optional.<ScheduleStatus>absent(),
http://git-wip-us.apache.org/repos/asf/aurora/blob/2dc1d59f/src/test/java/org/apache/aurora/scheduler/state/TaskStateMachineTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/state/TaskStateMachineTest.java b/src/test/java/org/apache/aurora/scheduler/state/TaskStateMachineTest.java
index afbca61..b7326a6 100644
--- a/src/test/java/org/apache/aurora/scheduler/state/TaskStateMachineTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/state/TaskStateMachineTest.java
@@ -37,6 +37,10 @@ import org.apache.aurora.scheduler.storage.entities.IScheduledTask;
import org.junit.Before;
import org.junit.Test;
+import static org.apache.aurora.scheduler.state.StateChangeResult.ILLEGAL;
+import static org.apache.aurora.scheduler.state.StateChangeResult.ILLEGAL_WITH_SIDE_EFFECTS;
+import static org.apache.aurora.scheduler.state.StateChangeResult.NOOP;
+import static org.apache.aurora.scheduler.state.StateChangeResult.SUCCESS;
import static org.apache.aurora.scheduler.state.TaskStateMachine.TaskState.ASSIGNED;
import static org.apache.aurora.scheduler.state.TaskStateMachine.TaskState.DELETED;
import static org.apache.aurora.scheduler.state.TaskStateMachine.TaskState.DRAINING;
@@ -53,9 +57,7 @@ import static org.apache.aurora.scheduler.state.TaskStateMachine.TaskState.RUNNI
import static org.apache.aurora.scheduler.state.TaskStateMachine.TaskState.STARTING;
import static org.apache.aurora.scheduler.state.TaskStateMachine.TaskState.THROTTLED;
import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotEquals;
-import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
// TODO(wfarner): At this rate, it's probably best to exhaustively cover this class with a matrix
@@ -117,7 +119,11 @@ public class TaskStateMachineTest {
legalTransition(TaskState.valueOf(endState.name()), finalActions);
for (ScheduleStatus badTransition : Tasks.TERMINAL_STATES) {
- illegalTransition(TaskState.valueOf(badTransition.name()));
+ if (endState == badTransition) {
+ assertEquals(NOOP, stateMachine.updateState(Optional.of(badTransition)).getResult());
+ } else {
+ illegalTransition(TaskState.valueOf(badTransition.name()));
+ }
}
}
}
@@ -284,7 +290,7 @@ public class TaskStateMachineTest {
private void legalTransition(TaskState state, Set<SideEffect.Action> expectedActions) {
ScheduleStatus previousState = stateMachine.getPreviousState();
TransitionResult result = stateMachine.updateState(state.getStatus());
- assertTrue("Transition to " + state + " was not successful", result.isSuccess());
+ assertEquals("Transition to " + state + " was not successful", SUCCESS, result.getResult());
assertNotEquals(previousState, stateMachine.getPreviousState());
assertEquals(
FluentIterable.from(expectedActions).transform(TO_SIDE_EFFECT).toSet(),
@@ -305,9 +311,10 @@ public class TaskStateMachineTest {
}
private void illegalTransition(TaskState state, Set<SideEffect> sideEffects) {
- TransitionResult result = stateMachine.updateState(state.getStatus());
- assertFalse(result.isSuccess());
- assertEquals(sideEffects, result.getSideEffects());
+ TransitionResult expected = new TransitionResult(
+ sideEffects.isEmpty() ? ILLEGAL : ILLEGAL_WITH_SIDE_EFFECTS,
+ ImmutableSet.copyOf(sideEffects));
+ assertEquals(expected, stateMachine.updateState(state.getStatus()));
}
private static ScheduledTask makeTask(boolean service) {
@@ -324,34 +331,34 @@ public class TaskStateMachineTest {
}
private static final TransitionResult SAVE = new TransitionResult(
- true,
+ SUCCESS,
ImmutableSet.of(new SideEffect(Action.SAVE_STATE, Optional.<ScheduleStatus>absent())));
private static final TransitionResult SAVE_AND_KILL = new TransitionResult(
- true,
+ SUCCESS,
ImmutableSet.of(
new SideEffect(Action.SAVE_STATE, Optional.<ScheduleStatus>absent()),
new SideEffect(Action.KILL, Optional.<ScheduleStatus>absent())));
private static final TransitionResult SAVE_AND_RESCHEDULE = new TransitionResult(
- true,
+ SUCCESS,
ImmutableSet.of(
new SideEffect(Action.SAVE_STATE, Optional.<ScheduleStatus>absent()),
new SideEffect(Action.RESCHEDULE, Optional.<ScheduleStatus>absent())));
private static final TransitionResult SAVE_KILL_AND_RESCHEDULE = new TransitionResult(
- true,
+ SUCCESS,
ImmutableSet.of(
new SideEffect(Action.SAVE_STATE, Optional.<ScheduleStatus>absent()),
new SideEffect(Action.KILL, Optional.<ScheduleStatus>absent()),
new SideEffect(Action.RESCHEDULE, Optional.<ScheduleStatus>absent())));
private static final TransitionResult ILLEGAL_KILL = new TransitionResult(
- false,
+ ILLEGAL_WITH_SIDE_EFFECTS,
ImmutableSet.of(new SideEffect(Action.KILL, Optional.<ScheduleStatus>absent())));
private static final TransitionResult RECORD_FAILURE = new TransitionResult(
- true,
+ SUCCESS,
ImmutableSet.of(
new SideEffect(Action.SAVE_STATE, Optional.<ScheduleStatus>absent()),
new SideEffect(Action.INCREMENT_FAILURES, Optional.<ScheduleStatus>absent())));
private static final TransitionResult DELETE_TASK = new TransitionResult(
- true,
+ SUCCESS,
ImmutableSet.of(new SideEffect(Action.DELETE, Optional.<ScheduleStatus>absent())));
private static final class TestCase {
@@ -531,7 +538,11 @@ public class TaskStateMachineTest {
TransitionResult expectation = EXPECTATIONS.get(testCase);
if (expectation == null) {
- expectation = new TransitionResult(false, ImmutableSet.<SideEffect>of());
+ if (taskPresent && from == to || !taskPresent && to == DELETED) {
+ expectation = new TransitionResult(NOOP, ImmutableSet.<SideEffect>of());
+ } else {
+ expectation = new TransitionResult(ILLEGAL, ImmutableSet.<SideEffect>of());
+ }
}
TaskStateMachine machine;
http://git-wip-us.apache.org/repos/asf/aurora/blob/2dc1d59f/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java b/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
index 1ac1a28..21b4044 100644
--- a/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
@@ -92,6 +92,7 @@ import org.apache.aurora.scheduler.quota.QuotaManager;
import org.apache.aurora.scheduler.state.LockManager;
import org.apache.aurora.scheduler.state.LockManager.LockException;
import org.apache.aurora.scheduler.state.MaintenanceController;
+import org.apache.aurora.scheduler.state.StateChangeResult;
import org.apache.aurora.scheduler.state.StateManager;
import org.apache.aurora.scheduler.state.UUIDGenerator;
import org.apache.aurora.scheduler.storage.Storage.StorageException;
@@ -655,7 +656,7 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
TASK_ID,
Optional.<ScheduleStatus>absent(),
ScheduleStatus.KILLING,
- killedByMessage(USER))).andReturn(true);
+ killedByMessage(USER))).andReturn(StateChangeResult.SUCCESS);
}
@Test
@@ -885,7 +886,7 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
TASK_ID,
Optional.<ScheduleStatus>absent(),
ScheduleStatus.FAILED,
- Optional.of(transitionMessage(USER).get()))).andReturn(true);
+ Optional.of(transitionMessage(USER).get()))).andReturn(StateChangeResult.SUCCESS);
expectAuth(ROOT, true);
expectAuth(ROOT, false);
@@ -983,7 +984,7 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
TASK_ID,
Optional.<ScheduleStatus>absent(),
ScheduleStatus.RESTARTING,
- restartedByMessage(USER))).andReturn(true);
+ restartedByMessage(USER))).andReturn(StateChangeResult.SUCCESS);
control.replay();
http://git-wip-us.apache.org/repos/asf/aurora/blob/2dc1d59f/src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java b/src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java
index 4e7ff3b..7aa19d4 100644
--- a/src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java
+++ b/src/test/java/org/apache/aurora/scheduler/updater/JobUpdaterIT.java
@@ -69,6 +69,7 @@ import org.apache.aurora.scheduler.events.PubsubEvent;
import org.apache.aurora.scheduler.mesos.Driver;
import org.apache.aurora.scheduler.state.LockManager;
import org.apache.aurora.scheduler.state.LockManagerImpl;
+import org.apache.aurora.scheduler.state.StateChangeResult;
import org.apache.aurora.scheduler.state.StateManager;
import org.apache.aurora.scheduler.state.StateManagerImpl;
import org.apache.aurora.scheduler.state.UUIDGenerator;
@@ -125,7 +126,6 @@ import static org.apache.aurora.scheduler.storage.Storage.MutateWork.NoResult;
import static org.apache.aurora.scheduler.updater.UpdateFactory.UpdateFactoryImpl.expandInstanceIds;
import static org.easymock.EasyMock.expectLastCall;
import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
public class JobUpdaterIT extends EasyMockTest {
@@ -242,7 +242,7 @@ public class JobUpdaterIT extends EasyMockTest {
storage.write(new NoResult.Quiet() {
@Override
protected void execute(Storage.MutableStoreProvider storeProvider) {
- assertTrue(stateManager.changeState(
+ assertEquals(StateChangeResult.SUCCESS, stateManager.changeState(
storeProvider,
getTaskId(job, instanceId),
Optional.<ScheduleStatus>absent(),