Skip to content

Commit

Permalink
PS-4811 : 5.7 Merge and fixup for old DB-937 introduces possible regr…
Browse files Browse the repository at this point in the history
…ession

- Reverted refactoring and most of DB-937 as well as entire functionality of
  former feature pk_insert_mode.  DB-937 rendered the optimization ineffective
  by forcing the FT layer to do a unique check anyway when setting put flags to
  DB_NOOVERWRITE.  So, simply removing the feature entirely makes more sense to
  ensure index consistency, keep the code simpler and cleaner, and to allow the
  engine to adhere to the server requirement to properly return
  HA_ERR_FOUND_DUPP_KEY on REPLACE INTO/INSERT IGNORE when a record already
  exists.  This is mandatory for 8.0 and replication so to keep the trees
  aligned and maintain some sanity and consistency, we went this way rather than
  trying to keep the feature limping along, particularly now that we have the
  'fast insert/upsert' feature where a user can now use this explicitly.
  • Loading branch information
George O. Lorch III committed Sep 10, 2018
1 parent 4b2a4a4 commit 23c0a72
Show file tree
Hide file tree
Showing 11 changed files with 41 additions and 156 deletions.
4 changes: 1 addition & 3 deletions mysql-test/suite/tokudb.rpl/r/rpl_mixed_replace_into.result
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@ 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]
set default_storage_engine='tokudb';
drop table if exists testr;
CREATE TABLE testr (pk int(11) NOT NULL AUTO_INCREMENT, num int(11) DEFAULT NULL, txt varchar(32) DEFAULT NULL, PRIMARY KEY (pk) );
CREATE TABLE testr (pk int(11) NOT NULL AUTO_INCREMENT, num int(11) DEFAULT NULL, txt varchar(32) DEFAULT NULL, PRIMARY KEY (pk)) ENGINE=TOKUDB;
INSERT INTO testr VALUES (1,1,'one'),(2,2,'two'),(3,3,'three'),(4,4,'four');
replace into testr values (2,2,'twotwo');
select * from testr;
Expand Down
4 changes: 1 addition & 3 deletions mysql-test/suite/tokudb.rpl/r/rpl_row_replace_into.result
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@ 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]
set default_storage_engine='tokudb';
drop table if exists testr;
CREATE TABLE testr (pk int(11) NOT NULL AUTO_INCREMENT, num int(11) DEFAULT NULL, txt varchar(32) DEFAULT NULL, PRIMARY KEY (pk) );
CREATE TABLE testr (pk int(11) NOT NULL AUTO_INCREMENT, num int(11) DEFAULT NULL, txt varchar(32) DEFAULT NULL, PRIMARY KEY (pk)) ENGINE=TOKUDB;
INSERT INTO testr VALUES (1,1,'one'),(2,2,'two'),(3,3,'three'),(4,4,'four');
replace into testr values (2,2,'twotwo');
select * from testr;
Expand Down
4 changes: 1 addition & 3 deletions mysql-test/suite/tokudb.rpl/r/rpl_stmt_replace_into.result
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,7 @@ 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]
set default_storage_engine='tokudb';
drop table if exists testr;
CREATE TABLE testr (pk int(11) NOT NULL AUTO_INCREMENT, num int(11) DEFAULT NULL, txt varchar(32) DEFAULT NULL, PRIMARY KEY (pk) );
CREATE TABLE testr (pk int(11) NOT NULL AUTO_INCREMENT, num int(11) DEFAULT NULL, txt varchar(32) DEFAULT NULL, PRIMARY KEY (pk)) ENGINE=TOKUDB;
INSERT INTO testr VALUES (1,1,'one'),(2,2,'two'),(3,3,'three'),(4,4,'four');
replace into testr values (2,2,'twotwo');
select * from testr;
Expand Down
8 changes: 1 addition & 7 deletions mysql-test/suite/tokudb.rpl/t/rpl_mixed_replace_into.test
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,7 @@ source include/have_tokudb.inc;
source include/master-slave.inc;
source include/have_binlog_format_mixed.inc;

set default_storage_engine='tokudb';

disable_warnings;
drop table if exists testr;
enable_warnings;

CREATE TABLE testr (pk int(11) NOT NULL AUTO_INCREMENT, num int(11) DEFAULT NULL, txt varchar(32) DEFAULT NULL, PRIMARY KEY (pk) );
CREATE TABLE testr (pk int(11) NOT NULL AUTO_INCREMENT, num int(11) DEFAULT NULL, txt varchar(32) DEFAULT NULL, PRIMARY KEY (pk)) ENGINE=TOKUDB;
INSERT INTO testr VALUES (1,1,'one'),(2,2,'two'),(3,3,'three'),(4,4,'four');
replace into testr values (2,2,'twotwo');
select * from testr;
Expand Down
8 changes: 1 addition & 7 deletions mysql-test/suite/tokudb.rpl/t/rpl_row_replace_into.test
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,7 @@ source include/have_tokudb.inc;
source include/master-slave.inc;
source include/have_binlog_format_row.inc;

set default_storage_engine='tokudb';

disable_warnings;
drop table if exists testr;
enable_warnings;

CREATE TABLE testr (pk int(11) NOT NULL AUTO_INCREMENT, num int(11) DEFAULT NULL, txt varchar(32) DEFAULT NULL, PRIMARY KEY (pk) );
CREATE TABLE testr (pk int(11) NOT NULL AUTO_INCREMENT, num int(11) DEFAULT NULL, txt varchar(32) DEFAULT NULL, PRIMARY KEY (pk)) ENGINE=TOKUDB;
INSERT INTO testr VALUES (1,1,'one'),(2,2,'two'),(3,3,'three'),(4,4,'four');
replace into testr values (2,2,'twotwo');
select * from testr;
Expand Down
8 changes: 1 addition & 7 deletions mysql-test/suite/tokudb.rpl/t/rpl_stmt_replace_into.test
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,7 @@ source include/have_tokudb.inc;
source include/master-slave.inc;
source include/have_binlog_format_statement.inc;

set default_storage_engine='tokudb';

disable_warnings;
drop table if exists testr;
enable_warnings;

CREATE TABLE testr (pk int(11) NOT NULL AUTO_INCREMENT, num int(11) DEFAULT NULL, txt varchar(32) DEFAULT NULL, PRIMARY KEY (pk) );
CREATE TABLE testr (pk int(11) NOT NULL AUTO_INCREMENT, num int(11) DEFAULT NULL, txt varchar(32) DEFAULT NULL, PRIMARY KEY (pk)) ENGINE=TOKUDB;
INSERT INTO testr VALUES (1,1,'one'),(2,2,'two'),(3,3,'three'),(4,4,'four');
replace into testr values (2,2,'twotwo');
select * from testr;
Expand Down
10 changes: 6 additions & 4 deletions mysql-test/suite/tokudb/r/i_s_tokudb_lock_waits_released.result
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
set default_storage_engine='tokudb';
set tokudb_prelock_empty=false;
drop table if exists t;
create table t (id int primary key);
create table t (id int primary key) engine=tokudb;
t should be empty
select trx_id,trx_mysql_thread_id from information_schema.tokudb_trx;
trx_id trx_mysql_thread_id
Expand Down Expand Up @@ -53,10 +51,12 @@ replace into t values (1);
set autocommit=0;
set tokudb_lock_timeout=600000;
replace into t values (1);
should find the presence of a lock on 1st transaction
should find the presence of lock(s) on 1st transactioni
select * from information_schema.tokudb_locks;
locks_trx_id locks_mysql_thread_id locks_dname locks_key_left locks_key_right locks_table_schema locks_table_name locks_table_dictionary_name
TRX_ID MYSQL_ID ./test/t-main 0001000000 0001000000 test t main
TRX_ID MYSQL_ID ./test/t-main ff01000000 0001000000 test t main
TRX_ID MYSQL_ID ./test/t-main 0001000000 0001000000 test t main
should find the presence of a lock_wait on the 2nd transaction
select * from information_schema.tokudb_lock_waits;
requesting_trx_id blocking_trx_id lock_waits_dname lock_waits_key_left lock_waits_key_right lock_waits_start_time lock_waits_table_schema lock_waits_table_name lock_waits_table_dictionary_name
Expand All @@ -71,6 +71,8 @@ verify that the lock on the 1st transaction is released and replaced by the lock
select * from information_schema.tokudb_locks;
locks_trx_id locks_mysql_thread_id locks_dname locks_key_left locks_key_right locks_table_schema locks_table_name locks_table_dictionary_name
TRX_ID MYSQL_ID ./test/t-main 0001000000 0001000000 test t main
TRX_ID MYSQL_ID ./test/t-main ff01000000 0001000000 test t main
TRX_ID MYSQL_ID ./test/t-main 0001000000 0001000000 test t main
select * from information_schema.tokudb_lock_waits;
requesting_trx_id blocking_trx_id lock_waits_dname lock_waits_key_left lock_waits_key_right lock_waits_start_time lock_waits_table_schema lock_waits_table_name lock_waits_table_dictionary_name
commit;
Expand Down
8 changes: 4 additions & 4 deletions mysql-test/suite/tokudb/r/replace-ignore.result
Original file line number Diff line number Diff line change
Expand Up @@ -327,14 +327,14 @@ a b c
8 8 -1
explain select * from t where c = 80;
id select_type table partitions type possible_keys key key_len ref rows filtered Extra
1 SIMPLE t NULL ALL NULL NULL NULL NULL 52 10.00 Using where
1 SIMPLE t NULL ALL NULL NULL NULL NULL 51 10.00 Using where
Warnings:
Note 1003 /* select#1 */ select `test`.`t`.`a` AS `a`,`test`.`t`.`b` AS `b`,`test`.`t`.`c` AS `c` from `test`.`t` where (`test`.`t`.`c` = 80)
select * from t where c = 80;
a b c
explain select * from t where c = -1;
id select_type table partitions type possible_keys key key_len ref rows filtered Extra
1 SIMPLE t NULL ALL NULL NULL NULL NULL 52 10.00 Using where
1 SIMPLE t NULL ALL NULL NULL NULL NULL 51 10.00 Using where
Warnings:
Note 1003 /* select#1 */ select `test`.`t`.`a` AS `a`,`test`.`t`.`b` AS `b`,`test`.`t`.`c` AS `c` from `test`.`t` where (`test`.`t`.`c` = <cache>(-(1)))
select * from t where c = -1;
Expand All @@ -359,14 +359,14 @@ a b c
15 15 -1
explain select * from t where c = 150;
id select_type table partitions type possible_keys key key_len ref rows filtered Extra
1 SIMPLE t NULL ALL NULL NULL NULL NULL 53 10.00 Using where
1 SIMPLE t NULL ALL NULL NULL NULL NULL 51 10.00 Using where
Warnings:
Note 1003 /* select#1 */ select `test`.`t`.`a` AS `a`,`test`.`t`.`b` AS `b`,`test`.`t`.`c` AS `c` from `test`.`t` where (`test`.`t`.`c` = 150)
select * from t where c = 150;
a b c
explain select * from t where c = -1;
id select_type table partitions type possible_keys key key_len ref rows filtered Extra
1 SIMPLE t NULL ALL NULL NULL NULL NULL 53 10.00 Using where
1 SIMPLE t NULL ALL NULL NULL NULL NULL 51 10.00 Using where
Warnings:
Note 1003 /* select#1 */ select `test`.`t`.`a` AS `a`,`test`.`t`.`b` AS `b`,`test`.`t`.`c` AS `c` from `test`.`t` where (`test`.`t`.`c` = <cache>(-(1)))
select * from t where c = -1;
Expand Down
19 changes: 10 additions & 9 deletions mysql-test/suite/tokudb/t/i_s_tokudb_lock_waits_released.test
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,9 @@
# populated with 1 lock_wait and all transactions are present in information_schema.tokudb_trx for 2 clients

source include/have_tokudb.inc;
set default_storage_engine='tokudb';
set tokudb_prelock_empty=false;
disable_warnings;
drop table if exists t;
enable_warnings;

create table t (id int primary key);
create table t (id int primary key) engine=tokudb;

# verify that txn_a insert (1) blocks txn_b insert (1) and txn_b gets a duplicate key error

Expand Down Expand Up @@ -56,7 +52,7 @@ select * from information_schema.tokudb_locks;
select * from information_schema.tokudb_lock_waits;

connection conn_b;
--error 1062 # duplicate key
--error ER_DUP_ENTRY
reap;
commit;

Expand All @@ -81,13 +77,18 @@ set autocommit=0;
set tokudb_lock_timeout=600000; # set lock wait timeout to 10 minutes
send replace into t values (1);

--echo should find the presence of a lock on 1st transaction
# There will actually be three locks held by the same transaction
# but it is expected as there is a sub-transaction involved now and
# sub-transactions inherit lockability from their parent but still maintain
# their own independent lock. This can be seen/verified by removing the
# column masking of the transaction Ids.
--echo should find the presence of lock(s) on 1st transactioni
connection default;
let $wait_condition= select count(*)=1 from information_schema.processlist where info='replace into t values (1)' and state='update';
source include/wait_condition.inc;
real_sleep 1; # delay a little to shorten the update -> write row -> lock wait race

replace_column 1 TRX_ID 2 MYSQL_ID;
replace_column 1 TRX_ID 2 MYSQL_ID;
select * from information_schema.tokudb_locks;

--echo should find the presence of a lock_wait on the 2nd transaction
Expand All @@ -101,7 +102,7 @@ select trx_id,trx_mysql_thread_id from information_schema.tokudb_trx;
connection conn_a;
commit;
--echo verify that the lock on the 1st transaction is released and replaced by the lock for the 2nd transaction
let $wait_condition= select count(*)=1 from information_schema.tokudb_locks where locks_dname='./test/t-main';
let $wait_condition= select count(*)=3 from information_schema.tokudb_locks where locks_dname='./test/t-main';
source include/wait_condition.inc;
replace_column 1 TRX_ID 2 MYSQL_ID;
select * from information_schema.tokudb_locks;
Expand Down
122 changes: 15 additions & 107 deletions storage/tokudb/ha_tokudb.cc
Original file line number Diff line number Diff line change
Expand Up @@ -443,23 +443,6 @@ static inline bool is_replace_into(THD* thd) {
return thd->lex->duplicates == DUP_REPLACE;
}

static inline bool do_ignore_flag_optimization(
THD* thd,
TABLE* table,
bool opt_eligible) {

bool do_opt = false;
if (opt_eligible &&
(is_replace_into(thd) || is_insert_ignore(thd)) &&
!table->triggers &&
!(mysql_bin_log.is_open() &&
thd->variables.binlog_format != BINLOG_FORMAT_STMT)) {
do_opt = true;
}

return do_opt;
}

ulonglong ha_tokudb::table_flags() const {
return int_table_flags | HA_BINLOG_ROW_CAPABLE | HA_BINLOG_STMT_CAPABLE;
}
Expand Down Expand Up @@ -1572,39 +1555,6 @@ static int initialize_key_and_col_info(
return error;
}

bool ha_tokudb::can_replace_into_be_fast(
TABLE_SHARE* table_share,
KEY_AND_COL_INFO* kc_info,
uint pk) {

uint curr_num_DBs = table_share->keys + tokudb_test(hidden_primary_key);
bool ret_val;
if (curr_num_DBs == 1) {
ret_val = true;
goto exit;
}
ret_val = true;
for (uint curr_index = 0; curr_index < table_share->keys; curr_index++) {
if (curr_index == pk) continue;
KEY* curr_key_info = &table_share->key_info[curr_index];
for (uint i = 0; i < curr_key_info->user_defined_key_parts; i++) {
uint16 curr_field_index = curr_key_info->key_part[i].field->field_index;
if (!bitmap_is_set(&kc_info->key_filters[curr_index],curr_field_index)) {
ret_val = false;
goto exit;
}
if (bitmap_is_set(&kc_info->key_filters[curr_index], curr_field_index) &&
!bitmap_is_set(&kc_info->key_filters[pk], curr_field_index)) {
ret_val = false;
goto exit;
}

}
}
exit:
return ret_val;
}

int ha_tokudb::initialize_share(const char* name, int mode) {
int error = 0;
uint64_t num_rows = 0;
Expand Down Expand Up @@ -1704,11 +1654,6 @@ int ha_tokudb::initialize_share(const char* name, int mode) {
}
}
}
share->replace_into_fast =
can_replace_into_be_fast(
table_share,
&share->kc_info,
primary_key);

share->pk_has_string = false;
if (!hidden_primary_key) {
Expand Down Expand Up @@ -3836,41 +3781,15 @@ void ha_tokudb::test_row_packing(uchar* record, DBT* pk_key, DBT* pk_val) {
}

// set the put flags for the main dictionary
void ha_tokudb::set_main_dict_put_flags(
THD* thd,
uint32_t* put_flags) {

void ha_tokudb::set_main_dict_put_flags(THD* thd,
uint32_t* put_flags) {
uint32_t old_prelock_flags = 0;
uint curr_num_DBs = table->s->keys + tokudb_test(hidden_primary_key);
bool in_hot_index = share->num_DBs > curr_num_DBs;
bool using_ignore_flag_opt =
do_ignore_flag_optimization(
thd,
table,
share->replace_into_fast && !using_ignore_no_key);
//
// optimization for "REPLACE INTO..." (and "INSERT IGNORE") command
// if the command is "REPLACE INTO" and the only table
// is the main table (or all indexes are a subset of the pk),
// then we can simply insert the element
// with DB_YESOVERWRITE. If the element does not exist,
// it will act as a normal insert, and if it does exist, it
// will act as a replace, which is exactly what REPLACE INTO is supposed
// to do. We cannot do this if otherwise, because then we lose
// consistency between indexes
//
if (hidden_primary_key) {
*put_flags = old_prelock_flags;
} else if (!do_unique_checks(thd, in_rpl_write_rows | in_rpl_update_rows) &&
!is_replace_into(thd) &&
!is_insert_ignore(thd)) {
*put_flags = old_prelock_flags;
} else if (using_ignore_flag_opt && is_replace_into(thd) && !in_hot_index) {

if (hidden_primary_key ||
(!do_unique_checks(thd, in_rpl_write_rows | in_rpl_update_rows) &&
!is_replace_into(thd) && !is_insert_ignore(thd))) {
*put_flags = old_prelock_flags;
} else {
// GL on DB-937 : The server expects an SE to return ER_DUP_ENTRY on a
// dup key hit even when IGNORE is in use. This is so the server can
// set the correct warnings on the statement.
*put_flags = DB_NOOVERWRITE | old_prelock_flags;
}
}
Expand Down Expand Up @@ -3997,11 +3916,10 @@ int ha_tokudb::write_row(uchar * record) {
int error;
THD *thd = ha_thd();
bool has_null;
DB_TXN* sub_trans = NULL;
DB_TXN* txn = NULL;
tokudb_trx_data *trx = NULL;
DB_TXN* sub_trans = nullptr;
DB_TXN* txn = nullptr;
tokudb_trx_data* trx = nullptr;
uint curr_num_DBs;
bool create_sub_trans = false;
bool num_DBs_locked = false;

//
Expand Down Expand Up @@ -4078,25 +3996,15 @@ int ha_tokudb::write_row(uchar * record) {
goto cleanup;
}

create_sub_trans =
(using_ignore &&
!(do_ignore_flag_optimization(
thd,
table,
share->replace_into_fast && !using_ignore_no_key)));
if (create_sub_trans) {
error =
txn_begin(
db_env,
transaction,
&sub_trans,
DB_INHERIT_ISOLATION,
thd);
if (using_ignore) {
error = txn_begin(
db_env, transaction, &sub_trans, DB_INHERIT_ISOLATION, thd);
if (error) {
goto cleanup;
}
}
txn = create_sub_trans ? sub_trans : transaction;

txn = using_ignore ? sub_trans : transaction;
TOKUDB_HANDLER_TRACE_FOR_FLAGS(TOKUDB_DEBUG_TXN, "txn %p", txn);
if (TOKUDB_UNLIKELY(TOKUDB_DEBUG_FLAGS(TOKUDB_DEBUG_CHECK_KEY))) {
test_row_packing(record,&prim_key,&row);
Expand Down Expand Up @@ -4163,7 +4071,7 @@ int ha_tokudb::write_row(uchar * record) {
if (error == DB_KEYEXIST) {
error = HA_ERR_FOUND_DUPP_KEY;
}
if (sub_trans) {
if (using_ignore) {
// no point in recording error value of abort.
// nothing we can do about it anyway and it is not what
// we want to return.
Expand Down
2 changes: 0 additions & 2 deletions storage/tokudb/ha_tokudb.h
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,6 @@ class TOKUDB_SHARE {
bool try_table_lock;

bool has_unique_keys;
bool replace_into_fast;
tokudb::thread::rwlock_t _num_DBs_lock;
uint32_t num_DBs;

Expand Down Expand Up @@ -674,7 +673,6 @@ class ha_tokudb : public handler {
int write_key_name_to_status(DB* status_block, char* key_name, DB_TXN* txn);
int write_auto_inc_create(DB* db, ulonglong val, DB_TXN* txn);
void init_auto_increment();
bool can_replace_into_be_fast(TABLE_SHARE* table_share, KEY_AND_COL_INFO* kc_info, uint pk);
int initialize_share(const char* name, int mode);

void set_query_columns(uint keynr);
Expand Down

0 comments on commit 23c0a72

Please sign in to comment.