aurora-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From wfar...@apache.org
Subject aurora git commit: Use the application-level storage lock to prevent DB-level deadlock when GCing rows.
Date Fri, 17 Jul 2015 01:01:24 GMT
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 <wfarner@apache.org>
Authored: Thu Jul 16 18:01:08 2015 -0700
Committer: Bill Farner <wfarner@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]));


Mime
View raw message