From 50411cbb03a72e668948f53aa2718cc4d4e41105 Mon Sep 17 00:00:00 2001 From: Wan Shen Lim Date: Mon, 11 Jun 2018 10:38:02 -0400 Subject: [PATCH 01/13] formatting --- src/concurrency/transaction_context.cpp | 1 - src/concurrency/transaction_manager.cpp | 5 +- src/include/concurrency/transaction_context.h | 36 ++++++----- src/include/concurrency/transaction_manager.h | 61 ++++++++----------- 4 files changed, 47 insertions(+), 56 deletions(-) diff --git a/src/concurrency/transaction_context.cpp b/src/concurrency/transaction_context.cpp index d3972b94f05..3389b6001a4 100644 --- a/src/concurrency/transaction_context.cpp +++ b/src/concurrency/transaction_context.cpp @@ -101,7 +101,6 @@ RWType TransactionContext::GetRWType(const ItemPointer &location) { } void TransactionContext::RecordRead(const ItemPointer &location) { - auto rw_set_it = rw_set_.find(location); if (rw_set_it != rw_set_.end()) { UNUSED_ATTRIBUTE RWType rw_type = rw_set_it->second; diff --git a/src/concurrency/transaction_manager.cpp b/src/concurrency/transaction_manager.cpp index d3171213e32..4c7da8aa34e 100644 --- a/src/concurrency/transaction_manager.cpp +++ b/src/concurrency/transaction_manager.cpp @@ -102,8 +102,9 @@ bool TransactionManager::IsOccupied(TransactionContext *const current_txn, const void *position_ptr) { ItemPointer &position = *((ItemPointer *)position_ptr); - auto tile_group_header = - storage::StorageManager::GetInstance()->GetTileGroup(position.block)->GetHeader(); + auto tile_group_header = storage::StorageManager::GetInstance() + ->GetTileGroup(position.block) + ->GetHeader(); auto tuple_id = position.offset; txn_id_t tuple_txn_id = tile_group_header->GetTransactionId(tuple_id); diff --git a/src/include/concurrency/transaction_context.h b/src/include/concurrency/transaction_context.h index dbd717c1031..028de62d702 100644 --- a/src/include/concurrency/transaction_context.h +++ b/src/include/concurrency/transaction_context.h @@ -45,10 +45,10 @@ class TransactionContext : public Printable { public: TransactionContext(const size_t thread_id, const IsolationLevelType isolation, - const cid_t &read_id); + const cid_t &read_id); TransactionContext(const size_t thread_id, const IsolationLevelType isolation, - const cid_t &read_id, const cid_t &commit_id); + const cid_t &read_id, const cid_t &commit_id); /** * @brief Destroys the object. @@ -116,8 +116,9 @@ class TransactionContext : public Printable { * * @return The query strings. */ - inline const std::vector& GetQueryStrings() const { - return query_strings_; } + inline const std::vector &GetQueryStrings() const { + return query_strings_; + } /** * @brief Sets the commit identifier. @@ -132,7 +133,7 @@ class TransactionContext : public Printable { * @param[in] epoch_id The epoch identifier */ inline void SetEpochId(const eid_t epoch_id) { epoch_id_ = epoch_id; } - + /** * @brief Sets the timestamp. * @@ -145,18 +146,18 @@ class TransactionContext : public Printable { * * @param[in] query_string The query string */ - inline void AddQueryString(const char* query_string) { + inline void AddQueryString(const char *query_string) { query_strings_.push_back(std::string(query_string)); } void RecordCreate(oid_t database_oid, oid_t table_oid, oid_t index_oid) { - rw_object_set_.push_back(std::make_tuple(database_oid, table_oid, - index_oid, DDLType::CREATE)); + rw_object_set_.push_back( + std::make_tuple(database_oid, table_oid, index_oid, DDLType::CREATE)); } void RecordDrop(oid_t database_oid, oid_t table_oid, oid_t index_oid) { - rw_object_set_.push_back(std::make_tuple(database_oid, table_oid, - index_oid, DDLType::DROP)); + rw_object_set_.push_back( + std::make_tuple(database_oid, table_oid, index_oid, DDLType::DROP)); } void RecordRead(const ItemPointer &); @@ -262,17 +263,13 @@ class TransactionContext : public Printable { * * @return True if read only, False otherwise. */ - bool IsReadOnly() const { - return read_only_; - } + bool IsReadOnly() const { return read_only_; } /** * @brief mark this context as read only * */ - void SetReadOnly() { - read_only_ = true; - } + void SetReadOnly() { read_only_ = true; } /** * @brief Gets the isolation level. @@ -328,8 +325,8 @@ class TransactionContext : public Printable { ReadWriteSet rw_set_; CreateDropSet rw_object_set_; - /** - * this set contains data location that needs to be gc'd in the transaction. + /** + * this set contains data location that needs to be gc'd in the transaction. */ std::shared_ptr gc_set_; std::shared_ptr gc_object_set_; @@ -344,7 +341,8 @@ class TransactionContext : public Printable { std::unique_ptr on_commit_triggers_; - /** one default transaction is NOT 'read only' unless it is marked 'read only' explicitly*/ + /** one default transaction is NOT 'read only' unless it is marked 'read only' + * explicitly*/ bool read_only_ = false; }; diff --git a/src/include/concurrency/transaction_manager.h b/src/include/concurrency/transaction_manager.h index a1f17533a6d..6112e0e9a87 100644 --- a/src/include/concurrency/transaction_manager.h +++ b/src/include/concurrency/transaction_manager.h @@ -10,7 +10,6 @@ // //===----------------------------------------------------------------------===// - #pragma once #include @@ -58,8 +57,7 @@ class TransactionManager { */ virtual ~TransactionManager() {} - void Init(const ProtocolType protocol, - const IsolationLevelType isolation, + void Init(const ProtocolType protocol, const IsolationLevelType isolation, const ConflictAvoidanceType conflict) { protocol_ = protocol; isolation_level_ = isolation; @@ -74,9 +72,8 @@ class TransactionManager { * * @return True if occupied, False otherwise. */ - bool IsOccupied( - TransactionContext *const current_txn, - const void *position_ptr); + bool IsOccupied(TransactionContext *const current_txn, + const void *position_ptr); /** * @brief Determines if visible. @@ -103,10 +100,9 @@ class TransactionManager { * * @return True if owner, False otherwise. */ - virtual bool IsOwner( - TransactionContext *const current_txn, - const storage::TileGroupHeader *const tile_group_header, - const oid_t &tuple_id) = 0; + virtual bool IsOwner(TransactionContext *const current_txn, + const storage::TileGroupHeader *const tile_group_header, + const oid_t &tuple_id) = 0; /** * This method tests whether any other transaction has owned this version. @@ -117,10 +113,9 @@ class TransactionManager { * * @return True if owned, False otherwise. */ - virtual bool IsOwned( - TransactionContext *const current_txn, - const storage::TileGroupHeader *const tile_group_header, - const oid_t &tuple_id) = 0; + virtual bool IsOwned(TransactionContext *const current_txn, + const storage::TileGroupHeader *const tile_group_header, + const oid_t &tuple_id) = 0; /** * Test whether the current transaction has created this version of the tuple. @@ -132,9 +127,9 @@ class TransactionManager { * @return True if written, False otherwise. */ virtual bool IsWritten( - TransactionContext *const current_txn, - const storage::TileGroupHeader *const tile_group_header, - const oid_t &tuple_id) = 0; + TransactionContext *const current_txn, + const storage::TileGroupHeader *const tile_group_header, + const oid_t &tuple_id) = 0; /** * Test whether it can obtain ownership. @@ -161,7 +156,7 @@ class TransactionManager { */ virtual bool AcquireOwnership( TransactionContext *const current_txn, - const storage::TileGroupHeader *const tile_group_header, + const storage::TileGroupHeader *const tile_group_header, const oid_t &tuple_id) = 0; /** @@ -173,8 +168,8 @@ class TransactionManager { */ virtual void YieldOwnership( TransactionContext *const current_txn, - // const oid_t &tile_group_id, - const storage::TileGroupHeader *const tile_group_header, + // const oid_t &tile_group_id, + const storage::TileGroupHeader *const tile_group_header, const oid_t &tuple_id) = 0; /** @@ -186,14 +181,13 @@ class TransactionManager { * @param index_entry_ptr The index entry pointer */ virtual void PerformInsert(TransactionContext *const current_txn, - const ItemPointer &location, + const ItemPointer &location, ItemPointer *index_entry_ptr = nullptr) = 0; virtual bool PerformRead(TransactionContext *const current_txn, - const ItemPointer &location, - storage::TileGroupHeader *tile_group_header, - bool acquire_ownership) = 0; - + const ItemPointer &location, + storage::TileGroupHeader *tile_group_header, + bool acquire_ownership) = 0; virtual void PerformUpdate(TransactionContext *const current_txn, const ItemPointer &old_location, @@ -215,7 +209,8 @@ class TransactionManager { * @param current_txn The current transaction * @param[in] result The result */ - void SetTransactionResult(TransactionContext *const current_txn, const ResultType result) { + void SetTransactionResult(TransactionContext *const current_txn, + const ResultType result) { current_txn->SetResult(result); } @@ -223,9 +218,9 @@ class TransactionManager { return BeginTransaction(0, type, false); } - TransactionContext *BeginTransaction(const size_t thread_id = 0, - const IsolationLevelType type = isolation_level_, - bool read_only = false); + TransactionContext *BeginTransaction( + const size_t thread_id = 0, + const IsolationLevelType type = isolation_level_, bool read_only = false); /** * @brief Ends a transaction. @@ -245,7 +240,8 @@ class TransactionManager { virtual ResultType CommitTransaction( TransactionContext *const current_txn) = 0; - virtual ResultType AbortTransaction(TransactionContext *const current_txn) = 0; + virtual ResultType AbortTransaction( + TransactionContext *const current_txn) = 0; /** * This function generates the maximum commit id of committed transactions. @@ -263,15 +259,12 @@ class TransactionManager { * * @return The isolation level. */ - IsolationLevelType GetIsolationLevel() { - return isolation_level_; - } + IsolationLevelType GetIsolationLevel() { return isolation_level_; } protected: static ProtocolType protocol_; static IsolationLevelType isolation_level_; static ConflictAvoidanceType conflict_avoidance_; - }; } // namespace storage } // namespace peloton From e506f17d043581af0e77369899edf01b0174bd94 Mon Sep 17 00:00:00 2001 From: Wan Shen Lim Date: Mon, 11 Jun 2018 10:42:02 -0400 Subject: [PATCH 02/13] remove default read_only for transactions (#1396:1-3) --- src/concurrency/transaction_context.cpp | 12 +++++---- src/concurrency/transaction_manager.cpp | 13 ++++------ src/include/concurrency/transaction_context.h | 26 ++++++++----------- src/include/concurrency/transaction_manager.h | 12 ++++++--- 4 files changed, 32 insertions(+), 31 deletions(-) diff --git a/src/concurrency/transaction_context.cpp b/src/concurrency/transaction_context.cpp index 3389b6001a4..1168750147a 100644 --- a/src/concurrency/transaction_context.cpp +++ b/src/concurrency/transaction_context.cpp @@ -48,24 +48,26 @@ namespace concurrency { * i : insert */ -TransactionContext::TransactionContext(const size_t thread_id, +TransactionContext::TransactionContext(bool read_only, const size_t thread_id, const IsolationLevelType isolation, const cid_t &read_id) { - Init(thread_id, isolation, read_id); + Init(read_only, thread_id, isolation, read_id); } -TransactionContext::TransactionContext(const size_t thread_id, +TransactionContext::TransactionContext(bool read_only, const size_t thread_id, const IsolationLevelType isolation, const cid_t &read_id, const cid_t &commit_id) { - Init(thread_id, isolation, read_id, commit_id); + Init(read_only, thread_id, isolation, read_id, commit_id); } TransactionContext::~TransactionContext() {} -void TransactionContext::Init(const size_t thread_id, +void TransactionContext::Init(bool read_only, const size_t thread_id, const IsolationLevelType isolation, const cid_t &read_id, const cid_t &commit_id) { + read_only_ = read_only; + read_id_ = read_id; // commit id can be set at a transaction's commit phase. diff --git a/src/concurrency/transaction_manager.cpp b/src/concurrency/transaction_manager.cpp index 4c7da8aa34e..813155ddc7d 100644 --- a/src/concurrency/transaction_manager.cpp +++ b/src/concurrency/transaction_manager.cpp @@ -32,7 +32,7 @@ ConflictAvoidanceType TransactionManager::conflict_avoidance_ = ConflictAvoidanceType::ABORT; TransactionContext *TransactionManager::BeginTransaction( - const size_t thread_id, const IsolationLevelType type, bool read_only) { + bool read_only, const size_t thread_id, const IsolationLevelType type) { TransactionContext *txn = nullptr; if (type == IsolationLevelType::SNAPSHOT) { @@ -45,9 +45,10 @@ TransactionContext *TransactionManager::BeginTransaction( cid_t commit_id = EpochManagerFactory::GetInstance().EnterEpoch( thread_id, TimestampType::COMMIT); - txn = new TransactionContext(thread_id, type, read_id, commit_id); + txn = new TransactionContext(read_only, thread_id, type, read_id, + commit_id); } else { - txn = new TransactionContext(thread_id, type, read_id); + txn = new TransactionContext(read_only, thread_id, type, read_id); } } else { @@ -58,11 +59,7 @@ TransactionContext *TransactionManager::BeginTransaction( // transaction processing with decentralized epoch manager cid_t read_id = EpochManagerFactory::GetInstance().EnterEpoch( thread_id, TimestampType::READ); - txn = new TransactionContext(thread_id, type, read_id); - } - - if (read_only) { - txn->SetReadOnly(); + txn = new TransactionContext(read_only, thread_id, type, read_id); } txn->SetTimestamp(function::DateFunctions::Now()); diff --git a/src/include/concurrency/transaction_context.h b/src/include/concurrency/transaction_context.h index 028de62d702..4645855b17e 100644 --- a/src/include/concurrency/transaction_context.h +++ b/src/include/concurrency/transaction_context.h @@ -44,11 +44,12 @@ class TransactionContext : public Printable { TransactionContext(TransactionContext const &) = delete; public: - TransactionContext(const size_t thread_id, const IsolationLevelType isolation, - const cid_t &read_id); + TransactionContext(bool read_only, const size_t thread_id, + const IsolationLevelType isolation, const cid_t &read_id); - TransactionContext(const size_t thread_id, const IsolationLevelType isolation, - const cid_t &read_id, const cid_t &commit_id); + TransactionContext(bool read_only, const size_t thread_id, + const IsolationLevelType isolation, const cid_t &read_id, + const cid_t &commit_id); /** * @brief Destroys the object. @@ -56,13 +57,14 @@ class TransactionContext : public Printable { ~TransactionContext(); private: - void Init(const size_t thread_id, const IsolationLevelType isolation, - const cid_t &read_id) { - Init(thread_id, isolation, read_id, read_id); + void Init(bool read_only, const size_t thread_id, + const IsolationLevelType isolation, const cid_t &read_id) { + Init(read_only, thread_id, isolation, read_id, read_id); } - void Init(const size_t thread_id, const IsolationLevelType isolation, - const cid_t &read_id, const cid_t &commit_id); + void Init(bool read_only, const size_t thread_id, + const IsolationLevelType isolation, const cid_t &read_id, + const cid_t &commit_id); public: //===--------------------------------------------------------------------===// @@ -265,12 +267,6 @@ class TransactionContext : public Printable { */ bool IsReadOnly() const { return read_only_; } - /** - * @brief mark this context as read only - * - */ - void SetReadOnly() { read_only_ = true; } - /** * @brief Gets the isolation level. * diff --git a/src/include/concurrency/transaction_manager.h b/src/include/concurrency/transaction_manager.h index 6112e0e9a87..b52ed4e7916 100644 --- a/src/include/concurrency/transaction_manager.h +++ b/src/include/concurrency/transaction_manager.h @@ -214,13 +214,19 @@ class TransactionManager { current_txn->SetResult(result); } + TransactionContext *BeginTransaction() { return BeginTransaction(false); } + + TransactionContext *BeginTransaction(const size_t thread_id) { + return BeginTransaction(false, thread_id, isolation_level_); + } + TransactionContext *BeginTransaction(const IsolationLevelType type) { - return BeginTransaction(0, type, false); + return BeginTransaction(false, 0, type); } TransactionContext *BeginTransaction( - const size_t thread_id = 0, - const IsolationLevelType type = isolation_level_, bool read_only = false); + bool read_only, const size_t thread_id = 0, + const IsolationLevelType type = isolation_level_); /** * @brief Ends a transaction. From e0a93485b433d3a95e28ff76748b1b2ef8d85668 Mon Sep 17 00:00:00 2001 From: Wan Shen Lim Date: Mon, 11 Jun 2018 14:50:41 -0400 Subject: [PATCH 03/13] formatting --- ...timestamp_ordering_transaction_manager.cpp | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/src/concurrency/timestamp_ordering_transaction_manager.cpp b/src/concurrency/timestamp_ordering_transaction_manager.cpp index eb92b2398cb..f3886ce9c4e 100644 --- a/src/concurrency/timestamp_ordering_transaction_manager.cpp +++ b/src/concurrency/timestamp_ordering_transaction_manager.cpp @@ -171,10 +171,9 @@ void TimestampOrderingTransactionManager::YieldOwnership( tile_group_header->SetTransactionId(tuple_id, INITIAL_TXN_ID); } -bool TimestampOrderingTransactionManager::PerformRead(TransactionContext *const current_txn, - const ItemPointer &read_location, - storage::TileGroupHeader *tile_group_header, - bool acquire_ownership) { +bool TimestampOrderingTransactionManager::PerformRead( + TransactionContext *const current_txn, const ItemPointer &read_location, + storage::TileGroupHeader *tile_group_header, bool acquire_ownership) { ItemPointer location = read_location; ////////////////////////////////////////////////////////// @@ -374,7 +373,8 @@ void TimestampOrderingTransactionManager::PerformInsert( oid_t tuple_id = location.offset; auto storage_manager = storage::StorageManager::GetInstance(); - auto tile_group_header = storage_manager->GetTileGroup(tile_group_id)->GetHeader(); + auto tile_group_header = + storage_manager->GetTileGroup(tile_group_id)->GetHeader(); auto transaction_id = current_txn->GetTransactionId(); // check MVCC info @@ -420,9 +420,8 @@ void TimestampOrderingTransactionManager::PerformUpdate( // version. PELOTON_ASSERT(tile_group_header->GetTransactionId(old_location.offset) == transaction_id); - PELOTON_ASSERT( - tile_group_header->GetPrevItemPointer(old_location.offset).IsNull() == - true); + PELOTON_ASSERT(tile_group_header->GetPrevItemPointer(old_location.offset) + .IsNull() == true); // check whether the new version is empty. PELOTON_ASSERT(new_tile_group_header->GetTransactionId(new_location.offset) == @@ -529,9 +528,8 @@ void TimestampOrderingTransactionManager::PerformDelete( PELOTON_ASSERT(tile_group_header->GetTransactionId(old_location.offset) == transaction_id); // we must be deleting the latest version. - PELOTON_ASSERT( - tile_group_header->GetPrevItemPointer(old_location.offset).IsNull() == - true); + PELOTON_ASSERT(tile_group_header->GetPrevItemPointer(old_location.offset) + .IsNull() == true); // check whether the new version is empty. PELOTON_ASSERT(new_tile_group_header->GetTransactionId(new_location.offset) == @@ -588,7 +586,8 @@ void TimestampOrderingTransactionManager::PerformDelete( oid_t tuple_id = location.offset; auto storage_manager = storage::StorageManager::GetInstance(); - auto tile_group_header = storage_manager->GetTileGroup(tile_group_id)->GetHeader(); + auto tile_group_header = + storage_manager->GetTileGroup(tile_group_id)->GetHeader(); PELOTON_ASSERT(tile_group_header->GetTransactionId(tuple_id) == current_txn->GetTransactionId()); @@ -660,7 +659,8 @@ ResultType TimestampOrderingTransactionManager::CommitTransaction( oid_t tile_group_id = item_ptr.block; oid_t tuple_slot = item_ptr.offset; - auto tile_group_header = storage_manager->GetTileGroup(tile_group_id)->GetHeader(); + auto tile_group_header = + storage_manager->GetTileGroup(tile_group_id)->GetHeader(); if (tuple_entry.second == RWType::READ_OWN) { // A read operation has acquired ownership but hasn't done any further @@ -811,7 +811,8 @@ ResultType TimestampOrderingTransactionManager::AbortTransaction( ItemPointer item_ptr = tuple_entry.first; oid_t tile_group_id = item_ptr.block; oid_t tuple_slot = item_ptr.offset; - auto tile_group_header = storage_manager->GetTileGroup(tile_group_id)->GetHeader(); + auto tile_group_header = + storage_manager->GetTileGroup(tile_group_id)->GetHeader(); if (tuple_entry.second == RWType::READ_OWN) { // A read operation has acquired ownership but hasn't done any further From 8b1531b98f0d40bcd13edad89de8f7fb2cac0871 Mon Sep 17 00:00:00 2001 From: Wan Shen Lim Date: Mon, 11 Jun 2018 14:51:58 -0400 Subject: [PATCH 04/13] treat txn w/ no modifying queries as read-only (#1396:4) --- .../timestamp_ordering_transaction_manager.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/concurrency/timestamp_ordering_transaction_manager.cpp b/src/concurrency/timestamp_ordering_transaction_manager.cpp index f3886ce9c4e..d3ece6e2cb9 100644 --- a/src/concurrency/timestamp_ordering_transaction_manager.cpp +++ b/src/concurrency/timestamp_ordering_transaction_manager.cpp @@ -623,6 +623,14 @@ ResultType TimestampOrderingTransactionManager::CommitTransaction( //// handle other isolation levels ////////////////////////////////////////////////////////// + auto &rw_set = current_txn->GetReadWriteSet(); + + // if no modifying queries, treat as read-only + if (rw_set.empty()) { + EndTransaction(current_txn); + return ResultType::SUCCESS; + } + auto storage_manager = storage::StorageManager::GetInstance(); auto &log_manager = logging::LogManager::GetInstance(); @@ -631,7 +639,6 @@ ResultType TimestampOrderingTransactionManager::CommitTransaction( // generate transaction id. cid_t end_commit_id = current_txn->GetCommitId(); - auto &rw_set = current_txn->GetReadWriteSet(); auto &rw_object_set = current_txn->GetCreateDropSet(); auto gc_set = current_txn->GetGCSetPtr(); From 467834ca5cf6ae3182755220c9a0b4906cf85cbc Mon Sep 17 00:00:00 2001 From: Wan Shen Lim Date: Mon, 11 Jun 2018 14:58:37 -0400 Subject: [PATCH 05/13] mark single-statement select txn read-only (#1395) --- src/traffic_cop/traffic_cop.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/traffic_cop/traffic_cop.cpp b/src/traffic_cop/traffic_cop.cpp index 7bfffebb4c0..5b7bce864f2 100644 --- a/src/traffic_cop/traffic_cop.cpp +++ b/src/traffic_cop/traffic_cop.cpp @@ -164,7 +164,9 @@ executor::ExecutionResult TrafficCop::ExecuteHelper( // new txn, reset result status curr_state.second = ResultType::SUCCESS; single_statement_txn_ = true; - txn = txn_manager.BeginTransaction(thread_id); + // txn is read-only for single-statement select + bool read_only = statement_->GetQueryType() == QueryType::QUERY_SELECT; + txn = txn_manager.BeginTransaction(read_only, thread_id); tcop_txn_state_.emplace(txn, ResultType::SUCCESS); } From b2612aa3a69bacc57342662bfb7be259ba869f05 Mon Sep 17 00:00:00 2001 From: Wan Shen Lim Date: Mon, 11 Jun 2018 16:11:58 -0400 Subject: [PATCH 06/13] formatting --- test/include/concurrency/testing_transaction_util.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/include/concurrency/testing_transaction_util.h b/test/include/concurrency/testing_transaction_util.h index d7bb919a0f3..70cddc61f20 100644 --- a/test/include/concurrency/testing_transaction_util.h +++ b/test/include/concurrency/testing_transaction_util.h @@ -242,7 +242,8 @@ class TransactionThread { if (cur_seq == 0) { if (schedule->declared_ro == true) { /** starts a read only transaction*/ - txn = txn_manager->BeginTransaction(0, IsolationLevelType::SNAPSHOT, true); + txn = txn_manager->BeginTransaction(0, IsolationLevelType::SNAPSHOT, + true); } else { txn = txn_manager->BeginTransaction(); } @@ -354,7 +355,6 @@ class TransactionScheduler { table(datatable_), time(0), concurrent(false) { - for (size_t i = 0; i < num_txn; i++) { if (read_only_.find(i) != read_only_.end()) { schedules.emplace_back(i, true); From 3395f6c47fcbb5dcb57bf5e4664ea3f2e82315b7 Mon Sep 17 00:00:00 2001 From: Wan Shen Lim Date: Mon, 11 Jun 2018 16:13:13 -0400 Subject: [PATCH 07/13] reorder non-default parameters in TransactionThread::ExecuteNext --- test/include/concurrency/testing_transaction_util.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/include/concurrency/testing_transaction_util.h b/test/include/concurrency/testing_transaction_util.h index 70cddc61f20..14457503d6a 100644 --- a/test/include/concurrency/testing_transaction_util.h +++ b/test/include/concurrency/testing_transaction_util.h @@ -242,8 +242,8 @@ class TransactionThread { if (cur_seq == 0) { if (schedule->declared_ro == true) { /** starts a read only transaction*/ - txn = txn_manager->BeginTransaction(0, IsolationLevelType::SNAPSHOT, - true); + txn = txn_manager->BeginTransaction(true, 0, + IsolationLevelType::SNAPSHOT); } else { txn = txn_manager->BeginTransaction(); } From 6787337ef21bd817221549128e8ba7437ae759ca Mon Sep 17 00:00:00 2001 From: Wan Shen Lim Date: Tue, 12 Jun 2018 17:43:02 -0400 Subject: [PATCH 08/13] add LOG_TRACE when ending transaction early --- src/concurrency/timestamp_ordering_transaction_manager.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/concurrency/timestamp_ordering_transaction_manager.cpp b/src/concurrency/timestamp_ordering_transaction_manager.cpp index d3ece6e2cb9..923587bd4cc 100644 --- a/src/concurrency/timestamp_ordering_transaction_manager.cpp +++ b/src/concurrency/timestamp_ordering_transaction_manager.cpp @@ -627,6 +627,7 @@ ResultType TimestampOrderingTransactionManager::CommitTransaction( // if no modifying queries, treat as read-only if (rw_set.empty()) { + LOG_TRACE("Empty RW set, ending transaction."); EndTransaction(current_txn); return ResultType::SUCCESS; } From bd5ac57dec286819a26f5475490794ff79a5c607 Mon Sep 17 00:00:00 2001 From: Wan Shen Lim Date: Tue, 12 Jun 2018 17:43:25 -0400 Subject: [PATCH 09/13] set statement in copy_test --- test/executor/copy_test.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/test/executor/copy_test.cpp b/test/executor/copy_test.cpp index 9b9291f4111..86d7252525c 100644 --- a/test/executor/copy_test.cpp +++ b/test/executor/copy_test.cpp @@ -93,6 +93,7 @@ TEST_F(CopyTests, Copying) { std::vector result; TestingSQLUtil::counter_.store(1); + traffic_cop.SetStatement(statement); executor::ExecutionResult status = traffic_cop.ExecuteHelper( statement->GetPlanTree(), params, result, result_format); From 9f8acef0db09f20a858ce56d42f3e52318dc3417 Mon Sep 17 00:00:00 2001 From: Wan Shen Lim Date: Mon, 18 Jun 2018 12:27:01 -0400 Subject: [PATCH 10/13] Use is_written_ flag instead of rwset in commit --- .../timestamp_ordering_transaction_manager.cpp | 9 ++++----- src/concurrency/transaction_context.cpp | 1 + src/include/concurrency/transaction_context.h | 7 +++++++ 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/concurrency/timestamp_ordering_transaction_manager.cpp b/src/concurrency/timestamp_ordering_transaction_manager.cpp index 0ea4e665ddd..62c78af79d9 100644 --- a/src/concurrency/timestamp_ordering_transaction_manager.cpp +++ b/src/concurrency/timestamp_ordering_transaction_manager.cpp @@ -623,15 +623,14 @@ ResultType TimestampOrderingTransactionManager::CommitTransaction( //// handle other isolation levels ////////////////////////////////////////////////////////// - auto &rw_set = current_txn->GetReadWriteSet(); - - // if no modifying queries, treat as read-only - if (rw_set.empty()) { - LOG_TRACE("Empty RW set, ending transaction."); + if (!current_txn->IsWritten()) { + LOG_TRACE("Transaction not yet written, ending transaction."); EndTransaction(current_txn); return ResultType::SUCCESS; } + auto &rw_set = current_txn->GetReadWriteSet(); + auto storage_manager = storage::StorageManager::GetInstance(); auto &log_manager = logging::LogManager::GetInstance(); diff --git a/src/concurrency/transaction_context.cpp b/src/concurrency/transaction_context.cpp index 6feee86b918..58b906e0db8 100644 --- a/src/concurrency/transaction_context.cpp +++ b/src/concurrency/transaction_context.cpp @@ -112,6 +112,7 @@ void TransactionContext::RecordReadOwn(const ItemPointer &location) { (rw_set_[location] != RWType::DELETE && rw_set_[location] != RWType::INS_DEL)); rw_set_[location] = RWType::READ_OWN; + is_written_ = true; } void TransactionContext::RecordUpdate(const ItemPointer &location) { diff --git a/src/include/concurrency/transaction_context.h b/src/include/concurrency/transaction_context.h index d7dce2134f9..b8f81cc40e8 100644 --- a/src/include/concurrency/transaction_context.h +++ b/src/include/concurrency/transaction_context.h @@ -266,6 +266,13 @@ class TransactionContext : public Printable { */ bool IsReadOnly() const { return read_only_; } + /** + * @brief Determines if already written. + * + * @return True if already written, False otherwise. + */ + bool IsWritten() const { return is_written_; } + /** * @brief Gets the isolation level. * From 5739f18b0c4b4f380490a81b3b00a20e14f733b7 Mon Sep 17 00:00:00 2001 From: Wan Shen Lim Date: Tue, 26 Jun 2018 13:31:17 -0400 Subject: [PATCH 11/13] undo formatting changes --- ...timestamp_ordering_transaction_manager.cpp | 26 ++++----- src/concurrency/transaction_manager.cpp | 5 +- src/include/concurrency/transaction_context.h | 28 +++++----- src/include/concurrency/transaction_manager.h | 55 +++++++++++-------- .../concurrency/testing_transaction_util.h | 1 + 5 files changed, 61 insertions(+), 54 deletions(-) diff --git a/src/concurrency/timestamp_ordering_transaction_manager.cpp b/src/concurrency/timestamp_ordering_transaction_manager.cpp index f4f834ef0be..587abe00e06 100644 --- a/src/concurrency/timestamp_ordering_transaction_manager.cpp +++ b/src/concurrency/timestamp_ordering_transaction_manager.cpp @@ -148,9 +148,10 @@ void TimestampOrderingTransactionManager::YieldOwnership( tile_group_header->SetTransactionId(tuple_id, INITIAL_TXN_ID); } -bool TimestampOrderingTransactionManager::PerformRead( - TransactionContext *const current_txn, const ItemPointer &read_location, - storage::TileGroupHeader *tile_group_header, bool acquire_ownership) { +bool TimestampOrderingTransactionManager::PerformRead(TransactionContext *const current_txn, + const ItemPointer &read_location, + storage::TileGroupHeader *tile_group_header, + bool acquire_ownership) { ItemPointer location = read_location; ////////////////////////////////////////////////////////// @@ -346,8 +347,7 @@ void TimestampOrderingTransactionManager::PerformInsert( oid_t tuple_id = location.offset; auto storage_manager = storage::StorageManager::GetInstance(); - auto tile_group_header = - storage_manager->GetTileGroup(tile_group_id)->GetHeader(); + auto tile_group_header = storage_manager->GetTileGroup(tile_group_id)->GetHeader(); auto transaction_id = current_txn->GetTransactionId(); // check MVCC info @@ -391,8 +391,9 @@ void TimestampOrderingTransactionManager::PerformUpdate( // version. PELOTON_ASSERT(tile_group_header->GetTransactionId(old_location.offset) == transaction_id); - PELOTON_ASSERT(tile_group_header->GetPrevItemPointer(old_location.offset) - .IsNull() == true); + PELOTON_ASSERT( + tile_group_header->GetPrevItemPointer(old_location.offset).IsNull() == + true); // check whether the new version is empty. PELOTON_ASSERT(new_tile_group_header->GetTransactionId(new_location.offset) == @@ -496,8 +497,9 @@ void TimestampOrderingTransactionManager::PerformDelete( PELOTON_ASSERT(tile_group_header->GetTransactionId(old_location.offset) == transaction_id); // we must be deleting the latest version. - PELOTON_ASSERT(tile_group_header->GetPrevItemPointer(old_location.offset) - .IsNull() == true); + PELOTON_ASSERT( + tile_group_header->GetPrevItemPointer(old_location.offset).IsNull() == + true); // check whether the new version is empty. PELOTON_ASSERT(new_tile_group_header->GetTransactionId(new_location.offset) == @@ -552,8 +554,7 @@ void TimestampOrderingTransactionManager::PerformDelete( oid_t tuple_id = location.offset; auto storage_manager = storage::StorageManager::GetInstance(); - auto tile_group_header = - storage_manager->GetTileGroup(tile_group_id)->GetHeader(); + auto tile_group_header = storage_manager->GetTileGroup(tile_group_id)->GetHeader(); PELOTON_ASSERT(tile_group_header->GetTransactionId(tuple_id) == current_txn->GetTransactionId()); @@ -595,8 +596,6 @@ ResultType TimestampOrderingTransactionManager::CommitTransaction( return ResultType::SUCCESS; } - auto &rw_set = current_txn->GetReadWriteSet(); - auto storage_manager = storage::StorageManager::GetInstance(); auto &log_manager = logging::LogManager::GetInstance(); @@ -605,6 +604,7 @@ ResultType TimestampOrderingTransactionManager::CommitTransaction( // generate transaction id. cid_t end_commit_id = current_txn->GetCommitId(); + auto &rw_set = current_txn->GetReadWriteSet(); auto &rw_object_set = current_txn->GetCreateDropSet(); auto gc_set = current_txn->GetGCSetPtr(); diff --git a/src/concurrency/transaction_manager.cpp b/src/concurrency/transaction_manager.cpp index 813155ddc7d..e2f9bcbccfb 100644 --- a/src/concurrency/transaction_manager.cpp +++ b/src/concurrency/transaction_manager.cpp @@ -99,9 +99,8 @@ bool TransactionManager::IsOccupied(TransactionContext *const current_txn, const void *position_ptr) { ItemPointer &position = *((ItemPointer *)position_ptr); - auto tile_group_header = storage::StorageManager::GetInstance() - ->GetTileGroup(position.block) - ->GetHeader(); + auto tile_group_header = + storage::StorageManager::GetInstance()->GetTileGroup(position.block)->GetHeader(); auto tuple_id = position.offset; txn_id_t tuple_txn_id = tile_group_header->GetTransactionId(tuple_id); diff --git a/src/include/concurrency/transaction_context.h b/src/include/concurrency/transaction_context.h index d6d673341d0..c23ab1d59c9 100644 --- a/src/include/concurrency/transaction_context.h +++ b/src/include/concurrency/transaction_context.h @@ -118,9 +118,8 @@ class TransactionContext : public Printable { * * @return The query strings. */ - inline const std::vector &GetQueryStrings() const { - return query_strings_; - } + inline const std::vector& GetQueryStrings() const { + return query_strings_; } /** * @brief Sets the commit identifier. @@ -135,7 +134,7 @@ class TransactionContext : public Printable { * @param[in] epoch_id The epoch identifier */ inline void SetEpochId(const eid_t epoch_id) { epoch_id_ = epoch_id; } - + /** * @brief Sets the timestamp. * @@ -148,18 +147,18 @@ class TransactionContext : public Printable { * * @param[in] query_string The query string */ - inline void AddQueryString(const char *query_string) { + inline void AddQueryString(const char* query_string) { query_strings_.push_back(std::string(query_string)); } void RecordCreate(oid_t database_oid, oid_t table_oid, oid_t index_oid) { - rw_object_set_.push_back( - std::make_tuple(database_oid, table_oid, index_oid, DDLType::CREATE)); + rw_object_set_.push_back(std::make_tuple(database_oid, table_oid, + index_oid, DDLType::CREATE)); } void RecordDrop(oid_t database_oid, oid_t table_oid, oid_t index_oid) { - rw_object_set_.push_back( - std::make_tuple(database_oid, table_oid, index_oid, DDLType::DROP)); + rw_object_set_.push_back(std::make_tuple(database_oid, table_oid, + index_oid, DDLType::DROP)); } void RecordReadOwn(const ItemPointer &); @@ -262,7 +261,9 @@ class TransactionContext : public Printable { * * @return True if read only, False otherwise. */ - bool IsReadOnly() const { return read_only_; } + bool IsReadOnly() const { + return read_only_; + } /** * @brief Determines if already written. @@ -325,8 +326,8 @@ class TransactionContext : public Printable { ReadWriteSet rw_set_; CreateDropSet rw_object_set_; - /** - * this set contains data location that needs to be gc'd in the transaction. + /** + * this set contains data location that needs to be gc'd in the transaction. */ std::shared_ptr gc_set_; std::shared_ptr gc_object_set_; @@ -340,8 +341,7 @@ class TransactionContext : public Printable { std::unique_ptr on_commit_triggers_; - /** one default transaction is NOT 'read only' unless it is marked 'read only' - * explicitly*/ + /** one default transaction is NOT 'read only' unless it is marked 'read only' explicitly*/ bool read_only_ = false; }; diff --git a/src/include/concurrency/transaction_manager.h b/src/include/concurrency/transaction_manager.h index b52ed4e7916..e15eb4b6d33 100644 --- a/src/include/concurrency/transaction_manager.h +++ b/src/include/concurrency/transaction_manager.h @@ -10,6 +10,7 @@ // //===----------------------------------------------------------------------===// + #pragma once #include @@ -57,7 +58,8 @@ class TransactionManager { */ virtual ~TransactionManager() {} - void Init(const ProtocolType protocol, const IsolationLevelType isolation, + void Init(const ProtocolType protocol, + const IsolationLevelType isolation, const ConflictAvoidanceType conflict) { protocol_ = protocol; isolation_level_ = isolation; @@ -72,8 +74,9 @@ class TransactionManager { * * @return True if occupied, False otherwise. */ - bool IsOccupied(TransactionContext *const current_txn, - const void *position_ptr); + bool IsOccupied( + TransactionContext *const current_txn, + const void *position_ptr); /** * @brief Determines if visible. @@ -100,9 +103,10 @@ class TransactionManager { * * @return True if owner, False otherwise. */ - virtual bool IsOwner(TransactionContext *const current_txn, - const storage::TileGroupHeader *const tile_group_header, - const oid_t &tuple_id) = 0; + virtual bool IsOwner( + TransactionContext *const current_txn, + const storage::TileGroupHeader *const tile_group_header, + const oid_t &tuple_id) = 0; /** * This method tests whether any other transaction has owned this version. @@ -113,9 +117,10 @@ class TransactionManager { * * @return True if owned, False otherwise. */ - virtual bool IsOwned(TransactionContext *const current_txn, - const storage::TileGroupHeader *const tile_group_header, - const oid_t &tuple_id) = 0; + virtual bool IsOwned( + TransactionContext *const current_txn, + const storage::TileGroupHeader *const tile_group_header, + const oid_t &tuple_id) = 0; /** * Test whether the current transaction has created this version of the tuple. @@ -127,9 +132,9 @@ class TransactionManager { * @return True if written, False otherwise. */ virtual bool IsWritten( - TransactionContext *const current_txn, - const storage::TileGroupHeader *const tile_group_header, - const oid_t &tuple_id) = 0; + TransactionContext *const current_txn, + const storage::TileGroupHeader *const tile_group_header, + const oid_t &tuple_id) = 0; /** * Test whether it can obtain ownership. @@ -156,7 +161,7 @@ class TransactionManager { */ virtual bool AcquireOwnership( TransactionContext *const current_txn, - const storage::TileGroupHeader *const tile_group_header, + const storage::TileGroupHeader *const tile_group_header, const oid_t &tuple_id) = 0; /** @@ -168,8 +173,8 @@ class TransactionManager { */ virtual void YieldOwnership( TransactionContext *const current_txn, - // const oid_t &tile_group_id, - const storage::TileGroupHeader *const tile_group_header, + // const oid_t &tile_group_id, + const storage::TileGroupHeader *const tile_group_header, const oid_t &tuple_id) = 0; /** @@ -181,13 +186,14 @@ class TransactionManager { * @param index_entry_ptr The index entry pointer */ virtual void PerformInsert(TransactionContext *const current_txn, - const ItemPointer &location, + const ItemPointer &location, ItemPointer *index_entry_ptr = nullptr) = 0; virtual bool PerformRead(TransactionContext *const current_txn, - const ItemPointer &location, - storage::TileGroupHeader *tile_group_header, - bool acquire_ownership) = 0; + const ItemPointer &location, + storage::TileGroupHeader *tile_group_header, + bool acquire_ownership) = 0; + virtual void PerformUpdate(TransactionContext *const current_txn, const ItemPointer &old_location, @@ -209,8 +215,7 @@ class TransactionManager { * @param current_txn The current transaction * @param[in] result The result */ - void SetTransactionResult(TransactionContext *const current_txn, - const ResultType result) { + void SetTransactionResult(TransactionContext *const current_txn, const ResultType result) { current_txn->SetResult(result); } @@ -246,8 +251,7 @@ class TransactionManager { virtual ResultType CommitTransaction( TransactionContext *const current_txn) = 0; - virtual ResultType AbortTransaction( - TransactionContext *const current_txn) = 0; + virtual ResultType AbortTransaction(TransactionContext *const current_txn) = 0; /** * This function generates the maximum commit id of committed transactions. @@ -265,12 +269,15 @@ class TransactionManager { * * @return The isolation level. */ - IsolationLevelType GetIsolationLevel() { return isolation_level_; } + IsolationLevelType GetIsolationLevel() { + return isolation_level_; + } protected: static ProtocolType protocol_; static IsolationLevelType isolation_level_; static ConflictAvoidanceType conflict_avoidance_; + }; } // namespace storage } // namespace peloton diff --git a/test/include/concurrency/testing_transaction_util.h b/test/include/concurrency/testing_transaction_util.h index 14457503d6a..4fe3cb9df79 100644 --- a/test/include/concurrency/testing_transaction_util.h +++ b/test/include/concurrency/testing_transaction_util.h @@ -355,6 +355,7 @@ class TransactionScheduler { table(datatable_), time(0), concurrent(false) { + for (size_t i = 0; i < num_txn; i++) { if (read_only_.find(i) != read_only_.end()) { schedules.emplace_back(i, true); From 61fd663bfa97e765c7214e33e6033252824fbd63 Mon Sep 17 00:00:00 2001 From: Wan Shen Lim Date: Wed, 27 Jun 2018 15:38:54 -0400 Subject: [PATCH 12/13] move early txn ending to after GC --- .../timestamp_ordering_transaction_manager.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/concurrency/timestamp_ordering_transaction_manager.cpp b/src/concurrency/timestamp_ordering_transaction_manager.cpp index 587abe00e06..119f48ef25a 100644 --- a/src/concurrency/timestamp_ordering_transaction_manager.cpp +++ b/src/concurrency/timestamp_ordering_transaction_manager.cpp @@ -590,12 +590,6 @@ ResultType TimestampOrderingTransactionManager::CommitTransaction( //// handle other isolation levels ////////////////////////////////////////////////////////// - if (!current_txn->IsWritten()) { - LOG_TRACE("Transaction not yet written, ending transaction."); - EndTransaction(current_txn); - return ResultType::SUCCESS; - } - auto storage_manager = storage::StorageManager::GetInstance(); auto &log_manager = logging::LogManager::GetInstance(); @@ -619,6 +613,14 @@ ResultType TimestampOrderingTransactionManager::CommitTransaction( gc_object_set->emplace_back(database_oid, table_oid, index_oid); } + // see if we can end early + if (!current_txn->IsWritten()) { + LOG_TRACE("Transaction not yet written, ending transaction."); + log_manager.StopLogging(); + EndTransaction(current_txn); + return ResultType::SUCCESS; + } + // install everything. // 1. install a new version for update operations; // 2. install an empty version for delete operations; From ec4c796f61c0cb2cee13764845c8f8d6fdfe186f Mon Sep 17 00:00:00 2001 From: Wan Shen Lim Date: Thu, 28 Jun 2018 14:59:15 -0400 Subject: [PATCH 13/13] undo marking read-only until we can test it (#1395) --- src/traffic_cop/traffic_cop.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/traffic_cop/traffic_cop.cpp b/src/traffic_cop/traffic_cop.cpp index 5b7bce864f2..7bfffebb4c0 100644 --- a/src/traffic_cop/traffic_cop.cpp +++ b/src/traffic_cop/traffic_cop.cpp @@ -164,9 +164,7 @@ executor::ExecutionResult TrafficCop::ExecuteHelper( // new txn, reset result status curr_state.second = ResultType::SUCCESS; single_statement_txn_ = true; - // txn is read-only for single-statement select - bool read_only = statement_->GetQueryType() == QueryType::QUERY_SELECT; - txn = txn_manager.BeginTransaction(read_only, thread_id); + txn = txn_manager.BeginTransaction(thread_id); tcop_txn_state_.emplace(txn, ResultType::SUCCESS); }