aurora-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From wfar...@apache.org
Subject aurora git commit: DbTaskStore: delete unreferenced job keys and task configs.
Date Thu, 25 Jun 2015 19:52:30 GMT
Repository: aurora
Updated Branches:
  refs/heads/master ca15f25cc -> 58514e749


DbTaskStore: delete unreferenced job keys and task configs.

Bugs closed: AURORA-1298

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


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

Branch: refs/heads/master
Commit: 58514e749e35251c8acea201164da550f97fbb34
Parents: ca15f25
Author: Bill Farner <wfarner@apache.org>
Authored: Thu Jun 25 12:17:35 2015 -0700
Committer: Bill Farner <wfarner@apache.org>
Committed: Thu Jun 25 12:52:16 2015 -0700

----------------------------------------------------------------------
 .../aurora/scheduler/app/SchedulerMain.java     |   1 +
 .../scheduler/storage/db/DbJobUpdateStore.java  |   2 +-
 .../scheduler/storage/db/DbLockStore.java       |   2 +-
 .../aurora/scheduler/storage/db/DbModule.java   |  34 ++++++
 .../scheduler/storage/db/DbTaskStore.java       |  12 --
 .../storage/db/GarbageCollectedTableMapper.java |  33 ++++++
 .../scheduler/storage/db/JobKeyMapper.java      |  10 +-
 .../scheduler/storage/db/LockKeyMapper.java     |   4 +-
 .../storage/db/RowGarbageCollector.java         |  75 +++++++++++++
 .../scheduler/storage/db/TaskConfigManager.java |  22 +---
 .../scheduler/storage/db/TaskConfigMapper.java  |  12 +-
 .../scheduler/storage/db/JobKeyMapper.xml       |  18 +--
 .../scheduler/storage/db/TaskConfigMapper.xml   |  25 +----
 .../aurora/scheduler/storage/db/schema.sql      |   3 +-
 .../storage/AbstractTaskStoreTest.java          |   2 +-
 .../scheduler/storage/db/DbTaskStoreTest.java   |  27 -----
 .../storage/db/RowGarbageCollectorTest.java     | 111 +++++++++++++++++++
 17 files changed, 286 insertions(+), 107 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/aurora/blob/58514e74/src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java b/src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java
index c31446c..968aca6 100644
--- a/src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java
+++ b/src/main/java/org/apache/aurora/scheduler/app/SchedulerMain.java
@@ -156,6 +156,7 @@ public class SchedulerMain extends AbstractApplication {
         .add(getPersistentStorageModule())
         .add(new CronModule())
         .add(DbModule.productionModule(Bindings.annotatedKeyFactory(Storage.Volatile.class)))
+        .add(new DbModule.GarbageCollectorModule())
         .build();
   }
 

http://git-wip-us.apache.org/repos/asf/aurora/blob/58514e74/src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java b/src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java
index 11c9c4a..7728684 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/db/DbJobUpdateStore.java
@@ -83,7 +83,7 @@ public class DbJobUpdateStore implements JobUpdateStore.Mutable {
     }
 
     IJobUpdateKey key = update.getSummary().getKey();
-    jobKeyMapper.merge(key.getJob().newBuilder());
+    jobKeyMapper.merge(key.getJob());
     detailsMapper.insert(update.newBuilder());
 
     if (lockToken.isPresent()) {

http://git-wip-us.apache.org/repos/asf/aurora/blob/58514e74/src/main/java/org/apache/aurora/scheduler/storage/db/DbLockStore.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/db/DbLockStore.java b/src/main/java/org/apache/aurora/scheduler/storage/db/DbLockStore.java
index 6a127e4..4d47b21 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/db/DbLockStore.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/db/DbLockStore.java
@@ -46,7 +46,7 @@ class DbLockStore implements LockStore.Mutable {
   @Timed("lock_store_save_lock")
   @Override
   public void saveLock(ILock lock) {
-    lockKeyMapper.insert(lock.getKey().newBuilder());
+    lockKeyMapper.insert(lock.getKey());
     mapper.insert(lock.newBuilder());
   }
 

http://git-wip-us.apache.org/repos/asf/aurora/blob/58514e74/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 2dc3034..8bfb076 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
@@ -22,6 +22,8 @@ import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Optional;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.util.concurrent.AbstractScheduledService;
+import com.google.inject.AbstractModule;
 import com.google.inject.Key;
 import com.google.inject.Module;
 import com.google.inject.PrivateModule;
@@ -32,6 +34,7 @@ import com.twitter.common.inject.Bindings.KeyFactory;
 import com.twitter.common.quantity.Amount;
 import com.twitter.common.quantity.Time;
 
+import org.apache.aurora.scheduler.SchedulerServicesModule;
 import org.apache.aurora.scheduler.storage.AttributeStore;
 import org.apache.aurora.scheduler.storage.CronJobStore;
 import org.apache.aurora.scheduler.storage.JobUpdateStore;
@@ -67,6 +70,11 @@ public final class DbModule extends PrivateModule {
   private static final Arg<Amount<Long, Time>> SLOW_QUERY_LOG_THRESHOLD =
       Arg.create(Amount.of(25L, Time.MILLISECONDS));
 
+  @CmdLine(name = "db_row_gc_interval",
+      help = "Interval on which to scan the database for unused row references.")
+  private static final Arg<Amount<Long, Time>> DB_ROW_GC_INTERVAL =
+      Arg.create(Amount.of(2L, Time.HOURS));
+
   private static final Set<Class<?>> MAPPER_CLASSES = ImmutableSet.<Class<?>>builder()
       .add(AttributeMapper.class)
       .add(EnumValueMapper.class)
@@ -200,6 +208,9 @@ public final class DbModule extends PrivateModule {
 
     expose(DbStorage.class);
     expose(SqlSessionFactory.class);
+    expose(TaskMapper.class);
+    expose(TaskConfigMapper.class);
+    expose(JobKeyMapper.class);
   }
 
   /**
@@ -228,4 +239,27 @@ public final class DbModule extends PrivateModule {
       expose(TaskStore.Mutable.class);
     }
   }
+
+  /**
+   * Module that sets up a periodic database garbage-collection routine.
+   */
+  public static class GarbageCollectorModule extends AbstractModule {
+    @Override
+    protected void configure() {
+      install(new PrivateModule() {
+        @Override
+        protected void configure() {
+          bind(RowGarbageCollector.class).in(Singleton.class);
+          bind(AbstractScheduledService.Scheduler.class).toInstance(
+              AbstractScheduledService.Scheduler.newFixedRateSchedule(
+                  0L,
+                  DB_ROW_GC_INTERVAL.get().getValue(),
+                  DB_ROW_GC_INTERVAL.get().getUnit().getTimeUnit()));
+          expose(RowGarbageCollector.class);
+        }
+      });
+      SchedulerServicesModule.addSchedulerActiveServiceBinding(binder())
+          .to(RowGarbageCollector.class);
+    }
+  }
 }

http://git-wip-us.apache.org/repos/asf/aurora/blob/58514e74/src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java b/src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java
index 30e3469..a5acb3a 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/db/DbTaskStore.java
@@ -73,7 +73,6 @@ class DbTaskStore implements TaskStore.Mutable {
 
   private final TaskMapper taskMapper;
   private final TaskConfigManager configManager;
-  private final JobKeyMapper jobKeyMapper;
   private final Clock clock;
   private final long slowQueryThresholdNanos;
 
@@ -81,14 +80,12 @@ class DbTaskStore implements TaskStore.Mutable {
   DbTaskStore(
       TaskMapper taskMapper,
       TaskConfigManager configManager,
-      JobKeyMapper jobKeyMapper,
       Clock clock,
       Amount<Long, Time> slowQueryThreshold) {
 
     LOG.warning("DbTaskStore is experimental, and should not be used in production clusters!");
     this.taskMapper = requireNonNull(taskMapper);
     this.configManager = requireNonNull(configManager);
-    this.jobKeyMapper = requireNonNull(jobKeyMapper);
     this.clock = requireNonNull(clock);
     this.slowQueryThresholdNanos =  slowQueryThreshold.as(Time.NANOSECONDS);
   }
@@ -178,7 +175,6 @@ class DbTaskStore implements TaskStore.Mutable {
             .toSet()));
 
     for (IScheduledTask task : tasks) {
-      jobKeyMapper.merge(task.getAssignedTask().getTask().getJob().newBuilder());
       long configId = configCache.getUnchecked(task.getAssignedTask().getTask());
 
       ScheduledTaskWrapper wrappedTask = new ScheduledTaskWrapper(-1, configId, task.newBuilder());
@@ -213,7 +209,6 @@ class DbTaskStore implements TaskStore.Mutable {
   @Timed("db_storage_delete_all_tasks")
   @Override
   public void deleteAllTasks() {
-    // TODO(wfarner): Need to re-evaluate all task configs after deleting tasks.
     taskMapper.truncate();
   }
 
@@ -221,14 +216,7 @@ class DbTaskStore implements TaskStore.Mutable {
   @Override
   public void deleteTasks(Set<String> taskIds) {
     if (!taskIds.isEmpty()) {
-      // First fetch task configs referenced by these task IDs.
-      List<Long> configIds = configManager.getTaskConfigIds(taskIds);
-
       taskMapper.deleteTasks(taskIds);
-
-      if (!configIds.isEmpty()) {
-        configManager.maybeExpungeConfigs(ImmutableSet.copyOf(configIds));
-      }
     }
   }
 

http://git-wip-us.apache.org/repos/asf/aurora/blob/58514e74/src/main/java/org/apache/aurora/scheduler/storage/db/GarbageCollectedTableMapper.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/db/GarbageCollectedTableMapper.java
b/src/main/java/org/apache/aurora/scheduler/storage/db/GarbageCollectedTableMapper.java
new file mode 100644
index 0000000..83e42a9
--- /dev/null
+++ b/src/main/java/org/apache/aurora/scheduler/storage/db/GarbageCollectedTableMapper.java
@@ -0,0 +1,33 @@
+/**
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.aurora.scheduler.storage.db;
+
+import java.util.List;
+
+import org.apache.ibatis.annotations.Param;
+
+/**
+ * Base interface for a table mapper whose rows should be garbage-collected when unreferenced.
+ */
+interface GarbageCollectedTableMapper {
+  /**
+   * Selects the IDs of all rows in the table.
+   */
+  List<Long> selectAllRowIds();
+
+  /**
+   * Attempts to delete a row from the table.
+   */
+  void deleteRow(@Param("rowId") long rowId);
+}

http://git-wip-us.apache.org/repos/asf/aurora/blob/58514e74/src/main/java/org/apache/aurora/scheduler/storage/db/JobKeyMapper.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/db/JobKeyMapper.java b/src/main/java/org/apache/aurora/scheduler/storage/db/JobKeyMapper.java
index afdaa49..ac0a998 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/db/JobKeyMapper.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/db/JobKeyMapper.java
@@ -16,22 +16,18 @@ package org.apache.aurora.scheduler.storage.db;
 import java.util.List;
 
 import org.apache.aurora.gen.JobKey;
+import org.apache.aurora.scheduler.storage.entities.IJobKey;
 
 /**
  * MyBatis mapper class for JobKeyMapper.xml
  *
  * See http://mybatis.github.io/mybatis-3/sqlmap-xml.html for more details.
  */
-interface JobKeyMapper {
+interface JobKeyMapper extends GarbageCollectedTableMapper {
   /**
    * Saves the job key, updating the existing value if it exists.
    */
-  void merge(JobKey key);
-
-  /**
-   * Permanently deletes a job key from the database.
-   */
-  void delete(JobKey key);
+  void merge(IJobKey key);
 
   /**
    * Selects all job keys from the database.

http://git-wip-us.apache.org/repos/asf/aurora/blob/58514e74/src/main/java/org/apache/aurora/scheduler/storage/db/LockKeyMapper.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/db/LockKeyMapper.java b/src/main/java/org/apache/aurora/scheduler/storage/db/LockKeyMapper.java
index 5e9ba82..8f1ee7d 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/db/LockKeyMapper.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/db/LockKeyMapper.java
@@ -15,7 +15,7 @@ package org.apache.aurora.scheduler.storage.db;
 
 import com.google.inject.Inject;
 
-import org.apache.aurora.gen.LockKey;
+import org.apache.aurora.scheduler.storage.entities.ILockKey;
 
 import static java.util.Objects.requireNonNull;
 
@@ -39,7 +39,7 @@ class LockKeyMapper {
     this.jobKeyMapper = requireNonNull(jobKeyMapper);
   }
 
-  public void insert(LockKey key) {
+  public void insert(ILockKey key) {
     if (key.isSetJob()) {
       jobKeyMapper.merge(requireNonNull(key.getJob()));
     } else {

http://git-wip-us.apache.org/repos/asf/aurora/blob/58514e74/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
new file mode 100644
index 0000000..ba7c677
--- /dev/null
+++ b/src/main/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollector.java
@@ -0,0 +1,75 @@
+/**
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.aurora.scheduler.storage.db;
+
+import java.util.List;
+import java.util.logging.Logger;
+
+import javax.inject.Inject;
+
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableList;
+import com.google.common.util.concurrent.AbstractScheduledService;
+
+import org.apache.ibatis.exceptions.PersistenceException;
+import org.mybatis.guice.transactional.Transactional;
+
+import static java.util.Objects.requireNonNull;
+
+/**
+ * A periodic cleanup routine for unreferenced database relations.
+ */
+class RowGarbageCollector extends AbstractScheduledService {
+
+  private static final Logger LOG = Logger.getLogger(RowGarbageCollector.class.getName());
+
+  private final Scheduler iterationScheduler;
+  private final List<GarbageCollectedTableMapper> tables;
+
+  @Inject
+  RowGarbageCollector(
+      Scheduler iterationScheduler,
+      TaskConfigMapper taskConfigMapper,
+      JobKeyMapper jobKeyMapper) {
+
+    this.iterationScheduler = requireNonNull(iterationScheduler);
+    this.tables = ImmutableList.of(taskConfigMapper, jobKeyMapper);
+  }
+
+  @Override
+  protected Scheduler scheduler() {
+    return iterationScheduler;
+  }
+
+  @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.
+        }
+      }
+    }
+    LOG.info("Deleted " + deletedCount + " unreferenced rows.");
+  }
+}

http://git-wip-us.apache.org/repos/asf/aurora/blob/58514e74/src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java b/src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java
index 3ada628..46fa940 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigManager.java
@@ -34,14 +34,17 @@ import static java.util.Objects.requireNonNull;
 
 class TaskConfigManager {
   private final TaskConfigMapper configMapper;
+  private final JobKeyMapper jobKeyMapper;
 
   @Inject
-  TaskConfigManager(TaskConfigMapper configMapper) {
+  TaskConfigManager(TaskConfigMapper configMapper, JobKeyMapper jobKeyMapper) {
     this.configMapper = requireNonNull(configMapper);
+    this.jobKeyMapper = requireNonNull(jobKeyMapper);
   }
 
   long insert(ITaskConfig config) {
     InsertResult configInsert = new InsertResult();
+    jobKeyMapper.merge(config.getJob());
     configMapper.insert(config, configInsert);
     for (IConstraint constraint : config.getConstraints()) {
       InsertResult constraintResult = new InsertResult();
@@ -108,26 +111,11 @@ class TaskConfigManager {
     return configMapper.selectConfigsByJob(job);
   }
 
-  List<Long> getTaskConfigIds(Set<String> scheduledTaskIds) {
+  Set<Long> getTaskConfigIds(Set<String> scheduledTaskIds) {
     requireNonNull(scheduledTaskIds);
     return configMapper.selectConfigsByTaskId(scheduledTaskIds);
   }
 
-  /**
-   * Performs reference counting on configurations.  If there are no longer any references
to
-   * these configuration rows, they will be deleted.
-   * TODO(wfarner): Should we rely on foreign key constraints and opportunistically delete?
-   *
-   * @param configRowIds Configurations to delete if no references are found.
-   */
-  void maybeExpungeConfigs(Set<Long> configRowIds) {
-    if (configMapper.selectTasksByConfigId(configRowIds).isEmpty()) {
-      configMapper.delete(configRowIds);
-
-      // TODO(wfarner): Need to try removal from other tables as well, e.g. job keys.
-    }
-  }
-
   private static final Function<Map.Entry<String, String>, TaskLink> TO_LINK
=
       new Function<Map.Entry<String, String>, TaskLink>() {
         @Override

http://git-wip-us.apache.org/repos/asf/aurora/blob/58514e74/src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java b/src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java
index 7ee001f..19f67ad 100644
--- a/src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java
+++ b/src/main/java/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.java
@@ -30,7 +30,7 @@ import org.apache.ibatis.annotations.Param;
 /**
  * MyBatis mapper for task config objects.
  */
-interface TaskConfigMapper {
+interface TaskConfigMapper extends GarbageCollectedTableMapper {
 
   /**
    * Inserts fields from a task config into the {@code task_configs} table.
@@ -56,15 +56,7 @@ interface TaskConfigMapper {
    * @param taskIds Task IDs to look up.
    * @return Task config row IDs.
    */
-  List<Long> selectConfigsByTaskId(@Param("taskIds") Set<String> taskIds);
-
-  /**
-   * Looks up task config IDs by id.
-   *
-   * @param configIds Task config IDs.
-   * @return Task config row IDs.
-   */
-  List<Long> selectTasksByConfigId(@Param("configIds") Set<Long> configIds);
+  Set<Long> selectConfigsByTaskId(@Param("taskIds") Set<String> taskIds);
 
   /**
    * Inserts the constraint association within an {@link ITaskConfig}.

http://git-wip-us.apache.org/repos/asf/aurora/blob/58514e74/src/main/resources/org/apache/aurora/scheduler/storage/db/JobKeyMapper.xml
----------------------------------------------------------------------
diff --git a/src/main/resources/org/apache/aurora/scheduler/storage/db/JobKeyMapper.xml b/src/main/resources/org/apache/aurora/scheduler/storage/db/JobKeyMapper.xml
index f5829ac..80ff347 100644
--- a/src/main/resources/org/apache/aurora/scheduler/storage/db/JobKeyMapper.xml
+++ b/src/main/resources/org/apache/aurora/scheduler/storage/db/JobKeyMapper.xml
@@ -28,23 +28,25 @@
       #{name}
     )
   </insert>
+
   <sql id="job_key_clause">
     role = #{role}
     AND environment = #{environment}
     AND name = #{name}
   </sql>
-  <delete id="delete">
-    DELETE FROM job_keys
-    WHERE <include refid="job_key_clause"/>
-  </delete>
-  <select id="exists" resultType="org.apache.aurora.gen.JobKey">
-    SELECT * FROM job_keys
-    WHERE <include refid="job_key_clause"/>
-  </select>
+
   <select id="selectAll" resultType="org.apache.aurora.gen.JobKey">
     SELECT * FROM job_keys
   </select>
 
+  <select id="selectAllRowIds" resultType="long">
+    SELECT id FROM job_keys
+  </select>
+
+  <delete id="deleteRow">
+    DELETE FROM job_keys WHERE id = #{rowId}
+  </delete>
+
   <resultMap id="jobKeyMap" type="org.apache.aurora.gen.JobKey">
     <id column="id" />
   </resultMap>

http://git-wip-us.apache.org/repos/asf/aurora/blob/58514e74/src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
----------------------------------------------------------------------
diff --git a/src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
b/src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
index 8258fb1..3304728 100644
--- a/src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
+++ b/src/main/resources/org/apache/aurora/scheduler/storage/db/TaskConfigMapper.xml
@@ -195,18 +195,6 @@
     )
   </select>
 
-  <select id="selectTasksByConfigId" resultType="long">
-    SELECT
-      t.id AS id
-    FROM tasks AS t
-    INNER JOIN task_configs AS c ON c.id = t.task_config_row_id
-    WHERE c.id IN (
-      <foreach item="configId" collection="configIds" separator=",">
-        #{configId}
-      </foreach>
-    )
-  </select>
-
   <insert id="insertConstraint" useGeneratedKeys="true" keyColumn="id" keyProperty="result.id">
     INSERT INTO task_constraints (
       task_config_id,
@@ -312,12 +300,11 @@
     )
   </insert>
 
-  <delete id="delete">
-    DELETE FROM task_configs
-    WHERE id IN (
-      <foreach item="configId" collection="configIds" separator=",">
-        #{configId}
-      </foreach>
-    )
+  <select id="selectAllRowIds" resultType="long">
+    SELECT id FROM task_configs
+  </select>
+
+  <delete id="deleteRow">
+    DELETE FROM task_configs WHERE id = #{rowId}
   </delete>
 </mapper>

http://git-wip-us.apache.org/repos/asf/aurora/blob/58514e74/src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql
----------------------------------------------------------------------
diff --git a/src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql b/src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql
index 24cf526..0f77db7 100644
--- a/src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql
+++ b/src/main/resources/org/apache/aurora/scheduler/storage/db/schema.sql
@@ -72,8 +72,7 @@ CREATE TABLE host_attributes(
 
 CREATE TABLE host_attribute_values(
   id IDENTITY,
-  host_attribute_id BIGINT NOT NULL REFERENCES host_attributes(id)
-  ON DELETE CASCADE,
+  host_attribute_id BIGINT NOT NULL REFERENCES host_attributes(id) ON DELETE CASCADE,
   name VARCHAR NOT NULL,
   value VARCHAR NOT NULL,
 

http://git-wip-us.apache.org/repos/asf/aurora/blob/58514e74/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 63a784f..3a8e0d9 100644
--- a/src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/storage/AbstractTaskStoreTest.java
@@ -153,7 +153,7 @@ public abstract class AbstractTaskStoreTest {
     });
   }
 
-  private void deleteAllTasks() {
+  protected void deleteAllTasks() {
     storage.write(new Storage.MutateWork.NoResult.Quiet() {
       @Override
       protected void execute(Storage.MutableStoreProvider storeProvider) {

http://git-wip-us.apache.org/repos/asf/aurora/blob/58514e74/src/test/java/org/apache/aurora/scheduler/storage/db/DbTaskStoreTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/storage/db/DbTaskStoreTest.java b/src/test/java/org/apache/aurora/scheduler/storage/db/DbTaskStoreTest.java
index dda988d..102a06e 100644
--- a/src/test/java/org/apache/aurora/scheduler/storage/db/DbTaskStoreTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/storage/db/DbTaskStoreTest.java
@@ -13,7 +13,6 @@
  */
 package org.apache.aurora.scheduler.storage.db;
 
-import com.google.common.collect.ImmutableList;
 import com.google.inject.AbstractModule;
 import com.google.inject.Module;
 import com.google.inject.util.Modules;
@@ -21,24 +20,10 @@ import com.twitter.common.stats.StatsProvider;
 import com.twitter.common.util.Clock;
 import com.twitter.common.util.testing.FakeClock;
 
-import org.apache.aurora.scheduler.base.Tasks;
 import org.apache.aurora.scheduler.storage.AbstractTaskStoreTest;
-import org.apache.aurora.scheduler.storage.db.views.TaskConfigRow;
 import org.apache.aurora.scheduler.testing.FakeStatsProvider;
-import org.junit.Before;
-import org.junit.Test;
-
-import static org.junit.Assert.assertEquals;
 
 public class DbTaskStoreTest extends AbstractTaskStoreTest {
-
-  private TaskConfigManager configManager;
-
-  @Before
-  public void setUp() {
-    configManager = injector.getInstance(TaskConfigManager.class);
-  }
-
   @Override
   protected Module getStorageModule() {
     return Modules.combine(
@@ -51,16 +36,4 @@ public class DbTaskStoreTest extends AbstractTaskStoreTest {
           }
         });
   }
-
-  @Test
-  public void testRelationsRemoved() {
-    // When there are no remaining references to a task config, it should be removed.
-    saveTasks(TASK_A);
-    deleteTasks(Tasks.id(TASK_A));
-    assertEquals(
-        ImmutableList.<TaskConfigRow>of(),
-        configManager.getConfigs(TASK_A.getAssignedTask().getTask().getJob()));
-
-    // TODO(wfarner): Check that the job key was removed.
-  }
 }

http://git-wip-us.apache.org/repos/asf/aurora/blob/58514e74/src/test/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollectorTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollectorTest.java
b/src/test/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollectorTest.java
new file mode 100644
index 0000000..31feaea
--- /dev/null
+++ b/src/test/java/org/apache/aurora/scheduler/storage/db/RowGarbageCollectorTest.java
@@ -0,0 +1,111 @@
+/**
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.aurora.scheduler.storage.db;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
+import com.google.inject.AbstractModule;
+import com.google.inject.Guice;
+import com.google.inject.Injector;
+import com.twitter.common.stats.StatsProvider;
+import com.twitter.common.util.Clock;
+import com.twitter.common.util.testing.FakeClock;
+
+import org.apache.aurora.gen.JobKey;
+import org.apache.aurora.scheduler.base.TaskTestUtil;
+import org.apache.aurora.scheduler.storage.Storage;
+import org.apache.aurora.scheduler.storage.db.views.ScheduledTaskWrapper;
+import org.apache.aurora.scheduler.storage.db.views.TaskConfigRow;
+import org.apache.aurora.scheduler.storage.entities.IJobKey;
+import org.apache.aurora.scheduler.storage.entities.IScheduledTask;
+import org.apache.aurora.scheduler.storage.entities.ITaskConfig;
+import org.apache.aurora.scheduler.testing.FakeStatsProvider;
+import org.junit.Before;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+
+public class RowGarbageCollectorTest {
+
+  private static final IJobKey JOB_A = IJobKey.build(new JobKey("roleA", "envA", "jobA"));
+  private static final IJobKey JOB_B = IJobKey.build(new JobKey("roleB", "envB", "jobB"));
+  private static final IScheduledTask TASK_A2 = TaskTestUtil.makeTask("task_a2", JOB_A);
+  private static final ITaskConfig CONFIG_A =
+      ITaskConfig.build(TASK_A2.getAssignedTask().getTask().newBuilder().setRamMb(124246));
+  private static final ITaskConfig CONFIG_B = TaskTestUtil.makeConfig(JOB_B);
+
+  private JobKeyMapper jobKeyMapper;
+  private TaskMapper taskMapper;
+  private TaskConfigMapper taskConfigMapper;
+  private RowGarbageCollector rowGc;
+
+  @Before
+  public void setUp() {
+    Injector injector = Guice.createInjector(
+        DbModule.testModule(),
+        new DbModule.GarbageCollectorModule(),
+        new AbstractModule() {
+          @Override
+          protected void configure() {
+            bind(StatsProvider.class).toInstance(new FakeStatsProvider());
+            bind(Clock.class).toInstance(new FakeClock());
+          }
+        }
+    );
+
+    rowGc = injector.getInstance(RowGarbageCollector.class);
+    injector.getInstance(Storage.class).prepare();
+    taskMapper = injector.getInstance(TaskMapper.class);
+    jobKeyMapper = injector.getInstance(JobKeyMapper.class);
+    taskConfigMapper = injector.getInstance(TaskConfigMapper.class);
+  }
+
+  @Test
+  public void testNoop() {
+    rowGc.runOneIteration();
+  }
+
+  @Test
+  public void testGarbageCollection() {
+    rowGc.runOneIteration();
+    assertEquals(ImmutableList.<JobKey>of(), jobKeyMapper.selectAll());
+    assertEquals(ImmutableList.<TaskConfigRow>of(), taskConfigMapper.selectConfigsByJob(JOB_A));
+    assertEquals(ImmutableList.<TaskConfigRow>of(), taskConfigMapper.selectConfigsByJob(JOB_B));
+
+    jobKeyMapper.merge(JOB_A);
+    rowGc.runOneIteration();
+    assertEquals(ImmutableList.<JobKey>of(), jobKeyMapper.selectAll());
+    assertEquals(ImmutableList.<TaskConfigRow>of(), taskConfigMapper.selectConfigsByJob(JOB_A));
+    assertEquals(ImmutableList.<TaskConfigRow>of(), taskConfigMapper.selectConfigsByJob(JOB_B));
+
+    jobKeyMapper.merge(JOB_A);
+    taskConfigMapper.insert(CONFIG_A, new InsertResult());
+    InsertResult a2Insert = new InsertResult();
+    taskConfigMapper.insert(TASK_A2.getAssignedTask().getTask(), a2Insert);
+    taskMapper.insertScheduledTask(
+        new ScheduledTaskWrapper(-1, a2Insert.getId(), TASK_A2.newBuilder()));
+    jobKeyMapper.merge(JOB_B);
+    taskConfigMapper.insert(CONFIG_B, new InsertResult());
+    rowGc.runOneIteration();
+    // Only job A and config A2 are still referenced, other rows are deleted.
+    assertEquals(ImmutableList.of(JOB_A.newBuilder()), jobKeyMapper.selectAll());
+    // Note: Using the ramMb as a sentinel value, since relations in the TaskConfig are not
+    // populated, therefore full object equivalence cannot easily be used.
+    assertEquals(
+        TASK_A2.getAssignedTask().getTask().getRamMb(),
+        Iterables.getOnlyElement(taskConfigMapper.selectConfigsByJob(JOB_A))
+            .getConfig().getRamMb());
+    assertEquals(ImmutableList.<TaskConfigRow>of(), taskConfigMapper.selectConfigsByJob(JOB_B));
+  }
+}


Mime
View raw message