From 5da44f226459984b6b261297ff3e280ee3f741f8 Mon Sep 17 00:00:00 2001
From: Aleksei Statkevich <astatkevich@fb.com>
Date: Tue, 11 Oct 2016 14:08:38 -0700
Subject: [PATCH] Make shard cleanup more efficient

---
 .../com/facebook/presto/raptor/metadata/H2ShardDao.java   | 4 ++--
 .../facebook/presto/raptor/metadata/MySqlShardDao.java    | 6 +++---
 .../com/facebook/presto/raptor/metadata/ShardCleaner.java | 8 +++++++-
 .../com/facebook/presto/raptor/metadata/ShardDao.java     | 4 ++--
 4 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/presto-raptor/src/main/java/com/facebook/presto/raptor/metadata/H2ShardDao.java b/presto-raptor/src/main/java/com/facebook/presto/raptor/metadata/H2ShardDao.java
index 9298063e06040..57b3cae9da3ce 100644
--- a/presto-raptor/src/main/java/com/facebook/presto/raptor/metadata/H2ShardDao.java
+++ b/presto-raptor/src/main/java/com/facebook/presto/raptor/metadata/H2ShardDao.java
@@ -36,8 +36,8 @@ public interface H2ShardDao
 
     @SqlUpdate("DELETE FROM transactions\n" +
             "WHERE end_time < :maxEndTime\n" +
-            "  AND successful IS NOT NULL\n" +
+            "  AND successful IN (TRUE, FALSE)\n" +
             "  AND transaction_id NOT IN (SELECT transaction_id FROM created_shards)\n" +
             "LIMIT " + CLEANUP_TRANSACTIONS_BATCH_SIZE)
-    void deleteOldCompletedTransactions(@Bind("maxEndTime") Timestamp maxEndTime);
+    int deleteOldCompletedTransactions(@Bind("maxEndTime") Timestamp maxEndTime);
 }
diff --git a/presto-raptor/src/main/java/com/facebook/presto/raptor/metadata/MySqlShardDao.java b/presto-raptor/src/main/java/com/facebook/presto/raptor/metadata/MySqlShardDao.java
index bf6fecc1db0a8..c32090d096cb9 100644
--- a/presto-raptor/src/main/java/com/facebook/presto/raptor/metadata/MySqlShardDao.java
+++ b/presto-raptor/src/main/java/com/facebook/presto/raptor/metadata/MySqlShardDao.java
@@ -44,9 +44,9 @@ public interface MySqlShardDao
     // 'order by' is needed in this statement in order to make it compatible with statement-based replication
     @SqlUpdate("DELETE FROM transactions\n" +
             "WHERE end_time < :maxEndTime\n" +
-            "  AND successful IS NOT NULL\n" +
+            "  AND successful IN (TRUE, FALSE)\n" +
             "  AND transaction_id NOT IN (SELECT transaction_id FROM created_shards)\n" +
-            "ORDER BY transaction_id\n" +
+            "ORDER BY end_time, transaction_id\n" +
             "LIMIT " + CLEANUP_TRANSACTIONS_BATCH_SIZE)
-    void deleteOldCompletedTransactions(@Bind("maxEndTime") Timestamp maxEndTime);
+    int deleteOldCompletedTransactions(@Bind("maxEndTime") Timestamp maxEndTime);
 }
diff --git a/presto-raptor/src/main/java/com/facebook/presto/raptor/metadata/ShardCleaner.java b/presto-raptor/src/main/java/com/facebook/presto/raptor/metadata/ShardCleaner.java
index 6f1ef4051b3e7..e0af225c5bc5a 100644
--- a/presto-raptor/src/main/java/com/facebook/presto/raptor/metadata/ShardCleaner.java
+++ b/presto-raptor/src/main/java/com/facebook/presto/raptor/metadata/ShardCleaner.java
@@ -50,6 +50,7 @@
 import java.util.concurrent.atomic.AtomicBoolean;
 
 import static com.facebook.presto.raptor.metadata.ShardDao.CLEANABLE_SHARDS_BATCH_SIZE;
+import static com.facebook.presto.raptor.metadata.ShardDao.CLEANUP_TRANSACTIONS_BATCH_SIZE;
 import static com.google.common.collect.Sets.difference;
 import static com.google.common.collect.Sets.newConcurrentHashSet;
 import static io.airlift.concurrent.Threads.daemonThreadsNamed;
@@ -298,7 +299,12 @@ void deleteOldShards()
     @VisibleForTesting
     void deleteOldCompletedTransactions()
     {
-        dao.deleteOldCompletedTransactions(maxTimestamp(maxCompletedTransactionAge));
+        while (!Thread.currentThread().isInterrupted()) {
+            int deleted = dao.deleteOldCompletedTransactions(maxTimestamp(maxCompletedTransactionAge));
+            if (deleted < CLEANUP_TRANSACTIONS_BATCH_SIZE) {
+                break;
+            }
+        }
     }
 
     @VisibleForTesting
diff --git a/presto-raptor/src/main/java/com/facebook/presto/raptor/metadata/ShardDao.java b/presto-raptor/src/main/java/com/facebook/presto/raptor/metadata/ShardDao.java
index 29e99b9ca2439..bb591d9854e61 100644
--- a/presto-raptor/src/main/java/com/facebook/presto/raptor/metadata/ShardDao.java
+++ b/presto-raptor/src/main/java/com/facebook/presto/raptor/metadata/ShardDao.java
@@ -34,7 +34,7 @@
 public interface ShardDao
 {
     int CLEANABLE_SHARDS_BATCH_SIZE = 1000;
-    int CLEANUP_TRANSACTIONS_BATCH_SIZE = 1_000_000;
+    int CLEANUP_TRANSACTIONS_BATCH_SIZE = 10_000;
 
     @SqlUpdate("INSERT INTO nodes (node_identifier) VALUES (:nodeIdentifier)")
     @GetGeneratedKeys
@@ -206,5 +206,5 @@ void updateBucketNode(
             @Bind("bucketNumber") int bucketNumber,
             @Bind("nodeId") int nodeId);
 
-    void deleteOldCompletedTransactions(@Bind("maxEndTime") Timestamp maxEndTime);
+    int deleteOldCompletedTransactions(@Bind("maxEndTime") Timestamp maxEndTime);
 }