aurora-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kevi...@apache.org
Subject git commit: Deprecate CronCollisionPolicy.RUN_OVERLAP.
Date Fri, 17 Jan 2014 02:25:36 GMT
Updated Branches:
  refs/heads/master 5a4b9df49 -> ba25e6617


Deprecate CronCollisionPolicy.RUN_OVERLAP.

To prevent cluster starvation, the scheduler now ignores new cron jobs
with RUN_OVERLAP and behaves like CANCEL_NEW for existing ones. The
choice of CANCEL_NEW was arbitrary but IMO fits with the mental
preemption model of Aurora better (you're never guaranteed that a new
instance will schedule but Aurora doesn't tend to kill existing
instances outside of an established preemption routine).

Testing Done:
./gradlew build

Bugs closed: AURORA-38

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


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

Branch: refs/heads/master
Commit: ba25e6617d64de3ec1727d2aebcc37496d07cbac
Parents: 5a4b9df
Author: Kevin Sweeney <kevints@apache.org>
Authored: Thu Jan 16 18:25:23 2014 -0800
Committer: Kevin Sweeney <kevints@apache.org>
Committed: Thu Jan 16 18:25:23 2014 -0800

----------------------------------------------------------------------
 .../configuration/ConfigurationManager.java     |  6 ++++
 .../aurora/scheduler/state/CronJobManager.java  | 28 ++-------------
 .../thrift/org/apache/aurora/gen/api.thrift     |  3 +-
 .../configuration/ConfigurationManagerTest.java | 20 +++++++++--
 .../state/BaseSchedulerCoreImplTest.java        | 38 --------------------
 .../scheduler/state/CronJobManagerTest.java     | 27 --------------
 6 files changed, 28 insertions(+), 94 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/ba25e661/src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
b/src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
index f789018..c75241e 100644
--- a/src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
+++ b/src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java
@@ -32,6 +32,7 @@ import com.twitter.common.base.Closure;
 import com.twitter.common.base.MorePreconditions;
 
 import org.apache.aurora.gen.Constraint;
+import org.apache.aurora.gen.CronCollisionPolicy;
 import org.apache.aurora.gen.JobConfiguration;
 import org.apache.aurora.gen.LimitConstraint;
 import org.apache.aurora.gen.TaskConfig;
@@ -263,6 +264,11 @@ public final class ConfigurationManager {
           "A service task may not be run on a cron schedule: " + builder);
     }
 
+    if (CronCollisionPolicy.RUN_OVERLAP.equals(job.getCronCollisionPolicy())) {
+      throw new TaskDescriptionException(
+          "The RUN_OVERLAP collision policy has been removed (AURORA-38).");
+    }
+
     return IJobConfiguration.build(builder);
   }
 

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/ba25e661/src/main/java/org/apache/aurora/scheduler/state/CronJobManager.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/state/CronJobManager.java b/src/main/java/org/apache/aurora/scheduler/state/CronJobManager.java
index b772712..5a56a70 100644
--- a/src/main/java/org/apache/aurora/scheduler/state/CronJobManager.java
+++ b/src/main/java/org/apache/aurora/scheduler/state/CronJobManager.java
@@ -32,12 +32,9 @@ import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Function;
 import com.google.common.base.Optional;
 import com.google.common.base.Preconditions;
-import com.google.common.base.Predicates;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
-import com.google.common.collect.Iterables;
 import com.google.common.collect.Maps;
-import com.google.common.collect.Ordering;
 import com.google.common.eventbus.Subscribe;
 import com.google.common.util.concurrent.ThreadFactoryBuilder;
 import com.twitter.common.application.ShutdownRegistry;
@@ -51,11 +48,9 @@ import com.twitter.common.stats.Stats;
 import com.twitter.common.util.BackoffHelper;
 
 import org.apache.aurora.gen.CronCollisionPolicy;
-import org.apache.aurora.gen.ScheduleStatus;
 import org.apache.aurora.scheduler.base.JobKeys;
 import org.apache.aurora.scheduler.base.Query;
 import org.apache.aurora.scheduler.base.ScheduleException;
-import org.apache.aurora.scheduler.base.Tasks;
 import org.apache.aurora.scheduler.configuration.ConfigurationManager.TaskDescriptionException;
 import org.apache.aurora.scheduler.configuration.SanitizedConfiguration;
 import org.apache.aurora.scheduler.cron.CronException;
@@ -74,8 +69,6 @@ import org.apache.commons.lang.StringUtils;
 import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkNotNull;
 
-import static org.apache.aurora.gen.ScheduleStatus.PENDING;
-
 /**
  * A job scheduler that receives jobs that should be run periodically on a cron schedule.
  */
@@ -304,24 +297,9 @@ public class CronJobManager extends JobManager implements EventSubscriber
{
           break;
 
         case RUN_OVERLAP:
-          Map<Integer, IScheduledTask> byInstance =
-              Maps.uniqueIndex(activeTasks, Tasks.SCHEDULED_TO_INSTANCE_ID);
-          Map<Integer, ScheduleStatus> existingTasks =
-              Maps.transformValues(byInstance, Tasks.GET_STATUS);
-          if (existingTasks.isEmpty()) {
-            builder.putAll(config.getTaskConfigs());
-          } else if (Iterables.any(existingTasks.values(), Predicates.equalTo(PENDING)))
{
-            LOG.info("Job " + JobKeys.toPath(job) + " has pending tasks, suppressing run.");
-          } else {
-            // To safely overlap this run, we need to adjust the instance IDs of the overlapping
-            // run (maintaining the role/job/instance UUID invariant).
-            int instanceOffset = Ordering.natural().max(existingTasks.keySet()) + 1;
-            LOG.info("Adjusting instance IDs of " + JobKeys.toPath(job) + " by " + instanceOffset
-                + " for overlapping cron run.");
-            for (Map.Entry<Integer, ITaskConfig> entry : config.getTaskConfigs().entrySet())
{
-              builder.put(entry.getKey() + instanceOffset, entry.getValue());
-            }
-          }
+          LOG.severe("Ignoring trigger for job "
+              + JobKeys.toPath(job)
+              + " with deprecated collision policy RUN_OVERLAP due to unterminated active
tasks.");
           break;
 
         default:

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/ba25e661/src/main/thrift/org/apache/aurora/gen/api.thrift
----------------------------------------------------------------------
diff --git a/src/main/thrift/org/apache/aurora/gen/api.thrift b/src/main/thrift/org/apache/aurora/gen/api.thrift
index 927552a..724e39e 100644
--- a/src/main/thrift/org/apache/aurora/gen/api.thrift
+++ b/src/main/thrift/org/apache/aurora/gen/api.thrift
@@ -176,7 +176,8 @@ struct TaskConfig {
 enum CronCollisionPolicy {
   KILL_EXISTING = 0,  // Kills the existing job with the colliding name, and runs the new
cron job.
   CANCEL_NEW    = 1,  // Cancels execution of the new job, leaving the running job in tact.
-  RUN_OVERLAP   = 2   // Runs both jobs, effectively adding more tasks to the existing job.
+  RUN_OVERLAP   = 2   // DEPRECATED. For existing jobs, treated the same as CANCEL_NEW. createJob
+                      // will reject jobs with this policy.
 }
 
 // Description of an aurora job.

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/ba25e661/src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
b/src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
index c9ffc70..24e1bc4 100644
--- a/src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java
@@ -28,6 +28,8 @@ import org.apache.aurora.gen.LimitConstraint;
 import org.apache.aurora.gen.TaskConfig;
 import org.apache.aurora.gen.TaskConstraint;
 import org.apache.aurora.gen.ValueConstraint;
+import org.apache.aurora.scheduler.configuration.ConfigurationManager.TaskDescriptionException;
+import org.apache.aurora.scheduler.storage.entities.IJobConfiguration;
 import org.junit.Test;
 
 import static org.apache.aurora.gen.apiConstants.DEFAULT_ENVIRONMENT;
@@ -37,11 +39,10 @@ import static org.apache.aurora.scheduler.configuration.ConfigurationManager.isG
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
-// TODO(Sathya): Improve test coverage for this class.
+// TODO(kevints): Improve test coverage for this class.
 public class ConfigurationManagerTest {
-  // This job caused a crash when loaded in MESOS-3062
-  // TODO(ksweeney): Create a test fixtures resource file and move this to it.
   private static final JobConfiguration UNSANITIZED_JOB_CONFIGURATION = new JobConfiguration()
       .setKey(new JobKey("owner-role", DEFAULT_ENVIRONMENT, "email_stats"))
       .setCronSchedule("0 2 * * *")
@@ -99,4 +100,17 @@ public class ConfigurationManagerTest {
     assertTrue(copy.isSetKey());
     assertEquals(DEFAULT_ENVIRONMENT, copy.getKey().getEnvironment());
   }
+
+  @Test
+  public void testRunOverlapRejected() {
+    JobConfiguration copy = UNSANITIZED_JOB_CONFIGURATION.deepCopy();
+    ConfigurationManager.applyDefaultsIfUnset(copy);
+    copy.setCronCollisionPolicy(CronCollisionPolicy.RUN_OVERLAP);
+    try {
+      ConfigurationManager.validateAndPopulate(IJobConfiguration.build(copy));
+      fail("CronCollisionPolicy.RUN_OVERLAP was allowed.");
+    } catch (TaskDescriptionException e) {
+      // Expected.
+    }
+  }
 }

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/ba25e661/src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java
b/src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java
index 9291fe6..79f09cd 100644
--- a/src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/state/BaseSchedulerCoreImplTest.java
@@ -554,44 +554,6 @@ public abstract class BaseSchedulerCoreImplTest extends EasyMockTest
{
   }
 
   @Test
-  public void testStartRunningOverlapCronJob() throws Exception {
-    // Start a cron job that is already started by an earlier
-    // call and is PENDING. Make sure it follows the cron collision policy.
-    SanitizedConfiguration sanitizedConfiguration =
-        makeCronJob(KEY_A, 1, "1 1 1 1 1", CronCollisionPolicy.RUN_OVERLAP);
-    expect(cronScheduler.schedule(eq(sanitizedConfiguration.getJobConfig().getCronSchedule()),
-        EasyMock.<Runnable>anyObject()))
-        .andReturn("key");
-
-    control.replay();
-    buildScheduler();
-
-    scheduler.createJob(sanitizedConfiguration);
-    assertTaskCount(0);
-    assertTrue(cron.hasJob(KEY_A));
-
-    scheduler.startCronJob(KEY_A);
-    assertTaskCount(1);
-
-    String taskId = Tasks.id(getOnlyTask(Query.jobScoped(KEY_A)));
-
-    // Now start the same cron job immediately.
-    scheduler.startCronJob(KEY_A);
-
-    // Since the task never left PENDING, the second run should have been suppressed.
-    assertTaskCount(1);
-    assertEquals(PENDING, getTask(taskId).getStatus());
-
-    changeStatus(taskId, ASSIGNED);
-
-    scheduler.startCronJob(KEY_A);
-    assertTaskCount(2);
-    assertEquals(ASSIGNED, getTask(taskId).getStatus());
-
-    getOnlyTask(Query.unscoped().byStatus(ScheduleStatus.PENDING));
-  }
-
-  @Test
   public void testKillCreateCronJob() throws Exception {
     SanitizedConfiguration sanitizedConfiguration = makeCronJob(KEY_A, 1, "1 1 1 1 1");
     IJobConfiguration job = sanitizedConfiguration.getJobConfig();

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/ba25e661/src/test/java/org/apache/aurora/scheduler/state/CronJobManagerTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/state/CronJobManagerTest.java b/src/test/java/org/apache/aurora/scheduler/state/CronJobManagerTest.java
index c679123..e9886cd 100644
--- a/src/test/java/org/apache/aurora/scheduler/state/CronJobManagerTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/state/CronJobManagerTest.java
@@ -15,12 +15,10 @@
  */
 package org.apache.aurora.scheduler.state;
 
-import java.util.Map;
 import java.util.concurrent.Executor;
 
 import com.google.common.base.Optional;
 import com.google.common.collect.ImmutableList;
-import com.google.common.collect.ImmutableMap;
 import com.google.inject.AbstractModule;
 import com.google.inject.Guice;
 import com.google.inject.Injector;
@@ -28,11 +26,9 @@ import com.twitter.common.application.ShutdownRegistry;
 import com.twitter.common.base.ExceptionalCommand;
 import com.twitter.common.testing.easymock.EasyMockTest;
 
-import org.apache.aurora.gen.AssignedTask;
 import org.apache.aurora.gen.ExecutorConfig;
 import org.apache.aurora.gen.Identity;
 import org.apache.aurora.gen.JobConfiguration;
-import org.apache.aurora.gen.ScheduleStatus;
 import org.apache.aurora.gen.ScheduledTask;
 import org.apache.aurora.gen.TaskConfig;
 import org.apache.aurora.scheduler.base.JobKeys;
@@ -45,7 +41,6 @@ import org.apache.aurora.scheduler.events.PubsubEvent.SchedulerActive;
 import org.apache.aurora.scheduler.storage.Storage;
 import org.apache.aurora.scheduler.storage.entities.IJobConfiguration;
 import org.apache.aurora.scheduler.storage.entities.IScheduledTask;
-import org.apache.aurora.scheduler.storage.entities.ITaskConfig;
 import org.apache.aurora.scheduler.storage.testing.StorageTestUtil;
 import org.easymock.Capture;
 import org.easymock.EasyMock;
@@ -53,7 +48,6 @@ import org.easymock.IExpectationSetters;
 import org.junit.Before;
 import org.junit.Test;
 
-import static org.apache.aurora.gen.CronCollisionPolicy.RUN_OVERLAP;
 import static org.apache.aurora.gen.apiConstants.DEFAULT_ENVIRONMENT;
 import static org.easymock.EasyMock.anyObject;
 import static org.easymock.EasyMock.capture;
@@ -287,27 +281,6 @@ public class CronJobManagerTest extends EasyMockTest {
   }
 
   @Test
-  public void testRunOverlapNoShardCollision() throws Exception {
-    IScheduledTask scheduledTask = IScheduledTask.build(new ScheduledTask()
-        .setStatus(ScheduleStatus.RUNNING)
-        .setAssignedTask(new AssignedTask().setTask(defaultTask())));
-    sanitizedConfiguration = SanitizedConfiguration.fromUnsanitized(
-        IJobConfiguration.build(job.newBuilder().setCronCollisionPolicy(RUN_OVERLAP)));
-    expectJobAccepted();
-    expectJobFetch();
-    expectActiveTaskFetch(scheduledTask);
-
-    Map<Integer, ITaskConfig> newConfig =
-        ImmutableMap.of(1, sanitizedConfiguration.getJobConfig().getTaskConfig());
-    stateManager.insertPendingTasks(newConfig);
-
-    control.replay();
-
-    cron.receiveJob(sanitizedConfiguration);
-    cron.startJobNow(job.getKey());
-  }
-
-  @Test
   public void testConsistentState() throws Exception {
     IJobConfiguration updated =
         IJobConfiguration.build(makeJob().newBuilder().setCronSchedule("1 2 3 4 5"));


Mime
View raw message