revision-id: 85e37c7ae3185790ef89a7da660112c73ad6cec1 (fb-prod201903-261-g85e37c7ae31) parent(s): b6188f3823dedf32a53f312ae086564676328b0f author: Sergei Petrunia committer: Sergei Petrunia timestamp: 2019-11-28 17:50:20 +0300 message: Fix rocksdb.unique_seq_rev_cf test For reverse-ordered column families, we need to flip the endpoint types (like it is already done in ha_rocksdb::set_range_lock) --- storage/rocksdb/ha_rocksdb.cc | 20 ++++++++++++++------ storage/rocksdb/rdb_locking_iter.cc | 5 +++-- storage/rocksdb/rdb_locking_iter.h | 12 ++++++++---- 3 files changed, 25 insertions(+), 12 deletions(-) diff --git a/storage/rocksdb/ha_rocksdb.cc b/storage/rocksdb/ha_rocksdb.cc index 53d489b7474..9743b0b3ee9 100644 --- a/storage/rocksdb/ha_rocksdb.cc +++ b/storage/rocksdb/ha_rocksdb.cc @@ -2769,6 +2769,8 @@ class Rdb_transaction { rocksdb::Status lock_singlepoint_range(rocksdb::ColumnFamilyHandle *const cf, const rocksdb::Slice &point) { + // Normally, one needs to "flip" the endpoint type for reverse-ordered CFs. + // But here we are locking just one point so this is not necessary. rocksdb::Endpoint endp(point, false); return lock_range(cf, endp, endp); } @@ -3177,6 +3179,7 @@ class Rdb_transaction { virtual rocksdb::Iterator *get_iterator( const rocksdb::ReadOptions &options, rocksdb::ColumnFamilyHandle *column_family, + bool is_rev_cf, bool use_locking_iterator=false) = 0; virtual void multi_get(rocksdb::ColumnFamilyHandle *const column_family, @@ -3187,7 +3190,8 @@ class Rdb_transaction { rocksdb::Iterator *get_iterator( - rocksdb::ColumnFamilyHandle *const column_family, bool skip_bloom_filter, + rocksdb::ColumnFamilyHandle *const column_family, bool is_rev_cf, + bool skip_bloom_filter, bool fill_cache, const rocksdb::Slice &eq_cond_lower_bound, const rocksdb::Slice &eq_cond_upper_bound, bool read_current = false, bool create_snapshot = true, @@ -3220,7 +3224,7 @@ class Rdb_transaction { if (read_current) { options.snapshot = nullptr; } - return get_iterator(options, column_family, use_locking_iterator); + return get_iterator(options, column_family, is_rev_cf, use_locking_iterator); } virtual bool is_tx_started() const = 0; @@ -3608,10 +3612,11 @@ class Rdb_transaction_impl : public Rdb_transaction { rocksdb::Iterator *get_iterator( const rocksdb::ReadOptions &options, rocksdb::ColumnFamilyHandle *const column_family, + bool is_rev_cf, bool use_locking_iterator) override { global_stats.queries[QUERIES_RANGE].inc(); if (use_locking_iterator) - return GetLockingIterator(m_rocksdb_tx, options, column_family); + return GetLockingIterator(m_rocksdb_tx, options, column_family, is_rev_cf); else return m_rocksdb_tx->GetIterator(options, column_family); } @@ -3912,6 +3917,7 @@ class Rdb_writebatch_impl : public Rdb_transaction { rocksdb::Iterator *get_iterator( const rocksdb::ReadOptions &options, rocksdb::ColumnFamilyHandle *const /* column_family */, + bool is_rev_cf, bool use_locking_iterator) override { DBUG_ASSERT(!use_locking_iterator); const auto it = rdb->NewIterator(options); @@ -10058,7 +10064,8 @@ int ha_rocksdb::check_and_lock_sk(const uint key_id, } rocksdb::Iterator *const iter = row_info.tx->get_iterator( - kd.get_cf(), total_order_seek, fill_cache, lower_bound_slice, + kd.get_cf(), kd.m_is_reverse_cf, total_order_seek, fill_cache, + lower_bound_slice, upper_bound_slice, true /* read current data */, false /* acquire snapshot */); /* @@ -10711,7 +10718,8 @@ void ha_rocksdb::setup_scan_iterator(const Rdb_key_def &kd, read_opts.snapshot = m_scan_it_snapshot; m_scan_it = rdb->NewIterator(read_opts, kd.get_cf()); } else { - m_scan_it = tx->get_iterator(kd.get_cf(), skip_bloom, fill_cache, + m_scan_it = tx->get_iterator(kd.get_cf(), kd.m_is_reverse_cf, + skip_bloom, fill_cache, m_scan_it_lower_bound_slice, m_scan_it_upper_bound_slice, /*read_current*/ false, @@ -15545,7 +15553,7 @@ rocksdb::Iterator *rdb_tx_get_iterator( Rdb_transaction *tx, const rocksdb::ReadOptions &options, rocksdb::ColumnFamilyHandle *const column_family) { global_stats.queries[QUERIES_RANGE].inc(); - return tx->get_iterator(options, column_family); + return tx->get_iterator(options, column_family, false); } bool rdb_tx_started(Rdb_transaction *tx) { return tx->is_tx_started(); } diff --git a/storage/rocksdb/rdb_locking_iter.cc b/storage/rocksdb/rdb_locking_iter.cc index 5f5cc101e8a..bd2ff90fc83 100644 --- a/storage/rocksdb/rdb_locking_iter.cc +++ b/storage/rocksdb/rdb_locking_iter.cc @@ -11,8 +11,9 @@ namespace myrocks { rocksdb::Iterator* GetLockingIterator( rocksdb::Transaction *trx, const rocksdb::ReadOptions& read_options, - rocksdb::ColumnFamilyHandle* column_family) { - return new LockingIterator(trx, column_family, read_options); + rocksdb::ColumnFamilyHandle* column_family, + bool is_rev_cf) { + return new LockingIterator(trx, column_family, is_rev_cf, read_options); } /* diff --git a/storage/rocksdb/rdb_locking_iter.h b/storage/rocksdb/rdb_locking_iter.h index 632915d4b3d..e2373648d7e 100644 --- a/storage/rocksdb/rdb_locking_iter.h +++ b/storage/rocksdb/rdb_locking_iter.h @@ -34,6 +34,7 @@ class LockingIterator : public rocksdb::Iterator { rocksdb::Transaction *txn_; rocksdb::ColumnFamilyHandle* cfh_; + bool m_is_rev_cf; rocksdb::ReadOptions read_opts_; rocksdb::Iterator *iter_; rocksdb::Status status_; @@ -44,9 +45,10 @@ class LockingIterator : public rocksdb::Iterator { public: LockingIterator(rocksdb::Transaction *txn, rocksdb::ColumnFamilyHandle *cfh, + bool is_rev_cf, const rocksdb::ReadOptions& opts ) : - txn_(txn), cfh_(cfh), read_opts_(opts), iter_(nullptr), + txn_(txn), cfh_(cfh), m_is_rev_cf(is_rev_cf), read_opts_(opts), iter_(nullptr), status_(rocksdb::Status::InvalidArgument()), valid_(false) {} ~LockingIterator() { @@ -98,10 +100,11 @@ class LockingIterator : public rocksdb::Iterator { */ DEBUG_SYNC(current_thd, "rocksdb.locking_iter_scan"); auto end_key = iter_->key(); + bool endp_arg= m_is_rev_cf; if (forward) - status_ = txn_->GetRangeLock(cfh_, rocksdb::Endpoint(target), rocksdb::Endpoint(end_key)); + status_ = txn_->GetRangeLock(cfh_, rocksdb::Endpoint(target, endp_arg), rocksdb::Endpoint(end_key, endp_arg)); else - status_ = txn_->GetRangeLock(cfh_, rocksdb::Endpoint(end_key), rocksdb::Endpoint(target)); + status_ = txn_->GetRangeLock(cfh_, rocksdb::Endpoint(end_key, endp_arg), rocksdb::Endpoint(target, endp_arg)); if (!status_.ok()) { // Failed to get a lock (most likely lock wait timeout) @@ -169,6 +172,7 @@ class LockingIterator : public rocksdb::Iterator { rocksdb::Iterator* GetLockingIterator(rocksdb::Transaction *trx, const rocksdb::ReadOptions& read_options, - rocksdb::ColumnFamilyHandle* column_family); + rocksdb::ColumnFamilyHandle* column_family, + bool is_rev_cf); } // namespace myrocks