Return-Path: X-Original-To: apmail-aurora-commits-archive@minotaur.apache.org Delivered-To: apmail-aurora-commits-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 00DD510972 for ; Tue, 11 Feb 2014 19:46:11 +0000 (UTC) Received: (qmail 9854 invoked by uid 500); 11 Feb 2014 19:46:07 -0000 Delivered-To: apmail-aurora-commits-archive@aurora.apache.org Received: (qmail 9809 invoked by uid 500); 11 Feb 2014 19:46:07 -0000 Mailing-List: contact commits-help@aurora.incubator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@aurora.incubator.apache.org Delivered-To: mailing list commits@aurora.incubator.apache.org Received: (qmail 9776 invoked by uid 99); 11 Feb 2014 19:46:05 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 11 Feb 2014 19:46:05 +0000 X-ASF-Spam-Status: No, hits=-2000.6 required=5.0 tests=ALL_TRUSTED,RP_MATCHES_RCVD X-Spam-Check-By: apache.org Received: from [140.211.11.3] (HELO mail.apache.org) (140.211.11.3) by apache.org (qpsmtpd/0.29) with SMTP; Tue, 11 Feb 2014 19:46:01 +0000 Received: (qmail 8081 invoked by uid 99); 11 Feb 2014 19:45:39 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 11 Feb 2014 19:45:39 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id 519E392453B; Tue, 11 Feb 2014 19:45:39 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: maxim@apache.org To: commits@aurora.incubator.apache.org Message-Id: <7536db4dba004bee8508c8cbb6d48e52@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: git commit: Moving RUN_OVERLAP check into receiveJob() to prevent failure on startup. Date: Tue, 11 Feb 2014 19:45:39 +0000 (UTC) X-Virus-Checked: Checked by ClamAV on apache.org Updated Branches: refs/heads/master 1d1bf9d9f -> 6f547490c Moving RUN_OVERLAP check into receiveJob() to prevent failure on startup. Bugs closed: AURORA-210 Reviewed at https://reviews.apache.org/r/17955/ Project: http://git-wip-us.apache.org/repos/asf/incubator-aurora/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-aurora/commit/6f547490 Tree: http://git-wip-us.apache.org/repos/asf/incubator-aurora/tree/6f547490 Diff: http://git-wip-us.apache.org/repos/asf/incubator-aurora/diff/6f547490 Branch: refs/heads/master Commit: 6f547490cfd0f8ae817af2ca6ff1bd97d59572bd Parents: 1d1bf9d Author: Maxim Khutornenko Authored: Tue Feb 11 11:45:05 2014 -0800 Committer: Maxim Khutornenko Committed: Tue Feb 11 11:45:05 2014 -0800 ---------------------------------------------------------------------- .../configuration/ConfigurationManager.java | 6 --- .../aurora/scheduler/state/CronJobManager.java | 5 +++ .../configuration/ConfigurationManagerTest.java | 16 -------- .../scheduler/state/CronJobManagerTest.java | 43 ++++++++++++++------ 4 files changed, 36 insertions(+), 34 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/6f547490/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 388926c..82034e0 100644 --- a/src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java +++ b/src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java @@ -32,7 +32,6 @@ 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; @@ -268,11 +267,6 @@ 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/6f547490/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 b5d97af..70ccf03 100644 --- a/src/main/java/org/apache/aurora/scheduler/state/CronJobManager.java +++ b/src/main/java/org/apache/aurora/scheduler/state/CronJobManager.java @@ -353,6 +353,11 @@ public class CronJobManager extends JobManager implements EventSubscriber { return false; } + if (CronCollisionPolicy.RUN_OVERLAP.equals(job.getCronCollisionPolicy())) { + throw new ScheduleException( + "The RUN_OVERLAP collision policy has been removed (AURORA-38)."); + } + SanitizedCronJob cronJob = new SanitizedCronJob(config, cron); storage.write(new MutateWork.NoResult.Quiet() { @Override http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/6f547490/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 24e1bc4..4beb2cf 100644 --- a/src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java +++ b/src/test/java/org/apache/aurora/scheduler/configuration/ConfigurationManagerTest.java @@ -28,8 +28,6 @@ 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; @@ -39,7 +37,6 @@ 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(kevints): Improve test coverage for this class. public class ConfigurationManagerTest { @@ -100,17 +97,4 @@ 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/6f547490/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 0b565ab..380df10 100644 --- a/src/test/java/org/apache/aurora/scheduler/state/CronJobManagerTest.java +++ b/src/test/java/org/apache/aurora/scheduler/state/CronJobManagerTest.java @@ -431,29 +431,48 @@ public class CronJobManagerTest extends EasyMockTest { jobTriggerCapture.getValue().run(); } - @Test - public void testRunOverlapCollision() throws Exception { + @Test(expected = ScheduleException.class) + public void testScheduleFails() throws Exception { + expectJobValidated(job); + storageUtil.jobStore.saveAcceptedJob(MANAGER_KEY, sanitizedConfiguration.getJobConfig()); + expect(cronScheduler.schedule(eq(job.getCronSchedule()), EasyMock.anyObject())) + .andThrow(new CronException("injected")); + + control.replay(); + + cron.receiveJob(sanitizedConfiguration); + } + + @Test(expected = ScheduleException.class) + public void testRunOverlapRejected() throws Exception { IJobConfiguration killExisting = IJobConfiguration.build( job.newBuilder().setCronCollisionPolicy(CronCollisionPolicy.RUN_OVERLAP)); - Capture jobTriggerCapture = expectJobAccepted(killExisting); - expectActiveTaskFetch(TASK); control.replay(); cron.receiveJob(new SanitizedConfiguration(killExisting)); - jobTriggerCapture.getValue().run(); } - @Test(expected = ScheduleException.class) - public void testScheduleFails() throws Exception { - expectJobValidated(job); - storageUtil.jobStore.saveAcceptedJob(MANAGER_KEY, sanitizedConfiguration.getJobConfig()); - expect(cronScheduler.schedule(eq(job.getCronSchedule()), EasyMock.anyObject())) - .andThrow(new CronException("injected")); + @Test + public void testRunOverlapLoadedSuccessfully() throws Exception { + // Existing RUN_OVERLAP jobs should still load and map. + + expect(cronScheduler.startAsync()).andReturn(cronScheduler); + cronScheduler.awaitRunning(); + shutdownRegistry.addAction(EasyMock.>anyObject()); + + IJobConfiguration jobA = + IJobConfiguration.build(makeJob().newBuilder() + .setCronCollisionPolicy(CronCollisionPolicy.RUN_OVERLAP)); + + expect(storageUtil.jobStore.fetchJobs(MANAGER_KEY)).andReturn(ImmutableList.of(jobA)); + expect(cronScheduler.isValidSchedule(jobA.getCronSchedule())).andReturn(true); + expect(cronScheduler.schedule(eq(jobA.getCronSchedule()), EasyMock.anyObject())) + .andReturn("keyA"); control.replay(); - cron.receiveJob(sanitizedConfiguration); + cron.schedulerActive(new SchedulerActive()); } private IJobConfiguration makeJob() {