From 0343478959561d121a5d32db0ea66fcda753155c Mon Sep 17 00:00:00 2001 From: mbutrovich Date: Mon, 25 Jun 2018 13:56:12 -0400 Subject: [PATCH] Cherry-pick GC fixes from #1349, first part. --- src/common/init.cpp | 2 +- src/common/internal_types.cpp | 8 +++---- ...timestamp_ordering_transaction_manager.cpp | 21 ++++++++++++------- src/include/common/internal_types.h | 4 ++-- src/include/gc/gc_manager.h | 5 ++++- src/include/storage/data_table.h | 4 ++-- src/include/storage/tile_group.h | 1 - test/common/internal_types_test.cpp | 2 +- test/executor/loader_test.cpp | 14 ++++++------- test/performance/insert_performance_test.cpp | 14 ++++++------- test/sql/update_sql_test.cpp | 12 +++++------ 11 files changed, 48 insertions(+), 39 deletions(-) diff --git a/src/common/init.cpp b/src/common/init.cpp index c8b87133211..89f97eaff07 100644 --- 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 index 3b0c20900df..1ebf5a907ea 100644 --- a/src/common/internal_types.cpp +++ b/src/common/internal_types.cpp @@ -3001,8 +3001,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"; @@ -3031,8 +3031,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 index 1deeccd0609..760f86566a8 100644 --- 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 index b34d2971c70..1384fca8a6b 100644 --- a/src/include/common/internal_types.h +++ b/src/include/common/internal_types.h @@ -1236,9 +1236,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 index 1adce28a944..14973fac6bc 100644 --- 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 index 48708b9edb5..dd8e39ec37c 100644 --- a/src/include/storage/data_table.h +++ b/src/include/storage/data_table.h @@ -282,11 +282,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 index ea9218f06a8..7ca48008894 100644 --- 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 index a8ff6b0881c..9aa259ba4dd 100644 --- a/test/common/internal_types_test.cpp +++ b/test/common/internal_types_test.cpp @@ -1071,7 +1071,7 @@ TEST_F(InternalTypesTests, GCVersionTypeTest) { std::vector 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 index 14b88882ff4..8572f0a4ecb 100644 --- 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 index a65be8e7ead..afd5dcfe61e 100644 --- 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 index 6df7faa5903..068f9eb3199 100644 --- 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)");