From d2304554ac0565a0d5fd495619cb0f64f91b364c Mon Sep 17 00:00:00 2001 From: Monty Date: Sat, 20 Apr 2024 14:02:05 +0300 Subject: [PATCH] MDEV-33751 Assertion `thd' failed in int temp_file_size_cb_func(tmp_file_tracking*, int) Changes: - Fixed that MyISAM and Aria parallel repair works with tmp file limit. This required to add current_thd to all parallel workers and add protection in my_malloc_size_cb_func() and temp_file_size_cb_func() to be able to handle shared THD's. I removed the old code in MyISAM to set current_thd() as only worked when using with virtal indexed columns and I wanted to keep the Aria and MyISAM code identical. Other things: - Improved error messages from Aria parallel repair and create_internal_tmp_table_from_heap(). --- include/myisam.h | 1 + include/myisamchk.h | 4 +- mysql-test/main/repair_symlink-5543.result | 2 +- mysql-test/main/tmp_space_usage.result | 20 ++++++++++ mysql-test/main/tmp_space_usage.test | 22 +++++++++++ sql/mysqld.cc | 17 +++++++-- sql/sql_class.cc | 1 + sql/sql_class.h | 2 + sql/sql_select.cc | 2 +- storage/maria/ha_maria.cc | 29 ++++++++++++-- storage/maria/ma_check.c | 11 ++++-- storage/maria/ma_sort.c | 10 ++++- storage/maria/maria_def.h | 1 + storage/myisam/ha_myisam.cc | 44 +++++++++++++++------- storage/myisam/mi_check.c | 1 + storage/myisam/sort.c | 14 ++++--- 16 files changed, 147 insertions(+), 34 deletions(-) diff --git a/include/myisam.h b/include/myisam.h index dd4f9084b00..9d4e0ecab46 100644 --- a/include/myisam.h +++ b/include/myisam.h @@ -357,6 +357,7 @@ typedef struct st_mi_sort_param MEM_ROOT wordroot; uchar *record; MY_TMPDIR *tmpdir; + HA_CHECK *check_param; /* The next two are used to collect statistics, see update_key_parts for diff --git a/include/myisamchk.h b/include/myisamchk.h index 90acbdccdde..d1702357250 100644 --- a/include/myisamchk.h +++ b/include/myisamchk.h @@ -113,9 +113,9 @@ typedef struct st_handler_check_param uint progress_counter; /* How often to call _report_progress() */ ulonglong progress, max_progress; - void (*init_fix_record)(void *); int (*fix_record)(struct st_myisam_info *info, uchar *record, int keynum); - + void (*init_repair_thread)(void *); + void *init_repair_thread_arg; mysql_mutex_t print_msg_mutex; my_bool need_print_msg_lock; myf malloc_flags; diff --git a/mysql-test/main/repair_symlink-5543.result b/mysql-test/main/repair_symlink-5543.result index e99bd4ea312..e42acbc391d 100644 --- a/mysql-test/main/repair_symlink-5543.result +++ b/mysql-test/main/repair_symlink-5543.result @@ -10,7 +10,7 @@ create table t2 (a int) engine=aria data directory='MYSQL_TMP_DIR'; insert t2 values (1); repair table t2; Table Op Msg_type Msg_text -test.t2 repair error 20 for record at pos 256 +test.t2 repair error Got error 20 for record at pos 256 when creating index test.t2 repair Error File 'MYSQL_TMP_DIR/t2.MAD' not found (Errcode: 20 "") test.t2 repair status Operation failed drop table t2; diff --git a/mysql-test/main/tmp_space_usage.result b/mysql-test/main/tmp_space_usage.result index 355979aa66f..10104062eef 100644 --- a/mysql-test/main/tmp_space_usage.result +++ b/mysql-test/main/tmp_space_usage.result @@ -157,4 +157,24 @@ DROP TABLE t1; set max_tmp_session_space_usage = 1024*1024; select count(distinct concat(seq,repeat('x',1000))) from seq_1_to_1000; ERROR HY000: Global temporary space limit reached +# +# MDEV-33751 Assertion `thd' failed in int +# temp_file_size_cb_func(tmp_file_tracking*, int) +# +set @@global.max_tmp_total_space_usage=64*1024*1024; +set @@max_tmp_session_space_usage=1179648; +select @@max_tmp_session_space_usage; +@@max_tmp_session_space_usage +1179648 +set @save_aria_repair_threads=@@aria_repair_threads; +set @@aria_repair_threads=2; +set @save_max_heap_table_size=@@max_heap_table_size; +set @@max_heap_table_size=16777216; +CREATE TABLE t1 (a CHAR(255),b INT,INDEX (b)); +INSERT INTO t1 SELECT SEQ,SEQ FROM seq_1_to_100000; +SELECT * FROM t1 UNION SELECT * FROM t1; +ERROR HY000: Local temporary space limit reached +DROP TABLE t1; +set @@aria_repair_threads=@save_aria_repair_threads; +set @@max_heap_table_size=@save_max_heap_table_size; # End of 11.5 tests diff --git a/mysql-test/main/tmp_space_usage.test b/mysql-test/main/tmp_space_usage.test index 5c2a48681d6..390018a10d3 100644 --- a/mysql-test/main/tmp_space_usage.test +++ b/mysql-test/main/tmp_space_usage.test @@ -209,4 +209,26 @@ set max_tmp_session_space_usage = 1024*1024; --error 201 select count(distinct concat(seq,repeat('x',1000))) from seq_1_to_1000; +--echo # +--echo # MDEV-33751 Assertion `thd' failed in int +--echo # temp_file_size_cb_func(tmp_file_tracking*, int) +--echo # + +set @@global.max_tmp_session_space_usage=64*1024*1024; +set @@max_tmp_session_space_usage=1179648; +select @@max_tmp_session_space_usage; +set @save_aria_repair_threads=@@aria_repair_threads; +set @@aria_repair_threads=2; +set @save_max_heap_table_size=@@max_heap_table_size; +set @@max_heap_table_size=16777216; + +CREATE TABLE t1 (a CHAR(255),b INT,INDEX (b)); +INSERT INTO t1 SELECT SEQ,SEQ FROM seq_1_to_100000; +--error 200 +SELECT * FROM t1 UNION SELECT * FROM t1; +DROP TABLE t1; + +set @@aria_repair_threads=@save_aria_repair_threads; +set @@max_heap_table_size=@save_max_heap_table_size; + --echo # End of 11.5 tests diff --git a/sql/mysqld.cc b/sql/mysqld.cc index 256318c50ca..76920bd57a3 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -3760,7 +3760,7 @@ static void my_malloc_size_cb_func(long long size, my_bool is_thread_specific) DBUG_ASSERT((longlong) thd->status_var.local_memory_used >= 0 || !debug_assert_on_not_freed_memory); } - else if (likely(thd)) + else if (likely(thd) && likely(!thd->shared_thd)) { DBUG_PRINT("info", ("global thd memory_used: %lld size: %lld", (longlong) thd->status_var.global_memory_used, size)); @@ -3777,6 +3777,7 @@ static int temp_file_size_cb_func(struct tmp_file_tracking *track, int no_error) { THD *thd= current_thd; + int error= 0; longlong size_change= (longlong) (track->file_size - track->previous_file_size); DBUG_ENTER("temp_file_size_cb_func"); @@ -3794,6 +3795,8 @@ static int temp_file_size_cb_func(struct tmp_file_tracking *track, track->previous_file_size. */ DBUG_ASSERT(thd->status_var.tmp_space_used >= track->previous_file_size); + if (unlikely(thd->shared_thd)) + mysql_mutex_lock(&thd->LOCK_thd_data); global_tmp_space_used+= size_change; if (size_change > 0) @@ -3805,14 +3808,16 @@ static int temp_file_size_cb_func(struct tmp_file_tracking *track, global_max_tmp_space_usage) { global_tmp_space_used-= size_change; - DBUG_RETURN(my_errno= EE_GLOBAL_TMP_SPACE_FULL); + error= my_errno= EE_GLOBAL_TMP_SPACE_FULL; + goto exit; } if (thd->status_var.tmp_space_used + size_change > thd->variables.max_tmp_space_usage && !no_error && thd->variables.max_tmp_space_usage) { global_tmp_space_used-= size_change; - DBUG_RETURN(my_errno= EE_LOCAL_TMP_SPACE_FULL); + error= my_errno= EE_LOCAL_TMP_SPACE_FULL; + goto exit; } set_if_bigger(global_status_var.max_tmp_space_used, cached_space); } @@ -3828,8 +3833,12 @@ static int temp_file_size_cb_func(struct tmp_file_tracking *track, /* Record that we have registered the change */ track->previous_file_size= track->file_size; + +exit: + if (unlikely(thd->shared_thd)) + mysql_mutex_unlock(&thd->LOCK_thd_data); } - DBUG_RETURN(0); + DBUG_RETURN(error); } int json_escape_string(const char *str,const char *str_end, diff --git a/sql/sql_class.cc b/sql/sql_class.cc index 88d0b2c6577..4975fbbe12e 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -813,6 +813,7 @@ THD::THD(my_thread_id id, bool is_wsrep_applier) net.reading_or_writing= 0; client_capabilities= 0; // minimalistic client system_thread= NON_SYSTEM_THREAD; + shared_thd= 0; cleanup_done= free_connection_done= abort_on_warning= got_warning= 0; peer_port= 0; // For SHOW PROCESSLIST transaction= &default_transaction; diff --git a/sql/sql_class.h b/sql/sql_class.h index cae7b21fe92..bdcfaf4b99b 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -3127,6 +3127,8 @@ public: Trans_binlog_info *semisync_info; /* If this is a semisync slave connection. */ bool semi_sync_slave; + /* Several threads may share this thd. Used with parallel repair */ + bool shared_thd; ulonglong client_capabilities; /* What the client supports */ ulong max_client_packet_length; diff --git a/sql/sql_select.cc b/sql/sql_select.cc index 35b0513b162..07061eeac8c 100644 --- a/sql/sql_select.cc +++ b/sql/sql_select.cc @@ -22797,7 +22797,7 @@ create_internal_tmp_table_from_heap(THD *thd, TABLE *table, if (unlikely(thd->check_killed())) goto err_killed; } - if (!new_table.no_rows && new_table.file->ha_end_bulk_insert()) + if (!new_table.no_rows && (write_err= new_table.file->ha_end_bulk_insert())) goto err; /* copy row that filled HEAP table */ if (unlikely((write_err=new_table.file->ha_write_tmp_row(table->record[0])))) diff --git a/storage/maria/ha_maria.cc b/storage/maria/ha_maria.cc index cb13db946aa..703a2d082ca 100644 --- a/storage/maria/ha_maria.cc +++ b/storage/maria/ha_maria.cc @@ -1635,6 +1635,19 @@ int ha_maria::optimize(THD * thd, HA_CHECK_OPT *check_opt) return error; } +/* + Set current_thd() for parallel worker thread +*/ + +C_MODE_START +void maria_setup_thd_for_repair_thread(void *arg) +{ + THD *thd= (THD*) arg; + DBUG_ASSERT(thd->shared_thd); + set_current_thd(thd); +} +C_MODE_END + int ha_maria::repair(THD *thd, HA_CHECK *param, bool do_optimize) { @@ -1726,8 +1739,17 @@ int ha_maria::repair(THD *thd, HA_CHECK *param, bool do_optimize) my_snprintf(buf, 40, "Repair with %d threads", my_count_bits(key_map)); thd_proc_info(thd, buf); param->testflag|= T_REP_PARALLEL; + /* + Ensure that all threads are using the same THD. This is needed + to get limit of tmp files to work + */ + param->init_repair_thread= maria_setup_thd_for_repair_thread; + param->init_repair_thread_arg= table->in_use; + /* Mark that multiple threads are using the thd */ + table->in_use->shared_thd= 1; error= maria_repair_parallel(param, file, fixed_name, MY_TEST(param->testflag & T_QUICK)); + table->in_use->shared_thd= 0; /* to reset proc_info, as it was pointing to local buffer */ thd_proc_info(thd, "Repair done"); } @@ -2100,12 +2122,13 @@ int ha_maria::enable_indexes(key_map map, bool persist) This can be set when doing an ALTER TABLE and enabling unique keys */ if ((error= (repair(thd, param, 0) != HA_ADMIN_OK)) && param->retry_repair && + !file->s->internal_table && (my_errno != HA_ERR_FOUND_DUPP_KEY || !file->create_unique_index_by_sort)) { - sql_print_warning("Warning: Enabling keys got errno %d on %s.%s, " + sql_print_warning("Warning: Enabling keys got errno %d on %s, " "retrying", - my_errno, param->db_name, param->table_name); + my_errno, file->s->open_file_name); /* Repairing by sort failed. Now try standard repair method. */ param->testflag &= ~T_REP_BY_SORT; file->state->records= start_rows; @@ -2398,7 +2421,7 @@ int ha_maria::end_bulk_insert() can_enable_indexes= 0; if (first_error) my_errno= first_errno; - DBUG_RETURN(first_error); + DBUG_RETURN(first_errno); } diff --git a/storage/maria/ma_check.c b/storage/maria/ma_check.c index 8d48ee1fc97..1bcc5a99252 100644 --- a/storage/maria/ma_check.c +++ b/storage/maria/ma_check.c @@ -2978,8 +2978,9 @@ err: if (got_error) { if (! param->error_printed) - _ma_check_print_error(param,"%d for record at pos %s",my_errno, - llstr(sort_param.start_recpos,llbuff)); + _ma_check_print_error(param,"Got error %d for record at pos %s when creating index", + my_errno, + llstr(sort_param.start_recpos,llbuff)); (void)_ma_flush_table_files_before_swap(param, info); if (sort_info.new_info && sort_info.new_info != sort_info.info) { @@ -4218,7 +4219,7 @@ err: if (got_error) { if (! param->error_printed) - _ma_check_print_error(param,"%d when fixing table",my_errno); + _ma_check_print_error(param,"Got error %d when trying to repair table",my_errno); (void)_ma_flush_table_files_before_swap(param, info); if (sort_info.new_info && sort_info.new_info != sort_info.info) { @@ -4488,6 +4489,7 @@ int maria_repair_parallel(HA_CHECK *param, register MARIA_HA *info, for (i=key=0, istep=1 ; key < share->base.keys ; rec_per_key_part+=sort_param[i].keyinfo->keysegs, i+=istep, key++) { + sort_param[i].check_param= param; sort_param[i].key=key; sort_param[i].keyinfo=share->keyinfo+key; sort_param[i].seg=sort_param[i].keyinfo->seg; @@ -4793,7 +4795,8 @@ err: if (got_error) { if (! param->error_printed) - _ma_check_print_error(param,"%d when fixing table",my_errno); + _ma_check_print_error(param,"Got error %d when repairing table with parallel repair", + my_errno); (void)_ma_flush_table_files_before_swap(param, info); if (new_file >= 0) { diff --git a/storage/maria/ma_sort.c b/storage/maria/ma_sort.c index de6f5b8b288..8f057e85f90 100644 --- a/storage/maria/ma_sort.c +++ b/storage/maria/ma_sort.c @@ -543,8 +543,16 @@ pthread_handler_t _ma_thr_find_all_keys(void *arg) MARIA_SORT_PARAM *sort_param= (MARIA_SORT_PARAM*) arg; my_bool error= FALSE; /* If my_thread_init fails */ - if (my_thread_init() || _ma_thr_find_all_keys_exec(sort_param)) + if (my_thread_init()) error= TRUE; + else + { + HA_CHECK *check= sort_param->check_param; + if (check->init_repair_thread) + check->init_repair_thread(check->init_repair_thread_arg); + if (_ma_thr_find_all_keys_exec(sort_param)) + error= TRUE; + } /* Thread must clean up after itself. diff --git a/storage/maria/maria_def.h b/storage/maria/maria_def.h index 387a3c9a67f..c5e0f23d54a 100644 --- a/storage/maria/maria_def.h +++ b/storage/maria/maria_def.h @@ -396,6 +396,7 @@ typedef struct st_maria_sort_param MEM_ROOT wordroot; uchar *record; MY_TMPDIR *tmpdir; + HA_CHECK *check_param; /* The next two are used to collect statistics, see maria_update_key_parts for diff --git a/storage/myisam/ha_myisam.cc b/storage/myisam/ha_myisam.cc index b52c4b7e374..69626f881f5 100644 --- a/storage/myisam/ha_myisam.cc +++ b/storage/myisam/ha_myisam.cc @@ -710,16 +710,6 @@ my_bool mi_killed_in_mariadb(MI_INFO *info) return (((TABLE*) (info->external_ref))->in_use->killed != 0); } -static void init_compute_vcols(void *table) -{ - /* - To evaluate vcols we must have current_thd set. - This will set current_thd in all threads to the same THD, but it's - safe, because vcols are always evaluated under info->s->intern_lock. - */ - set_current_thd(static_cast(table)->in_use); -} - static int compute_vcols(MI_INFO *info, uchar *record, int keynum) { /* This mutex is needed for parallel repair */ @@ -1032,7 +1022,6 @@ void ha_myisam::setup_vcols_for_repair(HA_CHECK *param) } DBUG_ASSERT(file->s->base.reclength < file->s->vreclength || !table->s->stored_fields); - param->init_fix_record= init_compute_vcols; param->fix_record= compute_vcols; table->use_all_columns(); } @@ -1286,6 +1275,25 @@ int ha_myisam::optimize(THD* thd, HA_CHECK_OPT *check_opt) } +/* + Set current_thd() for parallel worker thread + This is needed to evaluate vcols as we must have current_thd set. + This will set current_thd in all threads to the same THD, but it's + safe, because vcols are always evaluated under info->s->intern_lock. + + This is also used temp_file_size_cb_func() to tmp_space_usage by THD. +*/ + +C_MODE_START +void myisam_setup_thd_for_repair_thread(void *arg) +{ + THD *thd= (THD*) arg; + DBUG_ASSERT(thd->shared_thd); + set_current_thd(thd); +} +C_MODE_END + + int ha_myisam::repair(THD *thd, HA_CHECK ¶m, bool do_optimize) { int error=0; @@ -1357,8 +1365,18 @@ int ha_myisam::repair(THD *thd, HA_CHECK ¶m, bool do_optimize) { /* TODO: respect myisam_repair_threads variable */ thd_proc_info(thd, "Parallel repair"); - error = mi_repair_parallel(¶m, file, fixed_name, - MY_TEST(param.testflag & T_QUICK)); + param.testflag|= T_REP_PARALLEL; + /* + Ensure that all threads are using the same THD. This is needed + to get limit of tmp files to work + */ + param.init_repair_thread= myisam_setup_thd_for_repair_thread; + param.init_repair_thread_arg= table->in_use; + /* Mark that multiple threads are using the thd */ + table->in_use->shared_thd= 1; + error= mi_repair_parallel(¶m, file, fixed_name, + MY_TEST(param.testflag & T_QUICK)); + table->in_use->shared_thd= 0; } else { diff --git a/storage/myisam/mi_check.c b/storage/myisam/mi_check.c index b9e3a4b1c1d..826c4ce5b8e 100644 --- a/storage/myisam/mi_check.c +++ b/storage/myisam/mi_check.c @@ -2813,6 +2813,7 @@ int mi_repair_parallel(HA_CHECK *param, register MI_INFO *info, for (i=key=0, istep=1 ; key < share->base.keys ; rec_per_key_part+=sort_param[i].keyinfo->keysegs, i+=istep, key++) { + sort_param[i].check_param= param; sort_param[i].key=key; sort_param[i].keyinfo=share->keyinfo+key; sort_param[i].seg=sort_param[i].keyinfo->seg; diff --git a/storage/myisam/sort.c b/storage/myisam/sort.c index 0fb475c4d72..15a8cc60574 100644 --- a/storage/myisam/sort.c +++ b/storage/myisam/sort.c @@ -530,13 +530,17 @@ pthread_handler_t thr_find_all_keys(void *arg) MI_SORT_PARAM *sort_param= (MI_SORT_PARAM*) arg; my_bool error= FALSE; - MI_SORT_INFO *si= sort_param->sort_info; - if (si->param->init_fix_record) - si->param->init_fix_record(si->info->external_ref); - /* If my_thread_init fails */ - if (my_thread_init() || thr_find_all_keys_exec(sort_param)) + if (my_thread_init()) error= TRUE; + else + { + HA_CHECK *check= sort_param->check_param; + if (check->init_repair_thread) + check->init_repair_thread(check->init_repair_thread_arg); + if (thr_find_all_keys_exec(sort_param)) + error= TRUE; + } /* Thread must clean up after itself.