[Commits] 85e37c7ae31: Fix rocksdb.unique_seq_rev_cf test

psergey sergey at mariadb.com
Thu Nov 28 16:50:20 EET 2019


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


More information about the commits mailing list