revision-id: 791d4c1f48fe30d7519b93508cc3670246063b7f (v5.8-1026-g791d4c1f4) parent(s): e4d7dbc47038638ed3d199c3d959ed5ead84f353 author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2019-01-15 00:24:54 +0300 message: Range Locking: disable key tracking when range locking is used --- utilities/transactions/pessimistic_transaction.cc | 24 ++++++--- .../transactions/pessimistic_transaction_db.cc | 2 +- utilities/transactions/transaction_base.cc | 63 +++++++++++----------- utilities/transactions/transaction_base.h | 7 +++ utilities/transactions/transaction_lock_mgr.cc | 3 +- utilities/transactions/transaction_lock_mgr.h | 3 +- 6 files changed, 60 insertions(+), 42 deletions(-) diff --git a/utilities/transactions/pessimistic_transaction.cc b/utilities/transactions/pessimistic_transaction.cc index 36e6ed6ac..7a9a0d11e 100644 --- a/utilities/transactions/pessimistic_transaction.cc +++ b/utilities/transactions/pessimistic_transaction.cc @@ -50,6 +50,7 @@ PessimisticTransaction::PessimisticTransaction( skip_concurrency_control_(false) { txn_db_impl_ = static_cast_with_check<PessimisticTransactionDB, TransactionDB>(txn_db); + do_key_tracking_ = ! txn_db_impl_->use_range_locking; db_impl_ = static_cast_with_check<DBImpl, DB>(db_); Initialize(txn_options); } @@ -131,7 +132,8 @@ WriteCommittedTxn::WriteCommittedTxn(TransactionDB* txn_db, Status PessimisticTransaction::CommitBatch(WriteBatch* batch) { TransactionKeyMap keys_to_unlock; - Status s = LockBatch(batch, &keys_to_unlock); + + Status s = do_key_tracking_? LockBatch(batch, &keys_to_unlock): Status::OK(); if (!s.ok()) { return s; @@ -162,7 +164,7 @@ Status PessimisticTransaction::CommitBatch(WriteBatch* batch) { s = Status::InvalidArgument("Transaction is not in state for commit."); } - txn_db_impl_->UnLock(this, &keys_to_unlock); + txn_db_impl_->UnLock(this, &keys_to_unlock, /*all_keys_hint=*/true); return s; } @@ -400,12 +402,14 @@ Status PessimisticTransaction::RollbackToSavePoint() { return Status::InvalidArgument("Transaction is beyond state for rollback."); } - // Unlock any keys locked since last transaction - const std::unique_ptr<TransactionKeyMap>& keys = - GetTrackedKeysSinceSavePoint(); + if (do_key_tracking_) { + // Unlock any keys locked since last transaction + const std::unique_ptr<TransactionKeyMap>& keys = + GetTrackedKeysSinceSavePoint(); - if (keys) { - txn_db_impl_->UnLock(this, keys.get()); + if (keys) { + txn_db_impl_->UnLock(this, keys.get()); + } } return TransactionBaseImpl::RollbackToSavePoint(); @@ -503,6 +507,12 @@ Status PessimisticTransaction::TryLock(ColumnFamilyHandle* column_family, bool previously_locked; bool lock_upgrade = false; + // If we are not doing key tracking, just get the lock and return (this + // also assumes locks are "idempotent") + if (!do_key_tracking_) { + return txn_db_impl_->TryLock(this, cfh_id, key_str, exclusive); + } + // lock this key if this transactions hasn't already locked it SequenceNumber tracked_at_seq = kMaxSequenceNumber; diff --git a/utilities/transactions/pessimistic_transaction_db.cc b/utilities/transactions/pessimistic_transaction_db.cc index 425b3ba73..df5ec2d6c 100644 --- a/utilities/transactions/pessimistic_transaction_db.cc +++ b/utilities/transactions/pessimistic_transaction_db.cc @@ -409,7 +409,7 @@ void PessimisticTransactionDB::UnLock(PessimisticTransaction* txn, if (use_range_locking) { if (all_keys_hint) - range_lock_mgr_.UnLockAll(txn, keys, GetEnv()); + range_lock_mgr_.UnLockAll(txn, GetEnv()); else range_lock_mgr_.UnLock(txn, keys, GetEnv()); } diff --git a/utilities/transactions/transaction_base.cc b/utilities/transactions/transaction_base.cc index ac459a256..d81149201 100644 --- a/utilities/transactions/transaction_base.cc +++ b/utilities/transactions/transaction_base.cc @@ -24,6 +24,7 @@ TransactionBaseImpl::TransactionBaseImpl(DB* db, cmp_(GetColumnFamilyUserComparator(db->DefaultColumnFamily())), start_time_(db_->GetEnv()->NowMicros()), write_batch_(cmp_, 0, true, 0), + do_key_tracking_(true), indexing_enabled_(true) { assert(dynamic_cast<DBImpl*>(db_) != nullptr); log_number_ = 0; @@ -137,35 +138,37 @@ Status TransactionBaseImpl::RollbackToSavePoint() { Status s = write_batch_.RollbackToSavePoint(); assert(s.ok()); - // Rollback any keys that were tracked since the last savepoint - const TransactionKeyMap& key_map = save_point.new_keys_; - for (const auto& key_map_iter : key_map) { - uint32_t column_family_id = key_map_iter.first; - auto& keys = key_map_iter.second; - - auto& cf_tracked_keys = tracked_keys_[column_family_id]; - - for (const auto& key_iter : keys) { - const std::string& key = key_iter.first; - uint32_t num_reads = key_iter.second.num_reads; - uint32_t num_writes = key_iter.second.num_writes; - - auto tracked_keys_iter = cf_tracked_keys.find(key); - assert(tracked_keys_iter != cf_tracked_keys.end()); - - // Decrement the total reads/writes of this key by the number of - // reads/writes done since the last SavePoint. - if (num_reads > 0) { - assert(tracked_keys_iter->second.num_reads >= num_reads); - tracked_keys_iter->second.num_reads -= num_reads; - } - if (num_writes > 0) { - assert(tracked_keys_iter->second.num_writes >= num_writes); - tracked_keys_iter->second.num_writes -= num_writes; - } - if (tracked_keys_iter->second.num_reads == 0 && - tracked_keys_iter->second.num_writes == 0) { - tracked_keys_[column_family_id].erase(tracked_keys_iter); + if (do_key_tracking_) { + // Rollback any keys that were tracked since the last savepoint + const TransactionKeyMap& key_map = save_point.new_keys_; + for (const auto& key_map_iter : key_map) { + uint32_t column_family_id = key_map_iter.first; + auto& keys = key_map_iter.second; + + auto& cf_tracked_keys = tracked_keys_[column_family_id]; + + for (const auto& key_iter : keys) { + const std::string& key = key_iter.first; + uint32_t num_reads = key_iter.second.num_reads; + uint32_t num_writes = key_iter.second.num_writes; + + auto tracked_keys_iter = cf_tracked_keys.find(key); + assert(tracked_keys_iter != cf_tracked_keys.end()); + + // Decrement the total reads/writes of this key by the number of + // reads/writes done since the last SavePoint. + if (num_reads > 0) { + assert(tracked_keys_iter->second.num_reads >= num_reads); + tracked_keys_iter->second.num_reads -= num_reads; + } + if (num_writes > 0) { + assert(tracked_keys_iter->second.num_writes >= num_writes); + tracked_keys_iter->second.num_writes -= num_writes; + } + if (tracked_keys_iter->second.num_reads == 0 && + tracked_keys_iter->second.num_writes == 0) { + tracked_keys_[column_family_id].erase(tracked_keys_iter); + } } } } @@ -573,7 +576,7 @@ void TransactionBaseImpl::TrackKey(TransactionKeyMap* key_map, uint32_t cfh_id, std::unique_ptr<TransactionKeyMap> TransactionBaseImpl::GetTrackedKeysSinceSavePoint() { - if (save_points_ != nullptr && !save_points_->empty()) { + if (save_points_ != nullptr && !save_points_->empty() && do_key_tracking_) { // Examine the number of reads/writes performed on all keys written // since the last SavePoint and compare to the total number of reads/writes // for each key. diff --git a/utilities/transactions/transaction_base.h b/utilities/transactions/transaction_base.h index 171e13588..8edc81974 100644 --- a/utilities/transactions/transaction_base.h +++ b/utilities/transactions/transaction_base.h @@ -320,6 +320,13 @@ class TransactionBaseImpl : public Transaction { // Optimistic Transactions will wait till commit time to do conflict checking. TransactionKeyMap tracked_keys_; + protected: + // Whether to do key key tracking (in tracked_keys_). + // By default, key tracking is enabled. Pessimistic transactions with + // range locking don't do key tracking. + bool do_key_tracking_; + + private: // If true, future Put/Merge/Deletes will be indexed in the // WriteBatchWithIndex. // If false, future Put/Merge/Deletes will be inserted directly into the diff --git a/utilities/transactions/transaction_lock_mgr.cc b/utilities/transactions/transaction_lock_mgr.cc index ed382cefd..eef61e724 100644 --- a/utilities/transactions/transaction_lock_mgr.cc +++ b/utilities/transactions/transaction_lock_mgr.cc @@ -844,8 +844,7 @@ void RangeLockMgr::UnLock(const PessimisticTransaction* txn, 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) { +void RangeLockMgr::UnLockAll(const PessimisticTransaction* txn, Env* env) { // owned_locks may hold nullptr if the transaction has never acquired any // locks. diff --git a/utilities/transactions/transaction_lock_mgr.h b/utilities/transactions/transaction_lock_mgr.h index 8777983e4..f0b406ab5 100644 --- a/utilities/transactions/transaction_lock_mgr.h +++ b/utilities/transactions/transaction_lock_mgr.h @@ -212,8 +212,7 @@ class RangeLockMgr : 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 UnLockAll(const PessimisticTransaction* txn, Env* env); void UnLock(PessimisticTransaction* txn, uint32_t column_family_id, const std::string& key, Env* env) override ;