- commits - lists.mariadb.org

[Commits] ef985d3de92: MDEV-23449: alias do not exist and a query do not report an error
by Varun 12 Aug '20

12 Aug '20

revision-id: ef985d3de923334248a03732b042a18f18925e8a (mariadb-10.1.43-251-gef985d3de92) parent(s): ab578bdf453c3cb0e9ca561cf373f64c96b22fda author: Varun Gupta committer: Varun Gupta timestamp: 2020-08-12 02:19:17 +0530 message: MDEV-23449: alias do not exist and a query do not report an error For an IN/ANY/ALL subquery without an aggregate function and HAVING clause, the GROUP BY clause is removed. Due to the GROUP BY list being removed, the invalid reference in the GROUP BY clause was never resolved. Remove the GROUP BY list only when the all the items in the GROUP BY list are resolved. --- mysql-test/r/subselect4.result | 8 ++++++++ mysql-test/t/subselect4.test | 11 +++++++++++ sql/sql_select.cc | 32 ++++++++++++++++---------------- 3 files changed, 35 insertions(+), 16 deletions(-) diff --git a/mysql-test/r/subselect4.result b/mysql-test/r/subselect4.result index e7655131fcf..a4fd1123227 100644 --- a/mysql-test/r/subselect4.result +++ b/mysql-test/r/subselect4.result @@ -2648,4 +2648,12 @@ a 1 2 DROP TABLE t1,t2; +# +# MDEV-23449: alias do not exist and a query do not report an error +# +CREATE TABLE t1(a INT, b INT); +INSERT INTO t1 VALUES (1,1), (2,2), (3,3), (4,4), (5,5); +SELECT a, b FROM t1 WHERE a IN (SELECT A.a FROM t1 A GROUP BY s.id); +ERROR 42S22: Unknown column 's.id' in 'group statement' +DROP TABLE t1; # end of 10.1 tests diff --git a/mysql-test/t/subselect4.test b/mysql-test/t/subselect4.test index 8f1ad51ca50..03929517126 100644 --- a/mysql-test/t/subselect4.test +++ b/mysql-test/t/subselect4.test @@ -2162,4 +2162,15 @@ SELECT t1.a FROM t1 WHERE t1.a IN ( SELECT A.a FROM t1 A UNION ALL SELECT B.a FR DROP TABLE t1,t2; +--echo # +--echo # MDEV-23449: alias do not exist and a query do not report an error +--echo # + +CREATE TABLE t1(a INT, b INT); +INSERT INTO t1 VALUES (1,1), (2,2), (3,3), (4,4), (5,5); + +--error ER_BAD_FIELD_ERROR +SELECT a, b FROM t1 WHERE a IN (SELECT A.a FROM t1 A GROUP BY s.id); +DROP TABLE t1; + --echo # end of 10.1 tests diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 4c6e87e4f27..7a1a7baaa1c 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -731,22 +731,6 @@ JOIN::prepare(Item ***rref_pointer_array, tables_list, select_lex->leaf_tables, FALSE, SELECT_ACL, SELECT_ACL, FALSE)) DBUG_RETURN(-1); - - /* - Permanently remove redundant parts from the query if - 1) This is a subquery - 2) This is the first time this query is optimized (since the - transformation is permanent - 3) Not normalizing a view. Removal should take place when a - query involving a view is optimized, not when the view - is created - */ - if (select_lex->master_unit()->item && // 1) - select_lex->first_cond_optimization && // 2) - !thd->lex->is_view_context_analysis()) // 3) - { - remove_redundant_subquery_clauses(select_lex); - } /* TRUE if the SELECT list mixes elements with and without grouping, @@ -814,6 +798,22 @@ JOIN::prepare(Item ***rref_pointer_array, ref_pointer_array= *rref_pointer_array; + /* + Permanently remove redundant parts from the query if + 1) This is a subquery + 2) This is the first time this query is optimized (since the + transformation is permanent + 3) Not normalizing a view. Removal should take place when a + query involving a view is optimized, not when the view + is created + */ + if (select_lex->master_unit()->item && // 1) + select_lex->first_cond_optimization && // 2) + !thd->lex->is_view_context_analysis()) // 3) + { + remove_redundant_subquery_clauses(select_lex); + } + /* Resolve the ORDER BY that was skipped, then remove it. */ if (skip_order_by && select_lex != select_lex->master_unit()->global_parameters())

1 0

[Commits] 402758a0f: Apply coding style cleanup from make check-format
by Sergei Petrunia 10 Aug '20

10 Aug '20

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(&ltm_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 = &ltm_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 = &ltm_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(&ltree_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(&ltree_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);

1 0

[Commits] 24ca9e111: Range Locking: Cleanup and more test coverage
by Sergei Petrunia 10 Aug '20

10 Aug '20

revision-id: 24ca9e111197ba779a007e5b13d14ed4f4e54b4f (v5.8-2697-g24ca9e111) parent(s): c91814aeffe1043714481723bdffb8b38034ec8a author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2020-08-10 18:03:07 +0300 message: Range Locking: Cleanup and more test coverage - Cleanup in test code - Add a test that makes locking calls similar to MyRocks' calls for UPDATE t WHERE range_cond --- TARGETS | 4 +- utilities/transactions/pessimistic_transaction.cc | 20 ++++---- utilities/transactions/range_locking_test.cc | 56 +++++++++++++++-------- utilities/transactions/transaction_lock_mgr.cc | 1 + 4 files changed, 51 insertions(+), 30 deletions(-) diff --git a/TARGETS b/TARGETS index 42ac1c695..8eeb5edd9 100644 --- a/TARGETS +++ b/TARGETS @@ -1407,7 +1407,9 @@ ROCKS_TESTS = [ [ "range_locking_test", "utilities/transactions/range_locking_test.cc", - "serial", + "parallel", + [], + [], ], [ "rate_limiter_test", diff --git a/utilities/transactions/pessimistic_transaction.cc b/utilities/transactions/pessimistic_transaction.cc index f6fd3a504..865e4ba4d 100644 --- a/utilities/transactions/pessimistic_transaction.cc +++ b/utilities/transactions/pessimistic_transaction.cc @@ -567,7 +567,8 @@ Status PessimisticTransaction::TryLock(ColumnFamilyHandle* column_family, previously_locked = status.locked; lock_upgrade = previously_locked && exclusive && !status.exclusive; } else { - previously_locked = false; + // If the record is tracked, we can assume it was locked, too. + previously_locked = assume_tracked; status.locked = false; lock_upgrade = false; } @@ -589,7 +590,8 @@ Status PessimisticTransaction::TryLock(ColumnFamilyHandle* column_family, SequenceNumber tracked_at_seq = status.locked ? status.seq : kMaxSequenceNumber; if (!do_validate || snapshot_ == nullptr) { - if (assume_tracked && !previously_locked && tracked_locks_->IsPointLockSupported()) { + if (assume_tracked && !previously_locked && + tracked_locks_->IsPointLockSupported()) { s = Status::InvalidArgument( "assume_tracked is set but it is not tracked yet"); } @@ -646,13 +648,13 @@ Status PessimisticTransaction::TryLock(ColumnFamilyHandle* column_family, TrackKey(cfh_id, key_str, tracked_at_seq, read_only, exclusive); } else { #ifndef NDEBUG -#if 0 // psergey-todo: this will fail with range lock manager! - PointLockStatus lock_status = - tracked_locks_->GetPointLockStatus(cfh_id, key_str); - assert(lock_status.locked); - assert(lock_status.seq <= tracked_at_seq); - assert(lock_status.exclusive == exclusive); -#endif + if (tracked_locks_->IsPointLockSupported()) { + PointLockStatus lock_status = + tracked_locks_->GetPointLockStatus(cfh_id, key_str); + assert(lock_status.locked); + assert(lock_status.seq <= tracked_at_seq); + assert(lock_status.exclusive == exclusive); + } #endif } } diff --git a/utilities/transactions/range_locking_test.cc b/utilities/transactions/range_locking_test.cc index 07914df6c..8a930ec4d 100644 --- a/utilities/transactions/range_locking_test.cc +++ b/utilities/transactions/range_locking_test.cc @@ -17,16 +17,6 @@ #include "rocksdb/perf_context.h" #include "rocksdb/utilities/transaction.h" #include "rocksdb/utilities/transaction_db.h" -#include "table/mock_table.h" -// #include "test_util/fault_injection_test_env.h" -#include "util/random.h" -#include "util/string_util.h" -#include "test_util/sync_point.h" -#include "test_util/testharness.h" -#include "test_util/testutil.h" -#include "test_util/transaction_test_util.h" -#include "utilities/merge_operators.h" -#include "utilities/merge_operators/string_append/stringappend.h" #include "utilities/transactions/pessimistic_transaction_db.h" #include "port/port.h" @@ -85,40 +75,36 @@ TEST_F(RangeLockingTest, BasicRangeLocking) { TransactionOptions txn_options; std::string value; ReadOptions read_options; + auto cf = db->DefaultColumnFamily(); Transaction* txn0 = db->BeginTransaction(write_options, txn_options); Transaction* txn1 = db->BeginTransaction(write_options, txn_options); // Get a range lock { - auto s= txn0->GetRangeLock(db->DefaultColumnFamily(), - 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(db->DefaultColumnFamily(), - 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, db->DefaultColumnFamily(), - 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(db->DefaultColumnFamily(), - Slice("d"), Slice("value")); + auto s= txn0->Put(cf, Slice("n"), Slice("value")); ASSERT_EQ(s, Status::OK()); - auto s2= txn1->GetRangeLock(db->DefaultColumnFamily(), - Endpoint("c"), Endpoint("e")); + auto s2= txn1->GetRangeLock(cf, Endpoint("m"), Endpoint("p")); ASSERT_TRUE(s2.IsTimedOut()); } @@ -129,6 +115,36 @@ TEST_F(RangeLockingTest, BasicRangeLocking) { delete txn1; } +TEST_F(RangeLockingTest, MyRocksLikeUpdate) { + WriteOptions write_options; + TransactionOptions txn_options; + Transaction* txn0 = db->BeginTransaction(write_options, txn_options); + auto cf= db->DefaultColumnFamily(); + Status s; + + // Get a range lock for the range we are about to update + s = txn0->GetRangeLock(cf, Endpoint("a"), Endpoint("c")); + ASSERT_EQ(s, Status::OK()); + + bool try_range_lock_called = false; + + ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->SetCallBack( + "RangeLockMgr::TryRangeLock:enter", + [&](void* /*arg*/) { try_range_lock_called = true; }); + ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->EnableProcessing(); + + // For performance reasons, the following must NOT call lock_mgr->TryLock(): + // We verify that by checking the value of try_range_lock_called. + s = txn0->Put(cf, Slice("b"), Slice("value"), /*assume_tracked=*/true); + ASSERT_EQ(s, Status::OK()); + + ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->DisableProcessing(); + ROCKSDB_NAMESPACE::SyncPoint::GetInstance()->ClearAllCallBacks(); + ASSERT_EQ(try_range_lock_called, false); + + txn0->Rollback(); +} + TEST_F(RangeLockingTest, SnapshotValidation) { Status s; Slice key_slice= Slice("k"); diff --git a/utilities/transactions/transaction_lock_mgr.cc b/utilities/transactions/transaction_lock_mgr.cc index aa1d6ac75..340bbdb5d 100644 --- a/utilities/transactions/transaction_lock_mgr.cc +++ b/utilities/transactions/transaction_lock_mgr.cc @@ -779,6 +779,7 @@ Status RangeLockMgr::TryRangeLock(PessimisticTransaction* txn, request.create(mutex_factory_); DBT start_key_dbt, end_key_dbt; + TEST_SYNC_POINT("RangeLockMgr::TryRangeLock:enter"); std::string start_key; std::string end_key; serialize_endpoint(start_endp, &start_key);

1 0

[Commits] c91814aef: Merge branch 'master' into spetrunia-range-locking-rocksdb-pr2-locktrack
by Sergei Petrunia 10 Aug '20

10 Aug '20

revision-id: c91814aeffe1043714481723bdffb8b38034ec8a (v5.8-2696-gc91814aef) parent(s): 18221ef35c107f54861ee4330a0108dc382db582 8a1da56b961baf1b491b00af67f0fe8ab5c9afc5 author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2020-08-10 15:21:23 +0300 message: Merge branch 'master' into spetrunia-range-locking-rocksdb-pr2-locktrack .circleci/cat_ignore_eagain | 54 + .circleci/config.yml | 212 +++- .circleci/ubsan_suppression_list.txt | 6 + .circleci/vs2015_install.ps1 | 23 + .circleci/vs2017_install.ps1 | 34 + .travis.yml | 11 +- CMakeLists.txt | 73 +- HISTORY.md | 53 +- LANGUAGE-BINDINGS.md | 4 +- Makefile | 1096 +++++++++----------- README.md | 5 +- TARGETS | 54 +- appveyor.yml | 13 +- buckifier/buckify_rocksdb.py | 75 +- buckifier/targets_builder.py | 24 + build_tools/build_detect_platform | 65 +- build_tools/fbcode_config.sh | 2 + build_tools/fbcode_config4.8.1.sh | 2 + build_tools/fbcode_config_platform007.sh | 2 + build_tools/rocksdb-lego-determinator | 230 +++- cache/cache_test.cc | 36 +- cache/lru_cache_test.cc | 5 +- db/arena_wrapped_db_iter.cc | 10 +- db/arena_wrapped_db_iter.h | 7 +- db/blob/blob_file_meta.cc | 9 +- db/blob/blob_file_meta.h | 22 +- {utilities/blob_db => db/blob}/blob_log_format.cc | 4 +- {utilities/blob_db => db/blob}/blob_log_format.h | 4 - {utilities/blob_db => db/blob}/blob_log_reader.cc | 17 +- {utilities/blob_db => db/blob}/blob_log_reader.h | 24 +- {utilities/blob_db => db/blob}/blob_log_writer.cc | 49 +- {utilities/blob_db => db/blob}/blob_log_writer.h | 29 +- db/blob/db_blob_index_test.cc | 2 +- db/builder.cc | 50 +- db/builder.h | 6 +- db/c.cc | 302 ++++++ db/c_test.c | 486 +++++++++ db/column_family.cc | 24 +- db/column_family.h | 2 + db/column_family_test.cc | 42 +- db/compact_files_test.cc | 9 +- db/compacted_db_impl.cc | 6 +- db/compaction/compaction.cc | 31 +- db/compaction/compaction.h | 5 + db/compaction/compaction_iterator.cc | 31 +- db/compaction/compaction_iterator.h | 3 +- db/compaction/compaction_job.cc | 179 +++- db/compaction/compaction_job.h | 6 +- db/compaction/compaction_job_stats_test.cc | 9 +- db/compaction/compaction_job_test.cc | 13 +- db/compaction/compaction_picker.cc | 17 +- db/compaction/compaction_picker.h | 9 +- db/compaction/compaction_picker_fifo.cc | 37 +- db/compaction/compaction_picker_fifo.h | 8 +- db/compaction/compaction_picker_level.cc | 18 +- db/compaction/compaction_picker_level.h | 3 +- db/compaction/compaction_picker_test.cc | 193 +++- db/compaction/compaction_picker_universal.cc | 34 +- db/compaction/compaction_picker_universal.h | 3 +- db/compaction/sst_partitioner.cc | 44 + db/comparator_db_test.cc | 5 +- db/corruption_test.cc | 86 +- db/cuckoo_table_db_test.cc | 5 +- db/db_basic_test.cc | 720 ++++++++++--- db/db_block_cache_test.cc | 6 +- db/db_compaction_filter_test.cc | 9 +- db/db_compaction_test.cc | 331 ++++-- db/db_dynamic_level_test.cc | 35 +- db/db_filesnapshot.cc | 51 - db/db_flush_test.cc | 10 +- db/db_impl/db_impl.cc | 222 +++- db/db_impl/db_impl.h | 63 +- db/db_impl/db_impl_compaction_flush.cc | 76 +- db/db_impl/db_impl_debug.cc | 12 +- db/db_impl/db_impl_experimental.cc | 6 +- db/db_impl/db_impl_files.cc | 58 +- db/db_impl/db_impl_open.cc | 127 ++- db/db_impl/db_impl_readonly.cc | 29 +- db/db_impl/db_impl_secondary.cc | 14 +- db/db_impl/db_impl_write.cc | 42 +- db/db_impl/db_secondary_test.cc | 2 +- db/db_info_dumper.cc | 5 +- db/db_info_dumper.h | 3 +- db/db_io_failure_test.cc | 9 +- db/db_iter.cc | 36 +- db/db_iter.h | 2 +- db/db_iter_stress_test.cc | 11 +- db/db_iterator_test.cc | 38 +- db/db_memtable_test.cc | 4 +- db/db_merge_operand_test.cc | 2 +- db/db_merge_operator_test.cc | 11 +- db/db_options_test.cc | 15 +- db/db_properties_test.cc | 36 +- db/db_range_del_test.cc | 29 +- db/db_sst_test.cc | 19 +- db/db_statistics_test.cc | 5 +- db/db_table_properties_test.cc | 28 +- db/db_test.cc | 204 ++-- db/db_test2.cc | 474 ++++++--- db/db_test_util.cc | 27 +- db/db_test_util.h | 81 +- db/db_universal_compaction_test.cc | 33 +- db/db_wal_test.cc | 362 ++++--- db/db_with_timestamp_basic_test.cc | 146 ++- db/db_write_test.cc | 8 +- db/dbformat.cc | 21 +- db/dbformat.h | 68 +- db/error_handler.cc | 203 +++- db/error_handler.h | 17 + db/error_handler_fs_test.cc | 786 +++++++++++++- db/event_helpers.cc | 4 +- db/external_sst_file_basic_test.cc | 294 +++++- db/external_sst_file_ingestion_job.cc | 167 ++- db/external_sst_file_ingestion_job.h | 11 + db/external_sst_file_test.cc | 64 +- db/fault_injection_test.cc | 6 +- db/filename_test.cc | 2 +- db/flush_job.cc | 8 +- db/flush_job.h | 5 +- db/flush_job_test.cc | 7 +- db/forward_iterator.cc | 16 +- db/import_column_family_test.cc | 22 +- db/internal_stats.cc | 2 +- db/listener_test.cc | 74 +- db/log_reader.cc | 1 + db/log_writer.cc | 2 + db/memtable.cc | 21 +- db/memtable_list.cc | 1 - db/obsolete_files_test.cc | 48 +- db/plain_table_db_test.cc | 23 +- db/prefix_test.cc | 13 +- db/repair.cc | 12 +- db/table_cache.cc | 80 +- db/table_cache.h | 16 +- db/version_builder.cc | 316 ++++-- db/version_builder.h | 3 +- db/version_builder_test.cc | 355 ++++++- db/version_edit.cc | 129 ++- db/version_edit.h | 85 +- db/version_edit_handler.cc | 26 +- db/version_edit_handler.h | 3 +- db/version_edit_test.cc | 205 ++++ db/version_set.cc | 254 ++--- db/version_set.h | 83 +- db/version_set_test.cc | 140 +-- db/wal_edit.cc | 175 ++++ db/wal_edit.h | 143 +++ db/wal_edit_test.cc | 127 +++ db/write_batch.cc | 3 +- db/write_callback_test.cc | 489 ++++----- db_stress_tool/cf_consistency_stress.cc | 29 +- db_stress_tool/db_stress_common.h | 57 +- db_stress_tool/db_stress_compaction_filter.h | 79 ++ db_stress_tool/db_stress_driver.cc | 18 +- db_stress_tool/db_stress_gflags.cc | 23 +- db_stress_tool/db_stress_test_base.cc | 129 ++- db_stress_tool/db_stress_test_base.h | 5 +- db_stress_tool/db_stress_tool.cc | 39 +- db_stress_tool/no_batched_ops_stress.cc | 2 +- env/composite_env_wrapper.h | 18 +- env/env.cc | 4 +- env/env_basic_test.cc | 44 +- env/env_chroot.cc | 3 +- env/env_encryption.cc | 566 +++++----- env/env_hdfs.cc | 6 +- env/env_posix.cc | 26 +- env/env_test.cc | 163 ++- env/file_system.cc | 2 +- env/file_system_tracer.cc | 319 ++++++ env/file_system_tracer.h | 330 ++++++ env/fs_posix.cc | 45 +- env/io_posix.cc | 21 +- env/mock_env.cc | 2 +- examples/CMakeLists.txt | 45 + examples/simple_example.cc | 2 +- file/delete_scheduler.cc | 4 +- file/delete_scheduler.h | 3 +- file/delete_scheduler_test.cc | 10 +- file/file_prefetch_buffer.cc | 13 +- file/file_prefetch_buffer.h | 10 +- file/file_util.cc | 110 ++ file/file_util.h | 19 +- file/random_access_file_reader.cc | 28 +- file/random_access_file_reader.h | 28 +- file/random_access_file_reader_test.cc | 262 ++++- file/sst_file_manager_impl.cc | 5 +- file/writable_file_writer.cc | 106 +- file/writable_file_writer.h | 76 +- hdfs/env_hdfs.h | 8 +- include/rocksdb/advanced_options.h | 4 +- include/rocksdb/c.h | 132 +++ include/rocksdb/compaction_filter.h | 2 + include/rocksdb/compression_type.h | 40 + include/rocksdb/convenience.h | 1 + include/rocksdb/db.h | 29 +- include/rocksdb/env.h | 22 +- include/rocksdb/env_encryption.h | 238 ++++- include/rocksdb/file_system.h | 21 +- include/rocksdb/listener.h | 72 +- include/rocksdb/merge_operator.h | 0 include/rocksdb/options.h | 100 +- include/rocksdb/persistent_cache.h | 6 + include/rocksdb/sst_partitioner.h | 135 +++ include/rocksdb/statistics.h | 8 + include/rocksdb/table.h | 48 +- include/rocksdb/table_properties.h | 13 + include/rocksdb/utilities/backupable_db.h | 92 +- include/rocksdb/utilities/leveldb_options.h | 3 +- include/rocksdb/utilities/stackable_db.h | 9 + include/rocksdb/version.h | 2 +- java/CMakeLists.txt | 8 +- java/Makefile | 4 + java/crossbuild/docker-build-linux-alpine.sh | 2 +- java/crossbuild/docker-build-linux-centos.sh | 2 +- java/{rocksjni.pom => pom.xml.template} | 40 +- java/rocksjni/options.cc | 42 +- java/rocksjni/portal.h | 69 ++ java/rocksjni/rocksjni.cc | 107 +- java/rocksjni/sst_partitioner.cc | 42 + java/rocksjni/table.cc | 25 +- .../java/org/rocksdb/BlockBasedTableConfig.java | 158 ++- java/src/main/java/org/rocksdb/ChecksumType.java | 6 +- .../main/java/org/rocksdb/ColumnFamilyOptions.java | 15 +- .../org/rocksdb/ColumnFamilyOptionsInterface.java | 17 + .../main/java/org/rocksdb/IndexShorteningMode.java | 60 ++ java/src/main/java/org/rocksdb/IndexType.java | 16 +- .../main/java/org/rocksdb/NativeLibraryLoader.java | 7 +- java/src/main/java/org/rocksdb/Options.java | 14 + java/src/main/java/org/rocksdb/RocksDB.java | 180 +++- .../java/org/rocksdb/SstPartitionerFactory.java | 15 + .../rocksdb/SstPartitionerFixedPrefixFactory.java | 19 + java/src/main/java/org/rocksdb/TickerType.java | 10 + .../org/rocksdb/BlockBasedTableConfigTest.java | 23 +- java/src/test/java/org/rocksdb/RocksDBTest.java | 7 + .../src/test/java/org/rocksdb/SecondaryDBTest.java | 135 +++ .../test/java/org/rocksdb/SstPartitionerTest.java | 43 + memtable/memtablerep_bench.cc | 2 +- monitoring/statistics.cc | 6 + monitoring/thread_status_updater_debug.cc | 3 +- options/cf_options.cc | 140 ++- options/cf_options.h | 12 +- options/db_options.cc | 32 +- options/db_options.h | 4 +- options/options.cc | 6 + options/options_helper.cc | 26 +- options/options_settable_test.cc | 76 +- options/options_test.cc | 166 +++ port/stack_trace.cc | 2 +- src.mk | 101 +- table/adaptive/adaptive_table_factory.cc | 10 +- table/adaptive/adaptive_table_factory.h | 3 +- table/block_based/binary_search_index_reader.cc | 10 +- table/block_based/binary_search_index_reader.h | 2 +- table/block_based/block.cc | 218 ++-- table/block_based/block.h | 296 +++--- table/block_based/block_based_filter_block.cc | 12 +- table/block_based/block_based_filter_block.h | 6 +- table/block_based/block_based_table_builder.cc | 65 +- table/block_based/block_based_table_builder.h | 3 +- table/block_based/block_based_table_factory.cc | 14 +- table/block_based/block_based_table_factory.h | 3 +- table/block_based/block_based_table_iterator.cc | 20 +- table/block_based/block_based_table_iterator.h | 78 +- table/block_based/block_based_table_reader.cc | 225 ++-- table/block_based/block_based_table_reader.h | 54 +- table/block_based/block_based_table_reader_test.cc | 20 +- table/block_based/block_test.cc | 54 +- table/block_based/data_block_hash_index_test.cc | 20 +- table/block_based/filter_block.h | 2 +- table/block_based/filter_policy.cc | 208 +++- table/block_based/filter_policy_internal.h | 10 + table/block_based/full_filter_block.cc | 12 +- table/block_based/full_filter_block.h | 6 +- table/block_based/hash_index_reader.cc | 5 +- table/block_based/hash_index_reader.h | 2 +- table/block_based/index_builder.cc | 29 +- table/block_based/partitioned_filter_block.cc | 41 +- table/block_based/partitioned_filter_block.h | 8 +- table/block_based/partitioned_index_iterator.h | 12 +- table/block_based/partitioned_index_reader.cc | 29 +- table/block_based/partitioned_index_reader.h | 4 +- table/block_based/reader_common.cc | 39 +- table/block_based/reader_common.h | 8 +- table/block_based/uncompression_dict_reader.cc | 10 +- table/block_based/uncompression_dict_reader.h | 6 +- table/block_fetcher.cc | 86 +- table/block_fetcher_test.cc | 310 +++--- table/cuckoo/cuckoo_table_builder.cc | 5 +- table/cuckoo/cuckoo_table_builder.h | 17 +- table/cuckoo/cuckoo_table_factory.cc | 5 +- table/cuckoo/cuckoo_table_factory.h | 3 +- table/format.cc | 15 +- table/format.h | 4 +- table/internal_iterator.h | 34 +- table/iterator_wrapper.h | 13 +- table/merger_test.cc | 8 +- table/merging_iterator.cc | 6 +- table/meta_blocks.cc | 57 +- table/meta_blocks.h | 3 +- table/mock_table.cc | 154 ++- table/mock_table.h | 147 +-- table/plain/plain_table_builder.cc | 5 +- table/plain/plain_table_builder.h | 3 +- table/plain/plain_table_factory.cc | 6 +- table/plain/plain_table_factory.h | 10 +- table/plain/plain_table_reader.cc | 2 +- table/sst_file_dumper.cc | 481 +++++++++ .../sst_dump_tool_imp.h => table/sst_file_dumper.h | 8 +- table/sst_file_writer.cc | 14 +- table/table_builder.h | 25 +- table/table_properties.cc | 8 + table/table_reader.h | 6 +- table/table_test.cc | 439 ++++---- test_util/sync_point.cc | 25 +- test_util/sync_point.h | 5 +- test_util/sync_point_impl.cc | 2 +- test_util/testharness.cc | 2 +- test_util/testutil.cc | 72 +- test_util/testutil.h | 13 +- test_util/testutil_test.cc | 3 +- tools/check_format_compatible.sh | 14 +- tools/db_bench_tool.cc | 13 + tools/db_crashtest.py | 83 +- tools/db_repl_stress.cc | 10 +- tools/ldb_cmd.cc | 21 +- tools/ldb_cmd_test.cc | 40 +- tools/ldb_tool.cc | 5 +- tools/reduce_levels_test.cc | 5 +- tools/sst_dump_tool.cc | 458 +------- trace_replay/block_cache_tracer.cc | 4 +- trace_replay/io_tracer.cc | 228 ++++ trace_replay/io_tracer.h | 153 +++ trace_replay/io_tracer_test.cc | 215 ++++ trace_replay/trace_replay.h | 6 + util/bloom_test.cc | 76 +- util/cast_util.h | 3 +- util/file_reader_writer_test.cc | 15 +- util/filelock_test.cc | 4 + util/filter_bench.cc | 28 +- util/random.cc | 18 + util/random.h | 6 + util/status.cc | 2 +- util/thread_local.cc | 2 +- util/threadpool_imp.cc | 42 +- util/threadpool_imp.h | 2 +- util/timer.h | 21 +- util/timer_test.cc | 125 ++- util/user_comparator_wrapper.h | 4 + utilities/backupable/backupable_db.cc | 508 +++++++-- utilities/backupable/backupable_db_test.cc | 576 +++++++++- utilities/blob_db/blob_compaction_filter.cc | 374 ++++--- utilities/blob_db/blob_compaction_filter.h | 111 +- utilities/blob_db/blob_db.h | 4 + utilities/blob_db/blob_db_impl.cc | 108 +- utilities/blob_db/blob_db_impl.h | 24 +- utilities/blob_db/blob_db_impl_filesnapshot.cc | 7 +- utilities/blob_db/blob_db_test.cc | 268 ++++- utilities/blob_db/blob_dump_tool.h | 3 +- utilities/blob_db/blob_file.cc | 13 +- utilities/blob_db/blob_file.h | 19 +- utilities/cassandra/cassandra_functional_test.cc | 4 +- utilities/checkpoint/checkpoint_impl.cc | 48 +- utilities/checkpoint/checkpoint_impl.h | 6 +- utilities/checkpoint/checkpoint_test.cc | 18 +- .../layered_compaction_filter_base.h | 37 + utilities/debug.cc | 5 +- utilities/env_timed_test.cc | 2 +- .../fault_injection_env.cc | 4 +- .../fault_injection_env.h | 6 +- .../fault_injection_fs.cc | 10 +- .../fault_injection_fs.h | 14 +- utilities/memory/memory_test.cc | 17 +- .../option_change_migration_test.cc | 10 +- utilities/options/options_util.cc | 2 +- utilities/options/options_util_test.cc | 4 +- .../persistent_cache/persistent_cache_test.cc | 9 +- utilities/persistent_cache/persistent_cache_test.h | 3 +- .../persistent_cache/persistent_cache_tier.cc | 4 + utilities/persistent_cache/persistent_cache_tier.h | 3 + utilities/transactions/lock/point_lock_tracker.cc | 2 +- utilities/transactions/lock/point_lock_tracker.h | 6 +- utilities/transactions/pessimistic_transaction.cc | 4 +- utilities/transactions/range_locking_test.cc | 2 +- .../transactions/transaction_lock_mgr_test.cc | 4 +- utilities/transactions/transaction_test.cc | 63 +- utilities/transactions/transaction_test.h | 9 +- utilities/transactions/transaction_util.cc | 3 +- .../write_prepared_transaction_test.cc | 32 +- utilities/transactions/write_prepared_txn_db.cc | 6 +- utilities/transactions/write_unprepared_txn.cc | 8 +- utilities/transactions/write_unprepared_txn_db.cc | 3 +- utilities/ttl/db_ttl_impl.h | 34 +- .../write_batch_with_index.cc | 4 +- .../write_batch_with_index_internal.cc | 4 +- 394 files changed, 18415 insertions(+), 6264 deletions(-) diff --cc CMakeLists.txt index 429d5d5dc,2731a60ef..5e51cf80a --- a/CMakeLists.txt +++ b/CMakeLists.txt @@@ -1171,10 -1165,10 +1196,11 @@@ if(WITH_TESTS utilities/transactions/transaction_lock_mgr_test.cc utilities/transactions/write_prepared_transaction_test.cc utilities/transactions/write_unprepared_transaction_test.cc + utilities/transactions/range_locking_test.cc utilities/ttl/ttl_test.cc utilities/write_batch_with_index/write_batch_with_index_test.cc - ) + ) + endif() if(WITH_LIBRADOS) list(APPEND TESTS utilities/env_librados_test.cc) endif() diff --cc Makefile index bce90ad25,6177c136c..996a8598c --- a/Makefile +++ b/Makefile @@@ -665,6 -542,6 +545,7 @@@ PARALLEL_TEST = table_test \ transaction_test \ transaction_lock_mgr_test \ ++ range_locking_test \ write_prepared_transaction_test \ write_unprepared_transaction_test \ @@@ -1578,314 -1498,317 +1502,320 @@@ env_timed_test: $(OBJ_DIR)/utilities/en $(AM_LINK) ifdef ROCKSDB_USE_LIBRADOS - env_librados_test: utilities/env_librados_test.o $(LIBOBJECTS) $(TESTHARNESS) - $(AM_V_CCLD)$(CXX) $^ $(EXEC_LDFLAGS) -o $@ $(LDFLAGS) $(COVERAGEFLAGS) + env_librados_test: $(OBJ_DIR)/utilities/env_librados_test.o $(TEST_LIBRARY) $(LIBRARY) + $(AM_LINK) endif - object_registry_test: utilities/object_registry_test.o $(LIBOBJECTS) $(TESTHARNESS) + object_registry_test: $(OBJ_DIR)/utilities/object_registry_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - ttl_test: utilities/ttl/ttl_test.o $(LIBOBJECTS) $(TESTHARNESS) + ttl_test: $(OBJ_DIR)/utilities/ttl/ttl_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - write_batch_with_index_test: utilities/write_batch_with_index/write_batch_with_index_test.o $(LIBOBJECTS) $(TESTHARNESS) + write_batch_with_index_test: $(OBJ_DIR)/utilities/write_batch_with_index/write_batch_with_index_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - flush_job_test: db/flush_job_test.o $(LIBOBJECTS) $(TESTHARNESS) + flush_job_test: $(OBJ_DIR)/db/flush_job_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - compaction_iterator_test: db/compaction/compaction_iterator_test.o $(LIBOBJECTS) $(TESTHARNESS) + compaction_iterator_test: $(OBJ_DIR)/db/compaction/compaction_iterator_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - compaction_job_test: db/compaction/compaction_job_test.o $(LIBOBJECTS) $(TESTHARNESS) + compaction_job_test: $(OBJ_DIR)/db/compaction/compaction_job_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - compaction_job_stats_test: db/compaction/compaction_job_stats_test.o $(LIBOBJECTS) $(TESTHARNESS) + compaction_job_stats_test: $(OBJ_DIR)/db/compaction/compaction_job_stats_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - compact_on_deletion_collector_test: utilities/table_properties_collectors/compact_on_deletion_collector_test.o $(LIBOBJECTS) $(TESTHARNESS) + compact_on_deletion_collector_test: $(OBJ_DIR)/utilities/table_properties_collectors/compact_on_deletion_collector_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - wal_manager_test: db/wal_manager_test.o $(LIBOBJECTS) $(TESTHARNESS) + wal_manager_test: $(OBJ_DIR)/db/wal_manager_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - dbformat_test: db/dbformat_test.o $(LIBOBJECTS) $(TESTHARNESS) + wal_edit_test: $(OBJ_DIR)/db/wal_edit_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - env_basic_test: env/env_basic_test.o $(LIBOBJECTS) $(TESTHARNESS) + dbformat_test: $(OBJ_DIR)/db/dbformat_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - env_test: env/env_test.o $(LIBOBJECTS) $(TESTHARNESS) + env_basic_test: $(OBJ_DIR)/env/env_basic_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - io_posix_test: env/io_posix_test.o $(LIBOBJECTS) $(TESTHARNESS) + env_test: $(OBJ_DIR)/env/env_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - fault_injection_test: db/fault_injection_test.o $(LIBOBJECTS) $(TESTHARNESS) + io_posix_test: $(OBJ_DIR)/env/io_posix_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - rate_limiter_test: util/rate_limiter_test.o db/db_test_util.o $(LIBOBJECTS) $(TESTHARNESS) + fault_injection_test: $(OBJ_DIR)/db/fault_injection_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - delete_scheduler_test: file/delete_scheduler_test.o $(LIBOBJECTS) $(TESTHARNESS) + rate_limiter_test: $(OBJ_DIR)/util/rate_limiter_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - filename_test: db/filename_test.o $(LIBOBJECTS) $(TESTHARNESS) + delete_scheduler_test: $(OBJ_DIR)/file/delete_scheduler_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - random_access_file_reader_test: file/random_access_file_reader_test.o $(LIBOBJECTS) $(TESTHARNESS) $(TESTUTIL) + filename_test: $(OBJ_DIR)/db/filename_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - file_reader_writer_test: util/file_reader_writer_test.o $(LIBOBJECTS) $(TESTHARNESS) + random_access_file_reader_test: $(OBJ_DIR)/file/random_access_file_reader_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - block_based_filter_block_test: table/block_based/block_based_filter_block_test.o $(LIBOBJECTS) $(TESTHARNESS) + file_reader_writer_test: $(OBJ_DIR)/util/file_reader_writer_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - block_based_table_reader_test: table/block_based/block_based_table_reader_test.o $(LIBOBJECTS) $(TESTHARNESS) + block_based_filter_block_test: $(OBJ_DIR)/table/block_based/block_based_filter_block_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - full_filter_block_test: table/block_based/full_filter_block_test.o $(LIBOBJECTS) $(TESTHARNESS) + block_based_table_reader_test: table/block_based/block_based_table_reader_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - partitioned_filter_block_test: table/block_based/partitioned_filter_block_test.o $(LIBOBJECTS) $(TESTHARNESS) + full_filter_block_test: $(OBJ_DIR)/table/block_based/full_filter_block_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - log_test: db/log_test.o $(LIBOBJECTS) $(TESTHARNESS) + partitioned_filter_block_test: $(OBJ_DIR)/table/block_based/partitioned_filter_block_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - cleanable_test: table/cleanable_test.o $(LIBOBJECTS) $(TESTHARNESS) + log_test: $(OBJ_DIR)/db/log_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - table_test: table/table_test.o $(LIBOBJECTS) $(TESTHARNESS) + cleanable_test: $(OBJ_DIR)/table/cleanable_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - block_fetcher_test: table/block_fetcher_test.o $(LIBOBJECTS) $(TESTHARNESS) + table_test: $(OBJ_DIR)/table/table_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - block_test: table/block_based/block_test.o $(LIBOBJECTS) $(TESTHARNESS) + block_fetcher_test: table/block_fetcher_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - data_block_hash_index_test: table/block_based/data_block_hash_index_test.o $(LIBOBJECTS) $(TESTHARNESS) + block_test: $(OBJ_DIR)/table/block_based/block_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - inlineskiplist_test: memtable/inlineskiplist_test.o $(LIBOBJECTS) $(TESTHARNESS) + data_block_hash_index_test: $(OBJ_DIR)/table/block_based/data_block_hash_index_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - skiplist_test: memtable/skiplist_test.o $(LIBOBJECTS) $(TESTHARNESS) + inlineskiplist_test: $(OBJ_DIR)/memtable/inlineskiplist_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - write_buffer_manager_test: memtable/write_buffer_manager_test.o $(LIBOBJECTS) $(TESTHARNESS) + skiplist_test: $(OBJ_DIR)/memtable/skiplist_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - version_edit_test: db/version_edit_test.o $(LIBOBJECTS) $(TESTHARNESS) + write_buffer_manager_test: $(OBJ_DIR)/memtable/write_buffer_manager_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - version_set_test: db/version_set_test.o $(LIBOBJECTS) $(TESTHARNESS) + version_edit_test: $(OBJ_DIR)/db/version_edit_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - compaction_picker_test: db/compaction/compaction_picker_test.o $(LIBOBJECTS) $(TESTHARNESS) + version_set_test: $(OBJ_DIR)/db/version_set_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - version_builder_test: db/version_builder_test.o $(LIBOBJECTS) $(TESTHARNESS) + compaction_picker_test: $(OBJ_DIR)/db/compaction/compaction_picker_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - file_indexer_test: db/file_indexer_test.o $(LIBOBJECTS) $(TESTHARNESS) + version_builder_test: $(OBJ_DIR)/db/version_builder_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - reduce_levels_test: tools/reduce_levels_test.o $(LIBOBJECTS) $(TESTHARNESS) + file_indexer_test: $(OBJ_DIR)/db/file_indexer_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - write_batch_test: db/write_batch_test.o $(LIBOBJECTS) $(TESTHARNESS) + reduce_levels_test: $(OBJ_DIR)/tools/reduce_levels_test.o $(TOOLS_LIBRARY) $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - write_controller_test: db/write_controller_test.o $(LIBOBJECTS) $(TESTHARNESS) + write_batch_test: $(OBJ_DIR)/db/write_batch_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - merge_helper_test: db/merge_helper_test.o $(LIBOBJECTS) $(TESTHARNESS) + write_controller_test: $(OBJ_DIR)/db/write_controller_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - memory_test: utilities/memory/memory_test.o $(LIBOBJECTS) $(TESTHARNESS) + merge_helper_test: $(OBJ_DIR)/db/merge_helper_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - merge_test: db/merge_test.o $(LIBOBJECTS) $(TESTHARNESS) + memory_test: $(OBJ_DIR)/utilities/memory/memory_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - merger_test: table/merger_test.o $(LIBOBJECTS) $(TESTHARNESS) + merge_test: $(OBJ_DIR)/db/merge_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - util_merge_operators_test: utilities/util_merge_operators_test.o $(LIBOBJECTS) $(TESTHARNESS) + merger_test: $(OBJ_DIR)/table/merger_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - options_file_test: db/options_file_test.o $(LIBOBJECTS) $(TESTHARNESS) + util_merge_operators_test: $(OBJ_DIR)/utilities/util_merge_operators_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - deletefile_test: db/deletefile_test.o db/db_test_util.o $(LIBOBJECTS) $(TESTHARNESS) + options_file_test: $(OBJ_DIR)/db/options_file_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - obsolete_files_test: db/obsolete_files_test.o db/db_test_util.o $(LIBOBJECTS) $(TESTHARNESS) + deletefile_test: $(OBJ_DIR)/db/deletefile_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - rocksdb_dump: tools/dump/rocksdb_dump.o $(LIBOBJECTS) + obsolete_files_test: $(OBJ_DIR)/db/obsolete_files_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - rocksdb_undump: tools/dump/rocksdb_undump.o $(LIBOBJECTS) + rocksdb_dump: $(OBJ_DIR)/tools/dump/rocksdb_dump.o $(LIBRARY) $(AM_LINK) - cuckoo_table_builder_test: table/cuckoo/cuckoo_table_builder_test.o $(LIBOBJECTS) $(TESTHARNESS) + rocksdb_undump: $(OBJ_DIR)/tools/dump/rocksdb_undump.o $(LIBRARY) $(AM_LINK) - cuckoo_table_reader_test: table/cuckoo/cuckoo_table_reader_test.o $(LIBOBJECTS) $(TESTHARNESS) + cuckoo_table_builder_test: $(OBJ_DIR)/table/cuckoo/cuckoo_table_builder_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - cuckoo_table_db_test: db/cuckoo_table_db_test.o $(LIBOBJECTS) $(TESTHARNESS) + cuckoo_table_reader_test: $(OBJ_DIR)/table/cuckoo/cuckoo_table_reader_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - listener_test: db/listener_test.o db/db_test_util.o $(LIBOBJECTS) $(TESTHARNESS) + cuckoo_table_db_test: $(OBJ_DIR)/db/cuckoo_table_db_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - thread_list_test: util/thread_list_test.o $(LIBOBJECTS) $(TESTHARNESS) + listener_test: $(OBJ_DIR)/db/listener_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - compact_files_test: db/compact_files_test.o $(LIBOBJECTS) $(TESTHARNESS) + thread_list_test: $(OBJ_DIR)/util/thread_list_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - options_test: options/options_test.o $(LIBOBJECTS) $(TESTHARNESS) + compact_files_test: $(OBJ_DIR)/db/compact_files_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - options_settable_test: options/options_settable_test.o $(LIBOBJECTS) $(TESTHARNESS) + options_test: $(OBJ_DIR)/options/options_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - options_util_test: utilities/options/options_util_test.o $(LIBOBJECTS) $(TESTHARNESS) + options_settable_test: $(OBJ_DIR)/options/options_settable_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - db_bench_tool_test: tools/db_bench_tool_test.o $(BENCHTOOLOBJECTS) $(TESTHARNESS) + options_util_test: $(OBJ_DIR)/utilities/options/options_util_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - trace_analyzer_test: tools/trace_analyzer_test.o $(LIBOBJECTS) $(ANALYZETOOLOBJECTS) $(TESTHARNESS) + db_bench_tool_test: $(OBJ_DIR)/tools/db_bench_tool_test.o $(BENCH_OBJECTS) $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - event_logger_test: logging/event_logger_test.o $(LIBOBJECTS) $(TESTHARNESS) + trace_analyzer_test: $(OBJ_DIR)/tools/trace_analyzer_test.o $(ANALYZE_OBJECTS) $(TOOLS_LIBRARY) $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - timer_queue_test: util/timer_queue_test.o $(LIBOBJECTS) $(TESTHARNESS) + event_logger_test: $(OBJ_DIR)/logging/event_logger_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - sst_dump_test: tools/sst_dump_test.o $(LIBOBJECTS) $(TESTHARNESS) + timer_queue_test: $(OBJ_DIR)/util/timer_queue_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - optimistic_transaction_test: utilities/transactions/optimistic_transaction_test.o $(LIBOBJECTS) $(TESTHARNESS) + sst_dump_test: $(OBJ_DIR)/tools/sst_dump_test.o $(TOOLS_LIBRARY) $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - mock_env_test : env/mock_env_test.o $(LIBOBJECTS) $(TESTHARNESS) + optimistic_transaction_test: $(OBJ_DIR)/utilities/transactions/optimistic_transaction_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - manual_compaction_test: db/manual_compaction_test.o $(LIBOBJECTS) $(TESTHARNESS) + mock_env_test : $(OBJ_DIR)/env/mock_env_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - filelock_test: util/filelock_test.o $(LIBOBJECTS) $(TESTHARNESS) + manual_compaction_test: $(OBJ_DIR)/db/manual_compaction_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - auto_roll_logger_test: logging/auto_roll_logger_test.o $(LIBOBJECTS) $(TESTHARNESS) + filelock_test: $(OBJ_DIR)/util/filelock_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - env_logger_test: logging/env_logger_test.o $(LIBOBJECTS) $(TESTHARNESS) + auto_roll_logger_test: $(OBJ_DIR)/logging/auto_roll_logger_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - memtable_list_test: db/memtable_list_test.o $(LIBOBJECTS) $(TESTHARNESS) + env_logger_test: $(OBJ_DIR)/logging/env_logger_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - write_callback_test: db/write_callback_test.o $(LIBOBJECTS) $(TESTHARNESS) + memtable_list_test: $(OBJ_DIR)/db/memtable_list_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - heap_test: util/heap_test.o $(GTEST) + write_callback_test: $(OBJ_DIR)/db/write_callback_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - transaction_lock_mgr_test: utilities/transactions/transaction_lock_mgr_test.o $(LIBOBJECTS) $(TESTHARNESS) + heap_test: $(OBJ_DIR)/util/heap_test.o $(GTEST) $(AM_LINK) - transaction_test: utilities/transactions/transaction_test.o $(LIBOBJECTS) $(TESTHARNESS) + transaction_lock_mgr_test: utilities/transactions/transaction_lock_mgr_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - write_prepared_transaction_test: utilities/transactions/write_prepared_transaction_test.o $(LIBOBJECTS) $(TESTHARNESS) + transaction_test: $(OBJ_DIR)/utilities/transactions/transaction_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - write_unprepared_transaction_test: utilities/transactions/write_unprepared_transaction_test.o $(LIBOBJECTS) $(TESTHARNESS) + write_prepared_transaction_test: $(OBJ_DIR)/utilities/transactions/write_prepared_transaction_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - sst_dump: tools/sst_dump.o $(LIBOBJECTS) + write_unprepared_transaction_test: $(OBJ_DIR)/utilities/transactions/write_unprepared_transaction_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - blob_dump: tools/blob_dump.o $(LIBOBJECTS) + sst_dump: $(OBJ_DIR)/tools/sst_dump.o $(TOOLS_LIBRARY) $(LIBRARY) $(AM_LINK) - repair_test: db/repair_test.o db/db_test_util.o $(LIBOBJECTS) $(TESTHARNESS) + blob_dump: $(OBJ_DIR)/tools/blob_dump.o $(TOOLS_LIBRARY) $(LIBRARY) $(AM_LINK) - ldb_cmd_test: tools/ldb_cmd_test.o $(LIBOBJECTS) $(TESTHARNESS) + repair_test: $(OBJ_DIR)/db/repair_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - ldb: tools/ldb.o $(LIBOBJECTS) + ldb_cmd_test: $(OBJ_DIR)/tools/ldb_cmd_test.o $(TOOLS_LIBRARY) $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - iostats_context_test: monitoring/iostats_context_test.o $(LIBOBJECTS) $(TESTHARNESS) + ldb: $(OBJ_DIR)/tools/ldb.o $(TOOLS_LIBRARY) $(LIBRARY) + $(AM_LINK) + + iostats_context_test: $(OBJ_DIR)/monitoring/iostats_context_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_V_CCLD)$(CXX) $^ $(EXEC_LDFLAGS) -o $@ $(LDFLAGS) - persistent_cache_test: utilities/persistent_cache/persistent_cache_test.o db/db_test_util.o $(LIBOBJECTS) $(TESTHARNESS) + persistent_cache_test: $(OBJ_DIR)/utilities/persistent_cache/persistent_cache_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - statistics_test: monitoring/statistics_test.o $(LIBOBJECTS) $(TESTHARNESS) + statistics_test: $(OBJ_DIR)/monitoring/statistics_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - stats_history_test: monitoring/stats_history_test.o db/db_test_util.o $(LIBOBJECTS) $(TESTHARNESS) + stats_history_test: $(OBJ_DIR)/monitoring/stats_history_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - lru_cache_test: cache/lru_cache_test.o $(LIBOBJECTS) $(TESTHARNESS) + lru_cache_test: $(OBJ_DIR)/cache/lru_cache_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - range_del_aggregator_test: db/range_del_aggregator_test.o db/db_test_util.o $(LIBOBJECTS) $(TESTHARNESS) + range_del_aggregator_test: $(OBJ_DIR)/db/range_del_aggregator_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - range_del_aggregator_bench: db/range_del_aggregator_bench.o $(LIBOBJECTS) $(TESTUTIL) + range_del_aggregator_bench: $(OBJ_DIR)/db/range_del_aggregator_bench.o $(LIBRARY) $(AM_LINK) - blob_db_test: utilities/blob_db/blob_db_test.o $(LIBOBJECTS) $(TESTHARNESS) + blob_db_test: $(OBJ_DIR)/utilities/blob_db/blob_db_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - repeatable_thread_test: util/repeatable_thread_test.o $(LIBOBJECTS) $(TESTHARNESS) + repeatable_thread_test: $(OBJ_DIR)/util/repeatable_thread_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - range_locking_test: utilities/transactions/range_locking_test.o $(LIBOBJECTS) $(TESTHARNESS) ++range_locking_test: utilities/transactions/range_locking_test.o $(TEST_LIBRARY) $(LIBRARY) + $(AM_LINK) + - range_tombstone_fragmenter_test: db/range_tombstone_fragmenter_test.o db/db_test_util.o $(LIBOBJECTS) $(TESTHARNESS) + range_tombstone_fragmenter_test: $(OBJ_DIR)/db/range_tombstone_fragmenter_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - sst_file_reader_test: table/sst_file_reader_test.o $(LIBOBJECTS) $(TESTHARNESS) + sst_file_reader_test: $(OBJ_DIR)/table/sst_file_reader_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - db_secondary_test: db/db_impl/db_secondary_test.o db/db_test_util.o $(LIBOBJECTS) $(TESTHARNESS) + db_secondary_test: $(OBJ_DIR)/db/db_impl/db_secondary_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - block_cache_tracer_test: trace_replay/block_cache_tracer_test.o trace_replay/block_cache_tracer.o $(LIBOBJECTS) $(TESTHARNESS) + block_cache_tracer_test: $(OBJ_DIR)/trace_replay/block_cache_tracer_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - block_cache_trace_analyzer_test: tools/block_cache_analyzer/block_cache_trace_analyzer_test.o tools/block_cache_analyzer/block_cache_trace_analyzer.o $(LIBOBJECTS) $(TESTHARNESS) + block_cache_trace_analyzer_test: $(OBJ_DIR)/tools/block_cache_analyzer/block_cache_trace_analyzer_test.o $(OBJ_DIR)/tools/block_cache_analyzer/block_cache_trace_analyzer.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - defer_test: util/defer_test.o $(LIBOBJECTS) $(TESTHARNESS) + defer_test: $(OBJ_DIR)/util/defer_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - blob_file_addition_test: db/blob/blob_file_addition_test.o $(LIBOBJECTS) $(TESTHARNESS) + blob_file_addition_test: $(OBJ_DIR)/db/blob/blob_file_addition_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - blob_file_garbage_test: db/blob/blob_file_garbage_test.o $(LIBOBJECTS) $(TESTHARNESS) + blob_file_garbage_test: $(OBJ_DIR)/db/blob/blob_file_garbage_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - timer_test: util/timer_test.o $(LIBOBJECTS) $(TESTHARNESS) + timer_test: $(OBJ_DIR)/util/timer_test.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) - testutil_test: test_util/testutil_test.o $(LIBOBJECTS) $(TESTHARNESS) + testutil_test: $(OBJ_DIR)/test_util/testutil_test.o $(TEST_LIBRARY) $(LIBRARY) + $(AM_LINK) + + io_tracer_test: $(OBJ_DIR)/trace_replay/io_tracer_test.o $(OBJ_DIR)/trace_replay/io_tracer.o $(TEST_LIBRARY) $(LIBRARY) $(AM_LINK) #------------------------------------------------- diff --cc utilities/transactions/pessimistic_transaction.cc index a8f6fea62,d92818528..f6fd3a504 --- a/utilities/transactions/pessimistic_transaction.cc +++ b/utilities/transactions/pessimistic_transaction.cc @@@ -558,19 -555,9 +558,19 @@@ Status PessimisticTransaction::TryLock( } uint32_t cfh_id = GetColumnFamilyID(column_family); std::string key_str = key.ToString(); - // - PointLockStatus status = tracked_locks_->GetPointLockStatus(cfh_id, key_str); - bool previously_locked = status.locked; - bool lock_upgrade = previously_locked && exclusive && !status.exclusive; ++ + PointLockStatus status; - bool previously_locked; + bool lock_upgrade; ++ bool previously_locked; + if (tracked_locks_->IsPointLockSupported()) { + status = tracked_locks_->GetPointLockStatus(cfh_id, key_str); + previously_locked = status.locked; + lock_upgrade = previously_locked && exclusive && !status.exclusive; + } else { + previously_locked = false; + status.locked = false; + lock_upgrade = false; + } // Lock this key if this transactions hasn't already locked it or we require // an upgrade. diff --cc utilities/transactions/range_locking_test.cc index 1de512079,000000000..07914df6c mode 100644,000000..100644 --- a/utilities/transactions/range_locking_test.cc +++ b/utilities/transactions/range_locking_test.cc @@@ -1,200 -1,0 +1,200 @@@ +#ifndef ROCKSDB_LITE + +#ifndef __STDC_FORMAT_MACROS +#define __STDC_FORMAT_MACROS +#endif + +#include "utilities/transactions/transaction_test.h" + +#include <algorithm> +#include <functional> +#include <string> +#include <thread> + +#include "db/db_impl/db_impl.h" +#include "rocksdb/db.h" +#include "rocksdb/options.h" +#include "rocksdb/perf_context.h" +#include "rocksdb/utilities/transaction.h" +#include "rocksdb/utilities/transaction_db.h" +#include "table/mock_table.h" - #include "test_util/fault_injection_test_env.h" ++// #include "test_util/fault_injection_test_env.h" +#include "util/random.h" +#include "util/string_util.h" +#include "test_util/sync_point.h" +#include "test_util/testharness.h" +#include "test_util/testutil.h" +#include "test_util/transaction_test_util.h" +#include "utilities/merge_operators.h" +#include "utilities/merge_operators/string_append/stringappend.h" +#include "utilities/transactions/pessimistic_transaction_db.h" + +#include "port/port.h" + +using std::string; + +namespace rocksdb { + + +class RangeLockingTest : public ::testing::Test { + public: + TransactionDB* db; + std::string dbname; + Options options; + + std::shared_ptr<RangeLockMgrHandle> range_lock_mgr; + TransactionDBOptions txn_db_options; + + RangeLockingTest() + : db(nullptr) { + options.create_if_missing = true; + dbname = test::PerThreadDBPath("range_locking_testdb"); + + DestroyDB(dbname, options); + Status s; + + range_lock_mgr.reset(rocksdb::NewRangeLockManager(nullptr)); + txn_db_options.lock_mgr_handle = range_lock_mgr; + + s = TransactionDB::Open(options, txn_db_options, dbname, &db); + assert(s.ok()); + + } + + ~RangeLockingTest() { + delete db; + db = nullptr; + // This is to skip the assert statement in FaultInjectionTestEnv. There + // seems to be a bug in btrfs that the makes readdir return recently + // unlink-ed files. By using the default fs we simply ignore errors resulted + // from attempting to delete such files in DestroyDB. + DestroyDB(dbname, options); + } + + PessimisticTransaction* NewTxn( + TransactionOptions txn_opt = TransactionOptions()) { + 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. +TEST_F(RangeLockingTest, BasicRangeLocking) { + WriteOptions write_options; + TransactionOptions txn_options; + std::string value; + ReadOptions read_options; + + Transaction* txn0 = db->BeginTransaction(write_options, txn_options); + Transaction* txn1 = db->BeginTransaction(write_options, txn_options); + + // Get a range lock + { + auto s= txn0->GetRangeLock(db->DefaultColumnFamily(), + Endpoint("a"), Endpoint("c")); + ASSERT_EQ(s, Status::OK()); + } + + + // Check that range Lock inhibits an overlapping range lock + { + auto s= txn1->GetRangeLock(db->DefaultColumnFamily(), + Endpoint("b"), Endpoint("z")); + ASSERT_TRUE(s.IsTimedOut()); + } + + // Check that range Lock inhibits an overlapping point lock + { + auto s= txn1->GetForUpdate(read_options, db->DefaultColumnFamily(), + Slice("b"), &value); + ASSERT_TRUE(s.IsTimedOut()); + } + + // Get a point lock, check that it inhibits range locks + { + auto s= txn0->Put(db->DefaultColumnFamily(), + Slice("d"), Slice("value")); + ASSERT_EQ(s, Status::OK()); + + auto s2= txn1->GetRangeLock(db->DefaultColumnFamily(), + Endpoint("c"), Endpoint("e")); + ASSERT_TRUE(s2.IsTimedOut()); + } + + ASSERT_OK(txn0->Commit()); + txn1->Rollback(); + + delete txn0; + delete txn1; +} + +TEST_F(RangeLockingTest, SnapshotValidation) { + Status s; + Slice key_slice= Slice("k"); + ColumnFamilyHandle *cfh= db->DefaultColumnFamily(); + + auto txn0 = NewTxn(); + txn0->Put(key_slice, Slice("initial")); + txn0->Commit(); + + // txn1 + auto txn1 = NewTxn(); + txn1->SetSnapshot(); + std::string val1; + s = txn1->Get(ReadOptions(), cfh, key_slice, &val1); + ASSERT_TRUE(s.ok()); + val1 = val1 + std::string("-txn1"); + + s = txn1->Put(cfh, key_slice, Slice(val1)); + ASSERT_TRUE(s.ok()); + + // txn2 + auto txn2 = NewTxn(); + txn2->SetSnapshot(); + std::string val2; + // This will see the original value as nothing is committed + // This is also Get, so it is doesn't acquire any locks. + s = txn2->Get(ReadOptions(), cfh, key_slice, &val2); + ASSERT_TRUE(s.ok()); + + // txn1 + s = txn1->Commit(); + ASSERT_TRUE(s.ok()); + + // txn2 + val2 = val2 + std::string("-txn2"); + // Now, this call should do Snapshot Validation and fail: + s = txn2->Put(cfh, key_slice, Slice(val2)); + ASSERT_TRUE(s.IsBusy()); + + s = txn2->Commit(); + ASSERT_TRUE(s.ok()); + + /* + // Not meaningful if s.IsBusy() above is true: + // Examine the result + auto txn3= NewTxn(); + std::string val3; + Status s3 = txn3->Get(ReadOptions(), cfh, key_slice, &val3); + fprintf(stderr, "Final: %s\n", val3.c_str()); + */ +} + +} // namespace rocksdb + +int main(int argc, char** argv) { + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} + +#else +#include <stdio.h> + +int main(int /*argc*/, char** /*argv*/) { + fprintf(stderr, + "SKIPPED as Transactions are not supported in ROCKSDB_LITE\n"); + return 0; +} + +#endif // ROCKSDB_LITE

1 0

[Commits] 0664e1e766d: MDEV-22338: Assertion `!current_stmt_is_commit || !rgi->tables_to_lock' failed in Query_log_event::do_apply_event
by sujatha 10 Aug '20

10 Aug '20

revision-id: 0664e1e766d303187b381ace1c53547d1821e35c (mariadb-10.1.43-252-g0664e1e766d) parent(s): 85bd5314c56c150d756066806d4a6cb5b682383f author: Sujatha committer: Sujatha timestamp: 2020-08-10 12:29:48 +0530 message: MDEV-22338: Assertion `!current_stmt_is_commit || !rgi->tables_to_lock' failed in Query_log_event::do_apply_event Analysis: ========= With row based replication, master can generate event groups like shown below. BEGIN TABLE_MAP COMMIT i.e A binlog event group, without any rows event in it. Such groups are possible when a query operating on transactional table, doesn't modify any data in the table. For example a transaction is trying to modify both trans and non-trans tables and a trigger exists on non-transtable which modifies a trans table. CREATE TABLE t1 (id int) engine=innodb; CREATE TABLE t2 (id int) engine=myisam; CREATE TRIGGER t2 BEFORE INSERT ON t2 FOR EACH ROW DELETE FROM t1 LIMIT 1; INSERT INTO t2 VALUES (1); Above insert will write a record in 't2' table and its corresponding binlog entry is generated. First table map events are added to appropriate caches. t2's Table_Map is written into statement_cache. t1's Table_Map is written into transaction_cache. 'Write_rows' event is added to t2's statement_cache. Since there are no rows to remove from table 't1' there will not be any 'Delete_rows' event in the transaction cache. There won't be any 'Xid' event as well. During flush operation 'COMMIT' will be added to the transaction cache to complete the group. "t2's statement_cache" "t1's transaction_cache" BEGIN BEGIN TABLE_MAP TABLE_MAP WRITE_ROWS COMMIT COMMIT On the slave side it has strict rules, that a 'Table_Map' event should be followed by a rows event. This rule gets violated for 't1' transaction and results in an assert. Fix: ==== Replace the assert with if-else block. In a case where TABLE_MAP event is not followed by rows event invoke 'rows_event_stmt_cleanup' to initiate appropriate cleanup. The fix is based on upstream patch: commit da01f3846b62a2e41111256f9d6f0a2bb2de23c4 Author: Andrei Elkin <aelkin(a)mysql.com> --- .../suite/rpl/r/rpl_no_rows_event_assert.result | 26 ++++++++++++ .../suite/rpl/t/rpl_no_rows_event_assert.test | 46 ++++++++++++++++++++++ sql/log_event.cc | 22 ++++++++++- 3 files changed, 92 insertions(+), 2 deletions(-) diff --git a/mysql-test/suite/rpl/r/rpl_no_rows_event_assert.result b/mysql-test/suite/rpl/r/rpl_no_rows_event_assert.result new file mode 100644 index 00000000000..7661bb62305 --- /dev/null +++ b/mysql-test/suite/rpl/r/rpl_no_rows_event_assert.result @@ -0,0 +1,26 @@ +include/master-slave.inc +[connection master] +######################################################################### +# Test case1: Transaction cache has 'Table_Map' event followed by # +# 'COMMIT'. # +######################################################################### +CREATE TABLE t1 (id int) ENGINE=INNODB; +CREATE TABLE t2 (id int) ENGINE=MYISAM; +CREATE TRIGGER t2 BEFORE INSERT ON t2 FOR EACH ROW DELETE FROM t1 LIMIT 1; +INSERT INTO t2 VALUES (1); +DROP TABLE t1,t2; +######################################################################### +# Test case2: Transaction cache has 'Query', 'Table_Map' followed by # +# 'COMMIT'. # +######################################################################### +CREATE TABLE t1(f INT) ENGINE=INNODB; +CREATE TABLE t2(f INT) ENGINE=MYISAM; +CREATE TABLE t3(id INT AUTO_INCREMENT, i INT, PRIMARY KEY (id)) ENGINE=INNODB; +CREATE TABLE t4(f INT) ENGINE=MYISAM; +CREATE TRIGGER trig1 BEFORE INSERT ON t2 FOR EACH ROW INSERT INTO t3(i) SELECT * FROM t4 LIMIT 0; +BEGIN; +DELETE FROM t1 WHERE f=0; +INSERT INTO t2 VALUES (2); +COMMIT; +DROP TABLE t1,t2,t3,t4; +include/rpl_end.inc diff --git a/mysql-test/suite/rpl/t/rpl_no_rows_event_assert.test b/mysql-test/suite/rpl/t/rpl_no_rows_event_assert.test new file mode 100644 index 00000000000..c5cc7634615 --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_no_rows_event_assert.test @@ -0,0 +1,46 @@ +--source include/have_innodb.inc +--source include/have_binlog_format_mixed_or_row.inc +--source include/master-slave.inc +# ==== References ==== +# +# MDEV-22338: Assertion `!current_stmt_is_commit || !rgi->tables_to_lock' +# failed in Query_log_event::do_apply_event +# + +--echo ######################################################################### +--echo # Test case1: Transaction cache has 'Table_Map' event followed by # +--echo # 'COMMIT'. # +--echo ######################################################################### +--connection master +CREATE TABLE t1 (id int) ENGINE=INNODB; +CREATE TABLE t2 (id int) ENGINE=MYISAM; +CREATE TRIGGER t2 BEFORE INSERT ON t2 FOR EACH ROW DELETE FROM t1 LIMIT 1; +INSERT INTO t2 VALUES (1); +--sync_slave_with_master + +# Cleanup +--connection master +DROP TABLE t1,t2; + +--echo ######################################################################### +--echo # Test case2: Transaction cache has 'Query', 'Table_Map' followed by # +--echo # 'COMMIT'. # +--echo ######################################################################### + +CREATE TABLE t1(f INT) ENGINE=INNODB; +CREATE TABLE t2(f INT) ENGINE=MYISAM; +CREATE TABLE t3(id INT AUTO_INCREMENT, i INT, PRIMARY KEY (id)) ENGINE=INNODB; +CREATE TABLE t4(f INT) ENGINE=MYISAM; +CREATE TRIGGER trig1 BEFORE INSERT ON t2 FOR EACH ROW INSERT INTO t3(i) SELECT * FROM t4 LIMIT 0; + +BEGIN; +DELETE FROM t1 WHERE f=0; +INSERT INTO t2 VALUES (2); +COMMIT; +--sync_slave_with_master + +# Cleanup +--connection master +DROP TABLE t1,t2,t3,t4; + +--source include/rpl_end.inc diff --git a/sql/log_event.cc b/sql/log_event.cc index a1a442df43f..c37512aa6c1 100644 --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -4329,8 +4329,26 @@ int Query_log_event::do_apply_event(rpl_group_info *rgi, thd->clear_error(1); current_stmt_is_commit= is_commit(); - DBUG_ASSERT(!current_stmt_is_commit || !rgi->tables_to_lock); - rgi->slave_close_thread_tables(thd); + if (strcmp("COMMIT", query) == 0 && rgi->tables_to_lock != NULL) + { + /* + Cleaning-up the last statement context: + Found a row based event group which didn't modfiy any rows. i.e. + BEGIN + TABLE_MAP + COMMIT + */ + int error; + if ((error = rows_event_stmt_cleanup(rgi, thd))) + { + rli->report(ERROR_LEVEL, error, "Error in cleaning up after an event " + "preceding the commit; the group log file/position: %s %llu", + rli->group_master_log_name, + (ulong) rli->group_master_log_pos); + } + } + else + rgi->slave_close_thread_tables(thd); /* Note: We do not need to execute reset_one_shot_variables() if this

1 0

[Commits] c776cad5f8e: MDEV-22702: Assertion `!field->is_null()' failed in my_decimal::my_decimal
by Varun 06 Aug '20

06 Aug '20

revision-id: c776cad5f8ecf2675510deeb55724d7255a52503 (mariadb-10.4.11-267-gc776cad5f8e) parent(s): cc0dca366357651ddb549e31a12b1ecd39c7380e author: Varun Gupta committer: Varun Gupta timestamp: 2020-07-08 14:58:17 +0530 message: MDEV-22702: Assertion `!field->is_null()' failed in my_decimal::my_decimal With implicit grouping with window functions, we need to make sure that all the fields inside the window functions are nullable as any non-aggregated field can produce a NULL value. --- mysql-test/main/win.result | 14 ++++++++++++++ mysql-test/main/win.test | 11 +++++++++++ sql/sql_lex.h | 2 ++ sql/sql_select.cc | 12 ++++++++++++ 4 files changed, 39 insertions(+) diff --git a/mysql-test/main/win.result b/mysql-test/main/win.result index 6ef3a10966a..e70e982807c 100644 --- a/mysql-test/main/win.result +++ b/mysql-test/main/win.result @@ -3856,3 +3856,17 @@ row_number() OVER (order by a) 2 3 drop table t1; +# +# MDEV-22702: Assertion `!field->is_null()' failed in my_decimal::my_decimal +# +CREATE TABLE t1(a INT, b DECIMAL(10, 0) NOT NULL); +SELECT a, bit_or(min(a)) OVER (ORDER BY b) FROM t1; +a bit_or(min(a)) OVER (ORDER BY b) +NULL 0 +# No implicit grouping here +SELECT a, bit_or(a) OVER (ORDER BY b) FROM t1; +a bit_or(a) OVER (ORDER BY b) +SELECT a, sum(a), bit_or(a) OVER (ORDER BY b) FROM t1; +a sum(a) bit_or(a) OVER (ORDER BY b) +NULL NULL 0 +DROP TABLE t1; diff --git a/mysql-test/main/win.test b/mysql-test/main/win.test index 824c5f9fa56..b5d00cd80da 100644 --- a/mysql-test/main/win.test +++ b/mysql-test/main/win.test @@ -2512,3 +2512,14 @@ create table t1 (a int); insert into t1 values (1),(2),(3); SELECT row_number() OVER (order by a) FROM t1 order by NAME_CONST('myname',NULL); drop table t1; + +--echo # +--echo # MDEV-22702: Assertion `!field->is_null()' failed in my_decimal::my_decimal +--echo # + +CREATE TABLE t1(a INT, b DECIMAL(10, 0) NOT NULL); +SELECT a, bit_or(min(a)) OVER (ORDER BY b) FROM t1; +--echo # No implicit grouping here +SELECT a, bit_or(a) OVER (ORDER BY b) FROM t1; +SELECT a, sum(a), bit_or(a) OVER (ORDER BY b) FROM t1; +DROP TABLE t1; diff --git a/sql/sql_lex.h b/sql/sql_lex.h index 0983cea44d0..16a0b3b08a6 100644 --- a/sql/sql_lex.h +++ b/sql/sql_lex.h @@ -1510,6 +1510,8 @@ class st_select_lex: public st_select_lex_node } bool have_window_funcs() const { return (window_funcs.elements !=0); } + uint32 get_number_of_window_funcs() const + { return (uint32)window_funcs.elements; } ORDER *find_common_window_func_partition_fields(THD *thd); bool cond_pushdown_is_allowed() const diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 0ca5ab23288..05463efe9a5 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -1201,6 +1201,18 @@ JOIN::prepare(TABLE_LIST *tables_init, break; } } + /* + If the query has a window function with an aggregate function, + then also we have a mix of elements with and without grouping. + Window function can be in the ORDER BY clause too so the check + is made separately. + Window function is inherited from Item_sum so each window function is + also registered as a sum item, so need to check that we have an + explicit aggregate function also in the query. + */ + if (select_lex->have_window_funcs() && + select_lex->get_number_of_window_funcs() < select_lex->n_sum_items) + mixed_implicit_grouping= true; } table_count= select_lex->leaf_tables.elements;

3 2

[Commits] 85bd5314c56: Better comment about TABLE::maybe_null
by Sergei Petrunia 06 Aug '20

06 Aug '20

revision-id: 85bd5314c56c150d756066806d4a6cb5b682383f (mariadb-10.1.43-251-g85bd5314c56) parent(s): ab578bdf453c3cb0e9ca561cf373f64c96b22fda author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2020-08-06 13:39:10 +0300 message: Better comment about TABLE::maybe_null --- sql/table.h | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/sql/table.h b/sql/table.h index 93795113fab..2ea9b514df4 100644 --- a/sql/table.h +++ b/sql/table.h @@ -1291,9 +1291,16 @@ struct TABLE /* number of select if it is derived table */ uint derived_select_number; /* - 0 or JOIN_TYPE_{LEFT|RIGHT}. Currently this is only compared to 0. - If maybe_null !=0, this table is inner w.r.t. some outer join operation, - and null_row may be true. + Possible values: + - 0 by default + - JOIN_TYPE_{LEFT|RIGHT} if the table is inner w.r.t an outer join + operation + - 1 if the SELECT has mixed_implicit_grouping=1. example: + select max(col1), col2 from t1. In this case, the query produces + one row with all columns having NULL values. + + Interpetation: If maybe_null!=0, all fields of the table are considered + NULLable (and have NULL values when null_row=true) */ uint maybe_null; int current_lock; /* Type of lock on table */

1 0

[Commits] (no subject)
by Varun 05 Aug '20

05 Aug '20

1 0

[Commits] 7907592eeb5: MDEV-23291: SUM column from a derived table returns invalid values
by Varun 05 Aug '20

05 Aug '20

revision-id: 7907592eeb5d0fd8ac1064ef5f77eb274ddb0b5b (mariadb-10.1.43-249-g7907592eeb5) parent(s): e00bafaa6162ff8967a0d8eed6aa12806cfd4552 author: Varun Gupta committer: Varun Gupta timestamp: 2020-08-05 15:07:18 +0530 message: MDEV-23291: SUM column from a derived table returns invalid values The issue here was the read_set bitmap was not set for a field which was used as a reference in an inner select. We need to make sure that if we are in an inner select and we have references from outer select then we update the table bitmaps for such references. Introduced a function in the class Item_subselect that would update bitmaps of table for the references within a subquery that are defined in outer selects. --- mysql-test/r/view.result | 15 +++++++++++++++ mysql-test/t/view.test | 16 ++++++++++++++++ sql/item_subselect.cc | 20 ++++++++++++++++++++ sql/item_subselect.h | 1 + 4 files changed, 52 insertions(+) diff --git a/mysql-test/r/view.result b/mysql-test/r/view.result index d73ef7c0ee4..ba09d694c23 100644 --- a/mysql-test/r/view.result +++ b/mysql-test/r/view.result @@ -6274,5 +6274,20 @@ t1col1 t1col2 t1col3 drop view v1; drop table t1,t2; # +# MDEV-23291: SUM column from a derived table returns invalid values +# +CREATE TABLE t1(a INT, b INT); +INSERT INTO t1 VALUES (1,1), (2,2); +CREATE view v1 AS +SELECT a as x, (select x) as y, (select y) as z FROM t1; +SELECT sum(z) FROM (SELECT a as x, (select x) as y, (select y) as z FROM t1) q; +sum(z) +3 +SELECT sum(z) FROM v1; +sum(z) +3 +DROP TABLE t1; +DROP VIEW v1; +# # End of 10.1 tests # diff --git a/mysql-test/t/view.test b/mysql-test/t/view.test index f7c9ffbc99b..df589e83122 100644 --- a/mysql-test/t/view.test +++ b/mysql-test/t/view.test @@ -6104,6 +6104,22 @@ select * from v1; drop view v1; drop table t1,t2; +--echo # +--echo # MDEV-23291: SUM column from a derived table returns invalid values +--echo # + +CREATE TABLE t1(a INT, b INT); +INSERT INTO t1 VALUES (1,1), (2,2); + +CREATE view v1 AS +SELECT a as x, (select x) as y, (select y) as z FROM t1; + +SELECT sum(z) FROM (SELECT a as x, (select x) as y, (select y) as z FROM t1) q; +SELECT sum(z) FROM v1; + +DROP TABLE t1; +DROP VIEW v1; + --echo # --echo # End of 10.1 tests --echo # diff --git a/sql/item_subselect.cc b/sql/item_subselect.cc index 0b28c15027d..236f5433777 100644 --- a/sql/item_subselect.cc +++ b/sql/item_subselect.cc @@ -424,6 +424,26 @@ bool Item_subselect::mark_as_dependent(THD *thd, st_select_lex *select, return FALSE; } +/* + @brief + Update the table bitmaps for the outer references used within a subquery +*/ + +bool Item_subselect::update_table_bitmaps_processor(uchar *arg) +{ + List_iterator<Ref_to_outside> it(upper_refs); + Ref_to_outside *upper; + + while ((upper= it++)) + { + if (upper->item && + upper->item->walk(&Item::update_table_bitmaps_processor, FALSE, arg)) + return TRUE; + } + return FALSE; +} + + /* Adjust attributes after our parent select has been merged into grandparent diff --git a/sql/item_subselect.h b/sql/item_subselect.h index ee8b31f4f17..006bcfcaf88 100644 --- a/sql/item_subselect.h +++ b/sql/item_subselect.h @@ -231,6 +231,7 @@ class Item_subselect :public Item_result_field, @retval FALSE otherwise */ bool is_expensive_processor(uchar *arg) { return is_expensive(); } + bool update_table_bitmaps_processor(uchar *arg); /** Get the SELECT_LEX structure associated with this Item.

1 0

[Commits] 4746d353f0f: MDEV-23291: SUM column from a derived table returns invalid values
by Varun 05 Aug '20

05 Aug '20

revision-id: 4746d353f0fcbb64fcda485f7b01e0135be61db8 (mariadb-10.1.43-249-g4746d353f0f) parent(s): e00bafaa6162ff8967a0d8eed6aa12806cfd4552 author: Varun Gupta committer: Varun Gupta timestamp: 2020-08-05 14:08:00 +0530 message: MDEV-23291: SUM column from a derived table returns invalid values The issue here was the read_set bitmap was not set for an item in the select list of a subquery which is used as an outer reference in an outer select. Introduced a function in the class Item_subselect that would update bitmaps of table for the references from within a subquery that is used in outer selects. --- mysql-test/r/view.result | 15 +++++++++++++++ mysql-test/t/view.test | 16 ++++++++++++++++ sql/item_subselect.cc | 21 +++++++++++++++++++++ sql/item_subselect.h | 1 + 4 files changed, 53 insertions(+) diff --git a/mysql-test/r/view.result b/mysql-test/r/view.result index d73ef7c0ee4..ba09d694c23 100644 --- a/mysql-test/r/view.result +++ b/mysql-test/r/view.result @@ -6274,5 +6274,20 @@ t1col1 t1col2 t1col3 drop view v1; drop table t1,t2; # +# MDEV-23291: SUM column from a derived table returns invalid values +# +CREATE TABLE t1(a INT, b INT); +INSERT INTO t1 VALUES (1,1), (2,2); +CREATE view v1 AS +SELECT a as x, (select x) as y, (select y) as z FROM t1; +SELECT sum(z) FROM (SELECT a as x, (select x) as y, (select y) as z FROM t1) q; +sum(z) +3 +SELECT sum(z) FROM v1; +sum(z) +3 +DROP TABLE t1; +DROP VIEW v1; +# # End of 10.1 tests # diff --git a/mysql-test/t/view.test b/mysql-test/t/view.test index f7c9ffbc99b..df589e83122 100644 --- a/mysql-test/t/view.test +++ b/mysql-test/t/view.test @@ -6104,6 +6104,22 @@ select * from v1; drop view v1; drop table t1,t2; +--echo # +--echo # MDEV-23291: SUM column from a derived table returns invalid values +--echo # + +CREATE TABLE t1(a INT, b INT); +INSERT INTO t1 VALUES (1,1), (2,2); + +CREATE view v1 AS +SELECT a as x, (select x) as y, (select y) as z FROM t1; + +SELECT sum(z) FROM (SELECT a as x, (select x) as y, (select y) as z FROM t1) q; +SELECT sum(z) FROM v1; + +DROP TABLE t1; +DROP VIEW v1; + --echo # --echo # End of 10.1 tests --echo # diff --git a/sql/item_subselect.cc b/sql/item_subselect.cc index 0b28c15027d..b6a3d10a790 100644 --- a/sql/item_subselect.cc +++ b/sql/item_subselect.cc @@ -424,6 +424,27 @@ bool Item_subselect::mark_as_dependent(THD *thd, st_select_lex *select, return FALSE; } +/* + @brief + Update the table bitmaps for the reference of a subquery used in upper + level selects. +*/ + +bool Item_subselect::update_table_bitmaps_processor(uchar *arg) +{ + List_iterator<Ref_to_outside> it(upper_refs); + Ref_to_outside *upper; + + while ((upper= it++)) + { + if (upper->item && + upper->item->walk(&Item::update_table_bitmaps_processor, FALSE, arg)) + return TRUE; + } + return FALSE; +} + + /* Adjust attributes after our parent select has been merged into grandparent diff --git a/sql/item_subselect.h b/sql/item_subselect.h index ee8b31f4f17..006bcfcaf88 100644 --- a/sql/item_subselect.h +++ b/sql/item_subselect.h @@ -231,6 +231,7 @@ class Item_subselect :public Item_result_field, @retval FALSE otherwise */ bool is_expensive_processor(uchar *arg) { return is_expensive(); } + bool update_table_bitmaps_processor(uchar *arg); /** Get the SELECT_LEX structure associated with this Item.

1 0