From eddb59de733bf72e2fa12a1bd249dc0aa514b050 Mon Sep 17 00:00:00 2001 From: Abhinav Sharma Date: Fri, 30 Oct 2020 10:27:20 -0700 Subject: [PATCH] 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 | 3 + mysql-test/suite/rocksdb/r/rocksdb.result | 1 + .../rocksdb/r/rocksdb_read_free_rpl.result | 6 +- ...sdb_skip_locks_if_skip_unique_check.result | 32 ++++++ ...cksdb_skip_locks_if_skip_unique_check.test | 48 +++++++++ ...ip_locks_if_skip_unique_check_basic.result | 100 ++++++++++++++++++ ...skip_locks_if_skip_unique_check_basic.test | 18 ++++ storage/rocksdb/ha_rocksdb.cc | 36 +++++-- storage/rocksdb/ha_rocksdb.h | 6 +- 9 files changed, 234 insertions(+), 16 deletions(-) create mode 100644 mysql-test/suite/rocksdb_rpl/r/rocksdb_skip_locks_if_skip_unique_check.result create mode 100644 mysql-test/suite/rocksdb_rpl/t/rocksdb_skip_locks_if_skip_unique_check.test create mode 100644 mysql-test/suite/rocksdb_sys_vars/r/rocksdb_skip_locks_if_skip_unique_check_basic.result create mode 100644 mysql-test/suite/rocksdb_sys_vars/t/rocksdb_skip_locks_if_skip_unique_check_basic.test diff --git a/mysql-test/r/mysqld--help-notwin.result b/mysql-test/r/mysqld--help-notwin.result index e1d7ee6b2fbb..a7cab41722b5 100644 --- a/mysql-test/r/mysqld--help-notwin.result +++ b/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 diff --git a/mysql-test/suite/rocksdb/r/rocksdb.result b/mysql-test/suite/rocksdb/r/rocksdb.result index 6ac410273916..0a4087737e91 100644 --- a/mysql-test/suite/rocksdb/r/rocksdb.result +++ b/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 diff --git a/mysql-test/suite/rocksdb/r/rocksdb_read_free_rpl.result b/mysql-test/suite/rocksdb/r/rocksdb_read_free_rpl.result index 7f4f5ddcd248..58329d03ebc9 100644 --- a/mysql-test/suite/rocksdb/r/rocksdb_read_free_rpl.result +++ b/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 diff --git a/mysql-test/suite/rocksdb_rpl/r/rocksdb_skip_locks_if_skip_unique_check.result b/mysql-test/suite/rocksdb_rpl/r/rocksdb_skip_locks_if_skip_unique_check.result new file mode 100644 index 000000000000..00fc402ba672 --- /dev/null +++ b/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 diff --git a/mysql-test/suite/rocksdb_rpl/t/rocksdb_skip_locks_if_skip_unique_check.test b/mysql-test/suite/rocksdb_rpl/t/rocksdb_skip_locks_if_skip_unique_check.test new file mode 100644 index 000000000000..c37a62cfa78b --- /dev/null +++ b/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; diff --git a/mysql-test/suite/rocksdb_sys_vars/r/rocksdb_skip_locks_if_skip_unique_check_basic.result b/mysql-test/suite/rocksdb_sys_vars/r/rocksdb_skip_locks_if_skip_unique_check_basic.result new file mode 100644 index 000000000000..96b78cf669eb --- /dev/null +++ b/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; diff --git a/mysql-test/suite/rocksdb_sys_vars/t/rocksdb_skip_locks_if_skip_unique_check_basic.test b/mysql-test/suite/rocksdb_sys_vars/t/rocksdb_skip_locks_if_skip_unique_check_basic.test new file mode 100644 index 000000000000..641a3871fd57 --- /dev/null +++ b/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; diff --git a/storage/rocksdb/ha_rocksdb.cc b/storage/rocksdb/ha_rocksdb.cc index 1dd73d957b2b..70bf87f4c3b9 100644 --- a/storage/rocksdb/ha_rocksdb.cc +++ b/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 rocksdb_row_lock_deadlocks(0); std::atomic 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); } diff --git a/storage/rocksdb/ha_rocksdb.h b/storage/rocksdb/ha_rocksdb.h index dfd2cffc6d1c..f91ff44f925b 100644 --- a/storage/rocksdb/ha_rocksdb.h +++ b/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__));