You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@aurora.apache.org by wf...@apache.org on 2014/05/13 20:14:30 UTC

git commit: Fix regression causing scheduling rate limiter to not be honored.

Repository: incubator-aurora
Updated Branches:
  refs/heads/master 00e0c57a5 -> 8db874e37


Fix regression causing scheduling rate limiter to not be honored.

Also added findbugs to the build.

Bugs closed: AURORA-416

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


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

Branch: refs/heads/master
Commit: 8db874e373ea375eb20d8fb948c537d0a0888ba5
Parents: 00e0c57
Author: Bill Farner <wf...@apache.org>
Authored: Tue May 13 11:14:02 2014 -0700
Committer: Bill Farner <wf...@apache.org>
Committed: Tue May 13 11:14:02 2014 -0700

----------------------------------------------------------------------
 build.gradle                                    |  9 +++
 config/findbugs/excludeFilter.xml               | 72 ++++++++++++++++++++
 .../aurora/scheduler/async/TaskGroups.java      |  4 +-
 .../aurora/scheduler/app/SchedulerIT.java       |  2 +-
 .../aurora/scheduler/async/TaskGroupsTest.java  |  6 +-
 .../aurora/scheduler/cron/noop/NoopCronIT.java  |  3 -
 .../scheduler/storage/mem/MemStorageTest.java   |  2 +-
 .../thrift/SchedulerThriftInterfaceTest.java    |  2 +-
 .../aurora/scheduler/thrift/ThriftIT.java       |  2 +-
 9 files changed, 92 insertions(+), 10 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/8db874e3/build.gradle
----------------------------------------------------------------------
diff --git a/build.gradle b/build.gradle
index 6c758f6..f2c729e 100644
--- a/build.gradle
+++ b/build.gradle
@@ -17,6 +17,7 @@
 apply plugin: 'about'
 apply plugin: 'application'
 apply plugin: 'checkstyle'
+apply plugin: 'findbugs'
 apply plugin: 'idea'
 apply plugin: 'jacoco'
 apply plugin: 'java'
@@ -235,6 +236,14 @@ checkstyle {
   sourceSets = [ sourceSets.main , sourceSets.test]
 }
 
+tasks.withType(FindBugs) {
+  reports {
+    xml.enabled = false
+    html.enabled = true
+  }
+  excludeFilter = rootProject.file('config/findbugs/excludeFilter.xml')
+}
+
 /**
  * Check if Apache Thrift is all ready installed and is the same version as we
  * depend on, otherwise compile the version in build-support. project.thrift will

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/8db874e3/config/findbugs/excludeFilter.xml
----------------------------------------------------------------------
diff --git a/config/findbugs/excludeFilter.xml b/config/findbugs/excludeFilter.xml
new file mode 100644
index 0000000..5b54100
--- /dev/null
+++ b/config/findbugs/excludeFilter.xml
@@ -0,0 +1,72 @@
+<FindBugsFilter>
+  <!-- Warnings triggered in thrift-generated code. -->
+  <Match>
+    <Or>
+      <Package name="org.apache.aurora.gen" />
+      <Package name="org.apache.aurora.gen.comm" />
+      <Package name="org.apache.aurora.gen.storage" />
+      <Package name="org.apache.aurora.gen.test" />
+      <!-- Un-namespaced structs used by the executor. -->
+      <Class name="ProcessState" />
+      <Class name="ProcessStatus" />
+      <Class name="RunnerHeader" />
+      <Class name="RunnerState" />
+      <Class name="TaskStatus" />
+    </Or>
+    <Or>
+      <Bug pattern="BC_IMPOSSIBLE_CAST" />
+      <Bug pattern="CN_IDIOM" />
+      <Bug pattern="DLS_DEAD_LOCAL_STORE" />
+      <Bug pattern="NM_CLASS_NAMING_CONVENTION" />
+    </Or>
+  </Match>
+
+  <!-- Method is intentionally only callable by EventBus. -->
+  <Match>
+    <Class name="org.apache.aurora.scheduler.events.PubsubEventModule$1" />
+    <Bug pattern="UMAC_UNCALLABLE_METHOD_OF_ANONYMOUS_CLASS" />
+  </Match>
+
+  <!-- Technical debt. -->
+  <Match>
+    <Class name="org.apache.aurora.scheduler.log.mesos.MesosLog$LogStream" />
+    <Bug pattern="IS2_INCONSISTENT_SYNC" />
+  </Match>
+  <Match>
+    <Class name="org.apache.aurora.scheduler.storage.mem.MemAttributeStore" />
+    <Bug pattern="RV_RETURN_VALUE_OF_PUTIFABSENT_IGNORED" />
+  </Match>
+  <Match>
+    <Class name="org.apache.aurora.scheduler.http.Utilization" />
+    <Bug pattern="RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE" />
+  </Match>
+  <Match>
+    <Class name="~org\.apache\.aurora.*$" />
+    <Bug pattern="NP_PARAMETER_MUST_BE_NONNULL_BUT_MARKED_AS_NULLABLE" />
+  </Match>
+  <Match>
+    <Or>
+      <Class name="org.apache.aurora.scheduler.log.testing.FileLogTest" />
+      <Class name="org.apache.aurora.scheduler.storage.backup.StorageBackup$StorageBackupImpl" />
+      <Class name="org.apache.aurora.scheduler.storage.backup.StorageBackupTest" />
+    </Or>
+    <Bug pattern="RV_RETURN_VALUE_IGNORED_BAD_PRACTICE" />
+  </Match>
+  <Match>
+    <Or>
+      <Class name="org.apache.aurora.scheduler.app.SchedulerIT$IntPosition" />
+      <Class name="org.apache.aurora.scheduler.log.mesos.MesosLog$LogStream$LogPosition" />
+      <Class name="org.apache.aurora.scheduler.log.testing.FileLog$FileStream$CounterPosition" />
+    </Or>
+    <Bug pattern="EQ_COMPARETO_USE_OBJECT_EQUALS" />
+  </Match>
+  <Match>
+    <Or>
+      <Class name="org.apache.aurora.scheduler.cron.testing.AbstractCronIT" />
+      <Class name="org.apache.aurora.scheduler.log.mesos.MesosLogTest" />
+      <Class name="org.apache.aurora.scheduler.thrift.ThriftIT" />
+      <Class name="org.apache.aurora.scheduler.thrift.ThriftIT$1" />
+    </Or>
+    <Bug pattern="DM_DEFAULT_ENCODING" />
+  </Match>
+</FindBugsFilter>

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/8db874e3/src/main/java/org/apache/aurora/scheduler/async/TaskGroups.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/async/TaskGroups.java b/src/main/java/org/apache/aurora/scheduler/async/TaskGroups.java
index ada5eaf..38e19aa 100644
--- a/src/main/java/org/apache/aurora/scheduler/async/TaskGroups.java
+++ b/src/main/java/org/apache/aurora/scheduler/async/TaskGroups.java
@@ -110,11 +110,11 @@ public class TaskGroups implements EventSubscriber {
 
     this.executor = checkNotNull(executor);
     checkNotNull(rateLimiter);
-    this.taskScheduler = checkNotNull(taskScheduler);
+    checkNotNull(taskScheduler);
     this.backoff = checkNotNull(backoff);
     this.rescheduleCalculator = checkNotNull(rescheduleCalculator);
 
-    final TaskScheduler ratelLimitedScheduler = new TaskScheduler() {
+    this.taskScheduler = new TaskScheduler() {
       @Override
       public boolean schedule(String taskId) {
         rateLimiter.acquire();

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/8db874e3/src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java b/src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java
index ddbb025..df5a5da 100644
--- a/src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java
+++ b/src/test/java/org/apache/aurora/scheduler/app/SchedulerIT.java
@@ -263,7 +263,7 @@ public class SchedulerIT extends BaseZooKeeperTest {
   }
 
   private AtomicInteger curPosition = new AtomicInteger();
-  private class IntPosition implements Position {
+  private static class IntPosition implements Position {
     private final int pos;
 
     IntPosition(int pos) {

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/8db874e3/src/test/java/org/apache/aurora/scheduler/async/TaskGroupsTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/async/TaskGroupsTest.java b/src/test/java/org/apache/aurora/scheduler/async/TaskGroupsTest.java
index e23ab5c..9003bd3 100644
--- a/src/test/java/org/apache/aurora/scheduler/async/TaskGroupsTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/async/TaskGroupsTest.java
@@ -49,6 +49,7 @@ public class TaskGroupsTest extends EasyMockTest {
   private ScheduledExecutorService executor;
   private BackoffStrategy backoffStrategy;
   private TaskScheduler taskScheduler;
+  private RateLimiter rateLimiter;
   private RescheduleCalculator rescheduleCalculator;
 
   private TaskGroups taskGroups;
@@ -58,11 +59,12 @@ public class TaskGroupsTest extends EasyMockTest {
     executor = createMock(ScheduledExecutorService.class);
     backoffStrategy = createMock(BackoffStrategy.class);
     taskScheduler = createMock(TaskScheduler.class);
+    rateLimiter = createMock(RateLimiter.class);
     rescheduleCalculator = createMock(RescheduleCalculator.class);
     taskGroups = new TaskGroups(
         executor,
         backoffStrategy,
-        RateLimiter.create(10000),
+        rateLimiter,
         taskScheduler,
         rescheduleCalculator);
   }
@@ -78,6 +80,7 @@ public class TaskGroupsTest extends EasyMockTest {
         return null;
       }
     });
+    expect(rateLimiter.acquire()).andReturn(0D);
     expect(taskScheduler.schedule("a")).andReturn(true);
 
     control.replay();
@@ -99,6 +102,7 @@ public class TaskGroupsTest extends EasyMockTest {
     expect(backoffStrategy.calculateBackoffMs(0)).andReturn(0L).atLeastOnce();
     Capture<Runnable> evaluate = expectEvaluate();
 
+    expect(rateLimiter.acquire()).andReturn(0D);
     expect(taskScheduler.schedule(Tasks.id(task))).andAnswer(new IAnswer<Boolean>() {
       @Override
       public Boolean answer() {

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/8db874e3/src/test/java/org/apache/aurora/scheduler/cron/noop/NoopCronIT.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/cron/noop/NoopCronIT.java b/src/test/java/org/apache/aurora/scheduler/cron/noop/NoopCronIT.java
index a9b85d0..0d2f66f 100644
--- a/src/test/java/org/apache/aurora/scheduler/cron/noop/NoopCronIT.java
+++ b/src/test/java/org/apache/aurora/scheduler/cron/noop/NoopCronIT.java
@@ -18,7 +18,6 @@ package org.apache.aurora.scheduler.cron.noop;
 import com.google.inject.Guice;
 import com.google.inject.Injector;
 
-import org.apache.aurora.scheduler.cron.CronPredictor;
 import org.apache.aurora.scheduler.cron.CronScheduler;
 import org.junit.Before;
 import org.junit.Test;
@@ -30,13 +29,11 @@ public class NoopCronIT {
   private static final String SCHEDULE = "* * * * *";
 
   private CronScheduler cronScheduler;
-  private CronPredictor cronPredictor;
 
   @Before
   public void setUp() {
     Injector injector = Guice.createInjector(new NoopCronModule());
     cronScheduler = injector.getInstance(CronScheduler.class);
-    cronPredictor = injector.getInstance(CronPredictor.class);
   }
 
   @Test

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/8db874e3/src/test/java/org/apache/aurora/scheduler/storage/mem/MemStorageTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/storage/mem/MemStorageTest.java b/src/test/java/org/apache/aurora/scheduler/storage/mem/MemStorageTest.java
index de5cdcf..ae47018 100644
--- a/src/test/java/org/apache/aurora/scheduler/storage/mem/MemStorageTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/storage/mem/MemStorageTest.java
@@ -119,7 +119,7 @@ public class MemStorageTest extends TearDownTestCase {
                 .setEnvironment("env-" + taskId))));
   }
 
-  private class CustomException extends RuntimeException {
+  private static class CustomException extends RuntimeException {
   }
 
   private <T, E extends RuntimeException> void expectWriteFail(MutateWork<T, E> work) {

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/8db874e3/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 47d2fd6..405da0a 100644
--- a/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/thrift/SchedulerThriftInterfaceTest.java
@@ -957,7 +957,7 @@ public class SchedulerThriftInterfaceTest extends EasyMockTest {
 
   @Test
   public void testGetJobSummary() throws Exception {
-    int nextCronRunMs = 100;
+    long nextCronRunMs = 100;
     TaskConfig ownedCronJobTask = nonProductionTask()
         .setJobName(JobKeys.TO_JOB_NAME.apply(JOB_KEY))
         .setOwner(ROLE_IDENTITY)

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/8db874e3/src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java b/src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java
index e212174..488a545 100644
--- a/src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java
+++ b/src/test/java/org/apache/aurora/scheduler/thrift/ThriftIT.java
@@ -95,7 +95,7 @@ public class ThriftIT extends EasyMockTest {
     }
   };
 
-  private class CapabilityValidatorFake implements CapabilityValidator {
+  private static class CapabilityValidatorFake implements CapabilityValidator {
     private final SessionValidator validator;
 
     CapabilityValidatorFake(SessionValidator validator) {