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 2015/07/09 21:30:13 UTC
aurora git commit: Relax the transaction isolation in H2 to match
MemStorage behavior.
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 <wf...@apache.org>
Authored: Thu Jul 9 12:29:53 2015 -0700
Committer: Bill Farner <wf...@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);
}