aurora-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From wfar...@apache.org
Subject aurora git commit: Relax the transaction isolation in H2 to match MemStorage behavior.
Date Thu, 09 Jul 2015 19:30:13 GMT
Repository: aurora
Updated Branches:
  refs/heads/master 28f976ce3 -> 188cbd271


Relax the transaction isolation in H2 to match MemStorage behavior.

Bugs closed: AURORA-1386

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


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

Branch: refs/heads/master
Commit: 188cbd271e0bc7f61709fbc2b8f72a205970d6ac
Parents: 28f976c
Author: Bill Farner <wfarner@apache.org>
Authored: Thu Jul 9 12:29:53 2015 -0700
Committer: Bill Farner <wfarner@apache.org>
Committed: Thu Jul 9 12:29:53 2015 -0700

----------------------------------------------------------------------
 .../aurora/scheduler/storage/db/DbModule.java   | 33 ++++++++++++++---
 .../storage/AbstractTaskStoreTest.java          | 38 +++++++++++++++++++-
 2 files changed, 65 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/aurora/blob/188cbd27/src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java b/src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java
index 23bf0ac..49d8bcb 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/db/DbModule.java
@@ -13,12 +13,14 @@
  */
 package org.apache.aurora.scheduler.storage.db;
 
+import java.util.Map;
 import java.util.Set;
 import java.util.UUID;
 
 import javax.inject.Singleton;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Joiner;
 import com.google.common.base.Optional;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
@@ -94,11 +96,27 @@ public final class DbModule extends PrivateModule {
   private final Module taskStoresModule;
   private final String jdbcSchema;
 
-  private DbModule(KeyFactory keyFactory, Module taskStoresModule, String jdbcSchema) {
+  private DbModule(
+      KeyFactory keyFactory,
+      Module taskStoresModule,
+      String dbName,
+      Map<String, String> jdbcUriArgs) {
+
     this.keyFactory = requireNonNull(keyFactory);
     this.taskStoresModule = requireNonNull(taskStoresModule);
-    // We always disable the MvStore, as it is in beta as of this writing.
-    this.jdbcSchema = jdbcSchema + ";MV_STORE=false";
+
+    Map<String, String> args = ImmutableMap.<String, String>builder()
+        .putAll(jdbcUriArgs)
+        // We always disable the MvStore, as it is in beta as of this writing.
+        .put("MV_STORE", "false")
+        // In several scenarios, we initiate asynchronous work in the context of a transaction,
+        // which can cause the asynchronous work to read yet-to-be-committed data.  Since
this
+        // is currently only used as an in-memory store, we allow reads of uncommitted data
to match
+        // previous behavior of the map-based store, and allow this type of pattern to work
without
+        // regression.
+        .put("LOCK_MODE", "0")
+        .build();
+    this.jdbcSchema = dbName + Joiner.on(";").withKeyValueSeparator("=").join(args);
   }
 
   /**
@@ -109,7 +127,11 @@ public final class DbModule extends PrivateModule {
    * @return A new database module for production.
    */
   public static Module productionModule(KeyFactory keyFactory) {
-    return new DbModule(keyFactory, getTaskStoreModule(keyFactory), "aurora;DB_CLOSE_DELAY=-1");
+    return new DbModule(
+        keyFactory,
+        getTaskStoreModule(keyFactory),
+        "aurora",
+        ImmutableMap.of("DB_CLOSE_DELAY", "-1"));
   }
 
   /**
@@ -125,11 +147,12 @@ public final class DbModule extends PrivateModule {
     return new DbModule(
         keyFactory,
         taskStoreModule.isPresent() ? taskStoreModule.get() : getTaskStoreModule(keyFactory),
+        "testdb-" + UUID.randomUUID().toString(),
         // A non-zero close delay is used here to avoid eager database cleanup in tests that
         // make use of multiple threads.  Since all test databases are separately scoped
by the
         // included UUID, multiple DB instances will overlap in time but they should be distinct
         // in content.
-        "testdb-" + UUID.randomUUID().toString() + ";DB_CLOSE_DELAY=5;");
+        ImmutableMap.of("DB_CLOSE_DELAY", "5"));
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/aurora/blob/188cbd27/src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java
b/src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java
index c8a2d81..775bb71 100644
--- a/src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java
@@ -15,15 +15,18 @@ package org.apache.aurora.scheduler.storage;
 
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.Callable;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
 
 import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Maps;
+import com.google.common.testing.junit4.TearDownTestCase;
 import com.google.common.util.concurrent.ThreadFactoryBuilder;
 import com.google.inject.Guice;
 import com.google.inject.Injector;
@@ -44,6 +47,7 @@ import org.apache.aurora.gen.ScheduledTask;
 import org.apache.aurora.gen.TaskQuery;
 import org.apache.aurora.scheduler.base.JobKeys;
 import org.apache.aurora.scheduler.base.Query;
+import org.apache.aurora.scheduler.base.TaskTestUtil;
 import org.apache.aurora.scheduler.base.Tasks;
 import org.apache.aurora.scheduler.storage.TaskStore.Mutable.TaskMutation;
 import org.apache.aurora.scheduler.storage.entities.IHostAttributes;
@@ -62,7 +66,7 @@ import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
-public abstract class AbstractTaskStoreTest {
+public abstract class AbstractTaskStoreTest extends TearDownTestCase {
   protected static final IHostAttributes HOST_A = IHostAttributes.build(
       new HostAttributes(
           "hostA",
@@ -597,6 +601,38 @@ public abstract class AbstractTaskStoreTest {
     }
   }
 
+  @Test
+  public void testLegacyPermissiveTransactionIsolation() throws Exception {
+    // Ensures that a thread launched within a transaction can read the uncommitted changes
caused
+    // by the transaction.  This is not a pattern that we should embrace, but is necessary
for
+    // DbStorage to match behavior with MemStorage.
+    // TODO(wfarner): Create something like a transaction-aware Executor so that we can still
+    // asynchronously react to a completed transaction, but in a way that allows for more
strict
+    // transaction isolation.
+
+    ExecutorService executor = Executors.newFixedThreadPool(1,
+        new ThreadFactoryBuilder().setNameFormat("AsyncRead-%d").setDaemon(true).build());
+    addTearDown(() -> new ExecutorServiceShutdown(executor, Amount.of(1L, Time.SECONDS)).execute());
+
+    saveTasks(TASK_A);
+    storage.write(new Storage.MutateWork.NoResult<Exception>() {
+      @Override
+      protected void execute(Storage.MutableStoreProvider storeProvider) throws Exception
{
+        IScheduledTask taskARunning = TaskTestUtil.addStateTransition(TASK_A, RUNNING, 1000L);
+        saveTasks(taskARunning);
+
+        Future<ScheduleStatus> asyncReadState = executor.submit(new Callable<ScheduleStatus>()
{
+          @Override
+          public ScheduleStatus call() {
+            return Iterables.getOnlyElement(fetchTasks(Query.taskScoped(Tasks.id(TASK_A))))
+                .getStatus();
+          }
+        });
+        assertEquals(RUNNING, asyncReadState.get());
+      }
+    });
+  }
+
   private void assertStoreContents(IScheduledTask... tasks) {
     assertQueryResults(Query.unscoped(), tasks);
   }


Mime
View raw message