revision-id: b2be543c0d6084c1bd5ff99d6ffed90ff3d0bd24 parent(s): d629f934d907f506bd114d9d48852e6570d333da committer: Sergei Petrunia branch nick: modules timestamp: 2018-10-08 19:29:10 +0300 message: - Make RangeLockMgr::UnLock() only unlock the keys that it is asked to unlock. = In STO-mode, it used to unlock all keys. = Now fixed by leaving the STO-mode. This is the easiest fix, although probably not optimal. We could have walked the STO array from its back) - Add RangeLockMgr::UnLockAll() that does release all of transaction' locks. - Performance of unlock operation can still be improved. --- utilities/transactions/pessimistic_transaction.cc | 2 +- .../transactions/pessimistic_transaction_db.cc | 10 ++++++-- .../transactions/pessimistic_transaction_db.h | 3 ++- utilities/transactions/transaction_lock_mgr.cc | 29 ++++++++++++++++------ utilities/transactions/transaction_lock_mgr.h | 6 +++++ 5 files changed, 39 insertions(+), 11 deletions(-) diff --git a/utilities/transactions/pessimistic_transaction.cc b/utilities/transactions/pessimistic_transaction.cc index befa19f..a83db5e 100644 --- a/utilities/transactions/pessimistic_transaction.cc +++ b/utilities/transactions/pessimistic_transaction.cc @@ -97,7 +97,7 @@ PessimisticTransaction::~PessimisticTransaction() { } void PessimisticTransaction::Clear() { - txn_db_impl_->UnLock(this, &GetTrackedKeys()); + txn_db_impl_->UnLock(this, &GetTrackedKeys(), /*all_keys_hint=*/true); TransactionBaseImpl::Clear(); } diff --git a/utilities/transactions/pessimistic_transaction_db.cc b/utilities/transactions/pessimistic_transaction_db.cc index dcc420a..2325ea9 100644 --- a/utilities/transactions/pessimistic_transaction_db.cc +++ b/utilities/transactions/pessimistic_transaction_db.cc @@ -380,9 +380,15 @@ Status PessimisticTransactionDB::TryLock(PessimisticTransaction* txn, } void PessimisticTransactionDB::UnLock(PessimisticTransaction* txn, - const TransactionKeyMap* keys) { + const TransactionKeyMap* keys, + bool all_keys_hint) { if (use_range_locking) - range_lock_mgr_.UnLock(txn, keys, GetEnv()); + { + if (all_keys_hint) + range_lock_mgr_.UnLockAll(txn, keys, GetEnv()); + else + range_lock_mgr_.UnLock(txn, keys, GetEnv()); + } else lock_mgr_.UnLock(txn, keys, GetEnv()); } diff --git a/utilities/transactions/pessimistic_transaction_db.h b/utilities/transactions/pessimistic_transaction_db.h index 1876de5..262ca99 100644 --- a/utilities/transactions/pessimistic_transaction_db.h +++ b/utilities/transactions/pessimistic_transaction_db.h @@ -79,7 +79,8 @@ class PessimisticTransactionDB : public TransactionDB { Status TryLock(PessimisticTransaction* txn, uint32_t cfh_id, const std::string& key, bool exclusive); - void UnLock(PessimisticTransaction* txn, const TransactionKeyMap* keys); + void UnLock(PessimisticTransaction* txn, const TransactionKeyMap* keys, + bool all_keys_hint=false); void UnLock(PessimisticTransaction* txn, uint32_t cfh_id, const std::string& key); diff --git a/utilities/transactions/transaction_lock_mgr.cc b/utilities/transactions/transaction_lock_mgr.cc index 0ce5124..3482082 100644 --- a/utilities/transactions/transaction_lock_mgr.cc +++ b/utilities/transactions/transaction_lock_mgr.cc @@ -691,10 +691,11 @@ void TransactionLockMgr::UnLock(PessimisticTransaction* txn, } static void -another_lock_mgr_release_lock_int(toku::locktree *lt, +range_lock_mgr_release_lock_int(toku::locktree *lt, const PessimisticTransaction* txn, uint32_t column_family_id, - const std::string& key) + const std::string& key, + bool releasing_all_locks_hint= false) { DBT key_dbt; toku_fill_dbt(&key_dbt, key.data(), key.size()); @@ -708,31 +709,45 @@ another_lock_mgr_release_lock_int(toku::locktree *lt, void RangeLockMgr::UnLock(PessimisticTransaction* txn, uint32_t column_family_id, const std::string& key, Env* env) { - //fprintf(stderr, "RangeLockMgr::UnLock (key)\n"); - another_lock_mgr_release_lock_int(lt, txn, column_family_id, key); + range_lock_mgr_release_lock_int(lt, txn, column_family_id, key); toku::lock_request::retry_all_lock_requests(lt, nullptr /* lock_wait_needed_callback */); } void RangeLockMgr::UnLock(const PessimisticTransaction* txn, const TransactionKeyMap* key_map, Env* env) { + //TODO: if we collect all locks in a range buffer and then + // make one call to lock_tree::release_locks(), will that be faster? + for (auto& key_map_iter : *key_map) { + uint32_t column_family_id = key_map_iter.first; + auto& keys = key_map_iter.second; - //fprintf(stderr, "RangeLockMgr::UnLock(key_map)\n"); + for (auto& key_iter : keys) { + const std::string& key = key_iter.first; + range_lock_mgr_release_lock_int(lt, txn, column_family_id, key); + } + } + toku::lock_request::retry_all_lock_requests(lt, nullptr /* lock_wait_needed_callback */); +} +void RangeLockMgr::UnLockAll(const PessimisticTransaction* txn, + const TransactionKeyMap* key_map, Env* env) { + //TODO: collecting multiple locks into a buffer and then making one call + // to lock_tree::release_locks() will be faster. for (auto& key_map_iter : *key_map) { uint32_t column_family_id = key_map_iter.first; - //TODO: ^ What to do about the above? auto& keys = key_map_iter.second; for (auto& key_iter : keys) { const std::string& key = key_iter.first; - another_lock_mgr_release_lock_int(lt, txn, column_family_id, key); + range_lock_mgr_release_lock_int(lt, txn, column_family_id, key, true); } } toku::lock_request::retry_all_lock_requests(lt, nullptr /* lock_wait_needed_callback */); #if 0 + Original usage: void release_locks(TXNID txnid, const range_buffer *ranges); // release all of the locks this txn has ever successfully diff --git a/utilities/transactions/transaction_lock_mgr.h b/utilities/transactions/transaction_lock_mgr.h index 3580441..c2de992 100644 --- a/utilities/transactions/transaction_lock_mgr.h +++ b/utilities/transactions/transaction_lock_mgr.h @@ -92,6 +92,12 @@ class RangeLockMgr :public BaseLockMgr { void UnLock(const PessimisticTransaction* txn, const TransactionKeyMap* keys, Env* env) override ; + /* + Same as above, but *keys is guaranteed to hold all the locks obtained by + the transaction. + */ + void UnLockAll(const PessimisticTransaction* txn, const TransactionKeyMap* keys, + Env* env); void UnLock(PessimisticTransaction* txn, uint32_t column_family_id, const std::string& key, Env* env) override ;