Repository: aurora Updated Branches: refs/heads/master 7a803730c -> 24d2cafaa
Remove the rewriteConfigs thrift method Reviewed at https://reviews.apache.org/r/62601/ Project: http://git-wip-us.apache.org/repos/asf/aurora/repo Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/24d2cafa Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/24d2cafa Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/24d2cafa Branch: refs/heads/master Commit: 24d2cafaa2ef69bffcb27a8ad0db7432bcd889e7 Parents: 7a80373 Author: Bill Farner <[email protected]> Authored: Fri Sep 29 15:30:06 2017 -0700 Committer: Bill Farner <[email protected]> Committed: Fri Sep 29 15:30:06 2017 -0700 ---------------------------------------------------------------------- RELEASE-NOTES.md | 6 + .../thrift/org/apache/aurora/gen/api.thrift | 34 ---- .../thrift/org/apache/aurora/gen/storage.thrift | 7 +- .../aurora/scheduler/storage/TaskStore.java | 16 -- .../scheduler/storage/db/DbTaskStore.java | 19 -- .../scheduler/storage/log/LogStorage.java | 8 - .../storage/log/WriteAheadStorage.java | 14 -- .../scheduler/storage/mem/MemTaskStore.java | 19 -- .../thrift/SchedulerThriftInterface.java | 132 ------------- .../python/apache/aurora/client/api/__init__.py | 3 - .../storage/AbstractTaskStoreTest.java | 27 --- .../scheduler/storage/log/LogStorageTest.java | 32 --- .../thrift/SchedulerThriftInterfaceTest.java | 197 ------------------- src/test/python/apache/aurora/api_util.py | 3 - .../aurora/client/api/test_scheduler_client.py | 7 - 15 files changed, 7 insertions(+), 517 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/aurora/blob/24d2cafa/RELEASE-NOTES.md ---------------------------------------------------------------------- diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index fd2618f..c58e680 100644 --- a/RELEASE-NOTES.md +++ b/RELEASE-NOTES.md @@ -16,6 +16,12 @@ - Added the `thrift_method_interceptor_modules` scheduler flag that lets cluster operators inject custom Thrift method interceptors. +### Deprecations and removals + +- Removed the `rewriteConfigs` thrift API call in the scheduler. This was a last-ditch mechanism + to modify scheduler state on the fly. It was considered extremely risky to use since its + inception, and is safer to abandon due to its lack of use and likelihood for code rot. + 0.18.0 ====== http://git-wip-us.apache.org/repos/asf/aurora/blob/24d2cafa/api/src/main/thrift/org/apache/aurora/gen/api.thrift ---------------------------------------------------------------------- diff --git a/api/src/main/thrift/org/apache/aurora/gen/api.thrift b/api/src/main/thrift/org/apache/aurora/gen/api.thrift index 3749531..db2220d 100644 --- a/api/src/main/thrift/org/apache/aurora/gen/api.thrift +++ b/api/src/main/thrift/org/apache/aurora/gen/api.thrift @@ -1139,31 +1139,6 @@ service AuroraSchedulerManager extends ReadOnlyScheduler { Response pulseJobUpdate(1: JobUpdateKey key) } -struct InstanceConfigRewrite { - /** Key for the task to rewrite. */ - 1: InstanceKey instanceKey - /** The original configuration. */ - 2: TaskConfig oldTask - /** The rewritten configuration. */ - 3: TaskConfig rewrittenTask -} - -struct JobConfigRewrite { - /** The original job configuration. */ - 1: JobConfiguration oldJob - /** The rewritten job configuration. */ - 2: JobConfiguration rewrittenJob -} - -union ConfigRewrite { - 1: JobConfigRewrite jobRewrite - 2: InstanceConfigRewrite instanceRewrite -} - -struct RewriteConfigsRequest { - 1: list<ConfigRewrite> rewriteCommands -} - struct ExplicitReconciliationSettings { 1: optional i32 batchSize } @@ -1219,15 +1194,6 @@ service AuroraAdmin extends AuroraSchedulerManager { /** Start a storage snapshot and block until it completes. */ Response snapshot() - /** - * Forcibly rewrites the stored definition of user configurations. This is intended to be used - * in a controlled setting, primarily to migrate pieces of configurations that are opaque to the - * scheduler (e.g. executorConfig). - * The scheduler may do some validation of the rewritten configurations, but it is important - * that the caller take care to provide valid input and alter only necessary fields. - */ - Response rewriteConfigs(1: RewriteConfigsRequest request) - /** Tell scheduler to trigger an explicit task reconciliation with the given settings. */ Response triggerExplicitTaskReconciliation(1: ExplicitReconciliationSettings settings) http://git-wip-us.apache.org/repos/asf/aurora/blob/24d2cafa/api/src/main/thrift/org/apache/aurora/gen/storage.thrift ---------------------------------------------------------------------- diff --git a/api/src/main/thrift/org/apache/aurora/gen/storage.thrift b/api/src/main/thrift/org/apache/aurora/gen/storage.thrift index 9e4213f..9ee9eff 100644 --- a/api/src/main/thrift/org/apache/aurora/gen/storage.thrift +++ b/api/src/main/thrift/org/apache/aurora/gen/storage.thrift @@ -44,11 +44,6 @@ struct SaveTasks { 1: set<api.ScheduledTask> tasks } -struct RewriteTask { - 1: string taskId - 2: api.TaskConfig task -} - struct RemoveTasks { 1: set<string> taskIds } @@ -101,7 +96,7 @@ union Op { 8: SaveQuota saveQuota 9: RemoveQuota removeQuota 10: SaveHostAttributes saveHostAttributes - 11: RewriteTask rewriteTask + // 11: removed 12: SaveLock saveLock 13: RemoveLock removeLock 14: SaveJobUpdate saveJobUpdate http://git-wip-us.apache.org/repos/asf/aurora/blob/24d2cafa/src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java b/src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java index 1094a12..29d3b24 100644 --- a/src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java +++ b/src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java @@ -98,22 +98,6 @@ public interface TaskStore { Optional<IScheduledTask> mutateTask( String taskId, Function<IScheduledTask, IScheduledTask> mutator); - - /** - * Rewrites a task's configuration in-place. - * <p> - * <b>WARNING</b>: this is a dangerous operation, and should not be used without exercising - * great care. This feature should be used as a last-ditch effort to rewrite things that - * the scheduler otherwise can't (e.g. {@link ITaskConfig#executorConfig}) rewrite in a - * controlled/tested backfill operation. - * - * @param taskId ID of the task to alter. - * @param taskConfiguration Configuration object to swap with the existing task's - * configuration. - * @return {@code true} if the modification took effect, or {@code false} if the task does not - * exist in the store. - */ - boolean unsafeModifyInPlace(String taskId, ITaskConfig taskConfiguration); } final class Util { http://git-wip-us.apache.org/repos/asf/aurora/blob/24d2cafa/src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java b/src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java index 5af1a79..573d76d 100644 --- a/src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java +++ b/src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java @@ -32,7 +32,6 @@ import org.apache.aurora.common.inject.TimedInterceptor.Timed; import org.apache.aurora.common.quantity.Amount; import org.apache.aurora.common.quantity.Time; import org.apache.aurora.common.util.Clock; -import org.apache.aurora.gen.ScheduledTask; import org.apache.aurora.scheduler.base.Query; import org.apache.aurora.scheduler.base.Query.Builder; import org.apache.aurora.scheduler.base.Tasks; @@ -46,8 +45,6 @@ import org.slf4j.LoggerFactory; import static java.util.Objects.requireNonNull; -import static com.google.common.base.Preconditions.checkNotNull; - /** * A task store implementation based on a relational database. * <p> @@ -187,22 +184,6 @@ class DbTaskStore implements TaskStore.Mutable { }); } - @Timed("db_storage_unsafe_modify_in_place") - @Override - public boolean unsafeModifyInPlace(String taskId, ITaskConfig taskConfiguration) { - checkNotNull(taskId); - checkNotNull(taskConfiguration); - Optional<IScheduledTask> task = fetchTask(taskId); - if (task.isPresent()) { - deleteTasks(ImmutableSet.of(taskId)); - ScheduledTask builder = task.get().newBuilder(); - builder.getAssignedTask().setTask(taskConfiguration.newBuilder()); - saveTasks(ImmutableSet.of(IScheduledTask.build(builder))); - return true; - } - return false; - } - private FluentIterable<IScheduledTask> matches(Query.Builder query) { Iterable<DbScheduledTask> results; Predicate<IScheduledTask> filter; http://git-wip-us.apache.org/repos/asf/aurora/blob/24d2cafa/src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java b/src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java index 387350c..3c9bae4 100644 --- a/src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java +++ b/src/main/java/org/apache/aurora/scheduler/storage/log/LogStorage.java @@ -37,7 +37,6 @@ import org.apache.aurora.common.stats.SlidingStats; import org.apache.aurora.gen.HostAttributes; import org.apache.aurora.gen.storage.LogEntry; import org.apache.aurora.gen.storage.Op; -import org.apache.aurora.gen.storage.RewriteTask; import org.apache.aurora.gen.storage.SaveCronJob; import org.apache.aurora.gen.storage.SaveJobInstanceUpdateEvent; import org.apache.aurora.gen.storage.SaveJobUpdateEvent; @@ -67,7 +66,6 @@ import org.apache.aurora.scheduler.storage.entities.IJobUpdateEvent; import org.apache.aurora.scheduler.storage.entities.IJobUpdateKey; import org.apache.aurora.scheduler.storage.entities.ILock; import org.apache.aurora.scheduler.storage.entities.ILockKey; -import org.apache.aurora.scheduler.storage.entities.ITaskConfig; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -339,12 +337,6 @@ public class LogStorage implements NonVolatileStorage, DistributedSnapshotStore Op._Fields.SAVE_TASKS, op -> writeBehindTaskStore.saveTasks( thriftBackfill.backfillTasks(op.getSaveTasks().getTasks()))) - .put(Op._Fields.REWRITE_TASK, op -> { - RewriteTask rewriteTask = op.getRewriteTask(); - writeBehindTaskStore.unsafeModifyInPlace( - rewriteTask.getTaskId(), - ITaskConfig.build(rewriteTask.getTask())); - }) .put( Op._Fields.REMOVE_TASKS, op -> writeBehindTaskStore.deleteTasks(op.getRemoveTasks().getTaskIds())) http://git-wip-us.apache.org/repos/asf/aurora/blob/24d2cafa/src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java b/src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java index d0de063..325d3a1 100644 --- a/src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java +++ b/src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java @@ -26,7 +26,6 @@ import org.apache.aurora.gen.storage.RemoveJob; import org.apache.aurora.gen.storage.RemoveLock; import org.apache.aurora.gen.storage.RemoveQuota; import org.apache.aurora.gen.storage.RemoveTasks; -import org.apache.aurora.gen.storage.RewriteTask; import org.apache.aurora.gen.storage.SaveCronJob; import org.apache.aurora.gen.storage.SaveFrameworkId; import org.apache.aurora.gen.storage.SaveHostAttributes; @@ -57,7 +56,6 @@ import org.apache.aurora.scheduler.storage.entities.ILock; import org.apache.aurora.scheduler.storage.entities.ILockKey; import org.apache.aurora.scheduler.storage.entities.IResourceAggregate; import org.apache.aurora.scheduler.storage.entities.IScheduledTask; -import org.apache.aurora.scheduler.storage.entities.ITaskConfig; import org.slf4j.Logger; import uno.perk.forward.Forward; @@ -161,18 +159,6 @@ class WriteAheadStorage extends WriteAheadStorageForwarder implements } @Override - public boolean unsafeModifyInPlace(final String taskId, final ITaskConfig taskConfiguration) { - requireNonNull(taskId); - requireNonNull(taskConfiguration); - - boolean mutated = taskStore.unsafeModifyInPlace(taskId, taskConfiguration); - if (mutated) { - write(Op.rewriteTask(new RewriteTask(taskId, taskConfiguration.newBuilder()))); - } - return mutated; - } - - @Override public void deleteTasks(final Set<String> taskIds) { requireNonNull(taskIds); http://git-wip-us.apache.org/repos/asf/aurora/blob/24d2cafa/src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java b/src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java index 964e2fc..3bef9b7 100644 --- a/src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java +++ b/src/main/java/org/apache/aurora/scheduler/storage/mem/MemTaskStore.java @@ -42,7 +42,6 @@ import com.google.common.collect.Maps; import com.google.common.collect.Multimap; import com.google.common.collect.Multimaps; -import org.apache.aurora.common.base.MorePreconditions; import org.apache.aurora.common.inject.TimedInterceptor.Timed; import org.apache.aurora.common.quantity.Amount; import org.apache.aurora.common.quantity.Time; @@ -55,7 +54,6 @@ import org.apache.aurora.scheduler.base.Tasks; import org.apache.aurora.scheduler.storage.TaskStore; import org.apache.aurora.scheduler.storage.entities.IJobKey; import org.apache.aurora.scheduler.storage.entities.IScheduledTask; -import org.apache.aurora.scheduler.storage.entities.ITaskConfig; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -222,23 +220,6 @@ class MemTaskStore implements TaskStore.Mutable { }); } - @Timed("mem_storage_unsafe_modify_in_place") - @Override - public boolean unsafeModifyInPlace(String taskId, ITaskConfig taskConfiguration) { - MorePreconditions.checkNotBlank(taskId); - requireNonNull(taskConfiguration); - - Task stored = tasks.get(taskId); - if (stored == null) { - return false; - } else { - ScheduledTask updated = stored.storedTask.newBuilder(); - updated.getAssignedTask().setTask(taskConfiguration.newBuilder()); - tasks.put(taskId, toTask.apply(IScheduledTask.build(updated))); - return true; - } - } - private static Predicate<Task> queryFilter(Query.Builder query) { return Predicates.compose( Util.queryFilter(query), http://git-wip-us.apache.org/repos/asf/aurora/blob/24d2cafa/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 7ea6fc4..b7d9281 100644 --- a/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java +++ b/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java @@ -14,7 +14,6 @@ package org.apache.aurora.scheduler.thrift; import java.util.Comparator; -import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.atomic.AtomicLong; @@ -31,7 +30,6 @@ import com.google.common.collect.FluentIterable; import com.google.common.collect.HashMultimap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; -import com.google.common.collect.Lists; import com.google.common.collect.Maps; import com.google.common.collect.Multimap; import com.google.common.collect.Multimaps; @@ -39,7 +37,6 @@ import com.google.common.collect.Range; import com.google.common.collect.Sets; import org.apache.aurora.common.stats.StatsProvider; -import org.apache.aurora.gen.ConfigRewrite; import org.apache.aurora.gen.DrainHostsResult; import org.apache.aurora.gen.EndMaintenanceResult; import org.apache.aurora.gen.ExplicitReconciliationSettings; @@ -65,7 +62,6 @@ import org.apache.aurora.gen.ReadOnlyScheduler; import org.apache.aurora.gen.ResourceAggregate; import org.apache.aurora.gen.Response; import org.apache.aurora.gen.Result; -import org.apache.aurora.gen.RewriteConfigsRequest; import org.apache.aurora.gen.ScheduleStatus; import org.apache.aurora.gen.StartJobUpdateResult; import org.apache.aurora.gen.StartMaintenanceResult; @@ -90,19 +86,12 @@ 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; -import org.apache.aurora.scheduler.storage.Storage.MutableStoreProvider; import org.apache.aurora.scheduler.storage.Storage.MutateWork.NoResult; import org.apache.aurora.scheduler.storage.Storage.NonVolatileStorage; import org.apache.aurora.scheduler.storage.Storage.StoreProvider; import org.apache.aurora.scheduler.storage.backup.Recovery; import org.apache.aurora.scheduler.storage.backup.StorageBackup; -import org.apache.aurora.scheduler.storage.entities.IAssignedTask; -import org.apache.aurora.scheduler.storage.entities.IConfigRewrite; import org.apache.aurora.scheduler.storage.entities.IHostStatus; -import org.apache.aurora.scheduler.storage.entities.IInstanceConfigRewrite; -import org.apache.aurora.scheduler.storage.entities.IInstanceKey; -import org.apache.aurora.scheduler.storage.entities.IJobConfigRewrite; import org.apache.aurora.scheduler.storage.entities.IJobConfiguration; import org.apache.aurora.scheduler.storage.entities.IJobKey; import org.apache.aurora.scheduler.storage.entities.IJobUpdate; @@ -133,7 +122,6 @@ import static org.apache.aurora.common.base.MorePreconditions.checkNotBlank; import static org.apache.aurora.gen.ResponseCode.INVALID_REQUEST; import static org.apache.aurora.gen.ResponseCode.LOCK_ERROR; import static org.apache.aurora.gen.ResponseCode.OK; -import static org.apache.aurora.gen.ResponseCode.WARNING; import static org.apache.aurora.scheduler.base.Numbers.convertRanges; import static org.apache.aurora.scheduler.base.Numbers.toRanges; import static org.apache.aurora.scheduler.base.Tasks.ACTIVE_STATES; @@ -173,8 +161,6 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin { @VisibleForTesting static final String END_MAINTENANCE = STAT_PREFIX + "endMaintenance"; @VisibleForTesting - static final String REWRITE_CONFIGS = STAT_PREFIX + "rewriteConfigs"; - @VisibleForTesting static final String ADD_INSTANCES = STAT_PREFIX + "addInstances"; @VisibleForTesting static final String START_JOB_UPDATE = STAT_PREFIX + "startJobUpdate"; @@ -205,7 +191,6 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin { private final AtomicLong drainHostsCounter; private final AtomicLong maintenanceStatusCounter; private final AtomicLong endMaintenanceCounter; - private final AtomicLong rewriteConfigsCounter; private final AtomicLong addInstancesCounter; private final AtomicLong startJobUpdateCounter; @@ -252,7 +237,6 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin { this.drainHostsCounter = statsProvider.makeCounter(DRAIN_HOSTS); this.maintenanceStatusCounter = statsProvider.makeCounter(MAINTENANCE_STATUS); this.endMaintenanceCounter = statsProvider.makeCounter(END_MAINTENANCE); - this.rewriteConfigsCounter = statsProvider.makeCounter(REWRITE_CONFIGS); this.addInstancesCounter = statsProvider.makeCounter(ADD_INSTANCES); this.startJobUpdateCounter = statsProvider.makeCounter(START_JOB_UPDATE); } @@ -672,36 +656,6 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin { } @Override - public Response rewriteConfigs(RewriteConfigsRequest request) { - if (request.getRewriteCommandsSize() == 0) { - return addMessage(empty(), INVALID_REQUEST, "No rewrite commands provided."); - } - - return storage.write(storeProvider -> { - List<String> errors = Lists.newArrayList(); - - for (ConfigRewrite command : request.getRewriteCommands()) { - Optional<String> error = rewriteConfig(IConfigRewrite.build(command), storeProvider); - if (error.isPresent()) { - errors.add(error.get()); - } else { - rewriteConfigsCounter.incrementAndGet(); - } - } - - Response resp = empty(); - if (errors.isEmpty()) { - resp.setResponseCode(OK); - } else { - for (String error : errors) { - addMessage(resp, WARNING, error); - } - } - return resp; - }); - } - - @Override public Response triggerExplicitTaskReconciliation(ExplicitReconciliationSettings settings) throws TException { try { @@ -726,92 +680,6 @@ class SchedulerThriftInterface implements AnnotatedAuroraAdmin { return ok(); } - private Optional<String> rewriteJob(IJobConfigRewrite jobRewrite, CronJobStore.Mutable jobStore) { - IJobConfiguration existingJob = jobRewrite.getOldJob(); - IJobConfiguration rewrittenJob; - Optional<String> error = Optional.absent(); - try { - rewrittenJob = configurationManager.validateAndPopulate(jobRewrite.getRewrittenJob()); - } catch (TaskDescriptionException e) { - // We could add an error here, but this is probably a hint of something wrong in - // the client that's causing a bad configuration to be applied. - throw new RuntimeException(e); - } - - if (existingJob.getKey().equals(rewrittenJob.getKey())) { - Optional<IJobConfiguration> job = jobStore.fetchJob(existingJob.getKey()); - if (job.isPresent()) { - IJobConfiguration storedJob = job.get(); - if (storedJob.equals(existingJob)) { - jobStore.saveAcceptedJob(rewrittenJob); - } else { - error = Optional.of( - "CAS compare failed for " + JobKeys.canonicalString(storedJob.getKey())); - } - } else { - error = Optional.of( - "No jobs found for key " + JobKeys.canonicalString(existingJob.getKey())); - } - } else { - error = Optional.of("Disallowing rewrite attempting to change job key."); - } - - return error; - } - - private Optional<String> rewriteInstance( - IInstanceConfigRewrite instanceRewrite, - MutableStoreProvider storeProvider) { - - IInstanceKey instanceKey = instanceRewrite.getInstanceKey(); - Optional<String> error = Optional.absent(); - Iterable<IScheduledTask> tasks = storeProvider.getTaskStore().fetchTasks( - Query.instanceScoped(instanceKey.getJobKey(), - instanceKey.getInstanceId()) - .active()); - Optional<IAssignedTask> task = - Optional.fromNullable(Iterables.getOnlyElement(tasks, null)) - .transform(IScheduledTask::getAssignedTask); - - if (task.isPresent()) { - if (task.get().getTask().equals(instanceRewrite.getOldTask())) { - ITaskConfig newConfiguration = instanceRewrite.getRewrittenTask(); - boolean changed = storeProvider.getUnsafeTaskStore().unsafeModifyInPlace( - task.get().getTaskId(), newConfiguration); - if (!changed) { - error = Optional.of("Did not change " + task.get().getTaskId()); - } - } else { - error = Optional.of("CAS compare failed for " + instanceKey); - } - } else { - error = Optional.of("No active task found for " + instanceKey); - } - - return error; - } - - private Optional<String> rewriteConfig( - IConfigRewrite command, - MutableStoreProvider storeProvider) { - - Optional<String> error; - switch (command.getSetField()) { - case JOB_REWRITE: - error = rewriteJob(command.getJobRewrite(), storeProvider.getCronJobStore()); - break; - - case INSTANCE_REWRITE: - error = rewriteInstance(command.getInstanceRewrite(), storeProvider); - break; - - default: - throw new IllegalArgumentException("Unhandled command type " + command.getSetField()); - } - - return error; - } - @Override public Response addInstances(InstanceKey key, int count) { IJobKey jobKey = JobKeys.assertValid(IJobKey.build(key.getJobKey())); http://git-wip-us.apache.org/repos/asf/aurora/blob/24d2cafa/src/main/python/apache/aurora/client/api/__init__.py ---------------------------------------------------------------------- diff --git a/src/main/python/apache/aurora/client/api/__init__.py b/src/main/python/apache/aurora/client/api/__init__.py index 7efafd3..f6fd1dd 100644 --- a/src/main/python/apache/aurora/client/api/__init__.py +++ b/src/main/python/apache/aurora/client/api/__init__.py @@ -359,9 +359,6 @@ class AuroraClientAPI(object): def prune_tasks(self, query): return self._scheduler_proxy.pruneTasks(query) - def unsafe_rewrite_config(self, rewrite_request): - return self._scheduler_proxy.rewriteConfigs(rewrite_request) - def sla_get_job_uptime_vector(self, job_key): self._assert_valid_job_key(job_key) return Sla(self._scheduler_proxy).get_job_uptime_vector(job_key) http://git-wip-us.apache.org/repos/asf/aurora/blob/24d2cafa/src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java b/src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java index 69c35b1..76595c0 100644 --- a/src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java +++ b/src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java @@ -44,7 +44,6 @@ import org.apache.aurora.gen.AppcImage; import org.apache.aurora.gen.Attribute; import org.apache.aurora.gen.Container; import org.apache.aurora.gen.DockerImage; -import org.apache.aurora.gen.ExecutorConfig; import org.apache.aurora.gen.HostAttributes; import org.apache.aurora.gen.Image; import org.apache.aurora.gen.MaintenanceMode; @@ -74,9 +73,7 @@ import static org.apache.aurora.gen.ScheduleStatus.ASSIGNED; import static org.apache.aurora.gen.ScheduleStatus.RUNNING; import static org.apache.aurora.scheduler.base.TaskTestUtil.makeTask; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; public abstract class AbstractTaskStoreTest extends TearDownTestCase { protected static final IHostAttributes HOST_A = IHostAttributes.build( @@ -137,11 +134,6 @@ public abstract class AbstractTaskStoreTest extends TearDownTestCase { storeProvider -> storeProvider.getUnsafeTaskStore().mutateTask(taskId, mutation)); } - private boolean unsafeModifyInPlace(String taskId, ITaskConfig taskConfiguration) { - return storage.write(storeProvider -> - storeProvider.getUnsafeTaskStore().unsafeModifyInPlace(taskId, taskConfiguration)); - } - protected void deleteTasks(String... taskIds) { storage.write((NoResult.Quiet) storeProvider -> storeProvider.getUnsafeTaskStore().deleteTasks(ImmutableSet.copyOf(taskIds))); @@ -379,25 +371,6 @@ public abstract class AbstractTaskStoreTest extends TearDownTestCase { } @Test - public void testUnsafeModifyInPlace() { - ITaskConfig updated = ITaskConfig.build( - TASK_A.getAssignedTask() - .getTask() - .newBuilder() - .setExecutorConfig(new ExecutorConfig("aurora", "new_config"))); - - String taskId = Tasks.id(TASK_A); - assertFalse(unsafeModifyInPlace(taskId, updated)); - - saveTasks(TASK_A); - assertTrue(unsafeModifyInPlace(taskId, updated)); - assertEquals(updated, fetchTask(taskId).get().getAssignedTask().getTask()); - - deleteTasks(taskId); - assertFalse(unsafeModifyInPlace(taskId, updated)); - } - - @Test public void testDelete() { saveTasks(TASK_A, TASK_B, TASK_C, TASK_D); deleteTasks("a"); http://git-wip-us.apache.org/repos/asf/aurora/blob/24d2cafa/src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java b/src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java index 0eb54fd..17e75c5 100644 --- a/src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java +++ b/src/test/java/org/apache/aurora/scheduler/storage/log/LogStorageTest.java @@ -64,7 +64,6 @@ import org.apache.aurora.gen.storage.RemoveJob; import org.apache.aurora.gen.storage.RemoveLock; import org.apache.aurora.gen.storage.RemoveQuota; import org.apache.aurora.gen.storage.RemoveTasks; -import org.apache.aurora.gen.storage.RewriteTask; import org.apache.aurora.gen.storage.SaveCronJob; import org.apache.aurora.gen.storage.SaveFrameworkId; import org.apache.aurora.gen.storage.SaveHostAttributes; @@ -104,7 +103,6 @@ import org.apache.aurora.scheduler.storage.entities.ILock; import org.apache.aurora.scheduler.storage.entities.ILockKey; import org.apache.aurora.scheduler.storage.entities.IResourceAggregate; import org.apache.aurora.scheduler.storage.entities.IScheduledTask; -import org.apache.aurora.scheduler.storage.entities.ITaskConfig; import org.apache.aurora.scheduler.storage.log.LogStorage.SchedulingService; import org.apache.aurora.scheduler.storage.log.SnapshotDeduplicator.SnapshotDeduplicatorImpl; import org.apache.aurora.scheduler.storage.log.testing.LogOpMatcher; @@ -297,12 +295,6 @@ public class LogStorageTest extends EasyMockTest { builder.add(createTransaction(Op.saveTasks(saveTasks))); storageUtil.taskStore.saveTasks(ImmutableSet.of(expectedTask)); - RewriteTask rewriteTask = new RewriteTask("id1", new TaskConfig()); - builder.add(createTransaction(Op.rewriteTask(rewriteTask))); - expect(storageUtil.taskStore.unsafeModifyInPlace( - rewriteTask.getTaskId(), - ITaskConfig.build(rewriteTask.getTask()))).andReturn(true); - RemoveTasks removeTasks = new RemoveTasks(ImmutableSet.of("taskId1")); builder.add(createTransaction(Op.removeTasks(removeTasks))); storageUtil.taskStore.deleteTasks(removeTasks.getTaskIds()); @@ -580,30 +572,6 @@ public class LogStorageTest extends EasyMockTest { } @Test - public void testUnsafeModifyInPlace() throws Exception { - String taskId = "wilma"; - String taskId2 = "barney"; - ITaskConfig updatedConfig = task(taskId, ScheduleStatus.RUNNING).getAssignedTask().getTask(); - new AbstractMutationFixture() { - @Override - protected void setupExpectations() throws Exception { - storageUtil.expectWrite(); - expect(storageUtil.taskStore.unsafeModifyInPlace(taskId2, updatedConfig)).andReturn(false); - expect(storageUtil.taskStore.unsafeModifyInPlace(taskId, updatedConfig)).andReturn(true); - streamMatcher.expectTransaction( - Op.rewriteTask(new RewriteTask(taskId, updatedConfig.newBuilder()))) - .andReturn(position); - } - - @Override - protected void performMutations(MutableStoreProvider storeProvider) { - storeProvider.getUnsafeTaskStore().unsafeModifyInPlace(taskId2, updatedConfig); - storeProvider.getUnsafeTaskStore().unsafeModifyInPlace(taskId, updatedConfig); - } - }.run(); - } - - @Test public void testNestedTransactions() throws Exception { String taskId = "fred"; Function<IScheduledTask, IScheduledTask> mutation = Functions.identity(); http://git-wip-us.apache.org/repos/asf/aurora/blob/24d2cafa/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 9c44668..b48477a 100644 --- a/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java +++ b/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java @@ -30,20 +30,14 @@ import com.google.common.collect.Sets; import org.apache.aurora.common.testing.easymock.EasyMockTest; import org.apache.aurora.gen.AssignedTask; import org.apache.aurora.gen.AuroraAdmin; -import org.apache.aurora.gen.ConfigRewrite; import org.apache.aurora.gen.Constraint; import org.apache.aurora.gen.Container; import org.apache.aurora.gen.ExecutorConfig; import org.apache.aurora.gen.ExplicitReconciliationSettings; import org.apache.aurora.gen.HostStatus; import org.apache.aurora.gen.Hosts; -import org.apache.aurora.gen.Identity; -import org.apache.aurora.gen.InstanceConfigRewrite; -import org.apache.aurora.gen.InstanceKey; import org.apache.aurora.gen.InstanceTaskConfig; -import org.apache.aurora.gen.JobConfigRewrite; import org.apache.aurora.gen.JobConfiguration; -import org.apache.aurora.gen.JobKey; import org.apache.aurora.gen.JobUpdate; import org.apache.aurora.gen.JobUpdateInstructions; import org.apache.aurora.gen.JobUpdatePulseStatus; @@ -67,7 +61,6 @@ import org.apache.aurora.gen.Response; import org.apache.aurora.gen.ResponseCode; import org.apache.aurora.gen.ResponseDetail; import org.apache.aurora.gen.Result; -import org.apache.aurora.gen.RewriteConfigsRequest; import org.apache.aurora.gen.ScheduleStatus; import org.apache.aurora.gen.ScheduledTask; import org.apache.aurora.gen.StartJobUpdateResult; @@ -99,9 +92,7 @@ import org.apache.aurora.scheduler.storage.Storage.StorageException; import org.apache.aurora.scheduler.storage.backup.Recovery; import org.apache.aurora.scheduler.storage.backup.StorageBackup; import org.apache.aurora.scheduler.storage.entities.IHostStatus; -import org.apache.aurora.scheduler.storage.entities.IInstanceKey; import org.apache.aurora.scheduler.storage.entities.IJobConfiguration; -import org.apache.aurora.scheduler.storage.entities.IJobKey; import org.apache.aurora.scheduler.storage.entities.IJobUpdate; import org.apache.aurora.scheduler.storage.entities.ILockKey; import org.apache.aurora.scheduler.storage.entities.IMetadata; @@ -130,7 +121,6 @@ import static org.apache.aurora.gen.Resource.ramMb; import static org.apache.aurora.gen.ResponseCode.INVALID_REQUEST; import static org.apache.aurora.gen.ResponseCode.LOCK_ERROR; import static org.apache.aurora.gen.ResponseCode.OK; -import static org.apache.aurora.gen.ResponseCode.WARNING; import static org.apache.aurora.scheduler.configuration.ConfigurationManager.DEDICATED_ATTRIBUTE; import static org.apache.aurora.scheduler.storage.backup.Recovery.RecoveryException; import static org.apache.aurora.scheduler.thrift.Fixtures.CRON_JOB; @@ -166,7 +156,6 @@ import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.MAINTE import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.NOOP_JOB_UPDATE_MESSAGE; import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.NO_CRON; import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.RESTART_SHARDS; -import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.REWRITE_CONFIGS; import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.START_JOB_UPDATE; import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.START_MAINTENANCE; import static org.apache.aurora.scheduler.thrift.SchedulerThriftInterface.jobAlreadyExistsMessage; @@ -271,12 +260,6 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest { return SanitizedConfiguration.fromUnsanitized(TaskTestUtil.CONFIGURATION_MANAGER, job); } - private static IJobConfiguration validateAndPopulate(IJobConfiguration job) - throws TaskDescriptionException { - - return TaskTestUtil.CONFIGURATION_MANAGER.validateAndPopulate(job); - } - @Test public void testCreateJobNoLock() throws Exception { IJobConfiguration job = IJobConfiguration.build(makeProdJob()); @@ -1108,186 +1091,6 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest { } @Test - public void testRewriteShardTaskMissing() throws Exception { - InstanceKey instance = new InstanceKey(JobKeys.from("foo", "bar", "baz").newBuilder(), 0); - - storageUtil.expectTaskFetch( - Query.instanceScoped(IJobKey.build(instance.getJobKey()), instance.getInstanceId()) - .active()); - - control.replay(); - - RewriteConfigsRequest request = new RewriteConfigsRequest( - ImmutableList.of(ConfigRewrite.instanceRewrite( - new InstanceConfigRewrite(instance, productionTask(), productionTask())))); - assertResponse(WARNING, thrift.rewriteConfigs(request)); - assertEquals(0L, statsProvider.getLongValue(REWRITE_CONFIGS)); - } - - @Test - public void testRewriteNoCommands() throws Exception { - control.replay(); - - RewriteConfigsRequest request = new RewriteConfigsRequest(ImmutableList.of()); - assertResponse(INVALID_REQUEST, thrift.rewriteConfigs(request)); - assertEquals(0L, statsProvider.getLongValue(REWRITE_CONFIGS)); - } - - @Test(expected = RuntimeException.class) - public void testRewriteInvalidJob() throws Exception { - control.replay(); - - IJobConfiguration job = IJobConfiguration.build(makeJob()); - thrift.rewriteConfigs( - new RewriteConfigsRequest( - ImmutableList.of(ConfigRewrite.jobRewrite( - new JobConfigRewrite(job.newBuilder(), job.newBuilder().setTaskConfig(null)))))); - } - - @Test - public void testRewriteChangeJobKey() throws Exception { - control.replay(); - - IJobConfiguration job = IJobConfiguration.build(makeJob()); - JobKey rewrittenJobKey = JobKeys.from("a", "b", "c").newBuilder(); - Identity rewrittenIdentity = new Identity().setUser("steve"); - RewriteConfigsRequest request = new RewriteConfigsRequest( - ImmutableList.of(ConfigRewrite.jobRewrite(new JobConfigRewrite( - job.newBuilder(), - job.newBuilder() - .setTaskConfig(job.getTaskConfig().newBuilder().setJob(rewrittenJobKey) - .setOwner(rewrittenIdentity)) - .setOwner(rewrittenIdentity) - .setKey(rewrittenJobKey))))); - assertResponse(WARNING, thrift.rewriteConfigs(request)); - assertEquals(0L, statsProvider.getLongValue(REWRITE_CONFIGS)); - } - - @Test - public void testRewriteShardCasMismatch() throws Exception { - TaskConfig storedConfig = productionTask(); - TaskConfig modifiedConfig = - storedConfig.deepCopy().setExecutorConfig(new ExecutorConfig(EXECUTOR_NAME, "rewritten")); - IScheduledTask storedTask = IScheduledTask.build( - new ScheduledTask().setAssignedTask(new AssignedTask().setTask(storedConfig))); - InstanceKey instance = new InstanceKey(storedConfig.getJob(), 0); - - storageUtil.expectTaskFetch( - Query.instanceScoped(IInstanceKey.build(instance)).active(), storedTask); - - control.replay(); - - RewriteConfigsRequest request = new RewriteConfigsRequest( - ImmutableList.of(ConfigRewrite.instanceRewrite( - new InstanceConfigRewrite(instance, modifiedConfig, modifiedConfig)))); - assertResponse(WARNING, thrift.rewriteConfigs(request)); - assertEquals(0L, statsProvider.getLongValue(REWRITE_CONFIGS)); - } - - @Test - public void testRewriteInstance() throws Exception { - TaskConfig storedConfig = productionTask(); - ITaskConfig modifiedConfig = ITaskConfig.build( - storedConfig.deepCopy().setExecutorConfig(new ExecutorConfig(EXECUTOR_NAME, "rewritten"))); - String taskId = "task_id"; - IScheduledTask storedTask = IScheduledTask.build(new ScheduledTask().setAssignedTask( - new AssignedTask() - .setTaskId(taskId) - .setTask(storedConfig))); - InstanceKey instanceKey = new InstanceKey(storedConfig.getJob(), 0); - - storageUtil.expectTaskFetch( - Query.instanceScoped(IInstanceKey.build(instanceKey)).active(), storedTask); - expect(storageUtil.taskStore.unsafeModifyInPlace(taskId, modifiedConfig)) .andReturn(true); - - control.replay(); - - RewriteConfigsRequest request = new RewriteConfigsRequest( - ImmutableList.of(ConfigRewrite.instanceRewrite( - new InstanceConfigRewrite(instanceKey, storedConfig, modifiedConfig.newBuilder())))); - assertOkResponse(thrift.rewriteConfigs(request)); - assertEquals(1L, statsProvider.getLongValue(REWRITE_CONFIGS)); - } - - @Test - public void testRewriteInstanceUnchanged() throws Exception { - TaskConfig config = productionTask(); - String taskId = "task_id"; - IScheduledTask task = IScheduledTask.build(new ScheduledTask().setAssignedTask( - new AssignedTask() - .setTaskId(taskId) - .setTask(config))); - InstanceKey instanceKey = new InstanceKey(config.getJob(), 0); - - storageUtil.expectTaskFetch( - Query.instanceScoped(IInstanceKey.build(instanceKey)).active(), task); - expect(storageUtil.taskStore.unsafeModifyInPlace( - taskId, - ITaskConfig.build(config.deepCopy()))) - .andReturn(false); - - control.replay(); - - RewriteConfigsRequest request = new RewriteConfigsRequest( - ImmutableList.of(ConfigRewrite.instanceRewrite( - new InstanceConfigRewrite(instanceKey, config, config)))); - assertResponse(WARNING, thrift.rewriteConfigs(request)); - assertEquals(0L, statsProvider.getLongValue(REWRITE_CONFIGS)); - } - - @Test - public void testRewriteJobCasMismatch() throws Exception { - JobConfiguration oldJob = makeJob(productionTask()); - JobConfiguration newJob = oldJob.deepCopy(); - newJob.getTaskConfig().setExecutorConfig(new ExecutorConfig(EXECUTOR_NAME, "rewritten")); - expect(storageUtil.jobStore.fetchJob(IJobKey.build(oldJob.getKey()))) - .andReturn(Optional.of(IJobConfiguration.build(oldJob))); - - control.replay(); - - RewriteConfigsRequest request = new RewriteConfigsRequest( - ImmutableList.of(ConfigRewrite.jobRewrite( - new JobConfigRewrite(newJob, newJob)))); - assertResponse(WARNING, thrift.rewriteConfigs(request)); - assertEquals(0L, statsProvider.getLongValue(REWRITE_CONFIGS)); - } - - @Test - public void testRewriteJobNotFound() throws Exception { - JobConfiguration oldJob = makeJob(productionTask()); - JobConfiguration newJob = oldJob.deepCopy(); - newJob.getTaskConfig().setExecutorConfig(new ExecutorConfig(EXECUTOR_NAME, "rewritten")); - expect(storageUtil.jobStore.fetchJob(IJobKey.build(oldJob.getKey()))) - .andReturn(Optional.absent()); - - control.replay(); - - RewriteConfigsRequest request = new RewriteConfigsRequest( - ImmutableList.of(ConfigRewrite.jobRewrite( - new JobConfigRewrite(oldJob, newJob)))); - assertResponse(WARNING, thrift.rewriteConfigs(request)); - assertEquals(0L, statsProvider.getLongValue(REWRITE_CONFIGS)); - } - - @Test - public void testRewriteJob() throws Exception { - JobConfiguration oldJob = makeJob(productionTask()); - JobConfiguration newJob = oldJob.deepCopy(); - newJob.getTaskConfig().setExecutorConfig(new ExecutorConfig(EXECUTOR_NAME, "rewritten")); - expect(storageUtil.jobStore.fetchJob(IJobKey.build(oldJob.getKey()))) - .andReturn(Optional.of(IJobConfiguration.build(oldJob))); - storageUtil.jobStore.saveAcceptedJob(validateAndPopulate(IJobConfiguration.build(newJob))); - - control.replay(); - - RewriteConfigsRequest request = new RewriteConfigsRequest( - ImmutableList.of(ConfigRewrite.jobRewrite( - new JobConfigRewrite(oldJob, newJob)))); - assertOkResponse(thrift.rewriteConfigs(request)); - assertEquals(1L, statsProvider.getLongValue(REWRITE_CONFIGS)); - } - - @Test public void testUnauthorizedDedicatedJob() throws Exception { control.replay(); http://git-wip-us.apache.org/repos/asf/aurora/blob/24d2cafa/src/test/python/apache/aurora/api_util.py ---------------------------------------------------------------------- diff --git a/src/test/python/apache/aurora/api_util.py b/src/test/python/apache/aurora/api_util.py index 41441f3..3fc9b47 100644 --- a/src/test/python/apache/aurora/api_util.py +++ b/src/test/python/apache/aurora/api_util.py @@ -67,9 +67,6 @@ class SchedulerThriftApiSpec(ReadOnlyScheduler.Iface): def snapshot(self): pass - def rewriteConfigs(self, request): - pass - def createJob(self, description): pass http://git-wip-us.apache.org/repos/asf/aurora/blob/24d2cafa/src/test/python/apache/aurora/client/api/test_scheduler_client.py ---------------------------------------------------------------------- diff --git a/src/test/python/apache/aurora/client/api/test_scheduler_client.py b/src/test/python/apache/aurora/client/api/test_scheduler_client.py index b2fd4d9..f61f73a 100644 --- a/src/test/python/apache/aurora/client/api/test_scheduler_client.py +++ b/src/test/python/apache/aurora/client/api/test_scheduler_client.py @@ -45,7 +45,6 @@ from gen.apache.aurora.api.ttypes import ( Response, ResponseCode, ResponseDetail, - RewriteConfigsRequest, ScheduleStatus, TaskQuery ) @@ -288,12 +287,6 @@ class TestSchedulerProxyAdminInjection(TestSchedulerProxyInjection): self.mox.ReplayAll() self.make_scheduler_proxy().pruneTasks(t) - def test_rewriteConfigs(self): - self.mock_thrift_client.rewriteConfigs( - IsA(RewriteConfigsRequest)).AndReturn(DEFAULT_RESPONSE) - self.mox.ReplayAll() - self.make_scheduler_proxy().rewriteConfigs(RewriteConfigsRequest()) - def test_triggerExplicitTaskReconciliation(self): self.mock_thrift_client.triggerExplicitTaskReconciliation( IsA(ExplicitReconciliationSettings)).AndReturn(DEFAULT_RESPONSE)
