Improve locking iterator performance

Issue: locking iterator is several times slower than the default
iterator in queries based on secondary keys.

For example:

select COUNT(*) from sbtest1 where k % 10 = 1 for update;

This query ends up iterating over the entire table, and in the end locks
the entire table, but does so by lots of small locks and seeks every
time.

This diff shows that by simply locking the entire table instead we can
improve the performance of the iterator by 50% - it's still slower than
the default implementation, but now it's much closer to it.

This modification of course results in overlocking in queries such as

select COUNT(*) from sbtest1 where k % 10 = 1 for update limit 3000;

where instead of locking only a few thousands records, with this change
we lock the entire table instead.

This change is in this form a proof of concept, for a real patch I
think we could go in two directions:

* instead of a hard coded value, make this an adjustable session
  variable
* or instead of simply stopping after a threshold, continue locking in
  linear or exponential batches

Author: dutow

storage/rocksdb/rdb_locking_iter.cc

@@ -8,6 +8,8 @@
 /* This C++ file's header file */
 #include "./rdb_locking_iter.h"
 
+static const ulonglong max_lock_count = 1000;
+
 namespace myrocks {
 
 rocksdb::Iterator* GetLockingIterator(
@@ -119,11 +121,12 @@ void LockingIterator::lock_up_to(bool scan_forward,
     m_have_locked_until= true;
     m_locked_until.assign(end_key.data(), end_key.size());
     if (m_lock_count)  (*m_lock_count)++;
+    m_self_lock_count++;
   }
 }
 
 /*
-  Lock the range from target till the iterator end point that we are scaning
+  Lock the range from target till the iterator end point that we are scanning
   towards. If there's no iterator bound, use index start (or end, depending
   on the scan direction)
 */
@@ -141,7 +144,7 @@ void LockingIterator::lock_till_iterator_end(bool scan_forward,
       else
         m_kd->get_supremum_key(buf, &size);
 
-      DBUG_ASSERT(size == Rdb_key_def::INDEX_NUMBER_SIZE);
+      assert(size == Rdb_key_def::INDEX_NUMBER_SIZE);
       end = rocksdb::Slice((const char*)buf, size);
     }
   } else {
@@ -153,7 +156,7 @@ void LockingIterator::lock_till_iterator_end(bool scan_forward,
       else
         m_kd->get_infimum_key(buf, &size);
 
-      DBUG_ASSERT(size == Rdb_key_def::INDEX_NUMBER_SIZE);
+      assert(size == Rdb_key_def::INDEX_NUMBER_SIZE);
       end = rocksdb::Slice((const char*)buf, size);
     }
   }
@@ -171,6 +174,8 @@ void LockingIterator::lock_till_iterator_end(bool scan_forward,
 */
 void LockingIterator::Scan(bool scan_forward,
                            const rocksdb::Slice& target, bool call_next) {
+
+
   if (!m_iter->Valid()) {
     m_status = m_iter->status();
     m_valid = false;
@@ -180,6 +185,10 @@ void LockingIterator::Scan(bool scan_forward,
     }
     return;
   }
+  if (m_self_lock_count > max_lock_count) {
+    // already locked the entire index instead
+    return;
+  }
 
   while (1) {
 
@@ -197,11 +206,19 @@ void LockingIterator::Scan(bool scan_forward,
     auto end_key = m_iter->key();
     std::string end_key_copy= end_key.ToString();
 
-    lock_up_to(scan_forward, target, end_key);
-    if (!m_status.ok()) {
-      // Failed to get a lock (most likely lock wait timeout)
-      m_valid = false;
-      return;
+    if (m_self_lock_count == max_lock_count) 
+    {
+      // we can't handle too many small locks, just lock everything
+      lock_till_iterator_end(scan_forward, target);
+    } 
+    if (m_self_lock_count < max_lock_count) 
+    {
+      lock_up_to(scan_forward, target, end_key);
+      if (!m_status.ok()) {
+        // Failed to get a lock (most likely lock wait timeout)
+        m_valid = false;
+        return;
+      }
     }
 
     //Ok, now we have a lock which is inhibiting modifications in the range
@@ -265,7 +282,7 @@ void LockingIterator::Scan(bool scan_forward,
 */
 
 void LockingIterator::SeekToFirst() {
-  DBUG_ASSERT(0);
+  assert(0);
   m_status = rocksdb::Status::NotSupported("Not implemented");
   m_valid = false;
 }
@@ -276,7 +293,7 @@ void LockingIterator::SeekToFirst() {
 */
 
 void LockingIterator::SeekToLast() {
-  DBUG_ASSERT(0);
+  assert(0);
   m_status = rocksdb::Status::NotSupported("Not implemented");
   m_valid = false;
 }

storage/rocksdb/rdb_locking_iter.h

@@ -42,6 +42,7 @@ class LockingIterator : public rocksdb::Iterator {
   bool  m_valid;
 
   ulonglong *m_lock_count;
+  ulonglong m_self_lock_count = 0;
 
   // If true, m_locked_until has a valid key value.
   bool m_have_locked_until;