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(<ree_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