[Commits] d334cf128: Range Locking: make the range tree code invoke the user-provided comparator

Sergei Petrunia psergey at askmonty.org
Mon Apr 29 16:53:06 EEST 2019


revision-id: d334cf12818bfd2d44dc06c7bbe78c3a18bca543 (v5.8-1038-gd334cf128)
parent(s): 9af13f0e966171ef8688484b2da7fa576a1b4574
author: Sergei Petrunia
committer: Sergei Petrunia
timestamp: 2019-04-29 16:53:06 +0300
message:

Range Locking: make the range tree code invoke the user-provided comparator

---
 include/rocksdb/utilities/transaction.h        |  8 +++++
 utilities/transactions/transaction_lock_mgr.cc | 49 ++++++++++----------------
 utilities/transactions/transaction_lock_mgr.h  |  8 +----
 3 files changed, 28 insertions(+), 37 deletions(-)

diff --git a/include/rocksdb/utilities/transaction.h b/include/rocksdb/utilities/transaction.h
index 20f9b1bbd..b0f1308ed 100644
--- a/include/rocksdb/utilities/transaction.h
+++ b/include/rocksdb/utilities/transaction.h
@@ -80,6 +80,12 @@ using TransactionID = uint64_t;
 class Endpoint {
  public:
   Slice slice;
+
+  /*
+    true  : the key has an "infinity" suffix. A suffix that would compare as
+            greater than any other suffix
+    false : otherwise
+  */
   bool inf_suffix;
 
   Endpoint(const Slice &slice_arg, bool inf_suffix_arg=false) :
@@ -90,6 +96,8 @@ class Endpoint {
 
   Endpoint(const char* s, size_t size, bool inf_suffix_arg=false) :
     slice(s, size), inf_suffix(inf_suffix_arg) {}
+
+  Endpoint() : inf_suffix(false) {}
 };
 
 // Provides notification to the caller of SetSnapshotOnNextOperation when
diff --git a/utilities/transactions/transaction_lock_mgr.cc b/utilities/transactions/transaction_lock_mgr.cc
index c98803acb..c375b24f2 100644
--- a/utilities/transactions/transaction_lock_mgr.cc
+++ b/utilities/transactions/transaction_lock_mgr.cc
@@ -813,11 +813,12 @@ public:
   bool releasing_locks_;
 };
 
+static const char SUFFIX_INFIMUM= 0x0;
+static const char SUFFIX_SUPREMUM= 0x1;
+
 static
 void serialize_endpoint(const Endpoint &endp, std::string *buf) {
-  const char SUFFIX_INF= 0x0;
-  const char SUFFIX_SUP= 0x1;
-  buf->push_back(endp.inf_suffix ? SUFFIX_SUP : SUFFIX_INF);
+  buf->push_back(endp.inf_suffix ? SUFFIX_SUPREMUM : SUFFIX_INFIMUM);
   buf->append(endp.slice.data(), endp.slice.size());
 }
 
@@ -1044,11 +1045,6 @@ void RangeLockMgr::UnLockAll(const PessimisticTransaction* txn, Env*) {
 int RangeLockMgr::compare_dbt_endpoints(__toku_db*, void *arg,
                                         const DBT *a_key,
                                         const DBT *b_key) {
-  //RangeLockMgr* mgr= (RangeLockMgr*) arg;
-  // TODO: this should compare endpoints using the user-provided comparator +
-  // endpoint encoding.
-  // (just use one from any column family)
-  
   const char *a= (const char*)a_key->data;
   const char *b= (const char*)b_key->data;
 
@@ -1057,16 +1053,16 @@ int RangeLockMgr::compare_dbt_endpoints(__toku_db*, void *arg,
 
   size_t min_len= std::min(a_len, b_len);
 
-  //compare the values. Skip the first byte as it is the endpoint signifier
-
-  //TODO: use the upper layer' comparison function.
-  int res= memcmp(a+1, b+1, min_len-1);
+  // Compare the values. The first byte encodes the endpoint type, its value
+  // is either SUFFIX_INFIMUM or SUFFIX_SUPREMUM.
+  Comparator *cmp = (Comparator*) arg;
+  int res= cmp->Compare(Slice(a+1, min_len-1), Slice(b+1, min_len-1));
   if (!res)
   {
     if (b_len > min_len)
     {
       // a is shorter;
-      if (a[0] == 0)
+      if (a[0] == SUFFIX_INFIMUM)
         return  -1; //"a is smaller"
       else
       {
@@ -1077,7 +1073,7 @@ int RangeLockMgr::compare_dbt_endpoints(__toku_db*, void *arg,
     else if (a_len > min_len)
     {
       // the opposite of the above: b is shorter.
-      if (b[0] == 0)
+      if (b[0] == SUFFIX_INFIMUM)
         return  1; //"b is smaller"
       else
       {
@@ -1100,21 +1096,10 @@ int RangeLockMgr::compare_dbt_endpoints(__toku_db*, void *arg,
     return res;
 }
 
-int RangeLockMgr::compare_dbt_endpoints_rev(__toku_db* db, void *arg,
-                                            const DBT *a_key,
-                                            const DBT *b_key) {
-  return -compare_dbt_endpoints(db, arg, a_key, b_key);
-}
-
-
 RangeLockMgr::RangeLockMgr(std::shared_ptr<TransactionDBMutexFactory> mutex_factory) :
   mutex_factory_(mutex_factory),
   ltree_lookup_cache_(new ThreadLocalPtr(nullptr)) {
-
   ltm_.create(on_create, on_destroy, on_escalate, NULL, mutex_factory_);
-
-  fw_cmp_.create(compare_dbt_endpoints, (void*)this, NULL);
-  bw_cmp_.create(compare_dbt_endpoints, (void*)this, NULL);
 }
 
 
@@ -1168,8 +1153,6 @@ RangeLockMgr::~RangeLockMgr() {
     ltm_.release_lt(it.second);
   }
   ltm_.destroy();
-  fw_cmp_.destroy();
-  bw_cmp_.destroy();
 }
 
 uint64_t RangeLockMgr::get_escalation_count() {
@@ -1199,18 +1182,24 @@ void RangeLockMgr::AddColumnFamily(const ColumnFamilyHandle *cfh) {
   InstrumentedMutexLock l(&ltree_map_mutex_);
   if (ltree_map_.find(column_family_id) == ltree_map_.end()) {
     DICTIONARY_ID dict_id = { .dictid = column_family_id };
-    toku::comparator *ltree_cmp;
     // "RocksDB_SE_v3.10" // BytewiseComparator() ,ReverseBytewiseComparator()
+    /*
+    toku::comparator *ltree_cmp;
     if (!strcmp(cfh->GetComparator()->Name(), "RocksDB_SE_v3.10"))
       ltree_cmp = &fw_cmp_;
     else if (!strcmp(cfh->GetComparator()->Name(),"rev:RocksDB_SE_v3.10"))
-      ltree_cmp = &fw_cmp_; // temporary: use the same ordering
+      ltree_cmp = &bw_cmp_;
     else {
       assert(false);
       ltree_cmp= nullptr;
     }
-    toku::locktree *ltree= ltm_.get_lt(dict_id, *ltree_cmp,
+    */
+    toku::comparator cmp;
+    cmp.create(compare_dbt_endpoints, (void*)cfh->GetComparator(), NULL);
+    toku::locktree *ltree= ltm_.get_lt(dict_id, cmp,
                                        /* on_create_extra*/nullptr);
+    // This is ok to because get_lt has copied the comparator:
+    cmp.destroy();
     ltree_map_.emplace(column_family_id, ltree);
   } else {
     // column_family already exists in lock map
diff --git a/utilities/transactions/transaction_lock_mgr.h b/utilities/transactions/transaction_lock_mgr.h
index e41a4961a..d35bf5935 100644
--- a/utilities/transactions/transaction_lock_mgr.h
+++ b/utilities/transactions/transaction_lock_mgr.h
@@ -250,9 +250,6 @@ class RangeLockMgr :
  private:
   toku::locktree_manager ltm_;
 
-  toku::comparator fw_cmp_;
-  toku::comparator bw_cmp_;
-
   TransactionDB* my_txn_db_;
   std::shared_ptr<TransactionDBMutexFactory> mutex_factory_;
 
@@ -269,8 +266,7 @@ class RangeLockMgr :
   toku::locktree *get_locktree_by_cfid(uint32_t cf_id);
 
   static int compare_dbt_endpoints(__toku_db*, void *arg, const DBT *a_key, const DBT *b_key);
-  static int compare_dbt_endpoints_rev(__toku_db*, void *arg, const DBT *a_key, const DBT *b_key);
-  
+
   // Callbacks
   static int  on_create(locktree*, void*) { return 0; /* no error */ }
   static void on_destroy(locktree*) {}
@@ -279,7 +275,5 @@ class RangeLockMgr :
 
 };
 
-
-
 }  //  namespace rocksdb
 #endif  // ROCKSDB_LITE


More information about the commits mailing list