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 <[email protected]> Authored: Wed May 13 11:08:42 2015 -0700 Committer: Maxim Khutornenko <[email protected]> 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(),
