Taking row locks with unique_checks = 0

Summary: We should take row locks even with unique checks are disabled.

Differential Revision: D24745386

mysql-test/r/mysqld--help-notwin.result

@@ -2003,6 +2003,8 @@ The following options may be given as the first argument:
  Skip using bloom filter for reads
  --rocksdb-skip-fill-cache 
  Skip filling block cache on read requests
+ --rocksdb-skip-locks-if-skip-unique-check 
+ Skip row locking when unique checks are disabled.
  --rocksdb-sst-mgr-rate-bytes-per-sec=# 
  DBOptions::sst_file_manager rate_bytes_per_sec for
  RocksDB
@@ -3075,6 +3077,7 @@ rocksdb-signal-drop-index-thread FALSE
 rocksdb-sim-cache-size 0
 rocksdb-skip-bloom-filter-on-read FALSE
 rocksdb-skip-fill-cache FALSE
+rocksdb-skip-locks-if-skip-unique-check FALSE
 rocksdb-sst-mgr-rate-bytes-per-sec 0
 rocksdb-sst-props ON
 rocksdb-stats-dump-period-sec 600

mysql-test/suite/rocksdb/r/rocksdb.result

@@ -1016,6 +1016,7 @@ rocksdb_signal_drop_index_thread	OFF
 rocksdb_sim_cache_size	0
 rocksdb_skip_bloom_filter_on_read	OFF
 rocksdb_skip_fill_cache	OFF
+rocksdb_skip_locks_if_skip_unique_check	OFF
 rocksdb_sst_mgr_rate_bytes_per_sec	0
 rocksdb_stats_dump_period_sec	600
 rocksdb_stats_level	1

mysql-test/suite/rocksdb/r/rocksdb_read_free_rpl.result

@@ -133,7 +133,7 @@ include/sync_slave_sql_with_master.inc
 [connection slave]
 select case when variable_value-@up > 0 then 'false' else 'true' end as read_free from performance_schema.global_status where variable_name='rocksdb_num_get_for_update_calls';
 read_free
-true
+false
 [connection master]
 include/diff_tables.inc [master:t1, slave:t1]
 
@@ -163,7 +163,7 @@ include/sync_slave_sql_with_master.inc
 [connection slave]
 select case when variable_value-@up > 0 then 'false' else 'true' end as read_free from performance_schema.global_status where variable_name='rocksdb_num_get_for_update_calls';
 read_free
-true
+false
 [connection master]
 include/diff_tables.inc [master:t1, slave:t1]
 
@@ -407,7 +407,7 @@ include/sync_slave_sql_with_master.inc
 [connection slave]
 select case when variable_value-@up > 0 then 'false' else 'true' end as read_free from performance_schema.global_status where variable_name='rocksdb_num_get_for_update_calls';
 read_free
-true
+false
 select * from t2;
 id	i1	i2
 1	1	1

mysql-test/suite/rocksdb_rpl/r/rocksdb_skip_locks_if_skip_unique_check.result

@@ -0,0 +1,32 @@
+include/master-slave.inc
+Warnings:
+Note	####	Sending passwords in plain text without SSL/TLS is extremely insecure.
+Note	####	Storing MySQL user name or password information in the master info repository is not secure and is therefore not recommended. Please consider using the USER and PASSWORD connection options for START SLAVE; see the 'START SLAVE Syntax' in the MySQL Manual for more information.
+[connection master]
+create table t1 (a int primary key, b int) engine = rocksdb;
+set @@unique_checks = 0;
+insert into t1 values(1, 1);
+insert into t1 values(2, 2);
+include/sync_slave_sql_with_master.inc
+begin;
+update t1 set b = 20 where a = 2;
+set @@unique_checks = 0;
+insert into t1 values(2, 200);
+rollback;
+set @@global.rocksdb_skip_locks_if_skip_unique_check = 1;
+stop replica;
+start replica;
+begin;
+update t1 set b = 10 where a = 1;
+set @@unique_checks = 0;
+insert into t1 values(1, 100);
+include/sync_slave_sql_with_master.inc
+rollback;
+select * from t1;
+a	b
+1	100
+2	200
+set @@global.rocksdb_skip_locks_if_skip_unique_check = 0;
+drop table t1;
+include/sync_slave_sql_with_master.inc
+include/rpl_end.inc

mysql-test/suite/rocksdb_rpl/t/rocksdb_skip_locks_if_skip_unique_check.test

@@ -0,0 +1,48 @@
+source include/master-slave.inc;
+source include/have_rocksdb.inc;
+
+connection master;
+create table t1 (a int primary key, b int) engine = rocksdb;
+set @@unique_checks = 0;
+insert into t1 values(1, 1);
+insert into t1 values(2, 2);
+source include/sync_slave_sql_with_master.inc;
+
+connection slave;
+begin;
+update t1 set b = 20 where a = 2;
+
+connection master;
+set @@unique_checks = 0;
+insert into t1 values(2, 200);
+
+connection slave;
+let $wait_condition=
+    select count(*)= 1 from information_schema.processlist
+      where state = 'Waiting for row lock';
+source include/wait_condition.inc;
+rollback;
+
+
+# Now let's check if locks are not taken when # rocksdb_skip_locks_if_skip_unique_check is enabled
+connection slave;
+set @@global.rocksdb_skip_locks_if_skip_unique_check = 1;
+stop replica; start replica;
+begin;
+update t1 set b = 10 where a = 1;
+
+connection master;
+set @@unique_checks = 0;
+insert into t1 values(1, 100);
+source include/sync_slave_sql_with_master.inc;
+
+connection slave;
+rollback;
+select * from t1;
+set @@global.rocksdb_skip_locks_if_skip_unique_check = 0;
+
+connection master;
+drop table t1;
+source include/sync_slave_sql_with_master.inc;
+
+source include/rpl_end.inc;

mysql-test/suite/rocksdb_sys_vars/r/rocksdb_skip_locks_if_skip_unique_check_basic.result

@@ -0,0 +1,100 @@
+CREATE TABLE valid_values (value varchar(255)) ENGINE=myisam;
+INSERT INTO valid_values VALUES(1);
+INSERT INTO valid_values VALUES(0);
+INSERT INTO valid_values VALUES('on');
+CREATE TABLE invalid_values (value varchar(255)) ENGINE=myisam;
+INSERT INTO invalid_values VALUES('\'aaa\'');
+INSERT INTO invalid_values VALUES('\'bbb\'');
+SET @start_global_value = @@global.ROCKSDB_BULK_LOAD;
+SELECT @start_global_value;
+@start_global_value
+0
+SET @start_session_value = @@session.ROCKSDB_BULK_LOAD;
+SELECT @start_session_value;
+@start_session_value
+0
+'# Setting to valid values in global scope#'
+"Trying to set variable @@global.ROCKSDB_BULK_LOAD to 1"
+SET @@global.ROCKSDB_BULK_LOAD   = 1;
+SELECT @@global.ROCKSDB_BULK_LOAD;
+@@global.ROCKSDB_BULK_LOAD
+1
+"Setting the global scope variable back to default"
+SET @@global.ROCKSDB_BULK_LOAD = DEFAULT;
+SELECT @@global.ROCKSDB_BULK_LOAD;
+@@global.ROCKSDB_BULK_LOAD
+0
+"Trying to set variable @@global.ROCKSDB_BULK_LOAD to 0"
+SET @@global.ROCKSDB_BULK_LOAD   = 0;
+SELECT @@global.ROCKSDB_BULK_LOAD;
+@@global.ROCKSDB_BULK_LOAD
+0
+"Setting the global scope variable back to default"
+SET @@global.ROCKSDB_BULK_LOAD = DEFAULT;
+SELECT @@global.ROCKSDB_BULK_LOAD;
+@@global.ROCKSDB_BULK_LOAD
+0
+"Trying to set variable @@global.ROCKSDB_BULK_LOAD to on"
+SET @@global.ROCKSDB_BULK_LOAD   = on;
+SELECT @@global.ROCKSDB_BULK_LOAD;
+@@global.ROCKSDB_BULK_LOAD
+1
+"Setting the global scope variable back to default"
+SET @@global.ROCKSDB_BULK_LOAD = DEFAULT;
+SELECT @@global.ROCKSDB_BULK_LOAD;
+@@global.ROCKSDB_BULK_LOAD
+0
+'# Setting to valid values in session scope#'
+"Trying to set variable @@session.ROCKSDB_BULK_LOAD to 1"
+SET @@session.ROCKSDB_BULK_LOAD   = 1;
+SELECT @@session.ROCKSDB_BULK_LOAD;
+@@session.ROCKSDB_BULK_LOAD
+1
+"Setting the session scope variable back to default"
+SET @@session.ROCKSDB_BULK_LOAD = DEFAULT;
+SELECT @@session.ROCKSDB_BULK_LOAD;
+@@session.ROCKSDB_BULK_LOAD
+0
+"Trying to set variable @@session.ROCKSDB_BULK_LOAD to 0"
+SET @@session.ROCKSDB_BULK_LOAD   = 0;
+SELECT @@session.ROCKSDB_BULK_LOAD;
+@@session.ROCKSDB_BULK_LOAD
+0
+"Setting the session scope variable back to default"
+SET @@session.ROCKSDB_BULK_LOAD = DEFAULT;
+SELECT @@session.ROCKSDB_BULK_LOAD;
+@@session.ROCKSDB_BULK_LOAD
+0
+"Trying to set variable @@session.ROCKSDB_BULK_LOAD to on"
+SET @@session.ROCKSDB_BULK_LOAD   = on;
+SELECT @@session.ROCKSDB_BULK_LOAD;
+@@session.ROCKSDB_BULK_LOAD
+1
+"Setting the session scope variable back to default"
+SET @@session.ROCKSDB_BULK_LOAD = DEFAULT;
+SELECT @@session.ROCKSDB_BULK_LOAD;
+@@session.ROCKSDB_BULK_LOAD
+0
+'# Testing with invalid values in global scope #'
+"Trying to set variable @@global.ROCKSDB_BULK_LOAD to 'aaa'"
+SET @@global.ROCKSDB_BULK_LOAD   = 'aaa';
+Got one of the listed errors
+SELECT @@global.ROCKSDB_BULK_LOAD;
+@@global.ROCKSDB_BULK_LOAD
+0
+"Trying to set variable @@global.ROCKSDB_BULK_LOAD to 'bbb'"
+SET @@global.ROCKSDB_BULK_LOAD   = 'bbb';
+Got one of the listed errors
+SELECT @@global.ROCKSDB_BULK_LOAD;
+@@global.ROCKSDB_BULK_LOAD
+0
+SET @@global.ROCKSDB_BULK_LOAD = @start_global_value;
+SELECT @@global.ROCKSDB_BULK_LOAD;
+@@global.ROCKSDB_BULK_LOAD
+0
+SET @@session.ROCKSDB_BULK_LOAD = @start_session_value;
+SELECT @@session.ROCKSDB_BULK_LOAD;
+@@session.ROCKSDB_BULK_LOAD
+0
+DROP TABLE valid_values;
+DROP TABLE invalid_values;

mysql-test/suite/rocksdb_sys_vars/t/rocksdb_skip_locks_if_skip_unique_check_basic.test

@@ -0,0 +1,18 @@
+--source include/have_rocksdb.inc
+
+CREATE TABLE valid_values (value varchar(255)) ENGINE=myisam;
+INSERT INTO valid_values VALUES(1);
+INSERT INTO valid_values VALUES(0);
+INSERT INTO valid_values VALUES('on');
+
+CREATE TABLE invalid_values (value varchar(255)) ENGINE=myisam;
+INSERT INTO invalid_values VALUES('\'aaa\'');
+INSERT INTO invalid_values VALUES('\'bbb\'');
+
+--let $sys_var=ROCKSDB_BULK_LOAD
+--let $read_only=0
+--let $session=1
+--source ../include/rocksdb_sys_var.inc
+
+DROP TABLE valid_values;
+DROP TABLE invalid_values;

storage/rocksdb/ha_rocksdb.cc

@@ -781,6 +781,7 @@ static uint32_t rocksdb_select_bypass_rejected_query_history_size = 0;
 static uint32_t rocksdb_select_bypass_debug_row_delay = 0;
 static unsigned long long  // NOLINT(runtime/int)
     rocksdb_select_bypass_multiget_min = 0;
+static bool rocksdb_skip_locks_if_skip_unique_check = false;
 
 std::atomic<uint64_t> rocksdb_row_lock_deadlocks(0);
 std::atomic<uint64_t> rocksdb_row_lock_wait_timeouts(0);
@@ -2360,6 +2361,12 @@ static MYSQL_THDVAR_ULONG(mrr_batch_size, PLUGIN_VAR_RQCMDARG,
                           nullptr, nullptr, /* default */ 100, /* min */ 0,
                           /* max */ ROCKSDB_MAX_MRR_BATCH_SIZE, 0);
 
+static MYSQL_SYSVAR_BOOL(skip_locks_if_skip_unique_check,
+                         rocksdb_skip_locks_if_skip_unique_check,
+                         PLUGIN_VAR_RQCMDARG,
+                         "Skip row locking when unique checks are disabled.",
+                         nullptr, nullptr, false);
+
 static const int ROCKSDB_ASSUMED_KEY_VALUE_DISK_SIZE = 100;
 
 static struct SYS_VAR *rocksdb_system_variables[] = {
@@ -2545,6 +2552,7 @@ static struct SYS_VAR *rocksdb_system_variables[] = {
     MYSQL_SYSVAR(select_bypass_allow_filters),
     MYSQL_SYSVAR(select_bypass_debug_row_delay),
     MYSQL_SYSVAR(select_bypass_multiget_min),
+    MYSQL_SYSVAR(skip_locks_if_skip_unique_check),
     nullptr};
 
 static int rocksdb_compact_column_family(
@@ -10342,14 +10350,16 @@ int ha_rocksdb::get_pk_for_update(struct update_row_info *const row_info) {
 */
 int ha_rocksdb::check_and_lock_unique_pk(const uint key_id,
                                          const struct update_row_info &row_info,
-                                         bool *const found) {
+                                         bool *const found,
+                                         const bool skip_unique_check) {
   assert(found != nullptr);
 
   assert(row_info.old_pk_slice.size() == 0 ||
          row_info.new_pk_slice.compare(row_info.old_pk_slice) != 0);
 
   /* Ignore PK violations if this is a optimized 'replace into' */
-  const bool ignore_pk_unique_check = ha_thd()->lex->blind_replace_into;
+  const bool ignore_pk_unique_check =
+      ha_thd()->lex->blind_replace_into || skip_unique_check;
 
   /*
     Perform a read to determine if a duplicate entry exists. For primary
@@ -10421,9 +10431,10 @@ int ha_rocksdb::check_and_lock_unique_pk(const uint key_id,
     HA_EXIT_SUCCESS  OK
     other            HA_ERR error code (can be SE-specific)
 */
-int ha_rocksdb::check_and_lock_sk(const uint key_id,
-                                  const struct update_row_info &row_info,
-                                  bool *const found) {
+int ha_rocksdb::check_and_lock_sk(
+    const uint key_id, const struct update_row_info &row_info,
+    bool *const found,
+    const bool skip_unique_check MY_ATTRIBUTE((__unused__))) {
   assert(found != nullptr);
   *found = false;
 
@@ -10581,7 +10592,8 @@ int ha_rocksdb::check_and_lock_sk(const uint key_id,
     other            HA_ERR error code (can be SE-specific)
 */
 int ha_rocksdb::check_uniqueness_and_lock(
-    const struct update_row_info &row_info, bool pk_changed) {
+    const struct update_row_info &row_info, bool pk_changed,
+    bool skip_unique_check) {
   /*
     Go through each index and determine if the index has uniqueness
     requirements. If it does, then try to obtain a row lock on the new values.
@@ -10597,11 +10609,12 @@ int ha_rocksdb::check_uniqueness_and_lock(
         found = false;
         rc = HA_EXIT_SUCCESS;
       } else {
-        rc = check_and_lock_unique_pk(key_id, row_info, &found);
+        rc = check_and_lock_unique_pk(key_id, row_info, &found,
+                                      skip_unique_check);
         DEBUG_SYNC(ha_thd(), "rocksdb.after_unique_pk_check");
       }
     } else {
-      rc = check_and_lock_sk(key_id, row_info, &found);
+      rc = check_and_lock_sk(key_id, row_info, &found, skip_unique_check);
       DEBUG_SYNC(ha_thd(), "rocksdb.after_unique_sk_check");
     }
 
@@ -11038,12 +11051,15 @@ int ha_rocksdb::update_write_row(const uchar *const old_data,
     pk_changed = row_info.new_pk_slice.compare(row_info.old_pk_slice) != 0;
   }
 
-  if (!skip_unique_check) {
+  // Case: We skip both unique checks and rows locks only when bulk load is
+  // enabled or if rocksdb_skip_locks_if_skip_unique_check is ON
+  if (!THDVAR(table->in_use, bulk_load) &&
+      (!rocksdb_skip_locks_if_skip_unique_check || !skip_unique_check)) {
     /*
       Check to see if we are going to have failures because of unique
       keys.  Also lock the appropriate key values.
     */
-    rc = check_uniqueness_and_lock(row_info, pk_changed);
+    rc = check_uniqueness_and_lock(row_info, pk_changed, skip_unique_check);
     if (rc != HA_EXIT_SUCCESS) {
       DBUG_RETURN(rc);
     }

storage/rocksdb/ha_rocksdb.h

@@ -813,14 +813,14 @@ class ha_rocksdb : public my_core::handler {
   int get_pk_for_update(struct update_row_info *const row_info);
   int check_and_lock_unique_pk(const uint key_id,
                                const struct update_row_info &row_info,
-                               bool *const found)
+                               bool *const found, const bool skip_unique_check)
       MY_ATTRIBUTE((__warn_unused_result__));
   int check_and_lock_sk(const uint key_id,
                         const struct update_row_info &row_info,
-                        bool *const found)
+                        bool *const found, const bool skip_unique_check)
       MY_ATTRIBUTE((__warn_unused_result__));
   int check_uniqueness_and_lock(const struct update_row_info &row_info,
-                                bool pk_changed)
+                                bool pk_changed, const bool skip_unique_check)
       MY_ATTRIBUTE((__warn_unused_result__));
   bool over_bulk_load_threshold(int *err)
       MY_ATTRIBUTE((__warn_unused_result__));