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 4E5AF1014E for ; Tue, 27 May 2014 18:25:33 +0000 (UTC) Received: (qmail 71594 invoked by uid 500); 27 May 2014 18:25:33 -0000 Delivered-To: apmail-aurora-commits-archive@aurora.apache.org Received: (qmail 71560 invoked by uid 500); 27 May 2014 18:25:33 -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 71553 invoked by uid 99); 27 May 2014 18:25:33 -0000 Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136) by apache.org (qpsmtpd/0.29) with ESMTP; Tue, 27 May 2014 18:25:33 +0000 X-ASF-Spam-Status: No, hits=-2000.7 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, 27 May 2014 18:25:30 +0000 Received: (qmail 71479 invoked by uid 99); 27 May 2014 18:25:10 -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, 27 May 2014 18:25:10 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id B07D332A690; Tue, 27 May 2014 18:25:09 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: wfarner@apache.org To: commits@aurora.incubator.apache.org Date: Tue, 27 May 2014 18:25:09 -0000 Message-Id: <112ea7b4b21a4cb29aa24cf15bfa25ee@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [1/2] Enable some additional PMD rules, and fix sources to satisfy them. X-Virus-Checked: Checked by ClamAV on apache.org Repository: incubator-aurora Updated Branches: refs/heads/master 10da38a3a -> a8fa267f0 http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java b/src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java index a56f037..48b36c2 100644 --- a/src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java +++ b/src/main/java/org/apache/aurora/scheduler/base/AsyncUtil.java @@ -58,19 +58,21 @@ public final class AsyncUtil { // See java.util.concurrent.ThreadPoolExecutor#afterExecute(Runnable, Throwable) // for more details and an implementation example. super.afterExecute(runnable, throwable); - if (throwable != null) { - logger.log(Level.SEVERE, throwable.toString(), throwable); - } else if (runnable instanceof Future) { - try { - Future future = (Future) runnable; - if (future.isDone()) { - future.get(); + if (throwable == null) { + if (runnable instanceof Future) { + try { + Future future = (Future) runnable; + if (future.isDone()) { + future.get(); + } + } catch (InterruptedException ie) { + Thread.currentThread().interrupt(); + } catch (ExecutionException ee) { + logger.log(Level.SEVERE, ee.toString(), ee); } - } catch (InterruptedException ie) { - Thread.currentThread().interrupt(); - } catch (ExecutionException ee) { - logger.log(Level.SEVERE, ee.toString(), ee); } + } else { + logger.log(Level.SEVERE, throwable.toString(), throwable); } } }; http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java b/src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java index da06f12..d885b22 100644 --- a/src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java +++ b/src/main/java/org/apache/aurora/scheduler/base/CommandUtil.java @@ -28,7 +28,7 @@ public final class CommandUtil { } private static String uriBasename(String uri) { - int lastSlash = uri.lastIndexOf("/"); + int lastSlash = uri.lastIndexOf('/'); if (lastSlash == -1) { return uri; } else { http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/base/Query.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/base/Query.java b/src/main/java/org/apache/aurora/scheduler/base/Query.java index a5350c8..cfb1d16 100644 --- a/src/main/java/org/apache/aurora/scheduler/base/Query.java +++ b/src/main/java/org/apache/aurora/scheduler/base/Query.java @@ -148,11 +148,11 @@ public final class Query { public static final class Builder implements Supplier { private final TaskQuery query; - private Builder() { + Builder() { this.query = new TaskQuery(); } - private Builder(final TaskQuery query) { + Builder(final TaskQuery query) { this.query = checkNotNull(query); // It is expected that the caller calls deepCopy. } http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/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 f86a5b7..47cb70b 100644 --- a/src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java +++ b/src/main/java/org/apache/aurora/scheduler/configuration/ConfigurationManager.java @@ -324,7 +324,7 @@ public final class ConfigurationManager { IValueConstraint valueConstraint = constraint.getConstraint().getValue(); - if (!(valueConstraint.getValues().size() == 1)) { + if (valueConstraint.getValues().size() != 1) { throw new TaskDescriptionException("A dedicated constraint must have exactly one value"); } http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/configuration/Resources.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/configuration/Resources.java b/src/main/java/org/apache/aurora/scheduler/configuration/Resources.java index 5a38479..9ee2a85 100644 --- a/src/main/java/org/apache/aurora/scheduler/configuration/Resources.java +++ b/src/main/java/org/apache/aurora/scheduler/configuration/Resources.java @@ -112,7 +112,7 @@ public class Resources { .add(Resources.makeMesosResource(CPUS, numCpus)) .add(Resources.makeMesosResource(DISK_MB, disk.as(Data.MB))) .add(Resources.makeMesosResource(RAM_MB, ram.as(Data.MB))); - if (selectedPorts.size() > 0) { + if (selectedPorts.isEmpty()) { resourceBuilder.add(Resources.makeMesosRangeResource(Resources.PORTS, selectedPorts)); } http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/configuration/SanitizedConfiguration.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/configuration/SanitizedConfiguration.java b/src/main/java/org/apache/aurora/scheduler/configuration/SanitizedConfiguration.java index 7fc4930..d511ec0 100644 --- a/src/main/java/org/apache/aurora/scheduler/configuration/SanitizedConfiguration.java +++ b/src/main/java/org/apache/aurora/scheduler/configuration/SanitizedConfiguration.java @@ -14,6 +14,7 @@ package org.apache.aurora.scheduler.configuration; import java.util.Map; +import java.util.logging.Logger; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Functions; @@ -26,12 +27,15 @@ import com.google.common.collect.Range; import org.apache.aurora.scheduler.configuration.ConfigurationManager.TaskDescriptionException; import org.apache.aurora.scheduler.storage.entities.IJobConfiguration; import org.apache.aurora.scheduler.storage.entities.ITaskConfig; +import org.apache.commons.lang.StringUtils; /** * Wrapper for a configuration that has been fully-sanitized and populated with defaults. */ public final class SanitizedConfiguration { + private static final Logger LOG = Logger.getLogger(SanitizedConfiguration.class.getName()); + private final IJobConfiguration sanitized; private final Map tasks; @@ -73,6 +77,24 @@ public final class SanitizedConfiguration { return tasks; } + /** + * Determines whether this job is configured as a cron job. + * + * @return {@code true} if this is a cron job, otherwise {@code false}. + */ + public boolean isCron() { + if (getJobConfig().isSetCronSchedule()) { + if (StringUtils.isEmpty(getJobConfig().getCronSchedule())) { + // TODO(ksweeney): Remove this in 0.7.0 (AURORA-423). + LOG.warning("Got service config with empty string cron schedule. aurora-0.7.x " + + "will interpret this as cron job and cause an error."); + return false; + } + return true; + } + return false; + } + @Override public boolean equals(Object o) { if (!(o instanceof SanitizedConfiguration)) { http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java b/src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java index cc08137..57d874b 100644 --- a/src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java +++ b/src/main/java/org/apache/aurora/scheduler/cron/quartz/AuroraCronJob.java @@ -97,7 +97,7 @@ class AuroraCronJob implements Job { private final Map pendingTasks; private final Set activeTaskIds; - private DeferredLaunch(Map pendingTasks, Set activeTaskIds) { + DeferredLaunch(Map pendingTasks, Set activeTaskIds) { this.pendingTasks = pendingTasks; this.activeTaskIds = activeTaskIds; } http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronLifecycle.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronLifecycle.java b/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronLifecycle.java index 8f72a2d..02ba94a 100644 --- a/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronLifecycle.java +++ b/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronLifecycle.java @@ -32,6 +32,7 @@ import org.apache.aurora.scheduler.cron.SanitizedCronJob; import org.apache.aurora.scheduler.events.PubsubEvent; import org.apache.aurora.scheduler.storage.entities.IJobConfiguration; import org.quartz.Scheduler; +import org.quartz.SchedulerException; import static com.google.common.base.Preconditions.checkNotNull; @@ -79,7 +80,7 @@ class CronLifecycle extends AbstractIdleService implements PubsubEvent.EventSubs } @Override - protected void startUp() throws Exception { + protected void startUp() throws SchedulerException { LOG.info("Starting Quartz cron scheduler" + scheduler.getSchedulerName() + "."); scheduler.start(); RUNNING_FLAG.set(1); @@ -104,7 +105,7 @@ class CronLifecycle extends AbstractIdleService implements PubsubEvent.EventSubs } @Override - protected void shutDown() throws Exception { + protected void shutDown() throws SchedulerException { LOG.info("Shutting down Quartz cron scheduler."); scheduler.shutdown(); RUNNING_FLAG.set(0); http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronModule.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronModule.java b/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronModule.java index 0094c88..88cb360 100644 --- a/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronModule.java +++ b/src/main/java/org/apache/aurora/scheduler/cron/quartz/CronModule.java @@ -89,7 +89,7 @@ public class CronModule extends AbstractModule { } @Provides - private TimeZone provideTimeZone() { + TimeZone provideTimeZone() { TimeZone timeZone = TimeZone.getTimeZone(CRON_TIMEZONE.get()); TimeZone systemTimeZone = TimeZone.getDefault(); if (!timeZone.equals(systemTimeZone)) { @@ -109,7 +109,7 @@ public class CronModule extends AbstractModule { */ @Provides @Singleton - private static synchronized Scheduler provideScheduler(AuroraCronJobFactory jobFactory) { + static synchronized Scheduler provideScheduler(AuroraCronJobFactory jobFactory) { SimpleThreadPool threadPool = new SimpleThreadPool(NUM_THREADS.get(), Thread.NORM_PRIORITY); threadPool.setMakeThreadsDaemons(true); http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java b/src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java index 06a5f8f..2afbef8 100644 --- a/src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java +++ b/src/main/java/org/apache/aurora/scheduler/events/PubsubEvent.java @@ -34,13 +34,13 @@ public interface PubsubEvent { /** * Interface with no functionality, but identifies a class as supporting task pubsub events. */ - public interface EventSubscriber { + interface EventSubscriber { } /** * Event sent when tasks were deleted. */ - public static class TasksDeleted implements PubsubEvent { + class TasksDeleted implements PubsubEvent { private final Set tasks; public TasksDeleted(Set tasks) { @@ -76,8 +76,10 @@ public interface PubsubEvent { /** * Event sent when a task changed state. + *

+ * This class is final as it should only be constructed through declared factory methods. */ - public static final class TaskStateChange implements PubsubEvent { + final class TaskStateChange implements PubsubEvent { private final IScheduledTask task; private final Optional oldState; @@ -156,7 +158,7 @@ public interface PubsubEvent { /** * Event sent when a host changed maintenance state. */ - public static class HostMaintenanceStateChange implements PubsubEvent { + class HostMaintenanceStateChange implements PubsubEvent { private final HostStatus status; public HostMaintenanceStateChange(HostStatus status) { @@ -186,7 +188,7 @@ public interface PubsubEvent { /** * Event sent when a scheduling assignment was vetoed. */ - public static class Vetoed implements PubsubEvent { + class Vetoed implements PubsubEvent { private final String taskId; private final Set vetoes; @@ -220,7 +222,7 @@ public interface PubsubEvent { } } - public static class DriverRegistered implements PubsubEvent { + class DriverRegistered implements PubsubEvent { @Override public boolean equals(Object o) { return o != null && getClass().equals(o.getClass()); @@ -232,7 +234,7 @@ public interface PubsubEvent { } } - public static class DriverDisconnected implements PubsubEvent { + class DriverDisconnected implements PubsubEvent { @Override public boolean equals(Object o) { return o != null && getClass().equals(o.getClass()); @@ -244,7 +246,7 @@ public interface PubsubEvent { } } - public static class SchedulerActive implements PubsubEvent { + class SchedulerActive implements PubsubEvent { @Override public boolean equals(Object o) { return o != null && getClass().equals(o.getClass()); http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java b/src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java index d8daa68..b206830 100644 --- a/src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java +++ b/src/main/java/org/apache/aurora/scheduler/filter/SchedulingFilter.java @@ -32,7 +32,7 @@ public interface SchedulingFilter { * is only intended to be used for relative ranking of vetoes for determining which veto against * a scheduling assignment is 'weakest'. */ - public static class Veto { + class Veto { public static final int MAX_SCORE = 1000; private final String reason; http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/http/JerseyTemplateServlet.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/http/JerseyTemplateServlet.java b/src/main/java/org/apache/aurora/scheduler/http/JerseyTemplateServlet.java index c955640..96aec08 100644 --- a/src/main/java/org/apache/aurora/scheduler/http/JerseyTemplateServlet.java +++ b/src/main/java/org/apache/aurora/scheduler/http/JerseyTemplateServlet.java @@ -26,12 +26,14 @@ import org.antlr.stringtemplate.StringTemplate; /** * Base class for common functions needed in a jersey stringtemplate servlet. + * + * TODO(wfarner): This class should be composed rather than extended. Turn it into a helper class. */ -abstract class JerseyTemplateServlet { +class JerseyTemplateServlet { private final StringTemplateHelper templateHelper; - JerseyTemplateServlet(String templatePath) { + protected JerseyTemplateServlet(String templatePath) { templateHelper = new StringTemplateHelper(getClass(), templatePath, true); } http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java b/src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java index 9cb85bb..070c7da 100644 --- a/src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java +++ b/src/main/java/org/apache/aurora/scheduler/http/LeaderRedirect.java @@ -128,13 +128,13 @@ public class LeaderRedirect { .append(req.getScheme()) .append("://") .append(target.getHostText()) - .append(":") + .append(':') .append(target.getPort()) .append(req.getRequestURI()); String queryString = req.getQueryString(); if (queryString != null) { - redirect.append("?").append(queryString); + redirect.append('?').append(queryString); } return Optional.of(redirect.toString()); http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/http/Quotas.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/http/Quotas.java b/src/main/java/org/apache/aurora/scheduler/http/Quotas.java index b516470..438d127 100644 --- a/src/main/java/org/apache/aurora/scheduler/http/Quotas.java +++ b/src/main/java/org/apache/aurora/scheduler/http/Quotas.java @@ -89,7 +89,7 @@ public class Quotas { private final long ramMb; private final long diskMb; - private ResourceAggregateBean(double cpu, long ramMb, long diskMb) { + ResourceAggregateBean(double cpu, long ramMb, long diskMb) { this.cpu = cpu; this.ramMb = ramMb; this.diskMb = diskMb; http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/http/Slaves.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/http/Slaves.java b/src/main/java/org/apache/aurora/scheduler/http/Slaves.java index 56a8ce1..4c26db4 100644 --- a/src/main/java/org/apache/aurora/scheduler/http/Slaves.java +++ b/src/main/java/org/apache/aurora/scheduler/http/Slaves.java @@ -46,7 +46,7 @@ import static org.apache.aurora.scheduler.storage.Storage.Work; @Path("/slaves") public class Slaves extends JerseyTemplateServlet { private final String clusterName; - private Storage storage; + private final Storage storage; /** * Injected constructor. http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/http/StructDump.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/http/StructDump.java b/src/main/java/org/apache/aurora/scheduler/http/StructDump.java index 7bf2fba..a7fbcbf 100644 --- a/src/main/java/org/apache/aurora/scheduler/http/StructDump.java +++ b/src/main/java/org/apache/aurora/scheduler/http/StructDump.java @@ -123,10 +123,10 @@ public class StructDump extends JerseyTemplateServlet { public void execute(StringTemplate template) { template.setAttribute("id", id); Optional> struct = storage.weaklyConsistentRead(work); - if (!struct.isPresent()) { - template.setAttribute("exception", "Entity not found"); - } else { + if (struct.isPresent()) { template.setAttribute("structPretty", Util.prettyPrint(struct.get())); + } else { + template.setAttribute("exception", "Entity not found"); } } }); http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/local/IsolatedSchedulerModule.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/local/IsolatedSchedulerModule.java b/src/main/java/org/apache/aurora/scheduler/local/IsolatedSchedulerModule.java index a9d5f8f..e9d9bc4 100644 --- a/src/main/java/org/apache/aurora/scheduler/local/IsolatedSchedulerModule.java +++ b/src/main/java/org/apache/aurora/scheduler/local/IsolatedSchedulerModule.java @@ -14,16 +14,12 @@ package org.apache.aurora.scheduler.local; import java.util.List; -import java.util.concurrent.ExecutionException; -import java.util.concurrent.Future; import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.ScheduledThreadPoolExecutor; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; -import java.util.logging.Level; import java.util.logging.Logger; -import javax.annotation.Nullable; import javax.inject.Inject; import javax.inject.Provider; import javax.inject.Singleton; @@ -34,7 +30,6 @@ import com.google.common.collect.FluentIterable; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.eventbus.Subscribe; -import com.google.common.util.concurrent.ThreadFactoryBuilder; import com.google.inject.AbstractModule; import com.twitter.common.application.ShutdownRegistry; import com.twitter.common.base.Command; @@ -56,6 +51,7 @@ import org.apache.aurora.gen.TaskConfig; import org.apache.aurora.gen.comm.DeletedTasks; import org.apache.aurora.gen.comm.SchedulerMessage; import org.apache.aurora.scheduler.DriverFactory; +import org.apache.aurora.scheduler.base.AsyncUtil; import org.apache.aurora.scheduler.base.JobKeys; import org.apache.aurora.scheduler.configuration.ConfigurationManager; import org.apache.aurora.scheduler.configuration.Resources; @@ -136,24 +132,8 @@ public class IsolatedSchedulerModule extends AbstractModule { } private static ScheduledExecutorService createThreadPool(ShutdownRegistry shutdownRegistry) { - final ScheduledThreadPoolExecutor executor = new ScheduledThreadPoolExecutor( - 1, - new ThreadFactoryBuilder().setDaemon(true).setNameFormat("TaskScheduler-%d").build()) { - - @Override - protected void afterExecute(Runnable runnable, @Nullable Throwable throwable) { - if (throwable != null) { - LOG.log(Level.WARNING, "Error: " + throwable, throwable); - } else if (runnable instanceof Future) { - Future future = (Future) runnable; - try { - future.get(); - } catch (InterruptedException | ExecutionException e) { - e.printStackTrace(); - } - } - } - }; + final ScheduledThreadPoolExecutor executor = + AsyncUtil.loggingScheduledExecutor(1, "TaskScheduler-%d", LOG); Stats.exportSize("schedule_queue_size", executor.getQueue()); shutdownRegistry.addAction(new Command() { @Override http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/quota/QuotaCheckResult.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/quota/QuotaCheckResult.java b/src/main/java/org/apache/aurora/scheduler/quota/QuotaCheckResult.java index 396cecf..1a7b421 100644 --- a/src/main/java/org/apache/aurora/scheduler/quota/QuotaCheckResult.java +++ b/src/main/java/org/apache/aurora/scheduler/quota/QuotaCheckResult.java @@ -49,6 +49,10 @@ public class QuotaCheckResult { private Resource(String unit) { this.unit = unit; } + + String getUnit() { + return unit; + } } private final Optional details; @@ -107,7 +111,7 @@ public class QuotaCheckResult { .append(" quota exceeded by ") .append(String.format("%.2f", b - a)) .append(" ") - .append(resource.unit); + .append(resource.getUnit()); } return result; http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/state/LockManagerImpl.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/state/LockManagerImpl.java b/src/main/java/org/apache/aurora/scheduler/state/LockManagerImpl.java index 3390038..0568529 100644 --- a/src/main/java/org/apache/aurora/scheduler/state/LockManagerImpl.java +++ b/src/main/java/org/apache/aurora/scheduler/state/LockManagerImpl.java @@ -21,6 +21,7 @@ import com.google.common.base.Optional; import com.twitter.common.util.Clock; import org.apache.aurora.gen.Lock; +import org.apache.aurora.gen.LockKey._Fields; import org.apache.aurora.scheduler.base.JobKeys; import org.apache.aurora.scheduler.storage.LockStore; import org.apache.aurora.scheduler.storage.Storage; @@ -121,11 +122,8 @@ class LockManagerImpl implements LockManager { } private static String formatLockKey(ILockKey lockKey) { - switch (lockKey.getSetField()) { - case JOB: - return JobKeys.canonicalString(lockKey.getJob()); - default: - return "Unknown lock key type: " + lockKey.getSetField(); - } + return lockKey.getSetField() == _Fields.JOB + ? JobKeys.canonicalString(lockKey.getJob()) + : "Unknown lock key type: " + lockKey.getSetField(); } } http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java b/src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java index 46446a9..44ced61 100644 --- a/src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java +++ b/src/main/java/org/apache/aurora/scheduler/state/SchedulerCoreImpl.java @@ -49,7 +49,6 @@ import org.apache.aurora.scheduler.storage.entities.IJobConfiguration; 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.apache.commons.lang.StringUtils; import static com.google.common.base.Preconditions.checkNotNull; @@ -112,19 +111,6 @@ class SchedulerCoreImpl implements SchedulerCore { return hasActiveTasks || cronJobManager.hasJob(job.getKey()); } - private static boolean isCron(SanitizedConfiguration config) { - if (!config.getJobConfig().isSetCronSchedule()) { - return false; - } else if (StringUtils.isEmpty(config.getJobConfig().getCronSchedule())) { - // TODO(ksweeney): Remove this in 0.7.0 (AURORA-423). - LOG.warning("Got service config with empty string cron schedule. aurora-0.7.x " - + "will interpret this as cron job and cause an error."); - return false; - } else { - return true; - } - } - @Override public synchronized void createJob(final SanitizedConfiguration sanitizedConfiguration) throws ScheduleException { @@ -140,7 +126,7 @@ class SchedulerCoreImpl implements SchedulerCore { validateTaskLimits(job.getTaskConfig(), job.getInstanceCount()); // TODO(mchucarroll): deprecate cron as a part of create/kill job.(AURORA-454) - if (isCron(sanitizedConfiguration)) { + if (sanitizedConfiguration.isCron()) { try { LOG.warning("Deprecated behavior: scheduling job " + job.getKey() + " with cron via createJob (AURORA_454)"); http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/AttributeStore.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/storage/AttributeStore.java b/src/main/java/org/apache/aurora/scheduler/storage/AttributeStore.java index 616bdc4..6aa3e1b 100644 --- a/src/main/java/org/apache/aurora/scheduler/storage/AttributeStore.java +++ b/src/main/java/org/apache/aurora/scheduler/storage/AttributeStore.java @@ -46,7 +46,7 @@ public interface AttributeStore { * Attributes are considered mostly ephemeral and extremely low risk when inconsistency * is present. */ - public interface Mutable extends AttributeStore { + interface Mutable extends AttributeStore { /** * Deletes all attributes in the store. @@ -72,7 +72,7 @@ public interface AttributeStore { boolean setMaintenanceMode(String host, MaintenanceMode mode); } - public static final class Util { + final class Util { private Util() { } @@ -90,5 +90,4 @@ public interface AttributeStore { ? attributes.get().getAttributes() : ImmutableList.of(); } } - } http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/JobStore.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/storage/JobStore.java b/src/main/java/org/apache/aurora/scheduler/storage/JobStore.java index dcd0ab0..ad0d67a 100644 --- a/src/main/java/org/apache/aurora/scheduler/storage/JobStore.java +++ b/src/main/java/org/apache/aurora/scheduler/storage/JobStore.java @@ -50,7 +50,7 @@ public interface JobStore { */ Set fetchManagerIds(); - public interface Mutable extends JobStore { + interface Mutable extends JobStore { /** * Saves the job configuration for a job that has been accepted by the scheduler. Acts as an * update if the managerId already exists. http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/LockStore.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/storage/LockStore.java b/src/main/java/org/apache/aurora/scheduler/storage/LockStore.java index 4e2371c..596a378 100644 --- a/src/main/java/org/apache/aurora/scheduler/storage/LockStore.java +++ b/src/main/java/org/apache/aurora/scheduler/storage/LockStore.java @@ -39,7 +39,7 @@ public interface LockStore { */ Optional fetchLock(ILockKey lockKey); - public interface Mutable extends LockStore { + interface Mutable extends LockStore { /** * Saves a new lock or overwrites the existing one with same LockKey. * http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/QuotaStore.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/storage/QuotaStore.java b/src/main/java/org/apache/aurora/scheduler/storage/QuotaStore.java index 0270e58..688eb56 100644 --- a/src/main/java/org/apache/aurora/scheduler/storage/QuotaStore.java +++ b/src/main/java/org/apache/aurora/scheduler/storage/QuotaStore.java @@ -38,7 +38,7 @@ public interface QuotaStore { */ Map fetchQuotas(); - public interface Mutable extends QuotaStore { + interface Mutable extends QuotaStore { /** * Deletes all quotas. http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/ReadWriteLockManager.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/storage/ReadWriteLockManager.java b/src/main/java/org/apache/aurora/scheduler/storage/ReadWriteLockManager.java index c16f70d..fa49e22 100644 --- a/src/main/java/org/apache/aurora/scheduler/storage/ReadWriteLockManager.java +++ b/src/main/java/org/apache/aurora/scheduler/storage/ReadWriteLockManager.java @@ -41,6 +41,10 @@ public class ReadWriteLockManager { private LockType(LockMode mode) { this.mode = mode; } + + LockMode getMode() { + return mode; + } } private static class LockState { @@ -53,14 +57,14 @@ public class ReadWriteLockManager { initialLockMode = mode; stateChanged = true; } - if (initialLockMode == mode) { + if (initialLockMode.equals(mode)) { lockCount++; } return stateChanged; } private void lockReleased(LockMode mode) { - if (initialLockMode == mode) { + if (initialLockMode.equals(mode)) { lockCount--; if (lockCount == 0) { initialLockMode = LockMode.NONE; @@ -95,7 +99,7 @@ public class ReadWriteLockManager { lock.writeLock().lock(); } - return lockState.get().lockAcquired(type.mode); + return lockState.get().lockAcquired(type.getMode()); } /** @@ -112,7 +116,7 @@ public class ReadWriteLockManager { lock.writeLock().unlock(); } - lockState.get().lockReleased(type.mode); + lockState.get().lockReleased(type.getMode()); } /** http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/SchedulerStore.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/storage/SchedulerStore.java b/src/main/java/org/apache/aurora/scheduler/storage/SchedulerStore.java index f6a992d..057a2e6 100644 --- a/src/main/java/org/apache/aurora/scheduler/storage/SchedulerStore.java +++ b/src/main/java/org/apache/aurora/scheduler/storage/SchedulerStore.java @@ -27,7 +27,7 @@ public interface SchedulerStore { */ @Nullable String fetchFrameworkId(); - public interface Mutable extends SchedulerStore { + interface Mutable extends SchedulerStore { /** * Stores the given framework id overwriting any previously saved id. * http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/Storage.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/storage/Storage.java b/src/main/java/org/apache/aurora/scheduler/storage/Storage.java index 768a821..bbbd7dc 100644 --- a/src/main/java/org/apache/aurora/scheduler/storage/Storage.java +++ b/src/main/java/org/apache/aurora/scheduler/storage/Storage.java @@ -252,7 +252,7 @@ public interface Storage { /** * Utility functions for interacting with a Storage instance. */ - public final class Util { + final class Util { private Util() { // Utility class. http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java b/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java index d507666..8c20ab6 100644 --- a/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java +++ b/src/main/java/org/apache/aurora/scheduler/storage/StorageBackfill.java @@ -84,7 +84,9 @@ public final class StorageBackfill { // We want to keep exactly one task from this shard, so sort the IDs and keep the // highest (newest) in the hopes that it is legitimately running. String newestTask = Iterables.getLast(Sets.newTreeSet(activeTasksInShard)); - if (!Tasks.id(task).equals(newestTask)) { + if (Tasks.id(task).equals(newestTask)) { + LOG.info("Retaining task " + Tasks.id(task)); + } else { task.setStatus(ScheduleStatus.KILLED); task.addToTaskEvents(new TaskEvent(clock.nowMillis(), ScheduleStatus.KILLED) .setMessage("Killed duplicate shard.")); @@ -92,8 +94,6 @@ public final class StorageBackfill { // condition between the time the scheduler is actually available without hitting // IllegalStateException (see DriverImpl). // driver.killTask(Tasks.id(task)); - } else { - LOG.info("Retaining task " + Tasks.id(task)); } } } http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/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 40503b4..b76c937 100644 --- a/src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java +++ b/src/main/java/org/apache/aurora/scheduler/storage/TaskStore.java @@ -36,12 +36,12 @@ public interface TaskStore { */ ImmutableSet fetchTasks(Query.Builder query); - public interface Mutable extends TaskStore { + interface Mutable extends TaskStore { /** * A convenience interface to allow callers to more concisely implement a task mutation. */ - public interface TaskMutation extends Function { + interface TaskMutation extends Function { } /** http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/backup/BackupModule.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/storage/backup/BackupModule.java b/src/main/java/org/apache/aurora/scheduler/storage/backup/BackupModule.java index 17b79c3..4e2fb8b 100644 --- a/src/main/java/org/apache/aurora/scheduler/storage/backup/BackupModule.java +++ b/src/main/java/org/apache/aurora/scheduler/storage/backup/BackupModule.java @@ -119,13 +119,13 @@ public class BackupModule extends PrivateModule { } @Provides - private File provideBackupDir() { + File provideBackupDir() { if (!unvalidatedBackupDir.exists()) { - if (!unvalidatedBackupDir.mkdirs()) { + if (unvalidatedBackupDir.mkdirs()) { + LOG.info("Created backup dir " + unvalidatedBackupDir.getPath() + "."); + } else { throw new IllegalArgumentException( "Unable to create backup dir " + unvalidatedBackupDir.getPath() + "."); - } else { - LOG.info("Created backup dir " + unvalidatedBackupDir.getPath() + "."); } } @@ -138,7 +138,7 @@ public class BackupModule extends PrivateModule { } @Provides - private BackupConfig provideBackupConfig(File backupDir) { + BackupConfig provideBackupConfig(File backupDir) { return new BackupConfig(backupDir, MAX_SAVED_BACKUPS.get(), BACKUP_INTERVAL.get()); } } http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/backup/Recovery.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/storage/backup/Recovery.java b/src/main/java/org/apache/aurora/scheduler/storage/backup/Recovery.java index e3c09e0..f1511b8 100644 --- a/src/main/java/org/apache/aurora/scheduler/storage/backup/Recovery.java +++ b/src/main/java/org/apache/aurora/scheduler/storage/backup/Recovery.java @@ -93,7 +93,7 @@ public interface Recovery { * Thrown when a recovery operation could not be completed due to internal errors or improper * invocation order. */ - public static class RecoveryException extends Exception { + class RecoveryException extends Exception { RecoveryException(String message) { super(message); } http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/log/LogManager.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/storage/log/LogManager.java b/src/main/java/org/apache/aurora/scheduler/storage/log/LogManager.java index c31fe2d..496f13e 100644 --- a/src/main/java/org/apache/aurora/scheduler/storage/log/LogManager.java +++ b/src/main/java/org/apache/aurora/scheduler/storage/log/LogManager.java @@ -319,7 +319,7 @@ public final class LogManager { private final MessageDigest digest; private final int maxEntrySizeBytes; - private EntrySerializer(MessageDigest digest, Amount maxEntrySize) { + EntrySerializer(MessageDigest digest, Amount maxEntrySize) { this.digest = checkNotNull(digest); maxEntrySizeBytes = maxEntrySize.as(Data.BYTES); } @@ -374,7 +374,7 @@ public final class LogManager { new Transaction().setSchemaVersion(storageConstants.CURRENT_SCHEMA_VERSION); private final AtomicBoolean committed = new AtomicBoolean(false); - private StreamTransaction() { + StreamTransaction() { // supplied by factory method } http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/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 899c61a..7fcc072 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 @@ -343,7 +343,7 @@ public class LogStorage implements NonVolatileStorage, DistributedSnapshotStore } private static final class RecoveryFailedException extends SchedulerException { - private RecoveryFailedException(Throwable cause) { + RecoveryFailedException(Throwable cause) { super(cause); } } @@ -451,10 +451,10 @@ public class LogStorage implements NonVolatileStorage, DistributedSnapshotStore try { snapshot(); } catch (StorageException e) { - if (e.getCause() != null) { - LOG.log(Level.WARNING, e.getMessage(), e.getCause()); - } else { + if (e.getCause() == null) { LOG.log(Level.WARNING, "StorageException when attempting to snapshot.", e); + } else { + LOG.log(Level.WARNING, e.getMessage(), e.getCause()); } } } http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/log/testing/LogOpMatcher.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/storage/log/testing/LogOpMatcher.java b/src/main/java/org/apache/aurora/scheduler/storage/log/testing/LogOpMatcher.java index 87a442b..35e2e5a 100644 --- a/src/main/java/org/apache/aurora/scheduler/storage/log/testing/LogOpMatcher.java +++ b/src/main/java/org/apache/aurora/scheduler/storage/log/testing/LogOpMatcher.java @@ -69,7 +69,7 @@ public class LogOpMatcher implements IArgumentMatcher { public static final class StreamMatcher { private final Stream stream; - private StreamMatcher(Stream stream) { + StreamMatcher(Stream stream) { this.stream = Preconditions.checkNotNull(stream); } @@ -106,6 +106,6 @@ public class LogOpMatcher implements IArgumentMatcher { */ private static byte[] sameEntry(LogEntry entry) { EasyMock.reportMatcher(new LogOpMatcher(entry)); - return null; + return new byte[] {}; } } http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java b/src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java index 429691c..ff9e45c 100644 --- a/src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java +++ b/src/main/java/org/apache/aurora/scheduler/storage/mem/MemAttributeStore.java @@ -51,11 +51,11 @@ class MemAttributeStore implements Mutable { @Override public boolean setMaintenanceMode(String host, MaintenanceMode mode) { HostAttributes stored = hostAttributes.get(host); - if (stored != null) { + if (stored == null) { + return false; + } else { stored.setMode(mode); return true; - } else { - return false; } } http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobStore.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobStore.java b/src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobStore.java index 962861d..571636d 100644 --- a/src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobStore.java +++ b/src/main/java/org/apache/aurora/scheduler/storage/mem/MemJobStore.java @@ -88,10 +88,10 @@ class MemJobStore implements JobStore.Mutable { checkNotNull(jobKey); Optional manager = Optional.fromNullable(managers.getIfPresent(managerId)); - if (!manager.isPresent()) { - return Optional.absent(); - } else { + if (manager.isPresent()) { return Optional.fromNullable(manager.get().jobs.get(jobKey)); + } else { + return Optional.absent(); } } http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/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 6f3ebd3..29f64f7 100644 --- a/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java +++ b/src/main/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterface.java @@ -67,6 +67,7 @@ import org.apache.aurora.gen.JobSummaryResult; import org.apache.aurora.gen.ListBackupsResult; import org.apache.aurora.gen.Lock; import org.apache.aurora.gen.LockKey; +import org.apache.aurora.gen.LockKey._Fields; import org.apache.aurora.gen.LockValidation; import org.apache.aurora.gen.MaintenanceStatusResult; import org.apache.aurora.gen.PopulateJobResult; @@ -252,19 +253,6 @@ class SchedulerThriftInterface implements AuroraAdmin.Iface { return response; } - private static boolean isCron(SanitizedConfiguration config) { - if (!config.getJobConfig().isSetCronSchedule()) { - return false; - } else if (StringUtils.isEmpty(config.getJobConfig().getCronSchedule())) { - // TODO(ksweeney): Remove this in 0.7.0 (AURORA-424). - LOG.warning("Got service config with empty string cron schedule. aurora-0.7.x " - + "will interpret this as cron job and cause an error."); - return false; - } else { - return true; - } - } - @Override public Response scheduleCronJob( JobConfiguration mutableJob, @@ -290,7 +278,7 @@ class SchedulerThriftInterface implements AuroraAdmin.Iface { ILockKey.build(LockKey.job(jobKey.newBuilder())), Optional.fromNullable(mutableLock).transform(ILock.FROM_BUILDER)); - if (!isCron(sanitized)) { + if (!sanitized.isCron()) { LOG.info("Invalid attempt to schedule non-cron job " + sanitized.getJobConfig().getKey() + " with cron."); @@ -870,16 +858,97 @@ class SchedulerThriftInterface implements AuroraAdmin.Iface { } Response resp = new Response(); - if (!errors.isEmpty()) { - resp.setResponseCode(ResponseCode.WARNING).setMessage(Joiner.on(", ").join(errors)); - } else { + if (errors.isEmpty()) { resp.setResponseCode(OK).setMessage("All rewrites completed successfully."); + } else { + resp.setResponseCode(ResponseCode.WARNING).setMessage(Joiner.on(", ").join(errors)); } return resp; } }); } + private Optional rewriteJob(JobConfigRewrite jobRewrite, JobStore.Mutable jobStore) { + IJobConfiguration existingJob = IJobConfiguration.build(jobRewrite.getOldJob()); + IJobConfiguration rewrittenJob; + Optional error = Optional.absent(); + try { + rewrittenJob = ConfigurationManager.validateAndPopulate( + IJobConfiguration.build(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 Throwables.propagate(e); + } + + if (existingJob.getKey().equals(rewrittenJob.getKey())) { + if (existingJob.getOwner().equals(rewrittenJob.getOwner())) { + Multimap matches = jobsByKey(jobStore, existingJob.getKey()); + switch (matches.size()) { + case 0: + error = Optional.of( + "No jobs found for key " + JobKeys.canonicalString(existingJob.getKey())); + break; + + case 1: + Map.Entry match = + Iterables.getOnlyElement(matches.entries()); + IJobConfiguration storedJob = match.getValue(); + if (storedJob.equals(existingJob)) { + jobStore.saveAcceptedJob(match.getKey(), rewrittenJob); + } else { + error = Optional.of( + "CAS compare failed for " + JobKeys.canonicalString(storedJob.getKey())); + } + break; + + default: + error = Optional.of("Multiple jobs found for key " + + JobKeys.canonicalString(existingJob.getKey())); + } + } else { + error = Optional.of("Disallowing rewrite attempting to change job owner."); + } + } else { + error = Optional.of("Disallowing rewrite attempting to change job key."); + } + + return error; + } + + private Optional rewriteInstance( + InstanceConfigRewrite instanceRewrite, + MutableStoreProvider storeProvider) { + + InstanceKey instanceKey = instanceRewrite.getInstanceKey(); + Optional error = Optional.absent(); + Iterable tasks = storeProvider.getTaskStore().fetchTasks( + Query.instanceScoped(IJobKey.build(instanceKey.getJobKey()), + instanceKey.getInstanceId()) + .active()); + Optional task = + Optional.fromNullable(Iterables.getOnlyElement(tasks, null)) + .transform(Tasks.SCHEDULED_TO_ASSIGNED); + + if (task.isPresent()) { + if (task.get().getTask().newBuilder().equals(instanceRewrite.getOldTask())) { + ITaskConfig newConfiguration = ITaskConfig.build( + ConfigurationManager.applyDefaultsIfUnset(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 rewriteConfig( ConfigRewrite command, MutableStoreProvider storeProvider) { @@ -887,73 +956,11 @@ class SchedulerThriftInterface implements AuroraAdmin.Iface { Optional error = Optional.absent(); switch (command.getSetField()) { case JOB_REWRITE: - JobConfigRewrite jobRewrite = command.getJobRewrite(); - IJobConfiguration existingJob = IJobConfiguration.build(jobRewrite.getOldJob()); - IJobConfiguration rewrittenJob; - try { - rewrittenJob = ConfigurationManager.validateAndPopulate( - IJobConfiguration.build(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 Throwables.propagate(e); - } - if (!existingJob.getKey().equals(rewrittenJob.getKey())) { - error = Optional.of("Disallowing rewrite attempting to change job key."); - } else if (!existingJob.getOwner().equals(rewrittenJob.getOwner())) { - error = Optional.of("Disallowing rewrite attempting to change job owner."); - } else { - JobStore.Mutable jobStore = storeProvider.getJobStore(); - Multimap matches = - jobsByKey(jobStore, existingJob.getKey()); - switch (matches.size()) { - case 0: - error = Optional.of( - "No jobs found for key " + JobKeys.canonicalString(existingJob.getKey())); - break; - - case 1: - Map.Entry match = - Iterables.getOnlyElement(matches.entries()); - IJobConfiguration storedJob = match.getValue(); - if (!storedJob.equals(existingJob)) { - error = Optional.of( - "CAS compare failed for " + JobKeys.canonicalString(storedJob.getKey())); - } else { - jobStore.saveAcceptedJob(match.getKey(), rewrittenJob); - } - break; - - default: - error = Optional.of("Multiple jobs found for key " - + JobKeys.canonicalString(existingJob.getKey())); - } - } + error = rewriteJob(command.getJobRewrite(), storeProvider.getJobStore()); break; case INSTANCE_REWRITE: - InstanceConfigRewrite instanceRewrite = command.getInstanceRewrite(); - InstanceKey instanceKey = instanceRewrite.getInstanceKey(); - Iterable tasks = storeProvider.getTaskStore().fetchTasks( - Query.instanceScoped(IJobKey.build(instanceKey.getJobKey()), - instanceKey.getInstanceId()) - .active()); - Optional task = - Optional.fromNullable(Iterables.getOnlyElement(tasks, null)) - .transform(Tasks.SCHEDULED_TO_ASSIGNED); - if (!task.isPresent()) { - error = Optional.of("No active task found for " + instanceKey); - } else if (!task.get().getTask().newBuilder().equals(instanceRewrite.getOldTask())) { - error = Optional.of("CAS compare failed for " + instanceKey); - } else { - ITaskConfig newConfiguration = ITaskConfig.build( - ConfigurationManager.applyDefaultsIfUnset(instanceRewrite.getRewrittenTask())); - boolean changed = storeProvider.getUnsafeTaskStore().unsafeModifyInPlace( - task.get().getTaskId(), newConfiguration); - if (!changed) { - error = Optional.of("Did not change " + task.get().getTaskId()); - } - } + error = rewriteInstance(command.getInstanceRewrite(), storeProvider); break; default: @@ -1006,12 +1013,11 @@ class SchedulerThriftInterface implements AuroraAdmin.Iface { } private String getRoleFromLockKey(ILockKey lockKey) { - switch (lockKey.getSetField()) { - case JOB: - JobKeys.assertValid(lockKey.getJob()); - return lockKey.getJob().getRole(); - default: - throw new IllegalArgumentException("Unhandled LockKey: " + lockKey.getSetField()); + if (lockKey.getSetField() == _Fields.JOB) { + JobKeys.assertValid(lockKey.getJob()); + return lockKey.getJob().getRole(); + } else { + throw new IllegalArgumentException("Unhandled LockKey: " + lockKey.getSetField()); } } http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/a8fa267f/src/main/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptor.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptor.java b/src/main/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptor.java index 8eb52dd..0c1477c 100644 --- a/src/main/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptor.java +++ b/src/main/java/org/apache/aurora/scheduler/thrift/aop/FeatureToggleInterceptor.java @@ -33,13 +33,13 @@ public class FeatureToggleInterceptor implements MethodInterceptor { @Override public Object invoke(MethodInvocation invocation) throws Throwable { Method method = invocation.getMethod(); - if (!allowMethod.apply(method)) { + if (allowMethod.apply(method)) { + return invocation.proceed(); + } else { return Interceptors.properlyTypedResponse( method, ResponseCode.ERROR, "The " + method.getName() + " feature is currently disabled on this scheduler."); - } else { - return invocation.proceed(); } } }