revision-id: 402758a0f6899ec200860f13b5b606654cada766 (v5.8-2698-g402758a0f) parent(s): 24ca9e111197ba779a007e5b13d14ed4f4e54b4f author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2020-08-10 21:58:38 +0300 message: Apply coding style cleanup from make check-format --- include/rocksdb/utilities/transaction.h | 17 +- include/rocksdb/utilities/transaction_db.h | 9 +- utilities/transactions/lock/lock_tracker.h | 9 +- utilities/transactions/lock/point_lock_tracker.h | 5 +- utilities/transactions/lock/range_lock_tracker.cc | 14 +- utilities/transactions/lock/range_lock_tracker.h | 67 +++-- utilities/transactions/optimistic_transaction.cc | 4 +- utilities/transactions/pessimistic_transaction.cc | 23 +- utilities/transactions/pessimistic_transaction.h | 1 + .../transactions/pessimistic_transaction_db.cc | 35 ++- .../transactions/pessimistic_transaction_db.h | 19 +- utilities/transactions/range_locking_test.cc | 23 +- utilities/transactions/transaction_base.cc | 2 +- utilities/transactions/transaction_base.h | 24 +- utilities/transactions/transaction_lock_mgr.cc | 301 ++++++++++----------- utilities/transactions/transaction_lock_mgr.h | 87 +++--- .../transactions/transaction_lock_mgr_test.cc | 30 +- utilities/transactions/write_unprepared_txn.cc | 2 +- 18 files changed, 305 insertions(+), 367 deletions(-) diff --git a/include/rocksdb/utilities/transaction.h b/include/rocksdb/utilities/transaction.h index 1b0cfabcb..2f4804255 100644 --- a/include/rocksdb/utilities/transaction.h +++ b/include/rocksdb/utilities/transaction.h @@ -24,7 +24,6 @@ using TransactionName = std::string; using TransactionID = uint64_t; - /* class Endpoint allows to define prefix ranges. @@ -90,14 +89,14 @@ class Endpoint { */ bool inf_suffix; - Endpoint(const Slice &slice_arg, bool inf_suffix_arg=false) : - slice(slice_arg), inf_suffix(inf_suffix_arg) {} + Endpoint(const Slice& slice_arg, bool inf_suffix_arg = false) + : slice(slice_arg), inf_suffix(inf_suffix_arg) {} - Endpoint(const char* s, bool inf_suffix_arg=false) : - slice(s), inf_suffix(inf_suffix_arg) {} + Endpoint(const char* s, bool inf_suffix_arg = false) + : slice(s), inf_suffix(inf_suffix_arg) {} - Endpoint(const char* s, size_t size, bool inf_suffix_arg=false) : - slice(s, size), inf_suffix(inf_suffix_arg) {} + Endpoint(const char* s, size_t size, bool inf_suffix_arg = false) + : slice(s, size), inf_suffix(inf_suffix_arg) {} Endpoint() : inf_suffix(false) {} }; @@ -356,8 +355,8 @@ class Transaction { } // Get a range lock on [start_endpoint; end_endpoint]. - virtual Status GetRangeLock(ColumnFamilyHandle*, - const Endpoint&, const Endpoint&) { + virtual Status GetRangeLock(ColumnFamilyHandle*, const Endpoint&, + const Endpoint&) { return Status::NotSupported(); } diff --git a/include/rocksdb/utilities/transaction_db.h b/include/rocksdb/utilities/transaction_db.h index 34aed0df9..8d5db02ac 100644 --- a/include/rocksdb/utilities/transaction_db.h +++ b/include/rocksdb/utilities/transaction_db.h @@ -62,7 +62,7 @@ class RangeLockMgrHandle : public LockManagerHandle { }; virtual Counters GetStatus() = 0; - virtual ~RangeLockMgrHandle() {}; + virtual ~RangeLockMgrHandle(){}; }; // A factory function to create a Range Lock Manager. The created object should @@ -71,8 +71,7 @@ class RangeLockMgrHandle : public LockManagerHandle { // range-locking mode // 2. Used to control the lock manager when the DB is already open. RangeLockMgrHandle* NewRangeLockManager( - std::shared_ptr<TransactionDBMutexFactory> mutex_factory -); + std::shared_ptr<TransactionDBMutexFactory> mutex_factory); struct TransactionDBOptions { // Specifies the maximum number of keys that can be locked at the same time @@ -245,10 +244,10 @@ struct TransactionDBWriteOptimizations { struct KeyLockInfo { std::string key; - std::string key2; // Used when range locking is used + std::string key2; // Used when range locking is used std::vector<TransactionID> ids; bool exclusive; - bool has_key2 = false; // TRUE <=> key2 has a value + bool has_key2 = false; // TRUE <=> key2 has a value }; struct DeadlockInfo { diff --git a/utilities/transactions/lock/lock_tracker.h b/utilities/transactions/lock/lock_tracker.h index 924bfd98c..104b58f1b 100644 --- a/utilities/transactions/lock/lock_tracker.h +++ b/utilities/transactions/lock/lock_tracker.h @@ -10,7 +10,7 @@ #include "rocksdb/rocksdb_namespace.h" #include "rocksdb/status.h" #include "rocksdb/types.h" -#include "rocksdb/utilities/transaction_db.h" // for Endpoint +#include "rocksdb/utilities/transaction_db.h" // for Endpoint namespace ROCKSDB_NAMESPACE { @@ -199,17 +199,16 @@ class LockTracker { ColumnFamilyId /*column_family_id*/) const = 0; }; - -// An interface to LockTracker factory. LockTracker objects should only be +// An interface to LockTracker factory. LockTracker objects should only be // created through this interface's Create() method. -// +// // One can get the factory pointer e.g. from Lock Manager which overloads // BaseLockMgr::getLockTrackerFactory(). class LockTrackerFactory { public: // Caller owns the returned pointer. virtual LockTracker* Create() const = 0; - virtual ~LockTrackerFactory(){} + virtual ~LockTrackerFactory() {} }; } // namespace ROCKSDB_NAMESPACE diff --git a/utilities/transactions/lock/point_lock_tracker.h b/utilities/transactions/lock/point_lock_tracker.h index b38a83f26..faf6de121 100644 --- a/utilities/transactions/lock/point_lock_tracker.h +++ b/utilities/transactions/lock/point_lock_tracker.h @@ -81,9 +81,8 @@ class PointLockTracker : public LockTracker { TrackedKeys tracked_keys_; }; -class PointLockTrackerFactory : public LockTrackerFactory -{ -public: +class PointLockTrackerFactory : public LockTrackerFactory { + public: LockTracker* Create() const override { return new PointLockTracker; } static PointLockTrackerFactory instance; diff --git a/utilities/transactions/lock/range_lock_tracker.cc b/utilities/transactions/lock/range_lock_tracker.cc index 1bb23956b..001a28548 100644 --- a/utilities/transactions/lock/range_lock_tracker.cc +++ b/utilities/transactions/lock/range_lock_tracker.cc @@ -7,16 +7,14 @@ RangeLockTrackerFactory RangeLockTrackerFactory::instance; RangeLockList *RangeLockTracker::getOrCreateList() { RangeLockList *res; - if ((res = getList())) - return res; + if ((res = getList())) return res; // Doesn't exist, create range_list.reset(new RangeLockList()); return getList(); } - -void RangeLockTracker::Track(const PointLockRequest& lock_req) { +void RangeLockTracker::Track(const PointLockRequest &lock_req) { DBT key_dbt; std::string key; serialize_endpoint(Endpoint(lock_req.key, false), &key); @@ -25,12 +23,12 @@ void RangeLockTracker::Track(const PointLockRequest& lock_req) { rl->append(lock_req.column_family_id, &key_dbt, &key_dbt); } -void RangeLockTracker::Track(const RangeLockRequest& lock_req) { +void RangeLockTracker::Track(const RangeLockRequest &lock_req) { DBT start_dbt, end_dbt; std::string start_key, end_key; serialize_endpoint(lock_req.start_endp, &start_key); - serialize_endpoint(lock_req.end_endp, &end_key); + serialize_endpoint(lock_req.end_endp, &end_key); toku_fill_dbt(&start_dbt, start_key.data(), start_key.size()); toku_fill_dbt(&end_dbt, end_key.data(), end_key.size()); @@ -39,8 +37,8 @@ void RangeLockTracker::Track(const RangeLockRequest& lock_req) { rl->append(lock_req.column_family_id, &start_dbt, &end_dbt); } -PointLockStatus RangeLockTracker::GetPointLockStatus(ColumnFamilyId /*cf_id*/, - const std::string& /*key*/) const { +PointLockStatus RangeLockTracker::GetPointLockStatus( + ColumnFamilyId /*cf_id*/, const std::string & /*key*/) const { // TODO: what to do here if we are holding a range lock that is embedding the // point? diff --git a/utilities/transactions/lock/range_lock_tracker.h b/utilities/transactions/lock/range_lock_tracker.h index 70778e180..36787ce5e 100644 --- a/utilities/transactions/lock/range_lock_tracker.h +++ b/utilities/transactions/lock/range_lock_tracker.h @@ -9,18 +9,17 @@ #include <string> #include <unordered_map> -#include "utilities/transactions/lock/lock_tracker.h" #include "util/mutexlock.h" +#include "utilities/transactions/lock/lock_tracker.h" // Range Locking: -#include <locktree/locktree.h> #include <locktree/lock_request.h> +#include <locktree/locktree.h> namespace ROCKSDB_NAMESPACE { class RangeLockList; - /* Storage for locks that are currently held by a transaction. @@ -33,30 +32,30 @@ class RangeLockList; This property is currently harmless. */ class RangeLockList /*: public PessimisticTransaction::Lock--Storage */ { -public: - virtual ~RangeLockList() { - clear(); - } + public: + virtual ~RangeLockList() { clear(); } void clear() { - for(auto it : buffers_) { + for (auto it : buffers_) { it.second->destroy(); } buffers_.clear(); } - RangeLockList() : releasing_locks_(false) { - } + RangeLockList() : releasing_locks_(false) {} - void append(uint32_t cf_id, const DBT *left_key, const DBT *right_key) { + void append(uint32_t cf_id, const DBT* left_key, const DBT* right_key) { MutexLock l(&mutex_); // there's only one thread that calls this function. // the same thread will do lock release. assert(!releasing_locks_); - auto it= buffers_.find(cf_id); + auto it = buffers_.find(cf_id); if (it == buffers_.end()) { // create a new one - it= buffers_.emplace(cf_id, std::shared_ptr<toku::range_buffer>(new toku::range_buffer())).first; + it = buffers_ + .emplace(cf_id, std::shared_ptr<toku::range_buffer>( + new toku::range_buffer())) + .first; it->second->create(); } it->second->append(left_key, right_key); @@ -73,45 +72,43 @@ public: class RangeLockTracker : public LockTracker { public: - RangeLockTracker(): range_list(nullptr) {} + RangeLockTracker() : range_list(nullptr) {} RangeLockTracker(const RangeLockTracker&) = delete; RangeLockTracker& operator=(const RangeLockTracker&) = delete; - void Track(const PointLockRequest& ) override; - void Track(const RangeLockRequest& ) override ; - + void Track(const PointLockRequest&) override; + void Track(const RangeLockRequest&) override; + bool IsPointLockSupported() const override { return false; } bool IsRangeLockSupported() const override { return true; } // a Not-supported dummy implementation. - UntrackStatus Untrack( - const RangeLockRequest& /*lock_request*/) override { + UntrackStatus Untrack(const RangeLockRequest& /*lock_request*/) override { return UntrackStatus::NOT_TRACKED; } - UntrackStatus Untrack( - const PointLockRequest& /*lock_request*/) override { + UntrackStatus Untrack(const PointLockRequest& /*lock_request*/) override { return UntrackStatus::NOT_TRACKED; } // "If this method is not supported, leave it as a no-op." - void Merge(const LockTracker& ) override {} + void Merge(const LockTracker&) override {} // "If this method is not supported, leave it as a no-op." - void Subtract(const LockTracker& ) override {} - + void Subtract(const LockTracker&) override {} + void Clear() override; // "If this method is not supported, returns nullptr." virtual LockTracker* GetTrackedLocksSinceSavePoint( - const LockTracker& ) const override { + const LockTracker&) const override { return nullptr; } PointLockStatus GetPointLockStatus(ColumnFamilyId column_family_id, const std::string& key) const override; - + // The return value is only used for tests uint64_t GetNumPointLocks() const override { return 0; } @@ -119,23 +116,23 @@ class RangeLockTracker : public LockTracker { return nullptr; } - KeyIterator* GetKeyIterator(ColumnFamilyId /*column_family_id*/) const override { + KeyIterator* GetKeyIterator( + ColumnFamilyId /*column_family_id*/) const override { return nullptr; } - + // Non-override - RangeLockList *getList() { return range_list.get(); } - RangeLockList *getOrCreateList(); + RangeLockList* getList() { return range_list.get(); } + RangeLockList* getOrCreateList(); private: std::shared_ptr<RangeLockList> range_list; }; -class RangeLockTrackerFactory : public LockTrackerFactory -{ -public: - LockTracker* Create() const override { return new RangeLockTracker; } - +class RangeLockTrackerFactory : public LockTrackerFactory { + public: + LockTracker* Create() const override { return new RangeLockTracker; } + static RangeLockTrackerFactory instance; }; diff --git a/utilities/transactions/optimistic_transaction.cc b/utilities/transactions/optimistic_transaction.cc index 2c5191483..758373416 100644 --- a/utilities/transactions/optimistic_transaction.cc +++ b/utilities/transactions/optimistic_transaction.cc @@ -17,10 +17,10 @@ #include "rocksdb/utilities/optimistic_transaction_db.h" #include "util/cast_util.h" #include "util/string_util.h" -#include "utilities/transactions/transaction_util.h" +#include "utilities/transactions/lock/point_lock_tracker.h" #include "utilities/transactions/optimistic_transaction.h" #include "utilities/transactions/optimistic_transaction_db_impl.h" -#include "utilities/transactions/lock/point_lock_tracker.h" +#include "utilities/transactions/transaction_util.h" namespace ROCKSDB_NAMESPACE { diff --git a/utilities/transactions/pessimistic_transaction.cc b/utilities/transactions/pessimistic_transaction.cc index 865e4ba4d..3d676bfe9 100644 --- a/utilities/transactions/pessimistic_transaction.cc +++ b/utilities/transactions/pessimistic_transaction.cc @@ -38,9 +38,11 @@ TransactionID PessimisticTransaction::GenTxnID() { PessimisticTransaction::PessimisticTransaction( TransactionDB* txn_db, const WriteOptions& write_options, const TransactionOptions& txn_options, const bool init) - : TransactionBaseImpl(txn_db->GetRootDB(), write_options, - static_cast_with_check<PessimisticTransactionDB>(txn_db)-> - getLockMgr()->getLockTrackerFactory()), + : TransactionBaseImpl( + txn_db->GetRootDB(), write_options, + static_cast_with_check<PessimisticTransactionDB>(txn_db) + ->getLockMgr() + ->getLockTrackerFactory()), txn_db_impl_(nullptr), expiration_time_(0), txn_id_(0), @@ -94,7 +96,7 @@ void PessimisticTransaction::Initialize(const TransactionOptions& txn_options) { } PessimisticTransaction::~PessimisticTransaction() { - txn_db_impl_->UnLock(this, *tracked_locks_, /*all_keys_hint=*/true); + txn_db_impl_->UnLock(this, *tracked_locks_, /*all_keys_hint=*/true); if (expiration_time_ > 0) { txn_db_impl_->RemoveExpirableTransaction(txn_id_); } @@ -662,18 +664,17 @@ Status PessimisticTransaction::TryLock(ColumnFamilyHandle* column_family, return s; } -Status -PessimisticTransaction::GetRangeLock(ColumnFamilyHandle* column_family, - const Endpoint& start_endp, - const Endpoint& end_endp) { +Status PessimisticTransaction::GetRangeLock(ColumnFamilyHandle* column_family, + const Endpoint& start_endp, + const Endpoint& end_endp) { ColumnFamilyHandle* cfh = column_family ? column_family : db_impl_->DefaultColumnFamily(); - uint32_t cfh_id= GetColumnFamilyID(cfh); + uint32_t cfh_id = GetColumnFamilyID(cfh); - Status s= txn_db_impl_->TryRangeLock(this, cfh_id, start_endp, end_endp); + Status s = txn_db_impl_->TryRangeLock(this, cfh_id, start_endp, end_endp); if (s.ok()) { - RangeLockRequest req {cfh_id, start_endp, end_endp}; + RangeLockRequest req{cfh_id, start_endp, end_endp}; tracked_locks_->Track(req); } return s; diff --git a/utilities/transactions/pessimistic_transaction.h b/utilities/transactions/pessimistic_transaction.h index 824f7f074..aa52b0a0e 100644 --- a/utilities/transactions/pessimistic_transaction.h +++ b/utilities/transactions/pessimistic_transaction.h @@ -119,6 +119,7 @@ class PessimisticTransaction : public TransactionBaseImpl { virtual Status GetRangeLock(ColumnFamilyHandle* column_family, const Endpoint& start_key, const Endpoint& end_key); + protected: // Refer to // TransactionOptions::use_only_the_last_commit_time_batch_for_recovery diff --git a/utilities/transactions/pessimistic_transaction_db.cc b/utilities/transactions/pessimistic_transaction_db.cc index 9d779cb5f..e5ab97481 100644 --- a/utilities/transactions/pessimistic_transaction_db.cc +++ b/utilities/transactions/pessimistic_transaction_db.cc @@ -37,22 +37,21 @@ PessimisticTransactionDB::PessimisticTransactionDB( } void PessimisticTransactionDB::init_lock_manager() { - if (txn_db_options_.lock_mgr_handle) { // A custom lock manager was provided in options - lock_mgr_ = std::dynamic_pointer_cast<BaseLockMgr>(txn_db_options_.lock_mgr_handle); + lock_mgr_ = + std::dynamic_pointer_cast<BaseLockMgr>(txn_db_options_.lock_mgr_handle); range_lock_mgr_ = dynamic_cast<RangeLockMgr*>(lock_mgr_.get()); } else { // 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); + 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; } @@ -414,16 +413,14 @@ Status PessimisticTransactionDB::TryLock(PessimisticTransaction* txn, return lock_mgr_->TryLock(txn, cfh_id, key, GetEnv(), exclusive); } -Status -PessimisticTransactionDB::TryRangeLock(PessimisticTransaction *txn, - uint32_t cfh_id, - const Endpoint& start_endp, - const Endpoint& end_endp) { +Status PessimisticTransactionDB::TryRangeLock(PessimisticTransaction* txn, + uint32_t cfh_id, + const Endpoint& start_endp, + const Endpoint& end_endp) { if (range_lock_mgr_) { - return range_lock_mgr_->TryRangeLock(txn, cfh_id, start_endp, - end_endp, /*exclusive=*/true); - } - else + return range_lock_mgr_->TryRangeLock(txn, cfh_id, start_endp, end_endp, + /*exclusive=*/true); + } else return Status::NotSupported(); } diff --git a/utilities/transactions/pessimistic_transaction_db.h b/utilities/transactions/pessimistic_transaction_db.h index 837862e52..099715440 100644 --- a/utilities/transactions/pessimistic_transaction_db.h +++ b/utilities/transactions/pessimistic_transaction_db.h @@ -98,13 +98,11 @@ class PessimisticTransactionDB : public TransactionDB { Status TryLock(PessimisticTransaction* txn, uint32_t cfh_id, const std::string& key, bool exclusive); - Status TryRangeLock(PessimisticTransaction* txn, - uint32_t cfh_id, - const Endpoint& start_endp, - const Endpoint& end_endp); + Status TryRangeLock(PessimisticTransaction* txn, uint32_t cfh_id, + const Endpoint& start_endp, const Endpoint& end_endp); void UnLock(PessimisticTransaction* txn, const LockTracker& keys, - bool all_keys_hint=false); + bool all_keys_hint = false); void UnLock(PessimisticTransaction* txn, uint32_t cfh_id, const std::string& key); @@ -147,7 +145,7 @@ class PessimisticTransactionDB : public TransactionDB { virtual void UpdateCFComparatorMap(const std::vector<ColumnFamilyHandle*>&) {} virtual void UpdateCFComparatorMap(ColumnFamilyHandle*) {} - BaseLockMgr *getLockMgr() const { return lock_mgr_.get(); } + BaseLockMgr* getLockMgr() const { return lock_mgr_.get(); } protected: DBImpl* db_impl_; @@ -174,14 +172,15 @@ class PessimisticTransactionDB : public TransactionDB { friend class WriteUnpreparedTransactionTest_RecoveryTest_Test; friend class WriteUnpreparedTransactionTest_MarkLogWithPrepSection_Test; - // Lock manager being used. This is either a TransactionLockMgr or a RangeLockMgr + // Lock manager being used. This is either a TransactionLockMgr or a + // RangeLockMgr std::shared_ptr<BaseLockMgr> lock_mgr_; // Non-null if we are using a lock manager that supports range locking. - RangeLockMgr *range_lock_mgr_ = nullptr; - + RangeLockMgr* range_lock_mgr_ = nullptr; + void init_lock_manager(); - + // Must be held when adding/dropping column families. InstrumentedMutex column_family_mutex_; Transaction* BeginInternalTransaction(const WriteOptions& options); diff --git a/utilities/transactions/range_locking_test.cc b/utilities/transactions/range_locking_test.cc index 8a930ec4d..443703cde 100644 --- a/utilities/transactions/range_locking_test.cc +++ b/utilities/transactions/range_locking_test.cc @@ -25,7 +25,6 @@ using std::string; namespace rocksdb { - class RangeLockingTest : public ::testing::Test { public: TransactionDB* db; @@ -35,8 +34,7 @@ class RangeLockingTest : public ::testing::Test { std::shared_ptr<RangeLockMgrHandle> range_lock_mgr; TransactionDBOptions txn_db_options; - RangeLockingTest() - : db(nullptr) { + RangeLockingTest() : db(nullptr) { options.create_if_missing = true; dbname = test::PerThreadDBPath("range_locking_testdb"); @@ -48,7 +46,6 @@ class RangeLockingTest : public ::testing::Test { s = TransactionDB::Open(options, txn_db_options, dbname, &db); assert(s.ok()); - } ~RangeLockingTest() { @@ -66,7 +63,6 @@ class RangeLockingTest : public ::testing::Test { Transaction* txn = db->BeginTransaction(WriteOptions(), txn_opt); return reinterpret_cast<PessimisticTransaction*>(txn); } - }; // TODO: set a smaller lock wait timeout so that the test runs faster. @@ -82,29 +78,28 @@ TEST_F(RangeLockingTest, BasicRangeLocking) { // Get a range lock { - auto s= txn0->GetRangeLock(cf, Endpoint("a"), Endpoint("c")); + auto s = txn0->GetRangeLock(cf, Endpoint("a"), Endpoint("c")); ASSERT_EQ(s, Status::OK()); } - // Check that range Lock inhibits an overlapping range lock { - auto s= txn1->GetRangeLock(cf, Endpoint("b"), Endpoint("z")); + auto s = txn1->GetRangeLock(cf, Endpoint("b"), Endpoint("z")); ASSERT_TRUE(s.IsTimedOut()); } // Check that range Lock inhibits an overlapping point lock { - auto s= txn1->GetForUpdate(read_options, cf, Slice("b"), &value); + auto s = txn1->GetForUpdate(read_options, cf, Slice("b"), &value); ASSERT_TRUE(s.IsTimedOut()); } // Get a point lock, check that it inhibits range locks { - auto s= txn0->Put(cf, Slice("n"), Slice("value")); + auto s = txn0->Put(cf, Slice("n"), Slice("value")); ASSERT_EQ(s, Status::OK()); - auto s2= txn1->GetRangeLock(cf, Endpoint("m"), Endpoint("p")); + auto s2 = txn1->GetRangeLock(cf, Endpoint("m"), Endpoint("p")); ASSERT_TRUE(s2.IsTimedOut()); } @@ -119,7 +114,7 @@ TEST_F(RangeLockingTest, MyRocksLikeUpdate) { WriteOptions write_options; TransactionOptions txn_options; Transaction* txn0 = db->BeginTransaction(write_options, txn_options); - auto cf= db->DefaultColumnFamily(); + auto cf = db->DefaultColumnFamily(); Status s; // Get a range lock for the range we are about to update @@ -147,8 +142,8 @@ TEST_F(RangeLockingTest, MyRocksLikeUpdate) { TEST_F(RangeLockingTest, SnapshotValidation) { Status s; - Slice key_slice= Slice("k"); - ColumnFamilyHandle *cfh= db->DefaultColumnFamily(); + Slice key_slice = Slice("k"); + ColumnFamilyHandle* cfh = db->DefaultColumnFamily(); auto txn0 = NewTxn(); txn0->Put(key_slice, Slice("initial")); diff --git a/utilities/transactions/transaction_base.cc b/utilities/transactions/transaction_base.cc index 0053c0145..3303784f7 100644 --- a/utilities/transactions/transaction_base.cc +++ b/utilities/transactions/transaction_base.cc @@ -22,7 +22,7 @@ namespace ROCKSDB_NAMESPACE { TransactionBaseImpl::TransactionBaseImpl(DB* db, const WriteOptions& write_options, - const LockTrackerFactory *ltf) + const LockTrackerFactory* ltf) : db_(db), dbimpl_(static_cast_with_check<DBImpl>(db)), write_options_(write_options), diff --git a/utilities/transactions/transaction_base.h b/utilities/transactions/transaction_base.h index 4e3dd6dd4..3f5b48a5d 100644 --- a/utilities/transactions/transaction_base.h +++ b/utilities/transactions/transaction_base.h @@ -29,7 +29,7 @@ namespace ROCKSDB_NAMESPACE { class TransactionBaseImpl : public Transaction { public: TransactionBaseImpl(DB* db, const WriteOptions& write_options, - const LockTrackerFactory *ltf); + const LockTrackerFactory* ltf); virtual ~TransactionBaseImpl(); @@ -281,7 +281,7 @@ class TransactionBaseImpl : public Transaction { const Comparator* cmp_; - const LockTrackerFactory *ltf_; + const LockTrackerFactory* ltf_; // Stores that time the txn was constructed, in microseconds. uint64_t start_time_; @@ -308,7 +308,7 @@ class TransactionBaseImpl : public Transaction { SavePoint(std::shared_ptr<const Snapshot> snapshot, bool snapshot_needed, std::shared_ptr<TransactionNotifier> snapshot_notifier, uint64_t num_puts, uint64_t num_deletes, uint64_t num_merges, - const LockTrackerFactory *ltf) + const LockTrackerFactory* ltf) : snapshot_(snapshot), snapshot_needed_(snapshot_needed), snapshot_notifier_(snapshot_notifier), @@ -317,28 +317,28 @@ class TransactionBaseImpl : public Transaction { num_merges_(num_merges), new_locks_(ltf->Create()) {} - SavePoint(const LockTrackerFactory *ltf) : new_locks_(ltf->Create()) {} + SavePoint(const LockTrackerFactory* ltf) : new_locks_(ltf->Create()) {} - SavePoint(const SavePoint &s) {new_locks_ = s.new_locks_;} + SavePoint(const SavePoint& s) { new_locks_ = s.new_locks_; } }; // Records writes pending in this transaction WriteBatchWithIndex write_batch_; -public: + public: // For Pessimistic Transactions this is the set of acquired locks. // Optimistic Transactions will keep note the requested locks (not actually // locked), and do conflict checking until commit time based on the tracked // lock requests. -/* - psergey-merge: it's public because there are these users: - - RangeLockMgr::UnLockAll (probably solvable) - - RangeLockMgr::on_escalate -- HARDER! -*/ + /* + psergey-merge: it's public because there are these users: + - RangeLockMgr::UnLockAll (probably solvable) + - RangeLockMgr::on_escalate -- HARDER! + */ std::unique_ptr<LockTracker> tracked_locks_; -protected: + protected: // Stack of the Snapshot saved at each save point. Saved snapshots may be // nullptr if there was no snapshot at the time SetSavePoint() was called. std::unique_ptr<std::stack<TransactionBaseImpl::SavePoint, diff --git a/utilities/transactions/transaction_lock_mgr.cc b/utilities/transactions/transaction_lock_mgr.cc index 340bbdb5d..75e683b25 100644 --- a/utilities/transactions/transaction_lock_mgr.cc +++ b/utilities/transactions/transaction_lock_mgr.cc @@ -18,9 +18,9 @@ #include "util/cast_util.h" #include "util/hash.h" #include "util/thread_local.h" +#include "utilities/transactions/lock/range_lock_tracker.h" #include "utilities/transactions/pessimistic_transaction_db.h" #include "utilities/transactions/transaction_db_mutex_impl.h" -#include "utilities/transactions/lock/range_lock_tracker.h" namespace ROCKSDB_NAMESPACE { @@ -178,8 +178,8 @@ size_t LockMap::GetStripe(const std::string& key) const { return fastrange64(GetSliceNPHash64(key), num_stripes_); } -void TransactionLockMgr::AddColumnFamily(const ColumnFamilyHandle *cfh) { - uint32_t column_family_id= cfh->GetID(); +void TransactionLockMgr::AddColumnFamily(const ColumnFamilyHandle* cfh) { + uint32_t column_family_id = cfh->GetID(); InstrumentedMutexLock l(&lock_map_mutex_); if (lock_maps_.find(column_family_id) == lock_maps_.end()) { @@ -191,8 +191,8 @@ void TransactionLockMgr::AddColumnFamily(const ColumnFamilyHandle *cfh) { } } -void TransactionLockMgr::RemoveColumnFamily(const ColumnFamilyHandle *cfh) { - uint32_t column_family_id= cfh->GetID(); +void TransactionLockMgr::RemoveColumnFamily(const ColumnFamilyHandle* cfh) { + uint32_t column_family_id = cfh->GetID(); // Remove lock_map for this column family. Since the lock map is stored // as a shared ptr, concurrent transactions can still keep using it // until they release their references to it. @@ -742,13 +742,12 @@ void TransactionLockMgr::Resize(uint32_t target_size) { dlock_buffer_.Resize(target_size); } - ///////////////////////////////////////////////////////////////////////////// // RangeLockMgr - a lock manager that supports range locking ///////////////////////////////////////////////////////////////////////////// RangeLockMgrHandle* NewRangeLockManager( - std::shared_ptr<TransactionDBMutexFactory> mutex_factory) { + std::shared_ptr<TransactionDBMutexFactory> mutex_factory) { std::shared_ptr<TransactionDBMutexFactory> use_factory; if (mutex_factory) @@ -759,22 +758,19 @@ RangeLockMgrHandle* NewRangeLockManager( return new RangeLockMgr(use_factory); } +static const char SUFFIX_INFIMUM = 0x0; +static const char SUFFIX_SUPREMUM = 0x1; -static const char SUFFIX_INFIMUM= 0x0; -static const char SUFFIX_SUPREMUM= 0x1; - -void serialize_endpoint(const Endpoint &endp, std::string *buf) { +void serialize_endpoint(const Endpoint& endp, std::string* buf) { buf->push_back(endp.inf_suffix ? SUFFIX_SUPREMUM : SUFFIX_INFIMUM); buf->append(endp.slice.data(), endp.slice.size()); } - // Get a range lock on [start_key; end_key] range Status RangeLockMgr::TryRangeLock(PessimisticTransaction* txn, uint32_t column_family_id, - const Endpoint &start_endp, - const Endpoint &end_endp, - bool exclusive) { + const Endpoint& start_endp, + const Endpoint& end_endp, bool exclusive) { toku::lock_request request; request.create(mutex_factory_); DBT start_key_dbt, end_key_dbt; @@ -794,24 +790,22 @@ Status RangeLockMgr::TryRangeLock(PessimisticTransaction* txn, // locktree::kill_waiter call. Do we need this anymore? TransactionID wait_txn_id = txn->GetID(); - auto lt= get_locktree_by_cfid(column_family_id); + auto lt = get_locktree_by_cfid(column_family_id); request.set(lt, (TXNID)txn, &start_key_dbt, &end_key_dbt, - exclusive? toku::lock_request::WRITE: toku::lock_request::READ, - false /* not a big txn */, - (void*)wait_txn_id); - - uint64_t killed_time_msec = 0; // TODO: what should this have? + exclusive ? toku::lock_request::WRITE : toku::lock_request::READ, + false /* not a big txn */, (void*)wait_txn_id); + + uint64_t killed_time_msec = 0; // TODO: what should this have? uint64_t wait_time_msec = txn->GetLockTimeout(); // convert microseconds to milliseconds if (wait_time_msec != (uint64_t)-1) wait_time_msec = (wait_time_msec + 500) / 1000; std::vector<DeadlockInfo> di_path; - request.m_deadlock_cb = [&] (TXNID txnid, bool is_exclusive, - std::string key) { - di_path.push_back({((PessimisticTransaction*)txnid)->GetID(), - column_family_id, is_exclusive, key}); + request.m_deadlock_cb = [&](TXNID txnid, bool is_exclusive, std::string key) { + di_path.push_back({((PessimisticTransaction*)txnid)->GetID(), + column_family_id, is_exclusive, key}); }; request.start(); @@ -827,80 +821,77 @@ Status RangeLockMgr::TryRangeLock(PessimisticTransaction* txn, struct st_wait_info { PessimisticTransaction* txn; uint32_t column_family_id; - std::string *key_ptr; + std::string* key_ptr; autovector<TransactionID> wait_ids; - bool done= false; + bool done = false; - static void lock_wait_callback(void *cdata, TXNID /*waiter*/, TXNID waitee) { + static void lock_wait_callback(void* cdata, TXNID /*waiter*/, + TXNID waitee) { TEST_SYNC_POINT("RangeLockMgr::TryRangeLock:WaitingTxn"); - auto self= (struct st_wait_info*)cdata; + auto self = (struct st_wait_info*)cdata; // we know that the waiter is self->txn->GetID() (TODO: assert?) - if (!self->done) - { + if (!self->done) { self->wait_ids.push_back(waitee); self->txn->SetWaitingTxn(self->wait_ids, self->column_family_id, self->key_ptr); - self->done= true; + self->done = true; } } } wait_info; - wait_info.txn= txn; - wait_info.column_family_id= column_family_id; - wait_info.key_ptr= &key_str; - wait_info.done= false; + wait_info.txn = txn; + wait_info.column_family_id = column_family_id; + wait_info.key_ptr = &key_str; + wait_info.done = false; - const int r = request.wait(wait_time_msec, killed_time_msec, - nullptr, // killed_callback - st_wait_info::lock_wait_callback, - (void*)&wait_info); + const int r = + request.wait(wait_time_msec, killed_time_msec, + nullptr, // killed_callback + st_wait_info::lock_wait_callback, (void*)&wait_info); // Inform the txn that we are no longer waiting: txn->ClearWaitingTxn(); request.destroy(); switch (r) { - case 0: - break; /* fall through */ - case DB_LOCK_NOTGRANTED: - return Status::TimedOut(Status::SubCode::kLockTimeout); - case TOKUDB_OUT_OF_LOCKS: - return Status::Busy(Status::SubCode::kLockLimit); - case DB_LOCK_DEADLOCK: - { - std::reverse(di_path.begin(), di_path.end()); - dlock_buffer_.AddNewPath(DeadlockPath(di_path, request.get_start_time())); - return Status::Busy(Status::SubCode::kDeadlock); - } - default: + case 0: + break; /* fall through */ + case DB_LOCK_NOTGRANTED: + return Status::TimedOut(Status::SubCode::kLockTimeout); + case TOKUDB_OUT_OF_LOCKS: + return Status::Busy(Status::SubCode::kLockLimit); + case DB_LOCK_DEADLOCK: { + std::reverse(di_path.begin(), di_path.end()); + dlock_buffer_.AddNewPath(DeadlockPath(di_path, request.get_start_time())); + return Status::Busy(Status::SubCode::kDeadlock); + } + default: assert(0); return Status::Busy(Status::SubCode::kLockLimit); } // Don't: save the acquired lock in txn->owned_locks. // It is now responsibility of RangeLockMgr - // RangeLockList* range_list= ((RangeLockTracker*)txn->tracked_locks_.get())->getOrCreateList(); + // RangeLockList* range_list= + // ((RangeLockTracker*)txn->tracked_locks_.get())->getOrCreateList(); // range_list->append(column_family_id, &start_key_dbt, &end_key_dbt); return Status::OK(); } - // Get a singlepoint lock // (currently it is the same as getting a range lock) Status RangeLockMgr::TryLock(PessimisticTransaction* txn, - uint32_t column_family_id, - const std::string& key, Env*, - bool exclusive) { + uint32_t column_family_id, const std::string& key, + Env*, bool exclusive) { Endpoint endp(key.data(), key.size(), false); return TryRangeLock(txn, column_family_id, endp, endp, exclusive); } -static void -range_lock_mgr_release_lock_int(toku::locktree *lt, - const PessimisticTransaction* txn, - uint32_t /*column_family_id*/, - const std::string& key) { +static void range_lock_mgr_release_lock_int(toku::locktree* lt, + const PessimisticTransaction* txn, + uint32_t /*column_family_id*/, + const std::string& key) { DBT key_dbt; Endpoint endp(key.data(), key.size(), false); std::string endp_image; @@ -915,11 +906,12 @@ range_lock_mgr_release_lock_int(toku::locktree *lt, } void RangeLockMgr::UnLock(PessimisticTransaction* txn, - uint32_t column_family_id, - const std::string& key, Env*) { - auto lt= get_locktree_by_cfid(column_family_id); + uint32_t column_family_id, const std::string& key, + Env*) { + auto lt = get_locktree_by_cfid(column_family_id); range_lock_mgr_release_lock_int(lt, txn, column_family_id, key); - toku::lock_request::retry_all_lock_requests(lt, nullptr /* lock_wait_needed_callback */); + toku::lock_request::retry_all_lock_requests( + lt, nullptr /* lock_wait_needed_callback */); } void RangeLockMgr::UnLock(const PessimisticTransaction* /*txn*/, @@ -943,14 +935,12 @@ void RangeLockMgr::UnLock(const PessimisticTransaction* /*txn*/, } void RangeLockMgr::UnLockAll(const PessimisticTransaction* txn, Env*) { - // tracked_locks_->range_list may hold nullptr if the transaction has never // acquired any locks. - RangeLockList* range_list= - ((RangeLockTracker*)txn->tracked_locks_.get())->getList(); + RangeLockList* range_list = + ((RangeLockTracker*)txn->tracked_locks_.get())->getList(); - if (range_list) - { + if (range_list) { { MutexLock l(&range_list->mutex_); /* @@ -979,16 +969,16 @@ void RangeLockMgr::UnLockAll(const PessimisticTransaction* txn, Env*) { (the code in this function doesnt do that as there's only one thread that releases transaction's locks) */ - range_list->releasing_locks_= true; + range_list->releasing_locks_ = true; } // Don't try to call release_locks() if the buffer is empty! if we are - // not holding any locks, the lock tree might be in the STO-mode with - // another transaction, and our attempt to release an empty set of locks + // not holding any locks, the lock tree might be in the STO-mode with + // another transaction, and our attempt to release an empty set of locks // will cause an assertion failure. - for (auto it: range_list->buffers_) { + for (auto it : range_list->buffers_) { if (it.second->get_num_ranges()) { - toku::locktree *lt = get_locktree_by_cfid(it.first); + toku::locktree* lt = get_locktree_by_cfid(it.first); lt->release_locks((TXNID)txn, it.second.get(), true); it.second->destroy(); @@ -998,52 +988,42 @@ void RangeLockMgr::UnLockAll(const PessimisticTransaction* txn, Env*) { } } range_list->clear(); - range_list->releasing_locks_= false; + range_list->releasing_locks_ = false; } } +int RangeLockMgr::compare_dbt_endpoints(__toku_db*, void* arg, const DBT* a_key, + const DBT* b_key) { + const char* a = (const char*)a_key->data; + const char* b = (const char*)b_key->data; -int RangeLockMgr::compare_dbt_endpoints(__toku_db*, void *arg, - const DBT *a_key, - const DBT *b_key) { - const char *a= (const char*)a_key->data; - const char *b= (const char*)b_key->data; - - size_t a_len= a_key->size; - size_t b_len= b_key->size; + size_t a_len = a_key->size; + size_t b_len = b_key->size; - size_t min_len= std::min(a_len, b_len); + size_t min_len = std::min(a_len, b_len); // 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) - { + 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] == SUFFIX_INFIMUM) - return -1; //"a is smaller" - else - { + return -1; //"a is smaller" + else { // a is considered padded with 0xFF:FF:FF:FF... - return 1; // "a" is bigger + return 1; // "a" is bigger } - } - else if (a_len > min_len) - { + } else if (a_len > min_len) { // the opposite of the above: b is shorter. if (b[0] == SUFFIX_INFIMUM) - return 1; //"b is smaller" - else - { + return 1; //"b is smaller" + else { // b is considered padded with 0xFF:FF:FF:FF... - return -1; // "b" is bigger + return -1; // "b" is bigger } - } - else - { + } else { // the lengths are equal (and the key values, too) if (a[0] < b[0]) return -1; @@ -1052,15 +1032,15 @@ int RangeLockMgr::compare_dbt_endpoints(__toku_db*, void *arg, else return 0; } - } - else + } else return res; } -RangeLockMgr::RangeLockMgr(std::shared_ptr<TransactionDBMutexFactory> mutex_factory) : - mutex_factory_(mutex_factory), - ltree_lookup_cache_(new ThreadLocalPtr(nullptr)), - dlock_buffer_(10) { +RangeLockMgr::RangeLockMgr( + std::shared_ptr<TransactionDBMutexFactory> mutex_factory) + : mutex_factory_(mutex_factory), + ltree_lookup_cache_(new ThreadLocalPtr(nullptr)), + dlock_buffer_(10) { ltm_.create(on_create, on_destroy, on_escalate, NULL, mutex_factory_); } @@ -1076,18 +1056,19 @@ std::vector<DeadlockPath> RangeLockMgr::GetDeadlockInfoBuffer() { @brief Lock Escalation Callback function @param txnid Transaction whose locks got escalated - @param lt Lock Tree where escalation is happening (currently there is only one) + @param lt Lock Tree where escalation is happening (currently there is + only one) @param buffer Escalation result: list of locks that this transaction now owns in this lock tree. @param void* Callback context */ void RangeLockMgr::on_escalate(TXNID txnid, const locktree* lt, - const range_buffer &buffer, void *) { - auto txn= (PessimisticTransaction*)txnid; + const range_buffer& buffer, void*) { + auto txn = (PessimisticTransaction*)txnid; RangeLockList* trx_locks = - ((RangeLockTracker*)txn->tracked_locks_.get())->getList(); + ((RangeLockTracker*)txn->tracked_locks_.get())->getList(); MutexLock l(&trx_locks->mutex_); if (trx_locks->releasing_locks_) { @@ -1099,11 +1080,12 @@ void RangeLockMgr::on_escalate(TXNID txnid, const locktree* lt, return; } - // TODO: are we tracking this mem: lt->get_manager()->note_mem_released(trx_locks->ranges.buffer->total_memory_size()); + // TODO: are we tracking this mem: + // lt->get_manager()->note_mem_released(trx_locks->ranges.buffer->total_memory_size()); uint32_t cf_id = (uint32_t)lt->get_dict_id().dictid; - auto it= trx_locks->buffers_.find(cf_id); + auto it = trx_locks->buffers_.find(cf_id); it->second->destroy(); it->second->create(); @@ -1113,13 +1095,13 @@ void RangeLockMgr::on_escalate(TXNID txnid, const locktree* lt, it->second->append(rec.get_left_key(), rec.get_right_key()); iter.next(); } - // TODO: same as above: lt->get_manager()->note_mem_used(ranges.buffer->total_memory_size()); + // TODO: same as above: + // lt->get_manager()->note_mem_used(ranges.buffer->total_memory_size()); } - RangeLockMgr::~RangeLockMgr() { - //TODO: at this point, synchronization is not needed, right? - for (auto it: ltree_map_) { + // TODO: at this point, synchronization is not needed, right? + for (auto it : ltree_map_) { ltm_.release_lt(it.second); } ltm_.destroy(); @@ -1129,44 +1111,44 @@ RangeLockMgrHandle::Counters RangeLockMgr::GetStatus() { LTM_STATUS_S ltm_status_test; ltm_.get_status(<m_status_test); Counters res; - + // Searching status variable by its string name is how Toku's unit tests // do it (why didn't they make LTM_ESCALATION_COUNT constant visible?) // lookup keyname in status for (int i = 0; i < LTM_STATUS_S::LTM_STATUS_NUM_ROWS; i++) { - TOKU_ENGINE_STATUS_ROW status = <m_status_test.status[i]; - if (strcmp(status->keyname, "LTM_ESCALATION_COUNT") == 0) { - res.escalation_count = status->value.num; - continue; - } - if (strcmp(status->keyname, "LTM_SIZE_CURRENT") == 0) { - res.current_lock_memory = status->value.num; - } + TOKU_ENGINE_STATUS_ROW status = <m_status_test.status[i]; + if (strcmp(status->keyname, "LTM_ESCALATION_COUNT") == 0) { + res.escalation_count = status->value.num; + continue; + } + if (strcmp(status->keyname, "LTM_SIZE_CURRENT") == 0) { + res.current_lock_memory = status->value.num; + } } return res; } -void RangeLockMgr::AddColumnFamily(const ColumnFamilyHandle *cfh) { - uint32_t column_family_id= cfh->GetID(); +void RangeLockMgr::AddColumnFamily(const ColumnFamilyHandle* cfh) { + uint32_t column_family_id = cfh->GetID(); InstrumentedMutexLock l(<ree_map_mutex_); if (ltree_map_.find(column_family_id) == ltree_map_.end()) { - DICTIONARY_ID dict_id = { .dictid = column_family_id }; + DICTIONARY_ID dict_id = {.dictid = column_family_id}; 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); + 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 - //assert(false); + // assert(false); } } -void RangeLockMgr::RemoveColumnFamily(const ColumnFamilyHandle *cfh) { - uint32_t column_family_id= cfh->GetID(); +void RangeLockMgr::RemoveColumnFamily(const ColumnFamilyHandle* cfh) { + uint32_t column_family_id = cfh->GetID(); // Remove lock_map for this column family. Since the lock map is stored // as a shared ptr, concurrent transactions can still keep using it // until they release their references to it. @@ -1185,7 +1167,7 @@ void RangeLockMgr::RemoveColumnFamily(const ColumnFamilyHandle *cfh) { ltree_map_.erase(lock_maps_iter); } // lock_map_mutex_ - //TODO: why do we delete first and clear the caches second? Shouldn't this be + // TODO: why do we delete first and clear the caches second? Shouldn't this be // done in the reverse order? (if we do it in the reverse order, how will we // prevent somebody from re-populating the cache?) @@ -1195,8 +1177,7 @@ void RangeLockMgr::RemoveColumnFamily(const ColumnFamilyHandle *cfh) { ltree_lookup_cache_->Scrape(&local_caches, nullptr); } -toku::locktree *RangeLockMgr::get_locktree_by_cfid(uint32_t column_family_id) { - +toku::locktree* RangeLockMgr::get_locktree_by_cfid(uint32_t column_family_id) { // First check thread-local cache if (ltree_lookup_cache_->Get() == nullptr) { ltree_lookup_cache_->Reset(new LockTreeMap()); @@ -1218,7 +1199,7 @@ toku::locktree *RangeLockMgr::get_locktree_by_cfid(uint32_t column_family_id) { return nullptr; } else { // Found lock map. Store in thread-local cache and return. - //std::shared_ptr<LockMap>& lock_map = lock_map_iter->second; + // std::shared_ptr<LockMap>& lock_map = lock_map_iter->second; ltree_map_cache->insert({column_family_id, it->second}); return it->second; } @@ -1227,48 +1208,44 @@ toku::locktree *RangeLockMgr::get_locktree_by_cfid(uint32_t column_family_id) { } struct LOCK_PRINT_CONTEXT { - BaseLockMgr::LockStatusData *data; // Save locks here - uint32_t cfh_id; // Column Family whose tree we are traversing + BaseLockMgr::LockStatusData* data; // Save locks here + uint32_t cfh_id; // Column Family whose tree we are traversing }; -static -void push_into_lock_status_data(void* param, - const DBT *left, const DBT *right, - TXNID txnid_arg, bool is_shared, - TxnidVector *owners) { - struct LOCK_PRINT_CONTEXT *ctx= (LOCK_PRINT_CONTEXT*)param; +static void push_into_lock_status_data(void* param, const DBT* left, + const DBT* right, TXNID txnid_arg, + bool is_shared, TxnidVector* owners) { + struct LOCK_PRINT_CONTEXT* ctx = (LOCK_PRINT_CONTEXT*)param; struct KeyLockInfo info; info.key.append((const char*)left->data, (size_t)left->size); - info.exclusive= !is_shared; + info.exclusive = !is_shared; - if (!(left->size == right->size && - !memcmp(left->data, right->data, left->size))) - { - // not a single-point lock - info.has_key2= true; + if (!(left->size == right->size && + !memcmp(left->data, right->data, left->size))) { + // not a single-point lock + info.has_key2 = true; info.key2.append((const char*)right->data, right->size); } if (txnid_arg != TXNID_SHARED) { - TXNID txnid= ((PessimisticTransaction*)txnid_arg)->GetID(); + TXNID txnid = ((PessimisticTransaction*)txnid_arg)->GetID(); info.ids.push_back(txnid); } else { for (auto it : *owners) { - TXNID real_id= ((PessimisticTransaction*)it)->GetID(); + TXNID real_id = ((PessimisticTransaction*)it)->GetID(); info.ids.push_back(real_id); } } ctx->data->insert({ctx->cfh_id, info}); } - BaseLockMgr::LockStatusData RangeLockMgr::GetLockStatusData() { LockStatusData data; { InstrumentedMutexLock l(<ree_map_mutex_); for (auto it : ltree_map_) { - LOCK_PRINT_CONTEXT ctx = {&data, it.first }; + LOCK_PRINT_CONTEXT ctx = {&data, it.first}; it.second->dump_locks((void*)&ctx, push_into_lock_status_data); } } diff --git a/utilities/transactions/transaction_lock_mgr.h b/utilities/transactions/transaction_lock_mgr.h index 1b754ce8e..45fe228a3 100644 --- a/utilities/transactions/transaction_lock_mgr.h +++ b/utilities/transactions/transaction_lock_mgr.h @@ -17,14 +17,14 @@ #include "util/autovector.h" #include "util/hash_map.h" #include "util/thread_local.h" -#include "utilities/transactions/pessimistic_transaction.h" #include "utilities/transactions/lock/lock_tracker.h" #include "utilities/transactions/lock/point_lock_tracker.h" #include "utilities/transactions/lock/range_lock_tracker.h" +#include "utilities/transactions/pessimistic_transaction.h" // Range Locking: -#include <locktree/locktree.h> #include <locktree/lock_request.h> +#include <locktree/locktree.h> namespace ROCKSDB_NAMESPACE { @@ -63,34 +63,30 @@ class PessimisticTransactionDB; // class BaseLockMgr { public: - virtual LockTrackerFactory *getLockTrackerFactory() = 0; + virtual LockTrackerFactory* getLockTrackerFactory() = 0; - virtual void AddColumnFamily(const ColumnFamilyHandle *cfh) = 0; - virtual void RemoveColumnFamily(const ColumnFamilyHandle *cfh) = 0; + virtual void AddColumnFamily(const ColumnFamilyHandle* cfh) = 0; + virtual void RemoveColumnFamily(const ColumnFamilyHandle* cfh) = 0; - virtual - Status TryLock(PessimisticTransaction* txn, uint32_t column_family_id, - const std::string& key, Env* env, bool exclusive) = 0; - virtual - void UnLock(const PessimisticTransaction* txn, const LockTracker& tracker, - Env* env) = 0; - virtual - void UnLock(PessimisticTransaction* txn, uint32_t column_family_id, - const std::string& key, Env* env) = 0; + virtual Status TryLock(PessimisticTransaction* txn, uint32_t column_family_id, + const std::string& key, Env* env, bool exclusive) = 0; + virtual void UnLock(const PessimisticTransaction* txn, + const LockTracker& tracker, Env* env) = 0; + virtual void UnLock(PessimisticTransaction* txn, uint32_t column_family_id, + const std::string& key, Env* env) = 0; // Resize the deadlock info buffer - virtual void Resize(uint32_t)=0; - virtual std::vector<DeadlockPath> GetDeadlockInfoBuffer()= 0; + virtual void Resize(uint32_t) = 0; + virtual std::vector<DeadlockPath> GetDeadlockInfoBuffer() = 0; // TransactionDB will call this at start - virtual void init(TransactionDB*) {}; - virtual ~BaseLockMgr(){} + virtual void init(TransactionDB*){}; + virtual ~BaseLockMgr() {} using LockStatusData = std::unordered_multimap<uint32_t, KeyLockInfo>; - virtual LockStatusData GetLockStatusData()=0; + virtual LockStatusData GetLockStatusData() = 0; }; - // Point lock manager class TransactionLockMgr : public BaseLockMgr { public: @@ -109,11 +105,11 @@ class TransactionLockMgr : public BaseLockMgr { // Creates a new LockMap for this column family. Caller should guarantee // that this column family does not already exist. - void AddColumnFamily(const ColumnFamilyHandle *cfh); + void AddColumnFamily(const ColumnFamilyHandle* cfh); // Deletes the LockMap for this column family. Caller should guarantee that // this column family is no longer in use. - void RemoveColumnFamily(const ColumnFamilyHandle *cfh); + void RemoveColumnFamily(const ColumnFamilyHandle* cfh); // Attempt to lock key. If OK status is returned, the caller is responsible // for calling UnLock() on this key. @@ -132,7 +128,6 @@ class TransactionLockMgr : public BaseLockMgr { void Resize(uint32_t) override; private: - PessimisticTransactionDB* txn_db_impl_; // Default number of lock map stripes per column family @@ -198,25 +193,21 @@ class TransactionLockMgr : public BaseLockMgr { const autovector<TransactionID>& wait_ids); }; - using namespace toku; /* A lock manager that supports Range-based locking. */ -class RangeLockMgr : - public BaseLockMgr, - public RangeLockMgrHandle { +class RangeLockMgr : public BaseLockMgr, public RangeLockMgrHandle { public: - LockTrackerFactory* getLockTrackerFactory() override { return &RangeLockTrackerFactory::instance; } - void AddColumnFamily(const ColumnFamilyHandle *cfh) override; - void RemoveColumnFamily(const ColumnFamilyHandle *cfh) override; + void AddColumnFamily(const ColumnFamilyHandle* cfh) override; + void RemoveColumnFamily(const ColumnFamilyHandle* cfh) override; Status TryLock(PessimisticTransaction* txn, uint32_t column_family_id, - const std::string& key, Env* env, bool exclusive) override ; + const std::string& key, Env* env, bool exclusive) override; // Resize the deadlock-info buffer, does nothing currently void Resize(uint32_t) override; @@ -225,24 +216,20 @@ class RangeLockMgr : // Get a lock on a range // @note only exclusive locks are currently supported (requesting a // non-exclusive lock will get an exclusive one) - Status TryRangeLock(PessimisticTransaction* txn, - uint32_t column_family_id, - const Endpoint &start_endp, - const Endpoint &end_endp, + Status TryRangeLock(PessimisticTransaction* txn, uint32_t column_family_id, + const Endpoint& start_endp, const Endpoint& end_endp, bool exclusive); - + void UnLock(const PessimisticTransaction* txn, const LockTracker& tracker, - Env* env) override ; + Env* env) override; // Release all locks the transaction is holding void UnLockAll(const PessimisticTransaction* txn, Env* env); void UnLock(PessimisticTransaction* txn, uint32_t column_family_id, - const std::string& key, Env* env) override ; + const std::string& key, Env* env) override; RangeLockMgr(std::shared_ptr<TransactionDBMutexFactory> mutex_factory); - void init(TransactionDB *db_arg) override { - my_txn_db_ = db_arg; - } + void init(TransactionDB* db_arg) override { my_txn_db_ = db_arg; } ~RangeLockMgr(); @@ -250,9 +237,7 @@ class RangeLockMgr : return ltm_.set_max_lock_memory(max_lock_memory); } - size_t get_max_lock_memory() { - return ltm_.get_max_lock_memory(); - } + size_t get_max_lock_memory() { return ltm_.get_max_lock_memory(); } Counters GetStatus() override; @@ -278,19 +263,19 @@ class RangeLockMgr : DeadlockInfoBuffer dlock_buffer_; // Get the lock tree which stores locks for Column Family with given cf_id - toku::locktree *get_locktree_by_cfid(uint32_t cf_id); + 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(__toku_db*, void* arg, const DBT* a_key, + const DBT* b_key); // Callbacks - static int on_create(locktree*, void*) { return 0; /* no error */ } + static int on_create(locktree*, void*) { return 0; /* no error */ } static void on_destroy(locktree*) {} - static void on_escalate(TXNID txnid, const locktree *lt, - const range_buffer &buffer, void *extra); - + static void on_escalate(TXNID txnid, const locktree* lt, + const range_buffer& buffer, void* extra); }; -void serialize_endpoint(const Endpoint &endp, std::string *buf); +void serialize_endpoint(const Endpoint& endp, std::string* buf); } // namespace ROCKSDB_NAMESPACE #endif // ROCKSDB_LITE diff --git a/utilities/transactions/transaction_lock_mgr_test.cc b/utilities/transactions/transaction_lock_mgr_test.cc index 2d79f788a..9ebae4074 100644 --- a/utilities/transactions/transaction_lock_mgr_test.cc +++ b/utilities/transactions/transaction_lock_mgr_test.cc @@ -47,11 +47,9 @@ class TransactionLockMgrTest : public testing::Test { // If not using range locking, this test creates a separate lock manager // object (right, NOT the one TransactionDB is using!), and runs tests on // that. - locker_ = std::shared_ptr<TransactionLockMgr>( - new TransactionLockMgr(db_, txn_opt.num_stripes, - txn_opt.max_num_locks, - txn_opt.max_num_deadlocks, - mutex_factory_)); + locker_ = std::shared_ptr<TransactionLockMgr>(new TransactionLockMgr( + db_, txn_opt.num_stripes, txn_opt.max_num_locks, + txn_opt.max_num_deadlocks, mutex_factory_)); } } @@ -74,7 +72,7 @@ class TransactionLockMgrTest : public testing::Test { // With Range Locking, functions like GetLockStatusData() return range // endpoints, not the keys themselves. This function extracts the key. - std::string key_value(const std::string &s) { + std::string key_value(const std::string& s) { if (use_range_locking) return s.substr(1); else @@ -89,16 +87,14 @@ class TransactionLockMgrTest : public testing::Test { class AnyLockManagerTest : public TransactionLockMgrTest, public testing::WithParamInterface<bool> { -public: - AnyLockManagerTest() { - use_range_locking = GetParam(); - } + public: + AnyLockManagerTest() { use_range_locking = GetParam(); } }; - class MockColumnFamily : public ColumnFamilyHandle { uint32_t id_; std::string name; + public: ~MockColumnFamily() {} MockColumnFamily(uint32_t id_arg) : id_(id_arg) {} @@ -106,7 +102,7 @@ class MockColumnFamily : public ColumnFamilyHandle { const std::string& GetName() const override { return name; } - Status GetDescriptor(ColumnFamilyDescriptor* ) override { + Status GetDescriptor(ColumnFamilyDescriptor*) override { assert(0); return Status::NotSupported(); } @@ -132,7 +128,6 @@ TEST_F(TransactionLockMgrTest, LockNonExistingColumnFamily) { delete txn; } - TEST_P(AnyLockManagerTest, LockStatus) { locker_->AddColumnFamily(&cf_1024); locker_->AddColumnFamily(&cf_2048); @@ -299,11 +294,10 @@ port::Thread BlockUntilWaitingTxn(bool use_range_locking, std::function<void()> f) { std::atomic<bool> reached(false); const char* sync_point_name = - use_range_locking? "RangeLockMgr::TryRangeLock:WaitingTxn": - "TransactionLockMgr::AcquireWithTimeout:WaitingTxn"; + use_range_locking ? "RangeLockMgr::TryRangeLock:WaitingTxn" + : "TransactionLockMgr::AcquireWithTimeout:WaitingTxn"; ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->SetCallBack( - sync_point_name, - [&](void* /*arg*/) { reached.store(true); }); + sync_point_name, [&](void* /*arg*/) { reached.store(true); }); ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->EnableProcessing(); port::Thread t(f); @@ -330,7 +324,6 @@ TEST_P(AnyLockManagerTest, SharedLocks) { // Cleanup locker_->UnLock(txn1, 1, "k", env_); locker_->UnLock(txn2, 1, "k", env_); - } TEST_P(AnyLockManagerTest, Deadlock) { @@ -385,7 +378,6 @@ TEST_P(AnyLockManagerTest, Deadlock) { delete txn1; } - // This test doesn't work with Range Lock Manager, because Range Lock Manager // doesn't support deadlock_detect_depth. diff --git a/utilities/transactions/write_unprepared_txn.cc b/utilities/transactions/write_unprepared_txn.cc index 41fab015b..d2dfda31c 100644 --- a/utilities/transactions/write_unprepared_txn.cc +++ b/utilities/transactions/write_unprepared_txn.cc @@ -800,7 +800,7 @@ Status WriteUnpreparedTxn::RollbackInternal() { void WriteUnpreparedTxn::Clear() { if (!recovered_txn_) { - txn_db_impl_->UnLock(this, *tracked_locks_, /*all_keys_hint=*/ true); + txn_db_impl_->UnLock(this, *tracked_locks_, /*all_keys_hint=*/true); } unprep_seqs_.clear(); flushed_save_points_.reset(nullptr);