[Commits] ded1df92d: Remove TransactionDB::get_range_lock_manager()

22 Apr 2019

revision-id: ded1df92d1992b63f9db12e8757cb65ea711a4e3 (v5.8-1034-gded1df92d)
parent(s): 3b7567781e50b1ced3583a60fa7f1a568cc42d2e
author: Sergei Petrunia
committer: Sergei Petrunia
timestamp: 2019-04-22 13:18:22 +0300
message:

Remove TransactionDB::get_range_lock_manager()

This function was used by the user to get a handle to control the range
lock manager.

Now, the range lock manager is created by the user and then provided to
TransactionDB::Open:

  mgr= rocksdb::NewRangeLockManager(...);
  TransactionDBOptions txn_db_opts;
  txn_db_opts.range_lock_mgr = mgr;
  TransactionDB::Open(... txn_db_opts ...);

---
 include/rocksdb/utilities/transaction_db.h         | 31 +++++++++++---------
 utilities/transactions/pessimistic_transaction.cc  |  3 +-
 .../transactions/pessimistic_transaction_db.cc     | 33 ++++++++++++----------
 .../transactions/pessimistic_transaction_db.h      |  8 ++----
 utilities/transactions/range_locking_test.cc       |  9 +++---
 utilities/transactions/transaction_lock_mgr.cc     | 18 ++++++++++--
 utilities/transactions/transaction_lock_mgr.h      | 13 +++++++--
 7 files changed, 69 insertions(+), 46 deletions(-)

diff --git a/include/rocksdb/utilities/transaction_db.h b/include/rocksdb/utilities/transaction_db.h
index b12c70785..8f0e018f2 100644
--- a/include/rocksdb/utilities/transaction_db.h
+++ b/include/rocksdb/utilities/transaction_db.h
@@ -33,6 +33,20 @@ enum TxnDBWritePolicy {
 
 const uint32_t kInitialMaxDeadlocks = 5;
 
+// A handle to control RangeLockMgr
+class RangeLockMgrHandle {
+ public:
+  virtual int set_max_lock_memory(size_t max_lock_memory) = 0;
+  virtual uint64_t get_escalation_count() = 0;
+  virtual ~RangeLockMgrHandle() {};
+};
+
+// A factory function to create a Range Lock Manager
+RangeLockMgrHandle* NewRangeLockManager(
+  std::shared_ptr<TransactionDBMutexFactory> mutex_factory
+);
+
+
 struct TransactionDBOptions {
   // Specifies the maximum number of keys that can be locked at the same time
   // per column family.
@@ -93,10 +107,9 @@ struct TransactionDBOptions {
   // logic in myrocks. This hack of simply not rolling back merge operands works
   // for the special way that myrocks uses this operands.
   bool rollback_merge_operands = false;
-  
-  // If true, range_locking_opts specifies options on range locking (filling
-  // the struct is mandatory)
-  bool use_range_locking = false;
+
+  // If non-null, range locking should be used, and the lock manager is passed.
+  std::shared_ptr<RangeLockMgrHandle> range_lock_mgr;
 };
 
 struct TransactionOptions {
@@ -197,15 +210,6 @@ struct DeadlockPath {
   bool empty() { return path.empty() && !limit_exceeded; }
 };
 
-// Interface for controlling Range Locking manager
-class RangeLockMgrControl {
- public:
-
-  virtual int set_max_lock_memory(size_t max_lock_memory) = 0;
-  virtual uint64_t get_escalation_count() = 0;
-
-  virtual ~RangeLockMgrControl(){}
-};
 
 class TransactionDB : public StackableDB {
  public:
@@ -281,7 +285,6 @@ class TransactionDB : public StackableDB {
   virtual std::vector<DeadlockPath> GetDeadlockInfoBuffer() = 0;
   virtual void SetDeadlockInfoBufferSize(uint32_t target_size) = 0;
   
-  virtual RangeLockMgrControl* get_range_lock_manager() { return nullptr; }
  protected:
   // To Create an TransactionDB, call Open()
   // The ownership of db is transferred to the base StackableDB
diff --git a/utilities/transactions/pessimistic_transaction.cc b/utilities/transactions/pessimistic_transaction.cc
index ca6bc27b6..ff653fa0e 100644
--- a/utilities/transactions/pessimistic_transaction.cc
+++ b/utilities/transactions/pessimistic_transaction.cc
@@ -50,7 +50,8 @@ PessimisticTransaction::PessimisticTransaction(
       skip_concurrency_control_(false) {
   txn_db_impl_ =
       static_cast_with_check<PessimisticTransactionDB, TransactionDB>(txn_db);
-  do_key_tracking_ = !txn_db_impl_->get_range_lock_manager();
+
+  do_key_tracking_ = txn_db_impl_->ShouldDoKeyTracking();
   db_impl_ = static_cast_with_check<DBImpl, DB>(db_);
   Initialize(txn_options);
 }
diff --git a/utilities/transactions/pessimistic_transaction_db.cc b/utilities/transactions/pessimistic_transaction_db.cc
index 06981adf4..e50fb1dad 100644
--- a/utilities/transactions/pessimistic_transaction_db.cc
+++ b/utilities/transactions/pessimistic_transaction_db.cc
@@ -41,25 +41,28 @@ PessimisticTransactionDB::PessimisticTransactionDB(
 }
 
 void PessimisticTransactionDB::init_lock_manager() {
-  BaseLockMgr *lock_mgr;
 
-  std::shared_ptr<TransactionDBMutexFactory> mutex_factory = 
-      txn_db_options_.custom_mutex_factory?
-           txn_db_options_.custom_mutex_factory :
-             std::shared_ptr<TransactionDBMutexFactory>(
-               new TransactionDBMutexFactoryImpl());
-
-  if (txn_db_options_.use_range_locking) {
-    range_lock_mgr_= new RangeLockMgr(this, mutex_factory);
-    lock_mgr = range_lock_mgr_;
+  if (txn_db_options_.range_lock_mgr) {
+    // 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());
   } else {
-    lock_mgr = new TransactionLockMgr(this, txn_db_options_.num_stripes,
-                                      txn_db_options_.max_num_locks,
-                                      txn_db_options_.max_num_deadlocks,
-                                      mutex_factory);
+    // Use point lock manager by default
+    std::shared_ptr<TransactionDBMutexFactory> mutex_factory =
+        txn_db_options_.custom_mutex_factory?
+             txn_db_options_.custom_mutex_factory :
+               std::shared_ptr<TransactionDBMutexFactory>(
+                 new TransactionDBMutexFactoryImpl());
+    auto lock_mgr = new TransactionLockMgr(this, txn_db_options_.num_stripes,
+                                           txn_db_options_.max_num_locks,
+                                           txn_db_options_.max_num_deadlocks,
+                                           mutex_factory);
+    lock_mgr_.reset(lock_mgr);
     range_lock_mgr_ = nullptr;
   }
-  lock_mgr_ = std::shared_ptr<BaseLockMgr>(lock_mgr);
+  lock_mgr_->init(this);
 }
 
 // Support initiliazing PessimisticTransactionDB from a stackable db
diff --git a/utilities/transactions/pessimistic_transaction_db.h b/utilities/transactions/pessimistic_transaction_db.h
index 59da1634c..e88856342 100644
--- a/utilities/transactions/pessimistic_transaction_db.h
+++ b/utilities/transactions/pessimistic_transaction_db.h
@@ -127,6 +127,8 @@ class PessimisticTransactionDB : public TransactionDB {
   virtual void UpdateCFComparatorMap(const std::vector<ColumnFamilyHandle*>&) {}
   virtual void UpdateCFComparatorMap(ColumnFamilyHandle*) {}
 
+  // Key Tracking should be done only with point lock manager.
+  bool ShouldDoKeyTracking() const { return range_lock_mgr_ == nullptr; }
  protected:
   DBImpl* db_impl_;
   std::shared_ptr<Logger> info_log_;
@@ -157,12 +159,6 @@ class PessimisticTransactionDB : public TransactionDB {
   // Non-null if we are using a lock manager that supports range locking.
   RangeLockMgr *range_lock_mgr_ = nullptr;
  
- public:
-  // Return Range Lock Manager if we are actually using it
-  virtual RangeLockMgrControl* get_range_lock_manager() override { 
-    return range_lock_mgr_;
-  }
- private:
   void init_lock_manager();
  
   // Must be held when adding/dropping column families.
diff --git a/utilities/transactions/range_locking_test.cc b/utilities/transactions/range_locking_test.cc
index aef345be5..df01573e4 100644
--- a/utilities/transactions/range_locking_test.cc
+++ b/utilities/transactions/range_locking_test.cc
@@ -42,6 +42,7 @@ class RangeLockingTest : public ::testing::Test {
   std::string dbname;
   Options options;
 
+  std::shared_ptr<RangeLockMgrHandle> range_lock_mgr;
   TransactionDBOptions txn_db_options;
 
   RangeLockingTest()
@@ -51,13 +52,13 @@ class RangeLockingTest : public ::testing::Test {
 
     DestroyDB(dbname, options);
     Status s;
-    txn_db_options.use_range_locking = true;
+
+    range_lock_mgr.reset(rocksdb::NewRangeLockManager(nullptr));
+    txn_db_options.range_lock_mgr = range_lock_mgr;
+
     s = TransactionDB::Open(options, txn_db_options, dbname, &db);
     assert(s.ok());
 
-    rocksdb::RangeLockMgrControl *mgr= db->get_range_lock_manager();
-    assert(mgr);
-    // can also: mgr->set_max_lock_memory(rocksdb_max_lock_memory);
   }
 
   ~RangeLockingTest() {
diff --git a/utilities/transactions/transaction_lock_mgr.cc b/utilities/transactions/transaction_lock_mgr.cc
index 8b5907bc5..edec58261 100644
--- a/utilities/transactions/transaction_lock_mgr.cc
+++ b/utilities/transactions/transaction_lock_mgr.cc
@@ -28,6 +28,7 @@
 #include "util/sync_point.h"
 #include "util/thread_local.h"
 #include "utilities/transactions/pessimistic_transaction_db.h"
+#include "utilities/transactions/transaction_db_mutex_impl.h"
 
 namespace rocksdb {
 
@@ -753,6 +754,18 @@ void TransactionLockMgr::Resize(uint32_t target_size) {
 // RangeLockMgr - a lock manager that supports range locking
 /////////////////////////////////////////////////////////////////////////////
 
+RangeLockMgrHandle* NewRangeLockManager(
+  std::shared_ptr<TransactionDBMutexFactory> mutex_factory) {
+  std::shared_ptr<TransactionDBMutexFactory> use_factory;
+
+  if (mutex_factory)
+    use_factory = mutex_factory;
+  else
+    use_factory.reset(new TransactionDBMutexFactoryImpl());
+
+  return new RangeLockMgr(use_factory);
+}
+
 /*
   Storage for locks that are currently held by a transaction.
 
@@ -1070,9 +1083,8 @@ int RangeLockMgr::compare_dbt_endpoints(__toku_db*, void *arg,
 }
 
 
-RangeLockMgr::RangeLockMgr(TransactionDB* txn_db,
-                           std::shared_ptr<TransactionDBMutexFactory> mutex_factory) :
-                           my_txn_db_(txn_db), mutex_factory_(mutex_factory) {
+RangeLockMgr::RangeLockMgr(std::shared_ptr<TransactionDBMutexFactory> mutex_factory) :
+  mutex_factory_(mutex_factory) {
   ltm.create(on_create, on_destroy, on_escalate, NULL, mutex_factory_);
   cmp_.create(compare_dbt_endpoints, (void*)this, NULL);
   DICTIONARY_ID dict_id = { .dictid = 1 };
diff --git a/utilities/transactions/transaction_lock_mgr.h b/utilities/transactions/transaction_lock_mgr.h
index 5e9ee851c..02c68964e 100644
--- a/utilities/transactions/transaction_lock_mgr.h
+++ b/utilities/transactions/transaction_lock_mgr.h
@@ -76,6 +76,9 @@ class BaseLockMgr {
   // Resize the deadlock info buffer
   virtual void Resize(uint32_t)=0;
   virtual std::vector<DeadlockPath> GetDeadlockInfoBuffer()= 0;
+
+  // TransactionDB will call this at start
+  virtual void init(TransactionDB *db_arg) {};
   virtual ~BaseLockMgr(){}
 
   using LockStatusData = std::unordered_multimap<uint32_t, KeyLockInfo>;
@@ -194,7 +197,7 @@ using namespace toku;
 */
 class RangeLockMgr :
   public BaseLockMgr, 
-  public RangeLockMgrControl {
+  public RangeLockMgrHandle {
  public:
   void AddColumnFamily(uint32_t) override { /* do nothing */ }
   void RemoveColumnFamily(uint32_t) override { /* do nothing */ }
@@ -227,8 +230,12 @@ class RangeLockMgr :
   void UnLock(PessimisticTransaction* txn, uint32_t column_family_id,
               const std::string& key, Env* env) override ;
 
-  RangeLockMgr(TransactionDB* txn_db,
-               std::shared_ptr<TransactionDBMutexFactory> mutex_factory);
+  RangeLockMgr(std::shared_ptr<TransactionDBMutexFactory> mutex_factory);
+
+  void init(TransactionDB *db_arg) override {
+    my_txn_db_ = db_arg;
+  }
+
   ~RangeLockMgr();
 
   int set_max_lock_memory(size_t max_lock_memory) override

    