Repository: aurora Updated Branches: refs/heads/master a654b28c3 -> d74b02e87
Use the application-level storage lock to prevent DB-level deadlock when GCing rows. Bugs closed: AURORA-1401 Reviewed at https://reviews.apache.org/r/36561/ Project: http://git-wip-us.apache.org/repos/asf/aurora/repo Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/d74b02e8 Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/d74b02e8 Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/d74b02e8 Branch: refs/heads/master Commit: d74b02e877572c1a5cd744fc7974b1f4c48282e7 Parents: a654b28 Author: Bill Farner <wfar...@apache.org> Authored: Thu Jul 16 18:01:08 2015 -0700 Committer: Bill Farner <wfar...@apache.org> Committed: Thu Jul 16 18:01:08 2015 -0700 ---------------------------------------------------------------------- .../aurora/scheduler/storage/db/DbModule.java | 5 ++ .../storage/db/RowGarbageCollector.java | 53 +++++++++++++------- .../scheduler/app/local/LocalSchedulerMain.java | 1 + 3 files changed, 42 insertions(+), 17 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/aurora/blob/d74b02e8/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 0b76caf..ed92661 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 @@ -115,6 +115,11 @@ public final class DbModule extends PrivateModule { // previous behavior of the map-based store, and allow this type of pattern to work without // regression. .put("LOCK_MODE", "0") + // Error-level reporting for H2. + // See http://www.h2database.com/html/features.html#trace_options + // TODO(wfarner): H2 can ship these to slf4j, but is too noisy at our default level (info). + // Use this logging and reduce the default level for h2's logger. + .put("TRACE_LEVEL_SYSTEM_OUT", "1") .build(); this.jdbcSchema = dbName + ";" + Joiner.on(";").withKeyValueSeparator("=").join(args); } http://git-wip-us.apache.org/repos/asf/aurora/blob/d74b02e8/src/main/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollector.java ---------------------------------------------------------------------- diff --git a/src/main/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollector.java b/src/main/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollector.java index ba7c677..f1916a8 100644 --- a/src/main/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollector.java +++ b/src/main/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollector.java @@ -14,6 +14,7 @@ package org.apache.aurora.scheduler.storage.db; import java.util.List; +import java.util.concurrent.atomic.AtomicLong; import java.util.logging.Logger; import javax.inject.Inject; @@ -22,8 +23,10 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; import com.google.common.util.concurrent.AbstractScheduledService; +import org.apache.aurora.scheduler.storage.Storage; import org.apache.ibatis.exceptions.PersistenceException; -import org.mybatis.guice.transactional.Transactional; +import org.apache.ibatis.session.SqlSession; +import org.apache.ibatis.session.SqlSessionFactory; import static java.util.Objects.requireNonNull; @@ -34,17 +37,27 @@ class RowGarbageCollector extends AbstractScheduledService { private static final Logger LOG = Logger.getLogger(RowGarbageCollector.class.getName()); + // Note: these are deliberately ordered to remove 'parent' references first, but since + // this is an iterative process, it is not strictly necessary. + private static final List<Class<? extends GarbageCollectedTableMapper>> TABLES = + ImmutableList.of(TaskConfigMapper.class, JobKeyMapper.class); + private final Scheduler iterationScheduler; - private final List<GarbageCollectedTableMapper> tables; + private final SqlSessionFactory sessionFactory; + + // Note: Storage is only used to acquire the same application-level lock used by other storage + // mutations. This sidesteps the issue of DB deadlocks (e.g. AURORA-1401). + private final Storage storage; @Inject RowGarbageCollector( Scheduler iterationScheduler, - TaskConfigMapper taskConfigMapper, - JobKeyMapper jobKeyMapper) { + SqlSessionFactory sessionFactory, + Storage storage) { this.iterationScheduler = requireNonNull(iterationScheduler); - this.tables = ImmutableList.of(taskConfigMapper, jobKeyMapper); + this.sessionFactory = requireNonNull(sessionFactory); + this.storage = requireNonNull(storage); } @Override @@ -53,23 +66,29 @@ class RowGarbageCollector extends AbstractScheduledService { } @VisibleForTesting - @Transactional @Override public void runOneIteration() { - // Note: ordering of table scans is important here to remove 'parent' references first. LOG.info("Scanning database tables for unreferenced rows."); - long deletedCount = 0; - for (GarbageCollectedTableMapper table : tables) { - for (long rowId : table.selectAllRowIds()) { - try { - table.deleteRow(rowId); - deletedCount++; - } catch (PersistenceException e) { - // Expected for rows that are still referenced. + final AtomicLong deletedCount = new AtomicLong(); + for (Class<? extends GarbageCollectedTableMapper> tableClass : TABLES) { + storage.write(new Storage.MutateWork.NoResult.Quiet() { + @Override + protected void execute(Storage.MutableStoreProvider storeProvider) { + try (SqlSession session = sessionFactory.openSession(true)) { + GarbageCollectedTableMapper table = session.getMapper(tableClass); + for (long rowId : table.selectAllRowIds()) { + try { + table.deleteRow(rowId); + deletedCount.incrementAndGet(); + } catch (PersistenceException e) { + // Expected for rows that are still referenced. + } + } + } } - } + }); } - LOG.info("Deleted " + deletedCount + " unreferenced rows."); + LOG.info("Deleted " + deletedCount.get() + " unreferenced rows."); } } http://git-wip-us.apache.org/repos/asf/aurora/blob/d74b02e8/src/test/java/org/apache/aurora/scheduler/app/local/LocalSchedulerMain.java ---------------------------------------------------------------------- diff --git a/src/test/java/org/apache/aurora/scheduler/app/local/LocalSchedulerMain.java b/src/test/java/org/apache/aurora/scheduler/app/local/LocalSchedulerMain.java index 1597725..fc44389 100644 --- a/src/test/java/org/apache/aurora/scheduler/app/local/LocalSchedulerMain.java +++ b/src/test/java/org/apache/aurora/scheduler/app/local/LocalSchedulerMain.java @@ -101,6 +101,7 @@ public class LocalSchedulerMain extends SchedulerMain { .add("-shiro_ini_path=" + ResourceUtils.CLASSPATH_PREFIX + "org/apache/aurora/scheduler/http/api/security/shiro-example.ini") + .add("-enable_h2_console=true") .build(); AppLauncher.launch(LocalSchedulerMain.class, arguments.toArray(new String[0]));
