revision-id: c91095e08fbdacb69831a93ab403a80487dee435 (v5.8-1042-gc91095e08) parent(s): 1b423e3954d932c4624337308e3e1cd98481a495 author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2019-06-10 23:11:27 +0300 message: Range Locking: address review input --- include/rocksdb/utilities/transaction_db.h | 26 ++++++++++++++++++---- utilities/transactions/pessimistic_transaction.cc | 4 +--- .../transactions/pessimistic_transaction_db.cc | 8 +++---- utilities/transactions/transaction_lock_mgr.h | 3 +++ 4 files changed, 29 insertions(+), 12 deletions(-) diff --git a/include/rocksdb/utilities/transaction_db.h b/include/rocksdb/utilities/transaction_db.h index 7ebac3e06..84761c80e 100644 --- a/include/rocksdb/utilities/transaction_db.h +++ b/include/rocksdb/utilities/transaction_db.h @@ -33,9 +33,26 @@ enum TxnDBWritePolicy { const uint32_t kInitialMaxDeadlocks = 5; +class BaseLockMgr; + +// A lock manager handle +// The workflow is as follows: +// * Use a factory method (like NewRangeLockManager()) to create a lock +// manager and get its handle. +// * A Handle for a particular kind of lock manager will have extra +// methods and parameters to control the lock manager +// * Pass the handle to RocksDB in TransactionDBOptions::lock_mgr_handle. It +// will be used to perform locking. +class LockManagerHandle { + public: + // Get the underlying LockManager. To be used by RocksDB Internals + virtual BaseLockMgr* GetManager()=0; + virtual ~LockManagerHandle(){}; +}; + // A handle to control RangeLockMgr (Range-based lock manager) from outside // RocksDB -class RangeLockMgrHandle { +class RangeLockMgrHandle : public LockManagerHandle { public: virtual int set_max_lock_memory(size_t max_lock_memory) = 0; virtual uint64_t get_escalation_count() = 0; @@ -44,7 +61,7 @@ class RangeLockMgrHandle { // A factory function to create a Range Lock Manager. The created object should // be: -// 1. Passed in TransactionDBOptions::range_lock_mgr to open the database in +// 1. Passed in TransactionDBOptions::lock_mgr_handle to open the database in // range-locking mode // 2. Used to control the lock manager when the DB is already open. RangeLockMgrHandle* NewRangeLockManager( @@ -112,8 +129,9 @@ struct TransactionDBOptions { // for the special way that myrocks uses this operands. bool rollback_merge_operands = false; - // If non-null, range locking should be used, and the lock manager is passed. - std::shared_ptr<RangeLockMgrHandle> range_lock_mgr; + // nullptr means use default lock manager. + // Other value means the user provides a custom lock manager. + std::shared_ptr<LockManagerHandle> lock_mgr_handle; }; struct TransactionOptions { diff --git a/utilities/transactions/pessimistic_transaction.cc b/utilities/transactions/pessimistic_transaction.cc index 082934e75..4c0578444 100644 --- a/utilities/transactions/pessimistic_transaction.cc +++ b/utilities/transactions/pessimistic_transaction.cc @@ -537,8 +537,6 @@ Status PessimisticTransaction::TryLock(ColumnFamilyHandle* column_family, // an upgrade. if (!previously_locked || lock_upgrade) { s = txn_db_impl_->TryLock(this, cfh_id, key_str, exclusive); - if (!s.ok()) - return s; } SetSnapshotIfNeeded(); @@ -569,7 +567,7 @@ Status PessimisticTransaction::TryLock(ColumnFamilyHandle* column_family, // since the snapshot. This must be done after we locked the key. // If we already have validated an earilier snapshot it must has been // reflected in tracked_at_seq and ValidateSnapshot will return OK. - if (s.ok()) { //psergey-todo: this check seems to be meaningless, s.ok()==true always + if (s.ok()) { s = ValidateSnapshot(column_family, key, &tracked_at_seq); if (!s.ok()) { diff --git a/utilities/transactions/pessimistic_transaction_db.cc b/utilities/transactions/pessimistic_transaction_db.cc index 14b82de00..bacf95479 100644 --- a/utilities/transactions/pessimistic_transaction_db.cc +++ b/utilities/transactions/pessimistic_transaction_db.cc @@ -42,12 +42,10 @@ PessimisticTransactionDB::PessimisticTransactionDB( void PessimisticTransactionDB::init_lock_manager() { - if (txn_db_options_.range_lock_mgr) { + if (txn_db_options_.lock_mgr_handle) { // A custom lock manager was provided in options - std::shared_ptr<RangeLockMgr> tmp; - tmp = std::static_pointer_cast<RangeLockMgr>(txn_db_options_.range_lock_mgr); - lock_mgr_= tmp; - range_lock_mgr_ = static_cast<RangeLockMgr*>(lock_mgr_.get()); + lock_mgr_.reset(txn_db_options_.lock_mgr_handle->GetManager()); + range_lock_mgr_ = dynamic_cast<RangeLockMgr*>(lock_mgr_.get()); } else { // Use point lock manager by default std::shared_ptr<TransactionDBMutexFactory> mutex_factory = diff --git a/utilities/transactions/transaction_lock_mgr.h b/utilities/transactions/transaction_lock_mgr.h index 608d6f34f..54ae77a52 100644 --- a/utilities/transactions/transaction_lock_mgr.h +++ b/utilities/transactions/transaction_lock_mgr.h @@ -243,6 +243,9 @@ class RangeLockMgr : LockStatusData GetLockStatusData() override; + BaseLockMgr* GetManager() override { + return this; + } private: toku::locktree_manager ltm_;