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 <[email protected]> Authored: Thu Jul 9 12:29:53 2015 -0700 Committer: Bill Farner <[email protected]> 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); }
