Cherry-pick GC fixes from cmu-db#1349, first part.

mbutrovich · Jul 3, 2018 · 93cac12 · 93cac12
1 parent c949481
commit 93cac12
Show file tree

Hide file tree

Showing 11 changed files with 48 additions and 39 deletions.
diff --git a/src/common/init.cpp b/src/common/init.cpp
@@ -52,7 +52,7 @@ void PelotonInit::Initialize() {
   threadpool::MonoQueuePool::GetExecutionInstance().Startup();
 
   int parallelism = (CONNECTION_THREAD_COUNT + 3) / 4;
-  storage::DataTable::SetActiveTileGroupCount(parallelism);
+  storage::DataTable::SetDefaultActiveTileGroupCount(parallelism);
   storage::DataTable::SetActiveIndirectionArrayCount(parallelism);
 
   // start epoch.

diff --git a/src/common/internal_types.cpp b/src/common/internal_types.cpp
@@ -2967,8 +2967,8 @@ std::string GCVersionTypeToString(GCVersionType type) {
     case GCVersionType::ABORT_UPDATE: {
       return "ABORT_UPDATE";
     }
-    case GCVersionType::ABORT_DELETE: {
-      return "ABORT_DELETE";
+    case GCVersionType::TOMBSTONE: {
+      return "TOMBSTONE";
     }
     case GCVersionType::ABORT_INSERT: {
       return "ABORT_INSERT";
@@ -2997,8 +2997,8 @@ GCVersionType StringToGCVersionType(const std::string &str) {
     return GCVersionType::COMMIT_INS_DEL;
   } else if (upper_str == "ABORT_UPDATE") {
     return GCVersionType::ABORT_UPDATE;
-  } else if (upper_str == "ABORT_DELETE") {
-    return GCVersionType::ABORT_DELETE;
+  } else if (upper_str == "TOMBSTONE") {
+    return GCVersionType::TOMBSTONE;
   } else if (upper_str == "ABORT_INSERT") {
     return GCVersionType::ABORT_INSERT;
   } else if (upper_str == "ABORT_INS_DEL") {

diff --git a/src/concurrency/timestamp_ordering_transaction_manager.cpp b/src/concurrency/timestamp_ordering_transaction_manager.cpp
@@ -709,6 +709,9 @@ ResultType TimestampOrderingTransactionManager::CommitTransaction(
       gc_set->operator[](tile_group_id)[tuple_slot] =
           GCVersionType::COMMIT_DELETE;
 
+      gc_set->operator[](new_version.block)[new_version.offset] =
+          GCVersionType::TOMBSTONE;
+
       log_manager.LogDelete(ItemPointer(tile_group_id, tuple_slot));
 
     } else if (tuple_entry.second == RWType::INSERT) {
@@ -827,9 +830,11 @@ ResultType TimestampOrderingTransactionManager::AbortTransaction(
       // before we unlink the aborted version from version list
       ItemPointer *index_entry_ptr =
           tile_group_header->GetIndirection(tuple_slot);
-      UNUSED_ATTRIBUTE auto res = AtomicUpdateItemPointer(
-          index_entry_ptr, ItemPointer(tile_group_id, tuple_slot));
-      PELOTON_ASSERT(res == true);
+      if (index_entry_ptr) {
+        UNUSED_ATTRIBUTE auto res = AtomicUpdateItemPointer(
+            index_entry_ptr, ItemPointer(tile_group_id, tuple_slot));
+        PELOTON_ASSERT(res == true);
+      }
       //////////////////////////////////////////////////
 
       // we should set the version before releasing the lock.
@@ -875,9 +880,11 @@ ResultType TimestampOrderingTransactionManager::AbortTransaction(
       // before we unlink the aborted version from version list
       ItemPointer *index_entry_ptr =
           tile_group_header->GetIndirection(tuple_slot);
-      UNUSED_ATTRIBUTE auto res = AtomicUpdateItemPointer(
-          index_entry_ptr, ItemPointer(tile_group_id, tuple_slot));
-      PELOTON_ASSERT(res == true);
+      if (index_entry_ptr) {
+        UNUSED_ATTRIBUTE auto res = AtomicUpdateItemPointer(
+            index_entry_ptr, ItemPointer(tile_group_id, tuple_slot));
+        PELOTON_ASSERT(res == true);
+      }
       //////////////////////////////////////////////////
 
       // we should set the version before releasing the lock.
@@ -895,7 +902,7 @@ ResultType TimestampOrderingTransactionManager::AbortTransaction(
 
       // add the version to gc set.
       gc_set->operator[](new_version.block)[new_version.offset] =
-          GCVersionType::ABORT_DELETE;
+          GCVersionType::TOMBSTONE;
 
     } else if (tuple_entry.second == RWType::INSERT) {
       tile_group_header->SetBeginCommitId(tuple_slot, MAX_CID);

diff --git a/src/include/common/internal_types.h b/src/include/common/internal_types.h
@@ -1235,9 +1235,9 @@ enum class GCVersionType {
   COMMIT_DELETE,   // a version that is deleted during txn commit.
   COMMIT_INS_DEL,  // a version that is inserted and deleted during txn commit.
   ABORT_UPDATE,    // a version that is updated during txn abort.
-  ABORT_DELETE,    // a version that is deleted during txn abort.
   ABORT_INSERT,    // a version that is inserted during txn abort.
-  ABORT_INS_DEL,   // a version that is inserted and deleted during txn commit.
+  ABORT_INS_DEL,   // a version that is inserted and deleted during txn abort.
+  TOMBSTONE,       // a version that signifies that the tuple has been deleted.
 };
 std::string GCVersionTypeToString(GCVersionType type);
 GCVersionType StringToGCVersionType(const std::string &str);

diff --git a/src/include/gc/gc_manager.h b/src/include/gc/gc_manager.h
@@ -29,6 +29,7 @@ class TransactionContext;
 
 namespace storage {
 class TileGroup;
+class DataTable;
 }
 
 namespace gc {
@@ -65,10 +66,12 @@ class GCManager {
 
   virtual void StopGC() {}
 
-  virtual ItemPointer ReturnFreeSlot(const oid_t &table_id UNUSED_ATTRIBUTE) {
+  virtual ItemPointer GetRecycledTupleSlot(storage::DataTable *table UNUSED_ATTRIBUTE) {
     return INVALID_ITEMPOINTER;
   }
 
+  virtual void RecycleTupleSlot(const ItemPointer &location UNUSED_ATTRIBUTE) {}
+
   virtual void RegisterTable(const oid_t &table_id UNUSED_ATTRIBUTE) {}
 
   virtual void DeregisterTable(const oid_t &table_id UNUSED_ATTRIBUTE) {}

diff --git a/src/include/storage/data_table.h b/src/include/storage/data_table.h
@@ -303,11 +303,11 @@ class DataTable : public AbstractTable {
                        concurrency::TransactionContext *transaction,
                        ItemPointer **index_entry_ptr);
 
-  inline static size_t GetActiveTileGroupCount() {
+  inline static size_t GetDefaultActiveTileGroupCount() {
     return default_active_tilegroup_count_;
   }
 
-  static void SetActiveTileGroupCount(const size_t active_tile_group_count) {
+  static void SetDefaultActiveTileGroupCount(const size_t active_tile_group_count) {
     default_active_tilegroup_count_ = active_tile_group_count;
   }
 

diff --git a/src/include/storage/tile_group.h b/src/include/storage/tile_group.h
@@ -117,7 +117,6 @@ class TileGroup : public Printable {
 
   // this function is called only when building tile groups for aggregation
   // operations.
-  // FIXME: GC has recycled some of the tuples, so this count is not accurate
   uint32_t GetActiveTupleCount() const;
 
   uint32_t GetAllocatedTupleCount() const { return num_tuple_slots_; }

diff --git a/test/common/internal_types_test.cpp b/test/common/internal_types_test.cpp
@@ -1073,7 +1073,7 @@ TEST_F(InternalTypesTests, GCVersionTypeTest) {
   std::vector<GCVersionType> list = {
       GCVersionType::INVALID,       GCVersionType::COMMIT_UPDATE,
       GCVersionType::COMMIT_DELETE, GCVersionType::COMMIT_INS_DEL,
-      GCVersionType::ABORT_UPDATE,  GCVersionType::ABORT_DELETE,
+      GCVersionType::ABORT_UPDATE,  GCVersionType::TOMBSTONE,
       GCVersionType::ABORT_INSERT,  GCVersionType::ABORT_INS_DEL,
   };
 

diff --git a/test/executor/loader_test.cpp b/test/executor/loader_test.cpp
@@ -133,29 +133,29 @@ TEST_F(LoaderTests, LoadingTest) {
 
   int total_tuple_count = loader_threads_count * tilegroup_count_per_loader * TEST_TUPLES_PER_TILEGROUP;
   int max_cached_tuple_count =
-      TEST_TUPLES_PER_TILEGROUP * storage::DataTable::GetActiveTileGroupCount();
+      TEST_TUPLES_PER_TILEGROUP * storage::DataTable::GetDefaultActiveTileGroupCount();
   int max_unfill_cached_tuple_count =
       (TEST_TUPLES_PER_TILEGROUP - 1) *
-      storage::DataTable::GetActiveTileGroupCount();
+      storage::DataTable::GetDefaultActiveTileGroupCount();
 
   if (total_tuple_count - max_cached_tuple_count <= 0) {
     if (total_tuple_count <= max_unfill_cached_tuple_count) {
-      expected_tile_group_count = storage::DataTable::GetActiveTileGroupCount();
+      expected_tile_group_count = storage::DataTable::GetDefaultActiveTileGroupCount();
     } else {
       expected_tile_group_count =
-          storage::DataTable::GetActiveTileGroupCount() + total_tuple_count -
+          storage::DataTable::GetDefaultActiveTileGroupCount() + total_tuple_count -
           max_unfill_cached_tuple_count;
     }
   } else {
-    int filled_tile_group_count = total_tuple_count / max_cached_tuple_count * storage::DataTable::GetActiveTileGroupCount();
+    int filled_tile_group_count = total_tuple_count / max_cached_tuple_count * storage::DataTable::GetDefaultActiveTileGroupCount();
 
     if (total_tuple_count - filled_tile_group_count * TEST_TUPLES_PER_TILEGROUP - max_unfill_cached_tuple_count <= 0) {
       expected_tile_group_count = filled_tile_group_count +
-                                  storage::DataTable::GetActiveTileGroupCount();
+                                  storage::DataTable::GetDefaultActiveTileGroupCount();
     } else {
       expected_tile_group_count =
           filled_tile_group_count +
-          storage::DataTable::GetActiveTileGroupCount() +
+          storage::DataTable::GetDefaultActiveTileGroupCount() +
           (total_tuple_count - filled_tile_group_count -
            max_unfill_cached_tuple_count);
     }

diff --git a/test/performance/insert_performance_test.cpp b/test/performance/insert_performance_test.cpp
@@ -120,30 +120,30 @@ TEST_F(InsertPerformanceTests, LoadingTest) {
 
   int total_tuple_count = loader_threads_count * tilegroup_count_per_loader * TEST_TUPLES_PER_TILEGROUP;
   int max_cached_tuple_count =
-      TEST_TUPLES_PER_TILEGROUP * storage::DataTable::GetActiveTileGroupCount();
+      TEST_TUPLES_PER_TILEGROUP * storage::DataTable::GetDefaultActiveTileGroupCount();
   int max_unfill_cached_tuple_count =
       (TEST_TUPLES_PER_TILEGROUP - 1) *
-      storage::DataTable::GetActiveTileGroupCount();
+      storage::DataTable::GetDefaultActiveTileGroupCount();
 
   if (total_tuple_count - max_cached_tuple_count <= 0) {
     if (total_tuple_count <= max_unfill_cached_tuple_count) {
-      expected_tile_group_count = storage::DataTable::GetActiveTileGroupCount();
+      expected_tile_group_count = storage::DataTable::GetDefaultActiveTileGroupCount();
     } else {
       expected_tile_group_count =
-          storage::DataTable::GetActiveTileGroupCount() + total_tuple_count -
+          storage::DataTable::GetDefaultActiveTileGroupCount() + total_tuple_count -
           max_unfill_cached_tuple_count;
     }
   } else {
     int filled_tile_group_count = total_tuple_count / max_cached_tuple_count *
-                                  storage::DataTable::GetActiveTileGroupCount();
+                                  storage::DataTable::GetDefaultActiveTileGroupCount();
 
     if (total_tuple_count - filled_tile_group_count * TEST_TUPLES_PER_TILEGROUP - max_unfill_cached_tuple_count <= 0) {
       expected_tile_group_count = filled_tile_group_count +
-                                  storage::DataTable::GetActiveTileGroupCount();
+                                  storage::DataTable::GetDefaultActiveTileGroupCount();
     } else {
       expected_tile_group_count =
           filled_tile_group_count +
-          storage::DataTable::GetActiveTileGroupCount() +
+          storage::DataTable::GetDefaultActiveTileGroupCount() +
           (total_tuple_count - filled_tile_group_count -
            max_unfill_cached_tuple_count);
     }

diff --git a/test/sql/update_sql_test.cpp b/test/sql/update_sql_test.cpp
@@ -299,10 +299,10 @@ TEST_F(UpdateSQLTests, HalloweenProblemTest) {
   // it would have caused a second update on an already updated Tuple.
 
   size_t active_tilegroup_count = 3;
-  storage::DataTable::SetActiveTileGroupCount(active_tilegroup_count);
+  storage::DataTable::SetDefaultActiveTileGroupCount(active_tilegroup_count);
 
   LOG_DEBUG("Active tile group count = %zu",
-            storage::DataTable::GetActiveTileGroupCount());
+            storage::DataTable::GetDefaultActiveTileGroupCount());
   // Create a table first
   LOG_DEBUG("Creating a table...");
   LOG_DEBUG("Query: CREATE TABLE test(a INT, b INT)");
@@ -372,10 +372,10 @@ TEST_F(UpdateSQLTests, HalloweenProblemTestWithPK) {
 
   // active_tilegroup_count set to 3, [Reason: Refer to HalloweenProblemTest]
   size_t active_tilegroup_count = 3;
-  storage::DataTable::SetActiveTileGroupCount(active_tilegroup_count);
+  storage::DataTable::SetDefaultActiveTileGroupCount(active_tilegroup_count);
 
   LOG_DEBUG("Active tile group count = %zu",
-            storage::DataTable::GetActiveTileGroupCount());
+            storage::DataTable::GetDefaultActiveTileGroupCount());
   // Create a table first
   LOG_DEBUG("Creating a table...");
   LOG_DEBUG("Query: CREATE TABLE test(a INT PRIMARY KEY, b INT)");
@@ -469,10 +469,10 @@ TEST_F(UpdateSQLTests, MultiTileGroupUpdateSQLTest) {
 
   // active_tilegroup_count set to 3, [Reason: Refer to HalloweenProblemTest]
   size_t active_tilegroup_count = 3;
-  storage::DataTable::SetActiveTileGroupCount(active_tilegroup_count);
+  storage::DataTable::SetDefaultActiveTileGroupCount(active_tilegroup_count);
 
   LOG_DEBUG("Active tile group count = %zu",
-            storage::DataTable::GetActiveTileGroupCount());
+            storage::DataTable::GetDefaultActiveTileGroupCount());
   // Create a table first
   LOG_DEBUG("Creating a table...");
   LOG_DEBUG("Query: CREATE TABLE test(a INT PRIMARY KEY, b INT)");