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/11/11 04:33:51 UTC

incubator-aurora git commit: Add test coverage for WriteAheadStorage.

Repository: incubator-aurora
Updated Branches:
  refs/heads/master 5b3b75173 -> c3cce813e


Add test coverage for WriteAheadStorage.

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


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

Branch: refs/heads/master
Commit: c3cce813e9234994936c7f94e834d0b11316b9ba
Parents: 5b3b751
Author: Bill Farner <wf...@apache.org>
Authored: Mon Nov 10 19:32:10 2014 -0800
Committer: Bill Farner <wf...@apache.org>
Committed: Mon Nov 10 19:32:10 2014 -0800

----------------------------------------------------------------------
 config/pmd/logging-java.xml                     |   2 +
 .../scheduler/storage/log/LogStorage.java       |   3 +-
 .../storage/log/WriteAheadStorage.java          |  24 +--
 .../storage/log/WriteAheadStorageTest.java      | 203 +++++++++++++++++++
 4 files changed, 213 insertions(+), 19 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/c3cce813/config/pmd/logging-java.xml
----------------------------------------------------------------------
diff --git a/config/pmd/logging-java.xml b/config/pmd/logging-java.xml
index e4ec6ad..66fecbf 100644
--- a/config/pmd/logging-java.xml
+++ b/config/pmd/logging-java.xml
@@ -48,6 +48,7 @@ public class Foo {
      </example>
      </rule>
 
+<!-- It's valid to inject the logger, and in some cases preferable.
      <rule name="LoggerIsNotStaticFinal"
    		language="java"
      		since="2.0"
@@ -82,6 +83,7 @@ public class Foo{
 ]]>
      </example>
    </rule>
+-->
 
     <rule name="SystemPrintln"
    		language="java"

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/c3cce813/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 0195557..32890f5 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
@@ -304,7 +304,8 @@ public class LogStorage implements NonVolatileStorage, DistributedSnapshotStore
         lockStore,
         quotaStore,
         attributeStore,
-        jobUpdateStore);
+        jobUpdateStore,
+        Logger.getLogger(WriteAheadStorage.class.getName()));
 
     this.logEntryReplayActions = buildLogEntryReplayActions();
     this.transactionReplayActions = buildTransactionReplayActions();

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/c3cce813/src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java b/src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java
index 094d1c6..5790b84 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorage.java
@@ -24,7 +24,6 @@ import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Maps;
 import com.twitter.common.base.MorePreconditions;
-import com.twitter.common.inject.TimedInterceptor.Timed;
 
 import org.apache.aurora.gen.MaintenanceMode;
 import org.apache.aurora.gen.storage.Op;
@@ -85,8 +84,6 @@ class WriteAheadStorage extends ForwardingStore implements
     AttributeStore.Mutable,
     JobUpdateStore.Mutable {
 
-  private static final Logger LOG = Logger.getLogger(WriteAheadStorage.class.getName());
-
   private final TransactionManager transactionManager;
   private final SchedulerStore.Mutable schedulerStore;
   private final JobStore.Mutable jobStore;
@@ -95,6 +92,7 @@ class WriteAheadStorage extends ForwardingStore implements
   private final QuotaStore.Mutable quotaStore;
   private final AttributeStore.Mutable attributeStore;
   private final JobUpdateStore.Mutable jobUpdateStore;
+  private final Logger log;
 
   /**
    * Creates a new write-ahead storage that delegates to the providing default stores.
@@ -116,7 +114,8 @@ class WriteAheadStorage extends ForwardingStore implements
       LockStore.Mutable lockStore,
       QuotaStore.Mutable quotaStore,
       AttributeStore.Mutable attributeStore,
-      JobUpdateStore.Mutable jobUpdateStore) {
+      JobUpdateStore.Mutable jobUpdateStore,
+      Logger log) {
 
     super(
         schedulerStore,
@@ -135,6 +134,7 @@ class WriteAheadStorage extends ForwardingStore implements
     this.quotaStore = requireNonNull(quotaStore);
     this.attributeStore = requireNonNull(attributeStore);
     this.jobUpdateStore = requireNonNull(jobUpdateStore);
+    this.log = requireNonNull(log);
   }
 
   private void write(Op op) {
@@ -144,7 +144,6 @@ class WriteAheadStorage extends ForwardingStore implements
     transactionManager.log(op);
   }
 
-  @Timed("scheduler_log_save_framework_id")
   @Override
   public void saveFrameworkId(final String frameworkId) {
     requireNonNull(frameworkId);
@@ -153,7 +152,6 @@ class WriteAheadStorage extends ForwardingStore implements
     schedulerStore.saveFrameworkId(frameworkId);
   }
 
-  @Timed("scheduler_log_unsafe_modify_in_place")
   @Override
   public boolean unsafeModifyInPlace(final String taskId, final ITaskConfig taskConfiguration) {
     requireNonNull(taskId);
@@ -166,7 +164,6 @@ class WriteAheadStorage extends ForwardingStore implements
     return mutated;
   }
 
-  @Timed("scheduler_log_tasks_remove")
   @Override
   public void deleteTasks(final Set<String> taskIds) {
     requireNonNull(taskIds);
@@ -175,7 +172,6 @@ class WriteAheadStorage extends ForwardingStore implements
     taskStore.deleteTasks(taskIds);
   }
 
-  @Timed("scheduler_log_tasks_save")
   @Override
   public void saveTasks(final Set<IScheduledTask> newTasks) {
     requireNonNull(newTasks);
@@ -184,7 +180,6 @@ class WriteAheadStorage extends ForwardingStore implements
     taskStore.saveTasks(newTasks);
   }
 
-  @Timed("scheduler_log_tasks_mutate")
   @Override
   public ImmutableSet<IScheduledTask> mutateTasks(
       final Query.Builder query,
@@ -196,8 +191,8 @@ class WriteAheadStorage extends ForwardingStore implements
     ImmutableSet<IScheduledTask> mutated = taskStore.mutateTasks(query, mutator);
 
     Map<String, IScheduledTask> tasksById = Tasks.mapById(mutated);
-    if (LOG.isLoggable(Level.FINE)) {
-      LOG.fine("Storing updated tasks to log: "
+    if (log.isLoggable(Level.FINE)) {
+      log.fine("Storing updated tasks to log: "
           + Maps.transformValues(tasksById, Tasks.GET_STATUS));
     }
 
@@ -206,7 +201,6 @@ class WriteAheadStorage extends ForwardingStore implements
     return mutated;
   }
 
-  @Timed("scheduler_log_quota_save")
   @Override
   public void saveQuota(final String role, final IResourceAggregate quota) {
     requireNonNull(role);
@@ -216,7 +210,6 @@ class WriteAheadStorage extends ForwardingStore implements
     quotaStore.saveQuota(role, quota);
   }
 
-  @Timed("scheduler_save_host_attribute")
   @Override
   public void saveHostAttributes(final IHostAttributes attrs) {
     requireNonNull(attrs);
@@ -234,7 +227,6 @@ class WriteAheadStorage extends ForwardingStore implements
     }
   }
 
-  @Timed("scheduler_log_job_remove")
   @Override
   public void removeJob(final IJobKey jobKey) {
     requireNonNull(jobKey);
@@ -243,7 +235,6 @@ class WriteAheadStorage extends ForwardingStore implements
     jobStore.removeJob(jobKey);
   }
 
-  @Timed("scheduler_log_job_save")
   @Override
   public void saveAcceptedJob(final String managerId, final IJobConfiguration jobConfig) {
     requireNonNull(managerId);
@@ -253,7 +244,6 @@ class WriteAheadStorage extends ForwardingStore implements
     jobStore.saveAcceptedJob(managerId, jobConfig);
   }
 
-  @Timed("scheduler_log_quota_remove")
   @Override
   public void removeQuota(final String role) {
     requireNonNull(role);
@@ -262,7 +252,6 @@ class WriteAheadStorage extends ForwardingStore implements
     quotaStore.removeQuota(role);
   }
 
-  @Timed("scheduler_lock_save")
   @Override
   public void saveLock(final ILock lock) {
     requireNonNull(lock);
@@ -271,7 +260,6 @@ class WriteAheadStorage extends ForwardingStore implements
     lockStore.saveLock(lock);
   }
 
-  @Timed("scheduler_lock_remove")
   @Override
   public void removeLock(final ILockKey lockKey) {
     requireNonNull(lockKey);

http://git-wip-us.apache.org/repos/asf/incubator-aurora/blob/c3cce813/src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java b/src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java
new file mode 100644
index 0000000..792c9fe
--- /dev/null
+++ b/src/test/java/org/apache/aurora/scheduler/storage/log/WriteAheadStorageTest.java
@@ -0,0 +1,203 @@
+/**
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.aurora.scheduler.storage.log;
+
+import java.util.Set;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
+import com.google.common.base.Function;
+import com.google.common.base.Optional;
+import com.google.common.collect.ImmutableSet;
+import com.twitter.common.testing.easymock.EasyMockTest;
+
+import org.apache.aurora.gen.AssignedTask;
+import org.apache.aurora.gen.Attribute;
+import org.apache.aurora.gen.HostAttributes;
+import org.apache.aurora.gen.MaintenanceMode;
+import org.apache.aurora.gen.ScheduledTask;
+import org.apache.aurora.gen.storage.Op;
+import org.apache.aurora.gen.storage.PruneJobUpdateHistory;
+import org.apache.aurora.gen.storage.SaveHostAttributes;
+import org.apache.aurora.gen.storage.SaveTasks;
+import org.apache.aurora.scheduler.base.Query;
+import org.apache.aurora.scheduler.storage.AttributeStore;
+import org.apache.aurora.scheduler.storage.JobStore;
+import org.apache.aurora.scheduler.storage.JobUpdateStore;
+import org.apache.aurora.scheduler.storage.LockStore;
+import org.apache.aurora.scheduler.storage.QuotaStore;
+import org.apache.aurora.scheduler.storage.SchedulerStore;
+import org.apache.aurora.scheduler.storage.TaskStore;
+import org.apache.aurora.scheduler.storage.entities.IHostAttributes;
+import org.apache.aurora.scheduler.storage.entities.IScheduledTask;
+import org.easymock.EasyMock;
+import org.junit.Before;
+import org.junit.Test;
+
+import static org.easymock.EasyMock.expect;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+public class WriteAheadStorageTest extends EasyMockTest {
+
+  private LogStorage.TransactionManager transactionManager;
+  private SchedulerStore.Mutable schedulerStore;
+  private JobStore.Mutable jobStore;
+  private TaskStore.Mutable taskStore;
+  private LockStore.Mutable lockStore;
+  private QuotaStore.Mutable quotaStore;
+  private AttributeStore.Mutable attributeStore;
+  private JobUpdateStore.Mutable jobUpdateStore;
+  private Logger log;
+  private WriteAheadStorage storage;
+
+  @Before
+  public void setUp() {
+    transactionManager = createMock(LogStorage.TransactionManager.class);
+    schedulerStore = createMock(SchedulerStore.Mutable.class);
+    jobStore = createMock(JobStore.Mutable.class);
+    taskStore = createMock(TaskStore.Mutable.class);
+    lockStore = createMock(LockStore.Mutable.class);
+    quotaStore = createMock(QuotaStore.Mutable.class);
+    attributeStore = createMock(AttributeStore.Mutable.class);
+    jobUpdateStore = createMock(JobUpdateStore.Mutable.class);
+    log = createMock(Logger.class);
+
+    storage = new WriteAheadStorage(
+        transactionManager,
+        schedulerStore,
+        jobStore,
+        taskStore,
+        lockStore,
+        quotaStore,
+        attributeStore,
+        jobUpdateStore,
+        log);
+  }
+
+  private void expectOp(Op op) {
+    expect(transactionManager.hasActiveTransaction()).andReturn(true);
+    transactionManager.log(op);
+  }
+
+  @Test
+  public void testSetMaintenanceModeSaved() {
+    String host = "a";
+    MaintenanceMode mode = MaintenanceMode.DRAINED;
+    expect(attributeStore.setMaintenanceMode(host, mode)).andReturn(true);
+    IHostAttributes attribute = IHostAttributes.build(new HostAttributes()
+        .setHost(host)
+        .setMode(mode)
+        .setAttributes(ImmutableSet.of(
+            new Attribute().setName("os").setValues(ImmutableSet.of("linux")))));
+    expect(attributeStore.getHostAttributes(host)).andReturn(Optional.of(attribute));
+    expectOp(Op.saveHostAttributes(new SaveHostAttributes(attribute.newBuilder())));
+
+    control.replay();
+
+    assertTrue(storage.setMaintenanceMode(host, mode));
+  }
+
+  @Test
+  public void testSetMaintenanceModeNotSaved() {
+    String host = "a";
+    MaintenanceMode mode = MaintenanceMode.DRAINED;
+    expect(attributeStore.setMaintenanceMode(host, mode)).andReturn(false);
+
+    control.replay();
+
+    assertFalse(storage.setMaintenanceMode(host, mode));
+  }
+
+  @Test
+  public void testPruneHistory() {
+    Set<String> pruned = ImmutableSet.of("a", "b");
+    expect(jobUpdateStore.pruneHistory(1, 1)).andReturn(pruned);
+    expectOp(Op.pruneJobUpdateHistory(new PruneJobUpdateHistory(1, 1)));
+
+    control.replay();
+
+    storage.pruneHistory(1, 1);
+  }
+
+  @Test
+  public void testNoopPruneHistory() {
+    expect(jobUpdateStore.pruneHistory(1, 1)).andReturn(ImmutableSet.<String>of());
+
+    control.replay();
+
+    storage.pruneHistory(1, 1);
+  }
+
+  @Test
+  public void testMutate() {
+    Query.Builder query = Query.taskScoped("a");
+    Function<IScheduledTask, IScheduledTask> mutator =
+        createMock(new Clazz<Function<IScheduledTask, IScheduledTask>>() { });
+    ImmutableSet<IScheduledTask> mutated = ImmutableSet.of(IScheduledTask.build(
+            new ScheduledTask().setAssignedTask(new AssignedTask().setTaskId("a"))));
+
+    expect(taskStore.mutateTasks(query, mutator)).andReturn(mutated);
+    expect(log.isLoggable(Level.FINE)).andReturn(false);
+    expectOp(Op.saveTasks(new SaveTasks(IScheduledTask.toBuildersSet(mutated))));
+
+    // With increased logging.
+    expect(taskStore.mutateTasks(query, mutator)).andReturn(mutated);
+    expect(log.isLoggable(Level.FINE)).andReturn(true);
+    expectOp(Op.saveTasks(new SaveTasks(IScheduledTask.toBuildersSet(mutated))));
+    log.fine(EasyMock.anyString());
+
+    control.replay();
+
+    assertEquals(mutated, storage.mutateTasks(query, mutator));
+    assertEquals(mutated, storage.mutateTasks(query, mutator));
+  }
+
+  @Test(expected = UnsupportedOperationException.class)
+  public void testDeleteAllTasks() {
+    control.replay();
+    storage.deleteAllTasks();
+  }
+
+  @Test(expected = UnsupportedOperationException.class)
+  public void testDeleteHostAttributes() {
+    control.replay();
+    storage.deleteHostAttributes();
+  }
+
+  @Test(expected = UnsupportedOperationException.class)
+  public void testDeleteJobs() {
+    control.replay();
+    storage.deleteJobs();
+  }
+
+  @Test(expected = UnsupportedOperationException.class)
+  public void testDeleteQuotas() {
+    control.replay();
+    storage.deleteQuotas();
+  }
+
+  @Test(expected = UnsupportedOperationException.class)
+  public void testDeleteLocks() {
+    control.replay();
+    storage.deleteLocks();
+  }
+
+  @Test(expected = UnsupportedOperationException.class)
+  public void testDeleteAllUpdatesAndEvents() {
+    control.replay();
+    storage.deleteAllUpdatesAndEvents();
+  }
+}