From 33ecf8345d357cbf71bde2b03b641a612db7781f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 16 Jan 2018 13:55:45 +0200 Subject: [PATCH 1/4] Follow-up fix to MDEV-14441: Fix a potential race condition btr_cur_update_in_place(): Read block->index only once, so that it cannot change to NULL after the first read. When block->index != NULL, it must be equal to index. --- storage/innobase/btr/btr0cur.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/storage/innobase/btr/btr0cur.cc b/storage/innobase/btr/btr0cur.cc index 8146667f7c2..139e06ab996 100644 --- a/storage/innobase/btr/btr0cur.cc +++ b/storage/innobase/btr/btr0cur.cc @@ -3694,7 +3694,7 @@ btr_cur_update_in_place( #ifdef BTR_CUR_HASH_ADAPT { rw_lock_t* ahi_latch = block->index - ? btr_get_search_latch(block->index) : NULL; + ? btr_get_search_latch(index) : NULL; if (ahi_latch) { /* TO DO: Can we skip this if none of the fields index->search_info->curr_n_fields From be85c2dc889b668382106071e712213cd3b1cbcf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 16 Jan 2018 13:57:30 +0200 Subject: [PATCH 2/4] Mariabackup --prepare: Do not access transactions or data dictionary innobase_start_or_create_for_mysql(): Only start the data dictionary and transaction subsystems in normal server startup and during mariabackup --export. --- storage/innobase/srv/srv0start.cc | 29 ++++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/storage/innobase/srv/srv0start.cc b/storage/innobase/srv/srv0start.cc index 7bfd1715e9b..f45e4c98e81 100644 --- a/storage/innobase/srv/srv0start.cc +++ b/storage/innobase/srv/srv0start.cc @@ -2246,17 +2246,32 @@ files_checked: recv_sys->dblwr.pages.clear(); - if (err == DB_SUCCESS) { - /* Initialize the change buffer. */ - err = dict_boot(); - } - if (err != DB_SUCCESS) { return(srv_init_abort(err)); } - /* This must precede recv_apply_hashed_log_recs(true). */ - trx_sys_init_at_db_start(); + switch (srv_operation) { + case SRV_OPERATION_NORMAL: + case SRV_OPERATION_RESTORE_EXPORT: + /* Initialize the change buffer. */ + err = dict_boot(); + if (err != DB_SUCCESS) { + return(srv_init_abort(err)); + } + /* This must precede + recv_apply_hashed_log_recs(true). */ + trx_sys_init_at_db_start(); + break; + case SRV_OPERATION_RESTORE_DELTA: + case SRV_OPERATION_BACKUP: + ut_ad(!"wrong mariabackup mode"); + /* fall through */ + case SRV_OPERATION_RESTORE: + /* mariabackup --prepare only deals with + the redo log and the data files, not with + transactions or the data dictionary. */ + break; + } if (srv_force_recovery < SRV_FORCE_NO_LOG_REDO) { /* Apply the hashed log records to the From 81378b394763e56d57c4a4fc3c20244cc0ee9cc5 Mon Sep 17 00:00:00 2001 From: Alexander Barkov Date: Tue, 16 Jan 2018 16:09:51 +0400 Subject: [PATCH 3/4] Moving a change_list related methods from THD to Item_change_list 1. Moving the following methods from THD to Item_change_list: nocheck_register_item_tree_change() check_and_register_item_tree_change() rollback_item_tree_changes() as they work only with the "change_list" member and don't require anything else from THD. 2. Deriving THD from Item_change_list This change will help to fix "MDEV-14603 signal 11 with short stacktrace" easier. --- sql/sp_head.cc | 8 ++++---- sql/sql_class.cc | 14 +++++++++----- sql/sql_class.h | 37 +++++++++++++++++++++++-------------- sql/sql_parse.cc | 2 +- sql/sql_prepare.cc | 8 ++++---- 5 files changed, 41 insertions(+), 28 deletions(-) diff --git a/sql/sp_head.cc b/sql/sp_head.cc index 7816af398c2..8b05d14f2de 100644 --- a/sql/sp_head.cc +++ b/sql/sp_head.cc @@ -1249,7 +1249,7 @@ sp_head::execute(THD *thd, bool merge_da_on_success) We should also save Item tree change list to avoid rollback something too early in the calling query. */ - thd->change_list.move_elements_to(&old_change_list); + thd->Item_change_list::move_elements_to(&old_change_list); /* Cursors will use thd->packet, so they may corrupt data which was prepared for sending by upper level. OTOH cursors in the same routine can share this @@ -1389,8 +1389,8 @@ sp_head::execute(THD *thd, bool merge_da_on_success) /* Restore all saved */ thd->server_status= (thd->server_status & ~status_backup_mask) | old_server_status; old_packet.swap(thd->packet); - DBUG_ASSERT(thd->change_list.is_empty()); - old_change_list.move_elements_to(&thd->change_list); + DBUG_ASSERT(thd->Item_change_list::is_empty()); + old_change_list.move_elements_to(thd); thd->lex= old_lex; thd->set_query_id(old_query_id); DBUG_ASSERT(!thd->derived_tables); @@ -2976,7 +2976,7 @@ sp_lex_keeper::reset_lex_and_exec_core(THD *thd, uint *nextp, bool parent_modified_non_trans_table= thd->transaction.stmt.modified_non_trans_table; thd->transaction.stmt.modified_non_trans_table= FALSE; DBUG_ASSERT(!thd->derived_tables); - DBUG_ASSERT(thd->change_list.is_empty()); + DBUG_ASSERT(thd->Item_change_list::is_empty()); /* Use our own lex. We should not save old value since it is saved/restored in diff --git a/sql/sql_class.cc b/sql/sql_class.cc index c8eb8ed3128..d0b45441d83 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -2605,8 +2605,10 @@ struct Item_change_record: public ilink thd->mem_root (due to possible set_n_backup_active_arena called for thd). */ -void THD::nocheck_register_item_tree_change(Item **place, Item *old_value, - MEM_ROOT *runtime_memroot) +void +Item_change_list::nocheck_register_item_tree_change(Item **place, + Item *old_value, + MEM_ROOT *runtime_memroot) { Item_change_record *change; DBUG_ENTER("THD::nocheck_register_item_tree_change"); @@ -2647,8 +2649,10 @@ void THD::nocheck_register_item_tree_change(Item **place, Item *old_value, changes to substitute the same reference at both locations L1 and L2. */ -void THD::check_and_register_item_tree_change(Item **place, Item **new_value, - MEM_ROOT *runtime_memroot) +void +Item_change_list::check_and_register_item_tree_change(Item **place, + Item **new_value, + MEM_ROOT *runtime_memroot) { Item_change_record *change; I_List_iterator it(change_list); @@ -2663,7 +2667,7 @@ void THD::check_and_register_item_tree_change(Item **place, Item **new_value, } -void THD::rollback_item_tree_changes() +void Item_change_list::rollback_item_tree_changes() { I_List_iterator it(change_list); Item_change_record *change; diff --git a/sql/sql_class.h b/sql/sql_class.h index e5aaba9b407..bcd43cd62cd 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -1279,7 +1279,21 @@ public: */ struct Item_change_record; -typedef I_List Item_change_list; +class Item_change_list +{ + I_List change_list; +public: + void nocheck_register_item_tree_change(Item **place, Item *old_value, + MEM_ROOT *runtime_memroot); + void check_and_register_item_tree_change(Item **place, Item **new_value, + MEM_ROOT *runtime_memroot); + void rollback_item_tree_changes(); + void move_elements_to(Item_change_list *to) + { + change_list.move_elements_to(&to->change_list); + } + bool is_empty() { return change_list.is_empty(); } +}; /** @@ -2040,6 +2054,14 @@ void dbug_serve_apcs(THD *thd, int n_calls); */ class THD :public Statement, + /* + This is to track items changed during execution of a prepared + statement/stored procedure. It's created by + nocheck_register_item_tree_change() in memory root of THD, + and freed in rollback_item_tree_changes(). + For conventional execution it's always empty. + */ + public Item_change_list, public MDL_context_owner, public Open_tables_state { @@ -2510,14 +2532,6 @@ public: #ifdef SIGNAL_WITH_VIO_CLOSE Vio* active_vio; #endif - /* - This is to track items changed during execution of a prepared - statement/stored procedure. It's created by - nocheck_register_item_tree_change() in memory root of THD, and freed in - rollback_item_tree_changes(). For conventional execution it's always - empty. - */ - Item_change_list change_list; /* A permanent memory area of the statement. For conventional @@ -3599,11 +3613,6 @@ public: */ memcpy((char*) place, new_value, sizeof(*new_value)); } - void nocheck_register_item_tree_change(Item **place, Item *old_value, - MEM_ROOT *runtime_memroot); - void check_and_register_item_tree_change(Item **place, Item **new_value, - MEM_ROOT *runtime_memroot); - void rollback_item_tree_changes(); /* Cleanup statement parse state (parse tree, lex) and execution diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 6d068eb99ac..21abc1a248c 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -7910,7 +7910,7 @@ void mysql_parse(THD *thd, char *rawbuf, uint length, sp_cache_enforce_limit(thd->sp_func_cache, stored_program_cache_size); thd->end_statement(); thd->cleanup_after_query(); - DBUG_ASSERT(thd->change_list.is_empty()); + DBUG_ASSERT(thd->Item_change_list::is_empty()); } else { diff --git a/sql/sql_prepare.cc b/sql/sql_prepare.cc index 390b70877f5..4e847fb9ff3 100644 --- a/sql/sql_prepare.cc +++ b/sql/sql_prepare.cc @@ -3930,7 +3930,7 @@ bool Prepared_statement::prepare(const char *packet, uint packet_len) If called from a stored procedure, ensure that we won't rollback external changes when cleaning up after validation. */ - DBUG_ASSERT(thd->change_list.is_empty()); + DBUG_ASSERT(thd->Item_change_list::is_empty()); /* Marker used to release metadata locks acquired while the prepared @@ -4407,7 +4407,7 @@ Prepared_statement::execute_server_runnable(Server_runnable *server_runnable) bool error; Query_arena *save_stmt_arena= thd->stmt_arena; Item_change_list save_change_list; - thd->change_list.move_elements_to(&save_change_list); + thd->Item_change_list::move_elements_to(&save_change_list); state= STMT_CONVENTIONAL_EXECUTION; @@ -4426,7 +4426,7 @@ Prepared_statement::execute_server_runnable(Server_runnable *server_runnable) thd->restore_backup_statement(this, &stmt_backup); thd->stmt_arena= save_stmt_arena; - save_change_list.move_elements_to(&thd->change_list); + save_change_list.move_elements_to(thd); /* Items and memory will freed in destructor */ @@ -4654,7 +4654,7 @@ bool Prepared_statement::execute(String *expanded_query, bool open_cursor) If the free_list is not empty, we'll wrongly free some externally allocated items when cleaning up after execution of this statement. */ - DBUG_ASSERT(thd->change_list.is_empty()); + DBUG_ASSERT(thd->Item_change_list::is_empty()); /* The only case where we should have items in the thd->free_list is From f44017384afa7a10df9866423291fc764046043f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 16 Jan 2018 20:02:38 +0200 Subject: [PATCH 4/4] MDEV-14968 On upgrade, InnoDB reports "started; log sequence number 0" srv_prepare_to_delete_redo_log_files(): Initialize srv_start_lsn. --- storage/innobase/srv/srv0start.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/storage/innobase/srv/srv0start.cc b/storage/innobase/srv/srv0start.cc index f45e4c98e81..d3b7fb6bf94 100644 --- a/storage/innobase/srv/srv0start.cc +++ b/storage/innobase/srv/srv0start.cc @@ -1446,6 +1446,7 @@ srv_prepare_to_delete_redo_log_files( << " bytes; LSN=" << flushed_lsn; } + srv_start_lsn = flushed_lsn; /* Flush the old log files. */ log_mutex_exit();