diff options
author | Marko Mäkelä <marko.makela@mariadb.com> | 2019-08-12 18:50:54 +0300 |
---|---|---|
committer | Marko Mäkelä <marko.makela@mariadb.com> | 2019-08-12 18:50:54 +0300 |
commit | 609ea2f37b8169a7c282fe2d607c2412467ccbbb (patch) | |
tree | ce5f8317d4d659641799c560fe074abcc24d6ac6 /storage/innobase | |
parent | be33124c9dc284c4409d02e5405de568b467a167 (diff) | |
download | mariadb-git-609ea2f37b8169a7c282fe2d607c2412467ccbbb.tar.gz |
MDEV-17614: After-merge fix
MDEV-17614 flags INSERT…ON DUPLICATE KEY UPDATE unsafe for statement-based
replication when there are multiple unique indexes. This correctly fixes
something whose attempted fix in MySQL 5.7
in mysql/mysql-server@c93b0d9a972cb6f98fd445f2b69d924350f9128a
caused lock conflicts. That change was reverted in MySQL 5.7.26
in mysql/mysql-server@066b6fdd433aa6673622341f1a2f0a3a20018043
(with a substantial amount of other changes).
In MDEV-17073 we already disabled the unfortunate MySQL change when
statement-based replication was not being used. Now, thanks to MDEV-17614,
we can actually remove the change altogether.
This reverts commit 8a346f31b913daa011085afec2b2d38450c73e00 (MDEV-17073)
and mysql/mysql-server@c93b0d9a972cb6f98fd445f2b69d924350f9128a while
keeping the test cases.
Diffstat (limited to 'storage/innobase')
-rw-r--r-- | storage/innobase/include/ha_prototypes.h | 3 | ||||
-rw-r--r-- | storage/innobase/include/row0ins.h | 4 | ||||
-rw-r--r-- | storage/innobase/que/que0que.cc | 5 | ||||
-rw-r--r-- | storage/innobase/row/row0ins.cc | 175 | ||||
-rw-r--r-- | storage/innobase/row/row0mysql.cc | 3 |
5 files changed, 9 insertions, 181 deletions
diff --git a/storage/innobase/include/ha_prototypes.h b/storage/innobase/include/ha_prototypes.h index 7fd0de92562..693dcd15163 100644 --- a/storage/innobase/include/ha_prototypes.h +++ b/storage/innobase/include/ha_prototypes.h @@ -121,9 +121,6 @@ thd_is_replication_slave_thread( /*============================*/ THD* thd); /*!< in: thread handle */ -/** @return whether statement-based replication is active */ -extern "C" int thd_rpl_stmt_based(const THD* thd); - /******************************************************************//** Returns true if the transaction this thread is processing has edited non-transactional tables. Used by the deadlock detector when deciding diff --git a/storage/innobase/include/row0ins.h b/storage/innobase/include/row0ins.h index 2d4e9f67562..3a85d7a21c4 100644 --- a/storage/innobase/include/row0ins.h +++ b/storage/innobase/include/row0ins.h @@ -196,10 +196,6 @@ struct ins_node_t{ entry_list and sys fields are stored here; if this is NULL, entry list should be created and buffers for sys fields in row allocated */ - dict_index_t* duplicate; - /* This is the first index that reported - DB_DUPLICATE_KEY. Used in the case of REPLACE - or INSERT ... ON DUPLICATE UPDATE. */ ulint magic_n; }; diff --git a/storage/innobase/que/que0que.cc b/storage/innobase/que/que0que.cc index 9b98cdbcdcb..1d3d1573299 100644 --- a/storage/innobase/que/que0que.cc +++ b/storage/innobase/que/que0que.cc @@ -684,11 +684,6 @@ que_thr_stop( trx->lock.wait_thr = thr; thr->state = QUE_THR_LOCK_WAIT; - } else if (trx->duplicates && trx->error_state == DB_DUPLICATE_KEY - && thd_rpl_stmt_based(trx->mysql_thd)) { - - return(FALSE); - } else if (trx->error_state != DB_SUCCESS && trx->error_state != DB_LOCK_WAIT) { diff --git a/storage/innobase/row/row0ins.cc b/storage/innobase/row/row0ins.cc index ce394390679..ece291378b6 100644 --- a/storage/innobase/row/row0ins.cc +++ b/storage/innobase/row/row0ins.cc @@ -88,7 +88,6 @@ ins_node_create( node->select = NULL; node->trx_id = 0; - node->duplicate = NULL; node->entry_sys_heap = mem_heap_create(128); @@ -191,7 +190,6 @@ ins_node_set_new_row( node->state = INS_NODE_SET_IX_LOCK; node->index = NULL; node->entry = NULL; - node->duplicate = NULL; node->row = row; @@ -2320,12 +2318,6 @@ row_ins_duplicate_error_in_clust( true, ULINT_UNDEFINED, &heap); - ulint lock_type = - trx->isolation_level <= TRX_ISO_READ_COMMITTED - || (trx->mysql_thd - && !thd_rpl_stmt_based(trx->mysql_thd)) - ? LOCK_REC_NOT_GAP : LOCK_ORDINARY; - /* We set a lock on the possible duplicate: this is needed in logical logging of MySQL to make sure that in roll-forward we get the same duplicate @@ -2342,13 +2334,13 @@ row_ins_duplicate_error_in_clust( INSERT ON DUPLICATE KEY UPDATE). */ err = row_ins_set_exclusive_rec_lock( - lock_type, + LOCK_REC_NOT_GAP, btr_cur_get_block(cursor), rec, cursor->index, offsets, thr); } else { err = row_ins_set_shared_rec_lock( - lock_type, + LOCK_REC_NOT_GAP, btr_cur_get_block(cursor), rec, cursor->index, offsets, thr); } @@ -2363,7 +2355,10 @@ row_ins_duplicate_error_in_clust( if (row_ins_dupl_error_with_rec( rec, entry, cursor->index, offsets)) { - goto duplicate; +duplicate: + trx->error_info = cursor->index; + err = DB_DUPLICATE_KEY; + goto func_exit; } } } @@ -2406,10 +2401,7 @@ row_ins_duplicate_error_in_clust( if (row_ins_dupl_error_with_rec( rec, entry, cursor->index, offsets)) { -duplicate: - trx->error_info = cursor->index; - err = DB_DUPLICATE_KEY; - goto func_exit; + goto duplicate; } } @@ -3023,46 +3015,6 @@ row_ins_sec_index_entry_low( &cursor, 0, __FILE__, __LINE__, &mtr); } - if (!(flags & BTR_NO_LOCKING_FLAG) - && dict_index_is_unique(index) - && thr_get_trx(thr)->duplicates - && thr_get_trx(thr)->isolation_level >= TRX_ISO_REPEATABLE_READ - && thd_rpl_stmt_based(thr_get_trx(thr)->mysql_thd)) { - - /* In statement-based replication, when replicating a - REPLACE statement or ON DUPLICATE KEY UPDATE clause, a - gap lock is taken on the position of the to-be-inserted record, - to avoid other concurrent transactions from inserting the same - record. */ - - dberr_t err; - const rec_t* rec = page_rec_get_next_const( - btr_cur_get_rec(&cursor)); - - ut_ad(!page_rec_is_infimum(rec)); - - offsets = rec_get_offsets(rec, index, offsets, true, - ULINT_UNDEFINED, &offsets_heap); - - err = row_ins_set_exclusive_rec_lock( - LOCK_GAP, btr_cur_get_block(&cursor), rec, - index, offsets, thr); - - switch (err) { - case DB_SUCCESS: - case DB_SUCCESS_LOCKED_REC: - if (thr_get_trx(thr)->error_state != DB_DUPLICATE_KEY) { - break; - } - /* Fall through (skip actual insert) after we have - successfully acquired the gap lock. */ - default: - goto func_exit; - } - } - - ut_ad(thr_get_trx(thr)->error_state == DB_SUCCESS); - if (dup_chk_only) { goto func_exit; } @@ -3578,13 +3530,6 @@ row_ins( DBUG_PRINT("row_ins", ("table: %s", node->table->name.m_name)); - trx_t* trx = thr_get_trx(thr); - - if (node->duplicate) { - ut_ad(thd_rpl_stmt_based(trx->mysql_thd)); - trx->error_state = DB_DUPLICATE_KEY; - } - if (node->state == INS_NODE_ALLOC_ROW_ID) { row_ins_alloc_row_id_step(node); @@ -3610,91 +3555,7 @@ row_ins( if (node->index->type != DICT_FTS) { dberr_t err = row_ins_index_entry_step(node, thr); - switch (err) { - case DB_SUCCESS: - break; - case DB_NO_REFERENCED_ROW: - if (!dict_index_is_unique(node->index)) { - DBUG_RETURN(err); - } - /* fall through */ - case DB_DUPLICATE_KEY: - ut_ad(dict_index_is_unique(node->index)); - - if (trx->isolation_level - >= TRX_ISO_REPEATABLE_READ - && trx->duplicates - && !node->table->is_temporary() - && thd_rpl_stmt_based(trx->mysql_thd)) { - - /* When we are in REPLACE statement or - INSERT .. ON DUPLICATE UPDATE - statement, we process all the - unique secondary indexes, even after we - encounter a duplicate error. This is - done to take necessary gap locks in - secondary indexes to block concurrent - transactions from inserting the - searched records. */ - if (err == DB_NO_REFERENCED_ROW - && node->duplicate) { - /* A foreign key check on a - unique index may fail to - find the record. - - Consider as a example - following: - create table child(a int not null - primary key, b int not null, - c int, - unique key (b), - foreign key (b) references - parent (id)) engine=innodb; - - insert into child values - (1,1,2); - - insert into child(a) values - (1) on duplicate key update - c = 3; - - Now primary key value 1 - naturally causes duplicate - key error that will be - stored on node->duplicate. - If there was no duplicate - key error, we should return - the actual no referenced - row error. - - As value for - column b used in both unique - key and foreign key is not - provided, server uses 0 as a - search value. This is - naturally, not found leading - to DB_NO_REFERENCED_ROW. - But, we should update the - row with primay key value 1 - anyway. - - Return the - original DB_DUPLICATE_KEY - error after - placing all gaplocks. */ - err = DB_DUPLICATE_KEY; - break; - } else if (!node->duplicate) { - /* Save 1st dup error. Ignore - subsequent dup errors. */ - node->duplicate = node->index; - trx->error_state - = DB_DUPLICATE_KEY; - } - break; - } - // fall through - default: + if (err != DB_SUCCESS) { DBUG_RETURN(err); } } @@ -3711,31 +3572,13 @@ row_ins( node->index = dict_table_get_next_index(node->index); node->entry = UT_LIST_GET_NEXT(tuple_list, node->entry); } - - /* After encountering a duplicate key error, we process - remaining indexes just to place gap locks and no actual - insertion will take place. These gap locks are needed - only for unique indexes. So skipping non-unique indexes. */ - if (node->duplicate) { - ut_ad(thd_rpl_stmt_based(trx->mysql_thd)); - while (node->index - && !dict_index_is_unique(node->index)) { - - node->index = dict_table_get_next_index( - node->index); - node->entry = UT_LIST_GET_NEXT(tuple_list, - node->entry); - } - trx->error_state = DB_DUPLICATE_KEY; - } } ut_ad(node->entry == NULL); - trx->error_info = node->duplicate; node->state = INS_NODE_ALLOC_ROW_ID; - DBUG_RETURN(node->duplicate ? DB_DUPLICATE_KEY : DB_SUCCESS); + DBUG_RETURN(DB_SUCCESS); } /***********************************************************//** diff --git a/storage/innobase/row/row0mysql.cc b/storage/innobase/row/row0mysql.cc index fd87f8d03bc..f3d48edfb80 100644 --- a/storage/innobase/row/row0mysql.cc +++ b/storage/innobase/row/row0mysql.cc @@ -1435,7 +1435,6 @@ error_exit: goto run_again; } - node->duplicate = NULL; trx->op_info = ""; if (blob_heap != NULL) { @@ -1445,8 +1444,6 @@ error_exit: return(err); } - node->duplicate = NULL; - if (dict_table_has_fts_index(table)) { doc_id_t doc_id; |