aurora-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject incubator-aurora git commit: Improving messages in CronJobManager.
Date Thu, 27 Nov 2014 00:23:09 GMT
Repository: incubator-aurora
Updated Branches:
  refs/heads/master ae4d70fc7 -> 3310dc0da


Improving messages in CronJobManager.

Bugs closed: AURORA-924

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


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

Branch: refs/heads/master
Commit: 3310dc0da9502cc7de1fe203b6d15906f15930c0
Parents: ae4d70f
Author: Maxim Khutornenko <maxim@apache.org>
Authored: Wed Nov 26 16:22:50 2014 -0800
Committer: Maxim Khutornenko <maxim@apache.org>
Committed: Wed Nov 26 16:22:50 2014 -0800

----------------------------------------------------------------------
 .../cron/quartz/CronJobManagerImpl.java         | 26 ++++-----
 .../cron/quartz/CronJobManagerImplTest.java     | 60 ++++++++++++++++++++
 2 files changed, 72 insertions(+), 14 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/3310dc0d/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImpl.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImpl.java
b/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImpl.java
index 28f1ae7..c5cb8ca 100644
--- a/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImpl.java
+++ b/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImpl.java
@@ -90,7 +90,7 @@ class CronJobManagerImpl implements CronJobManager {
     } catch (SchedulerException e) {
       throw new CronException(e);
     }
-    LOG.info(String.format("Triggered cron job for %s.", JobKeys.canonicalString(jobKey)));
+    LOG.info(formatMessage("Triggered cron job for %s.", jobKey));
   }
 
   private static void checkNoRunOverlap(SanitizedCronJob cronJob) throws CronException {
@@ -140,30 +140,26 @@ class CronJobManagerImpl implements CronJobManager {
 
   private void checkNotExists(IJobKey jobKey, JobStore jobStore) throws CronException {
     if (jobStore.fetchJob(getManagerKey(), jobKey).isPresent()) {
-      throw new CronException(
-          String.format("Job already exists for %s.", JobKeys.canonicalString(jobKey)));
+      throw new CronException(formatMessage("Job already exists for %s.", jobKey));
     }
   }
 
   private void checkCronExists(IJobKey jobKey, JobStore jobStore) throws CronException {
     if (!jobStore.fetchJob(getManagerKey(), jobKey).isPresent()) {
-      throw new CronException(
-          String.format("No cron template found for %s.", JobKeys.canonicalString(jobKey)));
+      throw new CronException(formatMessage("No cron job found for %s.", jobKey));
     }
   }
 
   private void removeJob(IJobKey jobKey, JobStore.Mutable jobStore) {
     jobStore.removeJob(jobKey);
-    LOG.info(
-        String.format("Deleted cron job %s from storage.", JobKeys.canonicalString(jobKey)));
+    LOG.info(formatMessage("Deleted cron job %s from storage.", jobKey));
   }
 
   private void saveJob(SanitizedCronJob cronJob, JobStore.Mutable jobStore) {
     IJobConfiguration config = cronJob.getSanitizedConfig().getJobConfig();
 
     jobStore.saveAcceptedJob(getManagerKey(), config);
-    LOG.info(String.format(
-        "Saved new cron job %s to storage.", JobKeys.canonicalString(config.getKey())));
+    LOG.info(formatMessage("Saved new cron job %s to storage.", config.getKey()));
   }
 
   // TODO(ksweeney): Consider exposing this in the interface and making caller responsible.
@@ -175,8 +171,7 @@ class CronJobManagerImpl implements CronJobManager {
     } catch (SchedulerException e) {
       throw new CronException(e);
     }
-    LOG.info(String.format(
-        "Scheduled job %s with schedule %s.", JobKeys.canonicalString(jobKey), crontabEntry));
+    LOG.info(formatMessage("Scheduled job %s with schedule %s.", jobKey, crontabEntry));
   }
 
   @Override
@@ -221,16 +216,15 @@ class CronJobManagerImpl implements CronJobManager {
   }
 
   private void descheduleJob(IJobKey jobKey) {
-    String path = JobKeys.canonicalString(jobKey);
     try {
       // TODO(ksweeney): Consider interrupting the running job here.
       // There's a race here where an old running job could fail to find the old config.
That's
       // fine given that the behavior of AuroraCronJob is to log an error and exit if it's
unable
       // to find a job for its key.
       scheduler.deleteJob(Quartz.jobKey(jobKey));
-      LOG.info("Successfully descheduled " + path + ".");
+      LOG.info(formatMessage("Successfully descheduled %s.", jobKey));
     } catch (SchedulerException e) {
-      LOG.log(Level.WARNING, "Error when attempting to deschedule " + path + ": " + e, e);
+      LOG.log(Level.WARNING, formatMessage("Error descheduling %s: %s", jobKey, e), e);
     }
   }
 
@@ -257,4 +251,8 @@ class CronJobManagerImpl implements CronJobManager {
     }
     return scheduledJobs.build();
   }
+
+  private static String formatMessage(String format, IJobKey jobKey, Object... args) {
+    return String.format(format, JobKeys.canonicalString(jobKey), args);
+  }
 }

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/3310dc0d/src/test/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImplTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImplTest.java
b/src/test/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImplTest.java
index 934e9bb..701536f 100644
--- a/src/test/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImplTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/cron/quartz/CronJobManagerImplTest.java
@@ -20,6 +20,7 @@ import java.util.TimeZone;
 
 import com.google.common.base.Optional;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.twitter.common.testing.easymock.EasyMockTest;
@@ -40,11 +41,13 @@ import org.quartz.CronTrigger;
 import org.quartz.JobDetail;
 import org.quartz.JobKey;
 import org.quartz.Scheduler;
+import org.quartz.SchedulerException;
 import org.quartz.Trigger;
 import org.quartz.impl.matchers.GroupMatcher;
 
 import static org.easymock.EasyMock.anyObject;
 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;
@@ -75,6 +78,17 @@ public class CronJobManagerImplTest extends EasyMockTest {
   }
 
   @Test(expected = CronException.class)
+  public void testStartJobNowFails() throws Exception {
+    populateStorage();
+    scheduler.triggerJob(QuartzTestUtil.QUARTZ_JOB_KEY);
+    expectLastCall().andThrow(new SchedulerException());
+
+    control.replay();
+
+    cronJobManager.startJobNow(QuartzTestUtil.AURORA_JOB_KEY);
+  }
+
+  @Test(expected = CronException.class)
   public void testStartJobNowNonexistent() throws Exception {
     control.replay();
 
@@ -128,6 +142,18 @@ public class CronJobManagerImplTest extends EasyMockTest {
   }
 
   @Test(expected = CronException.class)
+  public void testCreateNonexistentJobFails() throws Exception {
+    SanitizedCronJob sanitizedCronJob = QuartzTestUtil.makeSanitizedCronJob();
+
+    expect(scheduler.scheduleJob(anyObject(JobDetail.class), anyObject(Trigger.class)))
+        .andThrow(new SchedulerException());
+
+    control.replay();
+
+    cronJobManager.createJob(sanitizedCronJob);
+  }
+
+  @Test(expected = CronException.class)
   public void testCreateExistingJobFails() throws Exception {
     SanitizedCronJob sanitizedCronJob = QuartzTestUtil.makeSanitizedCronJob();
     populateStorage();
@@ -184,6 +210,16 @@ public class CronJobManagerImplTest extends EasyMockTest {
   }
 
   @Test
+  public void testFailedDeleteJobDoesNotThrow() throws Exception {
+    expect(scheduler.deleteJob(QuartzTestUtil.QUARTZ_JOB_KEY)).andThrow(new SchedulerException());
+
+    control.replay();
+
+    populateStorage();
+    cronJobManager.deleteJob(QuartzTestUtil.AURORA_JOB_KEY);
+  }
+
+  @Test
   public void testGetScheduledJobs() throws Exception {
     CronTrigger cronTrigger = createMock(CronTrigger.class);
     expect(scheduler.getJobKeys(EasyMock.<GroupMatcher<JobKey>>anyObject()))
@@ -199,6 +235,30 @@ public class CronJobManagerImplTest extends EasyMockTest {
     assertEquals(CrontabEntry.parse("* * * * *"), scheduledJobs.get(QuartzTestUtil.AURORA_JOB_KEY));
   }
 
+  @Test
+  public void testGetScheduledJobsEmpty() throws Exception {
+    expect(scheduler.getJobKeys(EasyMock.<GroupMatcher<JobKey>>anyObject()))
+        .andReturn(ImmutableSet.of(QuartzTestUtil.QUARTZ_JOB_KEY));
+    EasyMock.
+        <List<? extends Trigger>>expect(scheduler.getTriggersOfJob(QuartzTestUtil.QUARTZ_JOB_KEY))
+        .andReturn(ImmutableList.<CronTrigger>of());
+
+    control.replay();
+    assertEquals(ImmutableMap.<IJobKey, CrontabEntry>of(), cronJobManager.getScheduledJobs());
+  }
+
+  @Test(expected = RuntimeException.class)
+  public void testGetScheduledJobsFails() throws Exception {
+    expect(scheduler.getJobKeys(EasyMock.<GroupMatcher<JobKey>>anyObject()))
+        .andReturn(ImmutableSet.of(QuartzTestUtil.QUARTZ_JOB_KEY));
+    EasyMock.
+        <List<? extends Trigger>>expect(scheduler.getTriggersOfJob(QuartzTestUtil.QUARTZ_JOB_KEY))
+        .andThrow(new SchedulerException());
+
+    control.replay();
+    cronJobManager.getScheduledJobs();
+  }
+
   private void populateStorage() throws Exception {
     storage.write(new Storage.MutateWork.NoResult<Exception>() {
       @Override


Mime
View raw message