From 823719ee89f6a51ecc745fe25d3b2160983aa9d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Sat, 3 Nov 2018 07:06:37 +0200 Subject: [PATCH] MDEV-17603: Revert the InnoDB changes for SBR MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a follow-up to MDEV-17073. It is simplest to refuse statement-based replication for REPLACE and INSERT…ON DUPLICATE KEY UPDATE statements, and to remove the extra locking in InnoDB. ha_innobase::external_lock(): Remove an inaccurate check. TODO: Do we still need ha_innobase::table_flags()? TODO: Does RocksDB really need thd_binlog_format(), thd_binlog_filter_ok()? Could those be removed altogether? --- sql/sql_class.cc | 5 - storage/innobase/handler/ha_innodb.cc | 35 ----- storage/innobase/handler/ha_innodb.h | 10 -- storage/innobase/include/ha_prototypes.h | 3 - storage/innobase/include/row0ins.h | 4 - storage/innobase/que/que0que.cc | 5 - storage/innobase/row/row0ins.cc | 165 +---------------------- storage/innobase/row/row0mysql.cc | 3 - 8 files changed, 4 insertions(+), 226 deletions(-) diff --git a/sql/sql_class.cc b/sql/sql_class.cc index 71d5b80eaa3..e581ed0af25 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -4523,11 +4523,6 @@ extern "C" int thd_rpl_is_parallel(const MYSQL_THD thd) return thd->rgi_slave && thd->rgi_slave->is_parallel_exec; } -extern "C" int thd_rpl_stmt_based(const MYSQL_THD thd) -{ - return !thd->is_current_stmt_binlog_format_row() && - !thd->is_current_stmt_binlog_disabled(); -} /* Returns high resolution timestamp for the start of the current query. */ diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index 84253d88983..0ee702df237 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -15687,41 +15687,6 @@ ha_innobase::external_lock( ut_ad(m_prebuilt->table); - /* Statement based binlogging does not work in isolation level - READ UNCOMMITTED and READ COMMITTED since the necessary - locks cannot be taken. In this case, we print an - informative error message and return with an error. - Note: decide_logging_format would give the same error message, - except it cannot give the extra details. */ - - if (lock_type == F_WRLCK - && !(table_flags() & HA_BINLOG_STMT_CAPABLE) - && thd_binlog_format(thd) == BINLOG_FORMAT_STMT - && thd_binlog_filter_ok(thd) - && thd_sqlcom_can_generate_row_events(thd)) { - - bool skip = false; - - /* used by test case */ - DBUG_EXECUTE_IF("no_innodb_binlog_errors", skip = true;); - - if (!skip) { -#ifdef WITH_WSREP - if (!wsrep_on(thd) || wsrep_thd_exec_mode(thd) == LOCAL_STATE) - { -#endif /* WITH_WSREP */ - my_error(ER_BINLOG_STMT_MODE_AND_ROW_ENGINE, MYF(0), - " InnoDB is limited to row-logging when" - " transaction isolation level is" - " READ COMMITTED or READ UNCOMMITTED."); - - DBUG_RETURN(HA_ERR_LOGGING_IMPOSSIBLE); -#ifdef WITH_WSREP - } -#endif /* WITH_WSREP */ - } - } - /* Check for UPDATEs in read-only mode. */ if (srv_read_only_mode && (thd_sql_command(thd) == SQLCOM_UPDATE diff --git a/storage/innobase/handler/ha_innodb.h b/storage/innobase/handler/ha_innodb.h index da2cc403796..891f408f775 100644 --- a/storage/innobase/handler/ha_innodb.h +++ b/storage/innobase/handler/ha_innodb.h @@ -530,16 +530,6 @@ but can be used for comparison. */ unsigned long long thd_start_utime(const MYSQL_THD thd); -/** Get the user thread's binary logging format -@param thd user thread -@return Value to be used as index into the binlog_format_names array */ -int thd_binlog_format(const MYSQL_THD thd); - -/** Check if binary logging is filtered for thread's current db. -@param thd Thread handle -@retval 1 the query is not filtered, 0 otherwise. */ -bool thd_binlog_filter_ok(const MYSQL_THD thd); - /** Check if the query may generate row changes which may end up in the binary. @param thd Thread handle @retval 1 the query may generate row changes, 0 otherwise. diff --git a/storage/innobase/include/ha_prototypes.h b/storage/innobase/include/ha_prototypes.h index 8af4d320997..86defe9b166 100644 --- a/storage/innobase/include/ha_prototypes.h +++ b/storage/innobase/include/ha_prototypes.h @@ -122,9 +122,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 a7320f9ed03..841fde7df3f 100644 --- a/storage/innobase/include/row0ins.h +++ b/storage/innobase/include/row0ins.h @@ -198,10 +198,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 99d8c70a2c0..b4d2e064fbd 100644 --- a/storage/innobase/que/que0que.cc +++ b/storage/innobase/que/que0que.cc @@ -689,11 +689,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 3362e5302b1..944b81f65c5 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; @@ -2299,12 +2297,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 @@ -2321,13 +2313,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); } @@ -3003,46 +2995,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; } @@ -3558,13 +3510,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); @@ -3590,91 +3535,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); } } @@ -3691,31 +3552,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 434d538a7ad..184a01ea0c7 100644 --- a/storage/innobase/row/row0mysql.cc +++ b/storage/innobase/row/row0mysql.cc @@ -1436,7 +1436,6 @@ row_insert_for_mysql( goto run_again; } - node->duplicate = NULL; trx->op_info = ""; if (blob_heap != NULL) { @@ -1446,8 +1445,6 @@ row_insert_for_mysql( return(err); } - node->duplicate = NULL; - if (dict_table_has_fts_index(table)) { doc_id_t doc_id; -- 2.19.1