From 519060da452925a2f8274b1d30c7518830299948 Mon Sep 17 00:00:00 2001 From: Jacob Mathew Date: Mon, 21 May 2018 19:17:03 -0700 Subject: [PATCH 01/26] MDEV-12900: spider tests failed in buildbot with valgrind The failures with valgrind occur as a result of Spider sometimes using the wrong transaction for operations in background threads that send requests to the data nodes. The use of the wrong transaction caused the networking to the data nodes to use the wrong thread in some cases. Valgrind eventually detects this when such a thread is destroyed before it is used to disconnect from the data node by that wrong transaction when it is freed. I have fixed the problem by correcting the transaction used in each of these cases. Author: Jacob Mathew. Reviewer: Kentoku Shiba. Cherry-Picked: Commit afe5a51 on branch 10.2 --- storage/spider/spd_conn.cc | 29 ++++++------------------ storage/spider/spd_ping_table.cc | 22 ------------------ storage/spider/spd_table.cc | 39 -------------------------------- 3 files changed, 7 insertions(+), 83 deletions(-) diff --git a/storage/spider/spd_conn.cc b/storage/spider/spd_conn.cc index fc3d9ecc25f..5de41ac0f98 100644 --- a/storage/spider/spd_conn.cc +++ b/storage/spider/spd_conn.cc @@ -69,9 +69,6 @@ extern PSI_thread_key spd_key_thd_bg_crd; extern PSI_thread_key spd_key_thd_bg_mon; #endif #endif - -extern pthread_mutex_t spider_global_trx_mutex; -extern SPIDER_TRX *spider_global_trx; #endif HASH spider_open_connections; @@ -2807,9 +2804,6 @@ void *spider_bg_sts_action( DBUG_RETURN(NULL); } share->bg_sts_thd = thd; -/* - spider.trx = spider_global_trx; -*/ spider.trx = trx; spider.share = share; spider.conns = conns; @@ -2922,13 +2916,11 @@ void *spider_bg_sts_action( { if (!conns[spider.search_link_idx]) { - pthread_mutex_lock(&spider_global_trx_mutex); spider_get_conn(share, spider.search_link_idx, share->conn_keys[spider.search_link_idx], - spider_global_trx, &spider, FALSE, FALSE, SPIDER_CONN_KIND_MYSQL, + trx, &spider, FALSE, FALSE, SPIDER_CONN_KIND_MYSQL, &error_num); conns[spider.search_link_idx]->error_mode = 0; - pthread_mutex_unlock(&spider_global_trx_mutex); /* if ( error_num && @@ -2937,7 +2929,7 @@ void *spider_bg_sts_action( ) { lex_start(thd); error_num = spider_ping_table_mon_from_table( - spider_global_trx, + trx, thd, share, (uint32) share->monitoring_sid[spider.search_link_idx], @@ -2958,7 +2950,6 @@ void *spider_bg_sts_action( } if (spider.search_link_idx != -1 && conns[spider.search_link_idx]) { - DBUG_ASSERT(!conns[spider.search_link_idx]->thd); #ifdef WITH_PARTITION_STORAGE_ENGINE if (spider_get_sts(share, spider.search_link_idx, share->bg_sts_try_time, &spider, @@ -2979,7 +2970,7 @@ void *spider_bg_sts_action( ) { lex_start(thd); error_num = spider_ping_table_mon_from_table( - spider_global_trx, + trx, thd, share, (uint32) share->monitoring_sid[spider.search_link_idx], @@ -3192,9 +3183,6 @@ void *spider_bg_crd_action( table.s = share->table_share; table.field = share->table_share->field; table.key_info = share->table_share->key_info; -/* - spider.trx = spider_global_trx; -*/ spider.trx = trx; spider.change_table_ptr(&table, share->table_share); spider.share = share; @@ -3308,13 +3296,11 @@ void *spider_bg_crd_action( { if (!conns[spider.search_link_idx]) { - pthread_mutex_lock(&spider_global_trx_mutex); spider_get_conn(share, spider.search_link_idx, share->conn_keys[spider.search_link_idx], - spider_global_trx, &spider, FALSE, FALSE, SPIDER_CONN_KIND_MYSQL, + trx, &spider, FALSE, FALSE, SPIDER_CONN_KIND_MYSQL, &error_num); conns[spider.search_link_idx]->error_mode = 0; - pthread_mutex_unlock(&spider_global_trx_mutex); /* if ( error_num && @@ -3323,7 +3309,7 @@ void *spider_bg_crd_action( ) { lex_start(thd); error_num = spider_ping_table_mon_from_table( - spider_global_trx, + trx, thd, share, (uint32) share->monitoring_sid[spider.search_link_idx], @@ -3344,7 +3330,6 @@ void *spider_bg_crd_action( } if (spider.search_link_idx != -1 && conns[spider.search_link_idx]) { - DBUG_ASSERT(!conns[spider.search_link_idx]->thd); #ifdef WITH_PARTITION_STORAGE_ENGINE if (spider_get_crd(share, spider.search_link_idx, share->bg_crd_try_time, &spider, &table, @@ -3365,7 +3350,7 @@ void *spider_bg_crd_action( ) { lex_start(thd); error_num = spider_ping_table_mon_from_table( - spider_global_trx, + trx, thd, share, (uint32) share->monitoring_sid[spider.search_link_idx], @@ -3717,7 +3702,7 @@ void *spider_bg_mon_action( { lex_start(thd); error_num = spider_ping_table_mon_from_table( - spider_global_trx, + trx, thd, share, (uint32) share->monitoring_sid[link_idx], diff --git a/storage/spider/spd_ping_table.cc b/storage/spider/spd_ping_table.cc index 77a2969d061..6f04e08dc93 100644 --- a/storage/spider/spd_ping_table.cc +++ b/storage/spider/spd_ping_table.cc @@ -52,11 +52,6 @@ extern PSI_mutex_key spd_key_mutex_mon_list_update_status; extern PSI_mutex_key spd_key_mutex_mon_table_cache; #endif -#ifndef WITHOUT_SPIDER_BG_SEARCH -extern pthread_mutex_t spider_global_trx_mutex; -extern SPIDER_TRX *spider_global_trx; -#endif - HASH *spider_udf_table_mon_list_hash; uint spider_udf_table_mon_list_hash_id; const char *spider_udf_table_mon_list_hash_func_name; @@ -130,7 +125,6 @@ SPIDER_TABLE_MON_LIST *spider_get_ping_table_mon_list( ) #endif { - DBUG_ASSERT(trx != spider_global_trx); if ( table_mon_list && table_mon_list->mon_table_cache_version != mon_table_cache_version @@ -608,29 +602,17 @@ SPIDER_CONN *spider_get_ping_table_tgt_conn( ) { SPIDER_CONN *conn; DBUG_ENTER("spider_get_ping_table_tgt_conn"); -#ifndef WITHOUT_SPIDER_BG_SEARCH - if (trx == spider_global_trx) - pthread_mutex_lock(&spider_global_trx_mutex); -#endif if ( !(conn = spider_get_conn( share, 0, share->conn_keys[0], trx, NULL, FALSE, FALSE, SPIDER_CONN_KIND_MYSQL, error_num)) ) { -#ifndef WITHOUT_SPIDER_BG_SEARCH - if (trx == spider_global_trx) - pthread_mutex_unlock(&spider_global_trx_mutex); -#endif my_error(ER_CONNECT_TO_FOREIGN_DATA_SOURCE, MYF(0), share->server_names[0]); *error_num = ER_CONNECT_TO_FOREIGN_DATA_SOURCE; goto error; } #ifndef DBUG_OFF - if (trx == spider_global_trx) - { - DBUG_ASSERT(!conn->thd); - } DBUG_PRINT("info",("spider conn->thd=%p", conn->thd)); if (conn->thd) { @@ -638,10 +620,6 @@ SPIDER_CONN *spider_get_ping_table_tgt_conn( } #endif conn->error_mode = 0; -#ifndef WITHOUT_SPIDER_BG_SEARCH - if (trx == spider_global_trx) - pthread_mutex_unlock(&spider_global_trx_mutex); -#endif DBUG_RETURN(conn); error: diff --git a/storage/spider/spd_table.cc b/storage/spider/spd_table.cc index 2f666fef2f4..130edc5ee8a 100644 --- a/storage/spider/spd_table.cc +++ b/storage/spider/spd_table.cc @@ -98,9 +98,6 @@ PSI_mutex_key spd_key_mutex_conn; PSI_mutex_key spd_key_mutex_hs_r_conn; PSI_mutex_key spd_key_mutex_hs_w_conn; #endif -#ifndef WITHOUT_SPIDER_BG_SEARCH -PSI_mutex_key spd_key_mutex_global_trx; -#endif PSI_mutex_key spd_key_mutex_open_conn; PSI_mutex_key spd_key_mutex_allocated_thds; PSI_mutex_key spd_key_mutex_mon_table_cache; @@ -144,9 +141,6 @@ static PSI_mutex_info all_spider_mutexes[]= #if defined(HS_HAS_SQLCOM) && defined(HAVE_HANDLERSOCKET) { &spd_key_mutex_hs_r_conn, "hs_r_conn", PSI_FLAG_GLOBAL}, { &spd_key_mutex_hs_w_conn, "hs_w_conn", PSI_FLAG_GLOBAL}, -#endif -#ifndef WITHOUT_SPIDER_BG_SEARCH - { &spd_key_mutex_global_trx, "global_trx", PSI_FLAG_GLOBAL}, #endif { &spd_key_mutex_open_conn, "open_conn", PSI_FLAG_GLOBAL}, { &spd_key_mutex_allocated_thds, "allocated_thds", PSI_FLAG_GLOBAL}, @@ -301,9 +295,6 @@ pthread_mutex_t spider_allocated_thds_mutex; #ifndef WITHOUT_SPIDER_BG_SEARCH pthread_attr_t spider_pt_attr; - -pthread_mutex_t spider_global_trx_mutex; -SPIDER_TRX *spider_global_trx; #endif extern pthread_mutex_t spider_mem_calc_mutex; @@ -6090,10 +6081,6 @@ int spider_db_done( do_delete_thd = TRUE; } -#ifndef WITHOUT_SPIDER_BG_SEARCH - spider_free_trx(spider_global_trx, TRUE); -#endif - for (roop_count = SPIDER_DBTON_SIZE - 1; roop_count >= 0; roop_count--) { if (spider_dbton[roop_count].deinit) @@ -6261,9 +6248,6 @@ int spider_db_done( pthread_mutex_destroy(&spider_mon_table_cache_mutex); pthread_mutex_destroy(&spider_allocated_thds_mutex); pthread_mutex_destroy(&spider_open_conn_mutex); -#ifndef WITHOUT_SPIDER_BG_SEARCH - pthread_mutex_destroy(&spider_global_trx_mutex); -#endif #if defined(HS_HAS_SQLCOM) && defined(HAVE_HANDLERSOCKET) pthread_mutex_destroy(&spider_hs_w_conn_mutex); pthread_mutex_destroy(&spider_hs_r_conn_mutex); @@ -6510,18 +6494,6 @@ int spider_db_init( error_num = HA_ERR_OUT_OF_MEM; goto error_conn_mutex_init; } -#ifndef WITHOUT_SPIDER_BG_SEARCH -#if MYSQL_VERSION_ID < 50500 - if (pthread_mutex_init(&spider_global_trx_mutex, MY_MUTEX_INIT_FAST)) -#else - if (mysql_mutex_init(spd_key_mutex_global_trx, - &spider_global_trx_mutex, MY_MUTEX_INIT_FAST)) -#endif - { - error_num = HA_ERR_OUT_OF_MEM; - goto error_global_trx_mutex_init; - } -#endif #if MYSQL_VERSION_ID < 50500 if (pthread_mutex_init(&spider_open_conn_mutex, MY_MUTEX_INIT_FAST)) #else @@ -6786,16 +6758,9 @@ int spider_db_init( } } -#ifndef WITHOUT_SPIDER_BG_SEARCH - if (!(spider_global_trx = spider_get_trx(NULL, FALSE, &error_num))) - goto error; -#endif - DBUG_RETURN(0); #ifndef WITHOUT_SPIDER_BG_SEARCH -error: - roop_count = SPIDER_DBTON_SIZE; error_init_dbton: for (roop_count--; roop_count >= 0; roop_count--) { @@ -6897,10 +6862,6 @@ error_hs_r_conn_mutex_init: #endif pthread_mutex_destroy(&spider_open_conn_mutex); error_open_conn_mutex_init: -#ifndef WITHOUT_SPIDER_BG_SEARCH - pthread_mutex_destroy(&spider_global_trx_mutex); -error_global_trx_mutex_init: -#endif pthread_mutex_destroy(&spider_conn_mutex); error_conn_mutex_init: pthread_mutex_destroy(&spider_lgtm_tblhnd_share_mutex); From 5797cbaf4ab93cdae191486c6c82be79b83c56d2 Mon Sep 17 00:00:00 2001 From: sachin Date: Fri, 18 May 2018 17:35:33 +0530 Subject: [PATCH 02/26] MDEV-10259 mysqld crash with certain statement length and... order with Galera and encrypt-tmp-files=1 Problem:- If trans_cache (IO_CACHE) uses encrypted tmp file then on next DML server will crash. Case:- Lets take a case , we have a table t1 , We try to do 2 inserts in t1 1. A really long insert so that trans_cache has to use temp_file 2. Just a small insert Analysis:- Actually server crashes from inside of galera library. /lib64/libc.so.6(abort+0x175)[0x7fb5ba779dc5] /usr/lib64/galera/libgalera_smm.so(_ZN6galera3FSMINS_9TrxHandle5State... mysys/stacktrace.c:247(my_print_stacktrace)[0x7fb5a714940e] sql/signal_handler.cc:160(handle_fatal_signal)[0x7fb5a715c1bd] sql/wsrep_hton.cc:257(wsrep_rollback)[0x7fb5bcce923a] sql/wsrep_hton.cc:268(wsrep_rollback)[0x7fb5bcce9368] sql/handler.cc:1658(ha_rollback_trans(THD*, bool))[0x7fb5bcd4f41a] sql/handler.cc:1483(ha_commit_trans(THD*, bool))[0x7fb5bcd4f804] but actual issue is not in galera but in mariadb, because for 2nd insert we should never call rollback. We are calling rollback because log_and_order fails it fails because write_cache fails , It fails because after reinit_io_cache(trans_cache) , my_b_bytes_in_cache says 0 so we look into tmp_file for data , which is obviously wrong since temp was used for previous insert and it no longer exist. wsrep_write_cache_inc() reads the IO_CACHE in a loop, filling it with my_b_fill() until it returns "0 bytes read". Later MYSQL_BIN_LOG::write_cache() does the same. wsrep_write_cache_inc() assumes that reading a zero bytes past EOF leaves the old data in the cache Solution:- There is two issue in my_b_encr_read 1st we should never equal read_end to info->buffer. I mean this does not make sense read_end should always point to end of buffer. 2nd For most of the case(apart from async IO_CACHE) info->pos_in_file should be equal to info->buffer position wrt to temp file , since in this case we are not changing info->buffer it should remain unchanged. --- .../galera/r/galera_encrypt_tmp_files.result | 35 +++++++++ .../galera/t/galera_encrypt_tmp_files.cnf | 8 +++ .../galera/t/galera_encrypt_tmp_files.test | 57 +++++++++++++++ sql/mf_iocache_encr.cc | 4 +- unittest/sql/mf_iocache-t.cc | 72 ++++++++++++++++++- 5 files changed, 173 insertions(+), 3 deletions(-) create mode 100644 mysql-test/suite/galera/r/galera_encrypt_tmp_files.result create mode 100644 mysql-test/suite/galera/t/galera_encrypt_tmp_files.cnf create mode 100644 mysql-test/suite/galera/t/galera_encrypt_tmp_files.test diff --git a/mysql-test/suite/galera/r/galera_encrypt_tmp_files.result b/mysql-test/suite/galera/r/galera_encrypt_tmp_files.result new file mode 100644 index 00000000000..63f6c281af1 --- /dev/null +++ b/mysql-test/suite/galera/r/galera_encrypt_tmp_files.result @@ -0,0 +1,35 @@ +SELECT VARIABLE_VALUE = 'Synced' FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_local_state_comment'; +VARIABLE_VALUE = 'Synced' +1 +SELECT VARIABLE_VALUE = 2 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size'; +VARIABLE_VALUE = 2 +1 +CREATE TABLE t1 (f1 INTEGER PRIMARY KEY) Engine=InnoDB; +INSERT INTO t1 VALUES (1); +SELECT VARIABLE_VALUE = 'Synced' FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_local_state_comment'; +VARIABLE_VALUE = 'Synced' +1 +SELECT VARIABLE_VALUE = 2 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size'; +VARIABLE_VALUE = 2 +1 +SELECT COUNT(*) = 1 FROM t1; +COUNT(*) = 1 +1 +DROP TABLE t1; +CREATE TABLE `t1` ( +`col1` int(11) NOT NULL, +`col2` varchar(64) NOT NULL DEFAULT '', +`col3` varchar(32) NOT NULL DEFAULT '0', +`col4` varchar(64) NOT NULL DEFAULT '', +`col5` tinyint(4) NOT NULL DEFAULT '0', +`col6` int(11) NOT NULL DEFAULT '0', +`col7` varchar(64) NOT NULL DEFAULT '', +`col8` tinyint(4) NOT NULL DEFAULT '0', +`col9` tinyint(4) NOT NULL DEFAULT '0', +`col10` text NOT NULL, +`col11` varchar(255) NOT NULL DEFAULT '', +`col12` tinyint(4) NOT NULL DEFAULT '1' +) ; +create table t2 (test int); +insert into t2 values (1); +drop table t1,t2; diff --git a/mysql-test/suite/galera/t/galera_encrypt_tmp_files.cnf b/mysql-test/suite/galera/t/galera_encrypt_tmp_files.cnf new file mode 100644 index 00000000000..0f7f80b7d0b --- /dev/null +++ b/mysql-test/suite/galera/t/galera_encrypt_tmp_files.cnf @@ -0,0 +1,8 @@ +!include ../galera_2nodes.cnf +[mysqld] + +encrypt-tmp-files = 1 +plugin-load-add= @ENV.FILE_KEY_MANAGEMENT_SO +file-key-management +loose-file-key-management-filename= @ENV.MYSQL_TEST_DIR/std_data/keys.txt +log-bin diff --git a/mysql-test/suite/galera/t/galera_encrypt_tmp_files.test b/mysql-test/suite/galera/t/galera_encrypt_tmp_files.test new file mode 100644 index 00000000000..c42c3dbd98a --- /dev/null +++ b/mysql-test/suite/galera/t/galera_encrypt_tmp_files.test @@ -0,0 +1,57 @@ +# This file tests that mariadb cluster should not crash when encrypt_tmp_file +# is enabled + +--source include/galera_cluster.inc +--source include/have_innodb.inc + +SELECT VARIABLE_VALUE = 'Synced' FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_local_state_comment'; +SELECT VARIABLE_VALUE = 2 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size'; + +CREATE TABLE t1 (f1 INTEGER PRIMARY KEY) Engine=InnoDB; +INSERT INTO t1 VALUES (1); + +--connection node_2 +SELECT VARIABLE_VALUE = 'Synced' FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_local_state_comment'; +SELECT VARIABLE_VALUE = 2 FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_cluster_size'; + +SELECT COUNT(*) = 1 FROM t1; + +DROP TABLE t1; + +--connection node_1 + +CREATE TABLE `t1` ( + `col1` int(11) NOT NULL, + `col2` varchar(64) NOT NULL DEFAULT '', + `col3` varchar(32) NOT NULL DEFAULT '0', + `col4` varchar(64) NOT NULL DEFAULT '', + `col5` tinyint(4) NOT NULL DEFAULT '0', + `col6` int(11) NOT NULL DEFAULT '0', + `col7` varchar(64) NOT NULL DEFAULT '', + `col8` tinyint(4) NOT NULL DEFAULT '0', + `col9` tinyint(4) NOT NULL DEFAULT '0', + `col10` text NOT NULL, + `col11` varchar(255) NOT NULL DEFAULT '', + `col12` tinyint(4) NOT NULL DEFAULT '1' +) ; + +#Although we just need $counter >= 907 for IO_CACHE to use +#encrypted temp file. Just on safe side I am using $counter +# = 1100 +--disable_query_log +--let $counter=1100 +--let $query= (1,'test','test','test',0,0,'-1',0,0,'','',-1) +while($counter) +{ + --let $query= $query ,(1,'test','test','test',0,0,'-1',0,0,'','',-1) + --dec $counter +} +--let $query= INSERT INTO t1 values $query ; +--eval $query +--enable_query_log +#INSERT INTO `t1` VALUE + +create table t2 (test int); +insert into t2 values (1); + +drop table t1,t2; diff --git a/sql/mf_iocache_encr.cc b/sql/mf_iocache_encr.cc index 149e6feb605..f26a437a25a 100644 --- a/sql/mf_iocache_encr.cc +++ b/sql/mf_iocache_encr.cc @@ -49,8 +49,8 @@ static int my_b_encr_read(IO_CACHE *info, uchar *Buffer, size_t Count) if (pos_in_file == info->end_of_file) { - info->read_pos= info->read_end= info->buffer; - info->pos_in_file= pos_in_file; + /* reading past EOF should not empty the cache */ + info->read_pos= info->read_end; info->error= 0; DBUG_RETURN(MY_TEST(Count)); } diff --git a/unittest/sql/mf_iocache-t.cc b/unittest/sql/mf_iocache-t.cc index 31f98562521..1b04f8eb0d3 100644 --- a/unittest/sql/mf_iocache-t.cc +++ b/unittest/sql/mf_iocache-t.cc @@ -187,10 +187,76 @@ void mdev9044() close_cached_file(&info); } +/* 2 Reads (with my_b_fill) in cache makes second read to fail */ +void mdev10259() +{ + int res; + uchar buf[200]; + memset(buf, FILL, sizeof(buf)); + + diag("MDEV-10259- mysqld crash with certain statement length and order with" + " Galera and encrypt-tmp-files=1"); + + init_io_cache_encryption(); + + res= open_cached_file(&info, 0, 0, CACHE_SIZE, 0); + ok(res == 0, "open_cached_file" INFO_TAIL); + + res= my_b_write(&info, buf, sizeof(buf)); + ok(res == 0 && info.pos_in_file == 0, "200 write" INFO_TAIL); + + res= my_b_flush_io_cache(&info, 1); + ok(res == 0, "flush" INFO_TAIL); + + ulong saved_pos= my_b_tell(&info); + res= reinit_io_cache(&info, READ_CACHE, 0, 0, 0); + ok(res == 0, "reinit READ_CACHE" INFO_TAIL); + + res= my_b_fill(&info); + ok(res == 200, "fill" INFO_TAIL); + + res= my_b_fill(&info); + ok(res == 0, "fill" INFO_TAIL); + + res= my_b_fill(&info); + ok(res == 0, "fill" INFO_TAIL); + + res= reinit_io_cache(&info, WRITE_CACHE, saved_pos, 0, 0); + ok(res == 0, "reinit WRITE_CACHE" INFO_TAIL); + + res= reinit_io_cache(&info, READ_CACHE, 0, 0, 0); + ok(res == 0, "reinit READ_CACHE" INFO_TAIL); + + ok(200 == my_b_bytes_in_cache(&info),"my_b_bytes_in_cache == 200"); + + res= my_b_fill(&info); + ok(res == 0, "fill" INFO_TAIL); + + res= my_b_fill(&info); + ok(res == 0, "fill" INFO_TAIL); + + res= my_b_fill(&info); + ok(res == 0, "fill" INFO_TAIL); + + res= reinit_io_cache(&info, WRITE_CACHE, saved_pos, 0, 0); + ok(res == 0, "reinit WRITE_CACHE" INFO_TAIL); + + res= reinit_io_cache(&info, READ_CACHE, 0, 0, 0); + ok(res == 0, "reinit READ_CACHE" INFO_TAIL); + + ok(200 == my_b_bytes_in_cache(&info),"my_b_bytes_in_cache == 200"); + + res= my_b_read(&info, buf, sizeof(buf)) || data_bad(buf, sizeof(buf)); + ok(res == 0 && info.pos_in_file == 0, "large read" INFO_TAIL); + + close_cached_file(&info); + +} + int main(int argc __attribute__((unused)),char *argv[]) { MY_INIT(argv[0]); - plan(29); + plan(46); /* temp files with and without encryption */ encrypt_tmp_files= 1; @@ -202,6 +268,10 @@ int main(int argc __attribute__((unused)),char *argv[]) /* regression tests */ mdev9044(); + encrypt_tmp_files= 1; + mdev10259(); + encrypt_tmp_files= 0; + my_end(0); return exit_status(); } From 2f3779d31cbbabcb171c22237253c82146118446 Mon Sep 17 00:00:00 2001 From: Monty Date: Sun, 20 May 2018 14:19:14 +0300 Subject: [PATCH 03/26] Fixes for Aria transaction handling with lock tables MDEV-10130 Assertion `share->in_trans == 0' failed in storage/maria/ma_close.c MDEV-10378 Assertion `trn' failed in virtual int ha_maria::start_stmt The problem was that maria_handler->trn was not properly reset at commit/rollback and ha_maria::exernal_lock() could get confused because. There was some old code in ha_maria::implicit_commit() that tried to take care of this, but it was not bullet proof. Fixed by adding list of all tables that is part of the maria transaction to TRN. A nice side effect was of the fix is that loops in ha_maria::implict_commit() got to be much simpler. Other things: - Fixed a bug in mysql_admin_table() where argument open_for_modify was wrongly reset for the next table in the chain - rollback admin command also in case of fatal error. - Split _ma_set_trn_for_table() to three version to simplify code and debugging. - Several new asserts to detect the original problem (that file was not properly removed from trn before calling ma_close()) --- mysql-test/suite/maria/lock.result | 10 +++ mysql-test/suite/maria/lock.test | 12 +++ sql/sql_admin.cc | 12 +-- sql/sql_handler.cc | 4 +- storage/maria/ha_maria.cc | 137 ++++++++++++----------------- storage/maria/ha_maria.h | 1 + storage/maria/ma_blockrec.c | 3 +- storage/maria/ma_close.c | 2 + storage/maria/ma_commit.c | 17 ++-- storage/maria/ma_extra.c | 4 +- storage/maria/ma_open.c | 4 +- storage/maria/ma_state.c | 48 ++++++++-- storage/maria/ma_state.h | 2 +- storage/maria/ma_trnman.h | 65 ++++++++++++++ storage/maria/maria_def.h | 14 +-- storage/maria/trnman.c | 1 + storage/maria/trnman.h | 3 +- 17 files changed, 219 insertions(+), 120 deletions(-) create mode 100644 storage/maria/ma_trnman.h diff --git a/mysql-test/suite/maria/lock.result b/mysql-test/suite/maria/lock.result index 90250568ef5..1c9d6b18100 100644 --- a/mysql-test/suite/maria/lock.result +++ b/mysql-test/suite/maria/lock.result @@ -99,3 +99,13 @@ f2 3 unlock tables; DROP TABLE t1,t2,tmp; +# +# MDEV-10378 Assertion `trn' failed in virtual int ha_maria::start_stmt +# +CREATE TABLE t1 (f1 VARCHAR(3), f2 INT, pk INT, PRIMARY KEY (pk)) ENGINE=Aria; +INSERT INTO t1 VALUES ('foo',10,1), ('foo',1,2); +LOCK TABLE t1 WRITE; +ALTER TABLE t1 ADD UNIQUE KEY (f1); +ERROR 23000: Duplicate entry 'foo' for key 'f1' +ALTER TABLE t1 ADD KEY (f2); +DROP TABLE t1; diff --git a/mysql-test/suite/maria/lock.test b/mysql-test/suite/maria/lock.test index 57447a18c55..4f3d4e8065e 100644 --- a/mysql-test/suite/maria/lock.test +++ b/mysql-test/suite/maria/lock.test @@ -105,3 +105,15 @@ INSERT INTO t2 (f2) SELECT f3 FROM tmp AS tmp_alias; select * from t2; unlock tables; DROP TABLE t1,t2,tmp; + +--echo # +--echo # MDEV-10378 Assertion `trn' failed in virtual int ha_maria::start_stmt +--echo # + +CREATE TABLE t1 (f1 VARCHAR(3), f2 INT, pk INT, PRIMARY KEY (pk)) ENGINE=Aria; +INSERT INTO t1 VALUES ('foo',10,1), ('foo',1,2); +LOCK TABLE t1 WRITE; +--error ER_DUP_ENTRY +ALTER TABLE t1 ADD UNIQUE KEY (f1); +ALTER TABLE t1 ADD KEY (f2); +DROP TABLE t1; diff --git a/sql/sql_admin.cc b/sql/sql_admin.cc index 4fe51a2189e..10fcbd56e18 100644 --- a/sql/sql_admin.cc +++ b/sql/sql_admin.cc @@ -302,7 +302,7 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables, HA_CHECK_OPT* check_opt, const char *operator_name, thr_lock_type lock_type, - bool open_for_modify, + bool org_open_for_modify, bool repair_table_use_frm, uint extra_open_options, int (*prepare_func)(THD *, TABLE_LIST *, @@ -359,10 +359,10 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables, for (table= tables; table; table= table->next_local) { char table_name[SAFE_NAME_LEN*2+2]; - char* db = table->db; + char *db= table->db; bool fatal_error=0; bool open_error; - + bool open_for_modify= org_open_for_modify; DBUG_PRINT("admin", ("table: '%s'.'%s'", table->db, table->table_name)); strxmov(table_name, db, ".", table->table_name, NullS); thd->open_options|= extra_open_options; @@ -395,8 +395,8 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables, /* CHECK TABLE command is allowed for views as well. Check on alter flags - to differentiate from ALTER TABLE...CHECK PARTITION on which view is not - allowed. + to differentiate from ALTER TABLE...CHECK PARTITION on which view is + not allowed. */ if (lex->alter_info.flags & Alter_info::ALTER_ADMIN_PARTITION || view_operator_func == NULL) @@ -1053,7 +1053,7 @@ send_result_message: } } /* Error path, a admin command failed. */ - if (thd->transaction_rollback_request) + if (thd->transaction_rollback_request || fatal_error) { /* Unlikely, but transaction rollback was requested by one of storage diff --git a/sql/sql_handler.cc b/sql/sql_handler.cc index 5fc7c20d409..52ce42e2e6a 100644 --- a/sql/sql_handler.cc +++ b/sql/sql_handler.cc @@ -155,10 +155,11 @@ static void mysql_ha_close_table(SQL_HANDLER *handler) { THD *thd= handler->thd; TABLE *table= handler->table; + DBUG_ENTER("mysql_ha_close_table"); /* check if table was already closed */ if (!table) - return; + DBUG_VOID_RETURN; if (!table->s->tmp_table) { @@ -184,6 +185,7 @@ static void mysql_ha_close_table(SQL_HANDLER *handler) } my_free(handler->lock); handler->init(); + DBUG_VOID_RETURN; } /* diff --git a/storage/maria/ha_maria.cc b/storage/maria/ha_maria.cc index dbd4a3331df..e56cc943390 100644 --- a/storage/maria/ha_maria.cc +++ b/storage/maria/ha_maria.cc @@ -37,6 +37,7 @@ C_MODE_START #include "ma_checkpoint.h" #include "ma_recovery.h" C_MODE_END +#include "ma_trnman.h" //#include "sql_priv.h" #include "protocol.h" @@ -1384,7 +1385,8 @@ int ha_maria::check(THD * thd, HA_CHECK_OPT * check_opt) } /* Reset trn, that may have been set by repair */ - _ma_set_trn_for_table(file, old_trn); + if (old_trn && old_trn != file->trn) + _ma_set_trn_for_table(file, old_trn); thd_proc_info(thd, old_proc_info); thd_progress_end(thd); return error ? HA_ADMIN_CORRUPT : HA_ADMIN_OK; @@ -1518,7 +1520,8 @@ int ha_maria::zerofill(THD * thd, HA_CHECK_OPT *check_opt) error=maria_zerofill(param, file, share->open_file_name.str); /* Reset trn, that may have been set by repair */ - _ma_set_trn_for_table(file, old_trn); + if (old_trn && old_trn != file->trn) + _ma_set_trn_for_table(file, old_trn); if (!error) { @@ -1758,7 +1761,8 @@ int ha_maria::repair(THD *thd, HA_CHECK *param, bool do_optimize) maria_lock_database(file, F_UNLCK); /* Reset trn, that may have been set by repair */ - _ma_set_trn_for_table(file, old_trn); + if (old_trn && old_trn != file->trn) + _ma_set_trn_for_table(file, old_trn); error= error ? HA_ADMIN_FAILED : (optimize_done ? (write_log_record_for_repair(param, file) ? HA_ADMIN_FAILED : @@ -2579,9 +2583,12 @@ int ha_maria::extra(enum ha_extra_function operation) without calling commit/rollback in between. If file->trn is not set we can't remove file->share from the transaction list in the extra() call. - We also ensure that we set file->trn to 0 if THD_TRN is 0 as in - this case we have already freed the trn. This can happen when one - implicit_commit() is called as part of alter table. + In current code we don't have to do this for HA_EXTRA_PREPARE_FOR_RENAME + as this is only used the intermediate table used by ALTER TABLE which + is not part of the transaction (it's not in the TRN list). Better to + keep this for now, to not break anything in a stable release. + When HA_EXTRA_PREPARE_FOR_RENAME is not handled below, we can change + the warnings in _ma_remove_table_from_trnman() to asserts. table->in_use is not set in the case this is a done as part of closefrm() as part of drop table. @@ -2594,7 +2601,7 @@ int ha_maria::extra(enum ha_extra_function operation) { THD *thd= table->in_use; TRN *trn= THD_TRN; - _ma_set_trn_for_table(file, trn); + _ma_set_tmp_trn_for_table(file, trn); } DBUG_ASSERT(file->s->base.born_transactional || file->trn == 0 || file->trn == &dummy_transaction_object); @@ -2710,6 +2717,7 @@ int ha_maria::external_lock(THD *thd, int lock_type) if (file->trn) { /* This can only happen with tables created with clone() */ + DBUG_PRINT("info",("file->trn: %p", file->trn)); trnman_increment_locked_tables(file->trn); } @@ -2730,7 +2738,7 @@ int ha_maria::external_lock(THD *thd, int lock_type) } else { - TRN *trn= THD_TRN; + TRN *trn= (file->trn != &dummy_transaction_object ? file->trn : 0); /* End of transaction */ /* @@ -2745,8 +2753,7 @@ int ha_maria::external_lock(THD *thd, int lock_type) */ if (_ma_reenable_logging_for_table(file, TRUE)) DBUG_RETURN(1); - /** @todo zero file->trn also in commit and rollback */ - _ma_set_trn_for_table(file, NULL); // Safety + _ma_reset_trn_for_table(file); /* Ensure that file->state points to the current number of rows. This is needed if someone calls maria_info() without first doing an @@ -2802,13 +2809,6 @@ int ha_maria::start_stmt(THD *thd, thr_lock_type lock_type) DBUG_ASSERT(lock_type != TL_UNLOCK); DBUG_ASSERT(file->trn == trn); - /* - If there was an implicit commit under this LOCK TABLES by a previous - statement (like a DDL), at least if that previous statement was about a - different ha_maria than 'this' then this->file->trn is a stale - pointer. We fix it: - */ - _ma_set_trn_for_table(file, trn); /* As external_lock() was already called, don't increment locked_tables. Note that we call the function below possibly several times when @@ -2833,6 +2833,23 @@ int ha_maria::start_stmt(THD *thd, thr_lock_type lock_type) } +/* + Reset THD_TRN and all file->trn related to the transaction + This is needed as some calls, like extra() or external_lock() may access + it before next transaction is started +*/ + +static void reset_thd_trn(THD *thd, MARIA_HA *first_table) +{ + DBUG_ENTER("reset_thd_trn"); + THD_TRN= NULL; + for (MARIA_HA *table= first_table; table ; + table= table->trn_next) + _ma_reset_trn_for_table(table); + DBUG_VOID_RETURN; +} + + /** Performs an implicit commit of the Maria transaction and creates a new one. @@ -2856,9 +2873,9 @@ int ha_maria::implicit_commit(THD *thd, bool new_trn) TRN *trn; int error; uint locked_tables; - DYNAMIC_ARRAY used_tables; - + MARIA_HA *used_tables, *trn_next; DBUG_ENTER("ha_maria::implicit_commit"); + if (!maria_hton || !(trn= THD_TRN)) DBUG_RETURN(0); if (!new_trn && (thd->locked_tables_mode == LTM_LOCK_TABLES || @@ -2876,48 +2893,16 @@ int ha_maria::implicit_commit(THD *thd, bool new_trn) locked_tables= trnman_has_locked_tables(trn); - if (new_trn && trn && trn->used_tables) - { - MARIA_USED_TABLES *tables; - /* - Save locked tables so that we can move them to another transaction - We are using a dynamic array as locked_tables in some cases can be - smaller than the used_tables list (for example when the server does - early unlock of tables. - */ - - my_init_dynamic_array2(&used_tables, sizeof(MARIA_SHARE*), (void*) 0, - locked_tables, 8, MYF(MY_THREAD_SPECIFIC)); - for (tables= (MARIA_USED_TABLES*) trn->used_tables; - tables; - tables= tables->next) - { - if (tables->share->base.born_transactional) - { - if (insert_dynamic(&used_tables, (uchar*) &tables->share)) - { - error= HA_ERR_OUT_OF_MEM; - goto end_and_free; - } - } - } - } - else - bzero(&used_tables, sizeof(used_tables)); - + used_tables= (MARIA_HA*) trn->used_instances; error= 0; if (unlikely(ma_commit(trn))) error= 1; if (!new_trn) { - /* - To be extra safe, we should also reset file->trn for all open - tables as some calls, like extra() may access it. We take care - of this in extra() by resetting file->trn if THD_TRN is 0. - */ - THD_TRN= NULL; + reset_thd_trn(thd, used_tables); goto end; } + /* We need to create a new transaction and put it in THD_TRN. Indeed, tables may be under LOCK TABLES, and so they will start the next @@ -2927,8 +2912,9 @@ int ha_maria::implicit_commit(THD *thd, bool new_trn) THD_TRN= trn; if (unlikely(trn == NULL)) { + reset_thd_trn(thd, used_tables); error= HA_ERR_OUT_OF_MEM; - goto end_and_free; + goto end; } /* Move all locked tables to the new transaction @@ -2938,35 +2924,25 @@ int ha_maria::implicit_commit(THD *thd, bool new_trn) in check table, we use the table without calling start_stmt(). */ - uint i; - for (i= 0 ; i < used_tables.elements ; i++) + for (MARIA_HA *handler= used_tables; handler ; + handler= trn_next) { - MARIA_SHARE *share; - LIST *handlers; + trn_next= handler->trn_next; + DBUG_ASSERT(handler->s->base.born_transactional); - share= *(dynamic_element(&used_tables, i, MARIA_SHARE**)); - /* Find table instances that was used in this transaction */ - for (handlers= share->open_list; handlers; handlers= handlers->next) + /* If handler uses versioning */ + if (handler->s->lock_key_trees) { - MARIA_HA *handler= (MARIA_HA*) handlers->data; - if (handler->external_ref && - ((TABLE*) handler->external_ref)->in_use == thd) - { - _ma_set_trn_for_table(handler, trn); - /* If handler uses versioning */ - if (handler->s->lock_key_trees) - { - if (_ma_setup_live_state(handler)) - error= HA_ERR_OUT_OF_MEM; - } - } + /* _ma_set_trn_for_table() will be called indirectly */ + if (_ma_setup_live_state(handler)) + error= HA_ERR_OUT_OF_MEM; } + else + _ma_set_trn_for_table(handler, trn); } /* This is just a commit, tables stay locked if they were: */ trnman_reset_locked_tables(trn, locked_tables); -end_and_free: - delete_dynamic(&used_tables); end: DBUG_RETURN(error); } @@ -3340,10 +3316,10 @@ static int maria_commit(handlerton *hton __attribute__ ((unused)), trnman_set_flags(trn, trnman_get_flags(trn) & ~TRN_STATE_INFO_LOGGED); /* statement or transaction ? */ - if ((thd->variables.option_bits & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN)) && !all) + if ((thd->variables.option_bits & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN)) && + !all) DBUG_RETURN(0); // end of statement - DBUG_PRINT("info", ("THD_TRN set to 0x0")); - THD_TRN= 0; + reset_thd_trn(thd, (MARIA_HA*) trn->used_instances); DBUG_RETURN(ma_commit(trn)); // end of transaction } @@ -3360,8 +3336,7 @@ static int maria_rollback(handlerton *hton __attribute__ ((unused)), trnman_rollback_statement(trn); DBUG_RETURN(0); // end of statement } - DBUG_PRINT("info", ("THD_TRN set to 0x0")); - THD_TRN= 0; + reset_thd_trn(thd, (MARIA_HA*) trn->used_instances); DBUG_RETURN(trnman_rollback_trn(trn) ? HA_ERR_OUT_OF_MEM : 0); // end of transaction } diff --git a/storage/maria/ha_maria.h b/storage/maria/ha_maria.h index 2b99c31ec5d..16adc0c19a2 100644 --- a/storage/maria/ha_maria.h +++ b/storage/maria/ha_maria.h @@ -193,6 +193,7 @@ public: private: DsMrr_impl ds_mrr; friend ICP_RESULT index_cond_func_maria(void *arg); + friend void reset_thd_trn(THD *thd); }; #endif /* HA_MARIA_INCLUDED */ diff --git a/storage/maria/ma_blockrec.c b/storage/maria/ma_blockrec.c index 9c92f8b63e5..ac46ac95101 100644 --- a/storage/maria/ma_blockrec.c +++ b/storage/maria/ma_blockrec.c @@ -271,6 +271,7 @@ #include "maria_def.h" #include "ma_blockrec.h" #include "trnman.h" +#include "ma_trnman.h" #include "ma_key_recover.h" #include "ma_recovery_util.h" #include @@ -7488,7 +7489,7 @@ void maria_ignore_trids(MARIA_HA *info) if (info->s->base.born_transactional) { if (!info->trn) - _ma_set_trn_for_table(info, &dummy_transaction_object); + _ma_set_tmp_trn_for_table(info, &dummy_transaction_object); /* Ignore transaction id when row is read */ info->trn->min_read_from= ~(TrID) 0; } diff --git a/storage/maria/ma_close.c b/storage/maria/ma_close.c index 4532b029126..75a996313e3 100644 --- a/storage/maria/ma_close.c +++ b/storage/maria/ma_close.c @@ -36,6 +36,8 @@ int maria_close(register MARIA_HA *info) /* Check that we have unlocked key delete-links properly */ DBUG_ASSERT(info->key_del_used == 0); + /* Check that file is not part of any uncommited transactions */ + DBUG_ASSERT(info->trn == 0 || info->trn == &dummy_transaction_object); if (share->reopen == 1) { diff --git a/storage/maria/ma_commit.c b/storage/maria/ma_commit.c index 46db3ca4ae5..7ad5e23e5f7 100644 --- a/storage/maria/ma_commit.c +++ b/storage/maria/ma_commit.c @@ -15,6 +15,7 @@ #include "maria_def.h" #include "trnman.h" +#include "ma_trnman.h" /** writes a COMMIT record to log and commits transaction in memory @@ -43,9 +44,9 @@ int ma_commit(TRN *trn) COMMIT record) and this is not an issue as * transaction's updates were not made visible to other transactions * "commit ok" was not sent to client - Alternatively, Recovery might commit trn (if MY_MIN(rec_lsn) is before COMMIT - record), which is ok too. All in all it means that "trn committed" is not - 100% equal to "COMMIT record written". + Alternatively, Recovery might commit trn (if MY_MIN(rec_lsn) is before + COMMIT record), which is ok too. All in all it means that "trn committed" + is not 100% equal to "COMMIT record written". - if COMMIT record is written after trnman_commit_trn(): if crash happens between the two, trn will be rolled back which is an issue (transaction's updates were made visible to other transactions). @@ -93,7 +94,12 @@ int ma_commit(TRN *trn) int maria_commit(MARIA_HA *info) { - return info->s->now_transactional ? ma_commit(info->trn) : 0; + TRN *trn; + if (!info->s->now_transactional) + return 0; + trn= info->trn; + info->trn= 0; /* checked in maria_close() */ + return ma_commit(trn); } @@ -120,10 +126,7 @@ int maria_begin(MARIA_HA *info) TRN *trn= trnman_new_trn(0); if (unlikely(!trn)) DBUG_RETURN(HA_ERR_OUT_OF_MEM); - - DBUG_PRINT("info", ("TRN set to 0x%lx", (ulong) trn)); _ma_set_trn_for_table(info, trn); } DBUG_RETURN(0); } - diff --git a/storage/maria/ma_extra.c b/storage/maria/ma_extra.c index d2e81184630..dc04e8aacde 100644 --- a/storage/maria/ma_extra.c +++ b/storage/maria/ma_extra.c @@ -345,7 +345,7 @@ int maria_extra(MARIA_HA *info, enum ha_extra_function function, _ma_decrement_open_count(info, 0); if (info->trn) { - _ma_remove_table_from_trnman(share, info->trn); + _ma_remove_table_from_trnman(info); /* Ensure we don't point to the deleted data in trn */ info->state= info->state_start= &share->state.state; } @@ -408,7 +408,7 @@ int maria_extra(MARIA_HA *info, enum ha_extra_function function, if (info->trn) { mysql_mutex_lock(&share->intern_lock); - _ma_remove_table_from_trnman(share, info->trn); + _ma_remove_table_from_trnman(info); /* Ensure we don't point to the deleted data in trn */ info->state= info->state_start= &share->state.state; mysql_mutex_unlock(&share->intern_lock); diff --git a/storage/maria/ma_open.c b/storage/maria/ma_open.c index 1db85180fcf..8fda846172b 100644 --- a/storage/maria/ma_open.c +++ b/storage/maria/ma_open.c @@ -19,6 +19,8 @@ #include "ma_sp_defs.h" #include "ma_rt_index.h" #include "ma_blockrec.h" +#include "trnman.h" +#include "ma_trnman.h" #include #if defined(MSDOS) || defined(__WIN__) @@ -183,7 +185,7 @@ static MARIA_HA *maria_clone_internal(MARIA_SHARE *share, if (!share->base.born_transactional) /* For transactional ones ... */ { /* ... force crash if no trn given */ - _ma_set_trn_for_table(&info, &dummy_transaction_object); + _ma_set_tmp_trn_for_table(&info, &dummy_transaction_object); info.state= &share->state.state; /* Change global values by default */ } else diff --git a/storage/maria/ma_state.c b/storage/maria/ma_state.c index 1b5a55cc91d..d3fa5acf198 100644 --- a/storage/maria/ma_state.c +++ b/storage/maria/ma_state.c @@ -66,7 +66,7 @@ my_bool _ma_setup_live_state(MARIA_HA *info) DBUG_RETURN(1); trn= info->trn; - for (tables= (MARIA_USED_TABLES*) info->trn->used_tables; + for (tables= (MARIA_USED_TABLES*) trn->used_tables; tables; tables= tables->next) { @@ -551,6 +551,7 @@ my_bool _ma_trnman_end_trans_hook(TRN *trn, my_bool commit, my_free(tables); } trn->used_tables= 0; + trn->used_instances= 0; DBUG_RETURN(error); } @@ -565,18 +566,25 @@ my_bool _ma_trnman_end_trans_hook(TRN *trn, my_bool commit, share->internal_lock must be locked when function is called */ -void _ma_remove_table_from_trnman(MARIA_SHARE *share, TRN *trn) +void _ma_remove_table_from_trnman(MARIA_HA *info) { + MARIA_SHARE *share= info->s; + TRN *trn= info->trn; MARIA_USED_TABLES *tables, **prev; + MARIA_HA *handler, **prev_file; DBUG_ENTER("_ma_remove_table_from_trnman"); DBUG_PRINT("enter", ("trn: %p used_tables: %p share: %p in_trans: %d", trn, trn->used_tables, share, share->in_trans)); mysql_mutex_assert_owner(&share->intern_lock); + + if (trn == &dummy_transaction_object) + DBUG_VOID_RETURN; - for (prev= (MARIA_USED_TABLES**) (char*) &trn->used_tables, tables= *prev; - tables; - tables= *prev) + /* First remove share from used_tables */ + for (prev= (MARIA_USED_TABLES**) (char*) &trn->used_tables; + (tables= *prev); + prev= &tables->next) { if (tables->share == share) { @@ -585,8 +593,36 @@ void _ma_remove_table_from_trnman(MARIA_SHARE *share, TRN *trn) my_free(tables); break; } - prev= &tables->next; } + if (tables != 0) + { + /* + This can only happens in case of rename of intermediate table as + part of alter table + */ + DBUG_PRINT("warning", ("share: %p where not in used_tables_list", share)); + } + + /* unlink table from used_instances */ + for (prev_file= (MARIA_HA**) &trn->used_instances; + (handler= *prev_file); + prev_file= &handler->trn_next) + { + if (handler == info) + { + *prev_file= info->trn_next; + break; + } + } + if (handler != 0) + { + /* + This can only happens in case of rename of intermediate table as + part of alter table + */ + DBUG_PRINT("warning", ("table: %p where not in used_instances", info)); + } + info->trn= 0; /* Not part of trans anymore */ DBUG_VOID_RETURN; } diff --git a/storage/maria/ma_state.h b/storage/maria/ma_state.h index d662b9899f8..d47595ee08d 100644 --- a/storage/maria/ma_state.h +++ b/storage/maria/ma_state.h @@ -84,5 +84,5 @@ my_bool _ma_row_visible_non_transactional_table(MARIA_HA *info); my_bool _ma_row_visible_transactional_table(MARIA_HA *info); void _ma_remove_not_visible_states_with_lock(struct st_maria_share *share, my_bool all); -void _ma_remove_table_from_trnman(struct st_maria_share *share, TRN *trn); +void _ma_remove_table_from_trnman(MARIA_HA *info); void _ma_reset_history(struct st_maria_share *share); diff --git a/storage/maria/ma_trnman.h b/storage/maria/ma_trnman.h new file mode 100644 index 00000000000..9bfd1f0d047 --- /dev/null +++ b/storage/maria/ma_trnman.h @@ -0,0 +1,65 @@ +/* Copyright (C) 2006-2008 MySQL AB, 2008-2009 Sun Microsystems, Inc. + + This program is free software; you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation; version 2 of the License. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program; if not, write to the Free Software + Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ + +#ifndef _ma_trnman_h +#define _ma_trnman_h + +/** + Sets table's trn and prints debug information + Links table into used_instances if new_trn is not 0 + + @param tbl MARIA_HA of table + @param newtrn what to put into tbl->trn +*/ + +static inline void _ma_set_trn_for_table(MARIA_HA *tbl, TRN *newtrn) +{ + DBUG_PRINT("info",("table: %p trn: %p -> %p", + tbl, tbl->trn, newtrn)); + + /* check that we are not calling this twice in a row */ + DBUG_ASSERT(newtrn->used_instances != (void*) tbl); + + tbl->trn= newtrn; + /* Link into used list */ + tbl->trn_next= (MARIA_HA*) newtrn->used_instances; + newtrn->used_instances= tbl; +} + + +/* + Same as _ma_set_trn_for_table(), but don't link table into used_instance list + Used when we want to temporary set trn for a table in extra() +*/ + +static inline void _ma_set_tmp_trn_for_table(MARIA_HA *tbl, TRN *newtrn) +{ + DBUG_PRINT("info",("table: %p trn: %p -> %p", + tbl, tbl->trn, newtrn)); + tbl->trn= newtrn; +} + + +/* + Reset TRN in table +*/ + +static inline void _ma_reset_trn_for_table(MARIA_HA *tbl) +{ + DBUG_PRINT("info",("table: %p trn: %p -> NULL", tbl, tbl->trn)); + tbl->trn= 0; +} + +#endif /* _ma_trnman_h */ diff --git a/storage/maria/maria_def.h b/storage/maria/maria_def.h index e4fd80d1151..bbf29d8b21a 100644 --- a/storage/maria/maria_def.h +++ b/storage/maria/maria_def.h @@ -576,6 +576,7 @@ struct st_maria_handler { MARIA_SHARE *s; /* Shared between open:s */ struct st_ma_transaction *trn; /* Pointer to active transaction */ + struct st_maria_handler *trn_next; MARIA_STATUS_INFO *state, state_save; MARIA_STATUS_INFO *state_start; /* State at start of transaction */ MARIA_USED_TABLES *used_tables; @@ -824,19 +825,6 @@ struct st_maria_handler #define get_pack_length(length) ((length) >= 255 ? 3 : 1) #define _ma_have_versioning(info) ((info)->row_flag & ROW_FLAG_TRANSID) -/** - Sets table's trn and prints debug information - @param tbl MARIA_HA of table - @param newtrn what to put into tbl->trn - @note cast of newtrn is because %p of NULL gives warning (NULL is int) -*/ -#define _ma_set_trn_for_table(tbl, newtrn) do { \ - DBUG_PRINT("info",("table: %p trn: %p -> %p", \ - (tbl), (tbl)->trn, (void *)(newtrn))); \ - (tbl)->trn= (newtrn); \ - } while (0) - - #define MARIA_MIN_BLOCK_LENGTH 20 /* Because of delete-link */ /* Don't use to small record-blocks */ #define MARIA_EXTEND_BLOCK_LENGTH 20 diff --git a/storage/maria/trnman.c b/storage/maria/trnman.c index daccf3550c2..1ad655b8a22 100644 --- a/storage/maria/trnman.c +++ b/storage/maria/trnman.c @@ -366,6 +366,7 @@ TRN *trnman_new_trn(WT_THD *wt) trn->commit_trid= MAX_TRID; trn->rec_lsn= trn->undo_lsn= trn->first_undo_lsn= 0; trn->used_tables= 0; + trn->used_instances= 0; trn->locked_tables= 0; trn->flags= 0; diff --git a/storage/maria/trnman.h b/storage/maria/trnman.h index 77e2916390a..057d9396473 100644 --- a/storage/maria/trnman.h +++ b/storage/maria/trnman.h @@ -46,7 +46,8 @@ struct st_ma_transaction LF_PINS *pins; WT_THD *wt; mysql_mutex_t state_lock; - void *used_tables; /**< Tables used by transaction */ + void *used_tables; /**< Table shares used by transaction */ + void *used_instances; /* table files used by transaction */ TRN *next, *prev; TrID trid, min_read_from, commit_trid; LSN rec_lsn, undo_lsn; From da71c1bad79ee11009b3b28a2a745709e04020cf Mon Sep 17 00:00:00 2001 From: Monty Date: Tue, 22 May 2018 14:36:06 +0300 Subject: [PATCH 04/26] MDEV-16229 Replication aborts with ER_VIEW_SELECT_TMPTABLE after half-failed RENAME Problem was that detection of temporary tables was all wrong for RENAME TABLE. (Temporary tables where opened by top level call to open_temporary_tables(), which can't detect if a temporary table was renamed to something and then reused). Fixed by adding proper parsing of rename list to check against the current name of a table at each rename stage. Also change do_rename_temporary() to check against the current state of temporary tables, not according to the state of start of RENAME TABLE. --- mysql-test/r/grant2.result | 3 +- mysql-test/r/rename.result | 66 ++++++++++++++++++++++++++++ mysql-test/suite/rpl/r/rename.result | 34 ++++++++++++++ mysql-test/suite/rpl/t/rename.test | 33 ++++++++++++++ mysql-test/t/grant2.test | 3 +- mysql-test/t/rename.test | 46 +++++++++++++++++++ sql/sql_parse.cc | 65 ++++++++++++++++++++++++++- sql/sql_rename.cc | 2 +- 8 files changed, 245 insertions(+), 7 deletions(-) create mode 100644 mysql-test/suite/rpl/r/rename.result create mode 100644 mysql-test/suite/rpl/t/rename.test diff --git a/mysql-test/r/grant2.result b/mysql-test/r/grant2.result index 9e9b3ffc4e5..70292240d84 100644 --- a/mysql-test/r/grant2.result +++ b/mysql-test/r/grant2.result @@ -705,10 +705,9 @@ LOAD INDEX INTO CACHE t3; Table Op Msg_type Msg_text mysqltest_db1.t3 preload_keys status OK # -# RENAME (doesn't work for temporary tables, thus should fail). +# RENAME should work for temporary tables # RENAME TABLE t3 TO t3_1; -ERROR 42000: INSERT, CREATE command denied to user 'mysqltest_u1'@'localhost' for table 't3_1' # # HANDLER OPEN/READ/CLOSE. # diff --git a/mysql-test/r/rename.result b/mysql-test/r/rename.result index 74370ba74dd..c90aaf24e9b 100644 --- a/mysql-test/r/rename.result +++ b/mysql-test/r/rename.result @@ -67,3 +67,69 @@ ERROR HY000: 'test.v1' is not BASE TABLE drop view v1; drop table t1; End of 5.0 tests +CREATE OR REPLACE TABLE t1 (a INT); +CREATE OR REPLACE TABLE t2 (a INT); +CREATE OR REPLACE TEMPORARY TABLE t1_tmp (b INT); +CREATE OR REPLACE TEMPORARY TABLE t2_tmp (b INT); +rename table t1 to t2; +ERROR 42S01: Table 't2' already exists +rename table t1 to tmp, tmp to t2; +ERROR 42S01: Table 't2' already exists +rename table t1_tmp to t2_tmp; +ERROR 42S01: Table 't2_tmp' already exists +rename table t1_tmp to tmp, tmp to t2_tmp; +ERROR 42S01: Table 't2_tmp' already exists +show create table t1_tmp; +Table Create Table +t1_tmp CREATE TEMPORARY TABLE `t1_tmp` ( + `b` int(11) DEFAULT NULL +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +show create table t2_tmp; +Table Create Table +t2_tmp CREATE TEMPORARY TABLE `t2_tmp` ( + `b` int(11) DEFAULT NULL +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +rename table t1 to t1_tmp; +rename table t2_tmp to t2; +rename table t2 to tmp, tmp to t2; +rename table t1_tmp to tmp, tmp to t1_tmp; +show tables; +Tables_in_test +t1_tmp +t2 +SHOW CREATE TABLE t1_tmp; +Table Create Table +t1_tmp CREATE TEMPORARY TABLE `t1_tmp` ( + `b` int(11) DEFAULT NULL +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +drop table t1_tmp; +SHOW CREATE TABLE t1_tmp; +Table Create Table +t1_tmp CREATE TABLE `t1_tmp` ( + `a` int(11) DEFAULT NULL +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +drop table t1_tmp; +SHOW CREATE TABLE t2; +Table Create Table +t2 CREATE TEMPORARY TABLE `t2` ( + `b` int(11) DEFAULT NULL +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +drop table t2; +SHOW CREATE TABLE t2; +Table Create Table +t2 CREATE TABLE `t2` ( + `a` int(11) DEFAULT NULL +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +drop table t2; +CREATE TABLE t1 (a INT); +insert into t1 values (1); +CREATE TEMPORARY TABLE t1 (b INT); +insert into t1 values (2); +RENAME TABLE t1 TO tmp, t1 TO t2; +select * from tmp; +b +2 +select * from t2; +a +1 +drop table tmp,t2; diff --git a/mysql-test/suite/rpl/r/rename.result b/mysql-test/suite/rpl/r/rename.result new file mode 100644 index 00000000000..5cedea51c86 --- /dev/null +++ b/mysql-test/suite/rpl/r/rename.result @@ -0,0 +1,34 @@ +include/master-slave.inc +[connection master] +# +# MDEV-16229 Replication aborts with ER_VIEW_SELECT_TMPTABLE after +# half-failed RENAME +# +CREATE TABLE t1 (a INT); +CREATE TEMPORARY TABLE t1 (b INT); +RENAME TABLE t1 TO tmp, tmp TO t1; +SHOW CREATE TABLE t1; +Table Create Table +t1 CREATE TEMPORARY TABLE `t1` ( + `b` int(11) DEFAULT NULL +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +CREATE VIEW v AS SELECT * FROM t1; +ERROR HY000: View's SELECT refers to a temporary table 't1' +RENAME TABLE t1 TO tmp, t1 TO t2; +SHOW CREATE TABLE tmp; +Table Create Table +tmp CREATE TEMPORARY TABLE `tmp` ( + `b` int(11) DEFAULT NULL +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +SHOW CREATE TABLE t2; +Table Create Table +t2 CREATE TABLE `t2` ( + `a` int(11) DEFAULT NULL +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +CREATE VIEW v AS SELECT * FROM tmp; +ERROR HY000: View's SELECT refers to a temporary table 'tmp' +CREATE VIEW v AS SELECT * FROM t2; +DROP VIEW v; +DROP TABLE tmp; +DROP TABLE t2; +include/rpl_end.inc diff --git a/mysql-test/suite/rpl/t/rename.test b/mysql-test/suite/rpl/t/rename.test new file mode 100644 index 00000000000..ac499157918 --- /dev/null +++ b/mysql-test/suite/rpl/t/rename.test @@ -0,0 +1,33 @@ +--source include/have_binlog_format_mixed.inc +--source include/master-slave.inc + +--echo # +--echo # MDEV-16229 Replication aborts with ER_VIEW_SELECT_TMPTABLE after +--echo # half-failed RENAME +--echo # + +CREATE TABLE t1 (a INT); +CREATE TEMPORARY TABLE t1 (b INT); +RENAME TABLE t1 TO tmp, tmp TO t1; +SHOW CREATE TABLE t1; +--error ER_VIEW_SELECT_TMPTABLE +CREATE VIEW v AS SELECT * FROM t1; + +RENAME TABLE t1 TO tmp, t1 TO t2; +SHOW CREATE TABLE tmp; +SHOW CREATE TABLE t2; +--error ER_VIEW_SELECT_TMPTABLE +CREATE VIEW v AS SELECT * FROM tmp; +CREATE VIEW v AS SELECT * FROM t2; + +--sync_slave_with_master + +# Cleanup + +--connection master + +DROP VIEW v; +DROP TABLE tmp; +DROP TABLE t2; + +--source include/rpl_end.inc diff --git a/mysql-test/t/grant2.test b/mysql-test/t/grant2.test index 8590dccd1d1..55eb21ed6f7 100644 --- a/mysql-test/t/grant2.test +++ b/mysql-test/t/grant2.test @@ -873,9 +873,8 @@ CACHE INDEX t3 IN keycache1; LOAD INDEX INTO CACHE t3; --echo # ---echo # RENAME (doesn't work for temporary tables, thus should fail). +--echo # RENAME should work for temporary tables --echo # ---error ER_TABLEACCESS_DENIED_ERROR RENAME TABLE t3 TO t3_1; --echo # diff --git a/mysql-test/t/rename.test b/mysql-test/t/rename.test index a55bc845acc..67732d5b5b9 100644 --- a/mysql-test/t/rename.test +++ b/mysql-test/t/rename.test @@ -95,3 +95,49 @@ drop table t1; --source include/wait_until_count_sessions.inc +# +# Test of rename with temporary tables +# + +CREATE OR REPLACE TABLE t1 (a INT); +CREATE OR REPLACE TABLE t2 (a INT); +CREATE OR REPLACE TEMPORARY TABLE t1_tmp (b INT); +CREATE OR REPLACE TEMPORARY TABLE t2_tmp (b INT); + +# Can't rename table over another one +--error ER_TABLE_EXISTS_ERROR +rename table t1 to t2; +--error ER_TABLE_EXISTS_ERROR +rename table t1 to tmp, tmp to t2; +--error ER_TABLE_EXISTS_ERROR +rename table t1_tmp to t2_tmp; +--error ER_TABLE_EXISTS_ERROR +rename table t1_tmp to tmp, tmp to t2_tmp; + +show create table t1_tmp; +show create table t2_tmp; + +# The following should work +rename table t1 to t1_tmp; +rename table t2_tmp to t2; +rename table t2 to tmp, tmp to t2; +rename table t1_tmp to tmp, tmp to t1_tmp; +show tables; +SHOW CREATE TABLE t1_tmp; +drop table t1_tmp; +SHOW CREATE TABLE t1_tmp; +drop table t1_tmp; +SHOW CREATE TABLE t2; +drop table t2; +SHOW CREATE TABLE t2; +drop table t2; + +CREATE TABLE t1 (a INT); +insert into t1 values (1); +CREATE TEMPORARY TABLE t1 (b INT); +insert into t1 values (2); +RENAME TABLE t1 TO tmp, t1 TO t2; +select * from tmp; +select * from t2; +drop table tmp,t2; + diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 1162aa901d1..28d11ef2e5c 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -495,6 +495,8 @@ void init_update_queries(void) There are other statements that deal with temporary tables and open them, but which are not listed here. The thing is that the order of pre-opening temporary tables for those statements is somewhat custom. + + Note that SQLCOM_RENAME_TABLE should not be in this list! */ sql_command_flags[SQLCOM_CREATE_TABLE]|= CF_PREOPEN_TMP_TABLES; sql_command_flags[SQLCOM_DROP_TABLE]|= CF_PREOPEN_TMP_TABLES; @@ -508,7 +510,6 @@ void init_update_queries(void) sql_command_flags[SQLCOM_INSERT_SELECT]|= CF_PREOPEN_TMP_TABLES; sql_command_flags[SQLCOM_DELETE]|= CF_PREOPEN_TMP_TABLES; sql_command_flags[SQLCOM_DELETE_MULTI]|= CF_PREOPEN_TMP_TABLES; - sql_command_flags[SQLCOM_RENAME_TABLE]|= CF_PREOPEN_TMP_TABLES; sql_command_flags[SQLCOM_REPLACE_SELECT]|= CF_PREOPEN_TMP_TABLES; sql_command_flags[SQLCOM_SELECT]|= CF_PREOPEN_TMP_TABLES; sql_command_flags[SQLCOM_SET_OPTION]|= CF_PREOPEN_TMP_TABLES; @@ -5333,6 +5334,60 @@ static bool execute_show_status(THD *thd, TABLE_LIST *all_tables) } +/* + Find out if a table is a temporary table + + A table is a temporary table if it's a temporary table or + there has been before a temporary table that has been renamed + to the current name. + + Some examples: + A->B B is a temporary table if and only if A is a temp. + A->B, B->C Second B is temp if A is temp + A->B, A->C Second A can't be temp as if A was temp then B is temp + and Second A can only be a normal table. C is also not temp +*/ + +static TABLE *find_temporary_table_for_rename(THD *thd, + TABLE_LIST *first_table, + TABLE_LIST *cur_table) +{ + TABLE_LIST *table; + TABLE *res= 0; + bool found= 0; + DBUG_ENTER("find_temporary_table_for_rename"); + + /* Find last instance when cur_table is in TO part */ + for (table= first_table; + table != cur_table; + table= table->next_local->next_local) + { + TABLE_LIST *next= table->next_local; + + if (!strcmp(table->get_db_name(), cur_table->get_db_name()) && + !strcmp(table->get_table_name(), cur_table->get_table_name())) + { + /* Table was moved away, can't be same as 'table' */ + found= 1; + res= 0; // Table can't be a temporary table + } + if (!strcmp(next->get_db_name(), cur_table->get_db_name()) && + !strcmp(next->get_table_name(), cur_table->get_table_name())) + { + /* + Table has matching name with new name of this table. cur_table should + have same temporary type as this table. + */ + found= 1; + res= table->table; + } + } + if (!found) + res= find_temporary_table(thd, table); + DBUG_RETURN(res); +} + + static bool execute_rename_table(THD *thd, TABLE_LIST *first_table, TABLE_LIST *all_tables) { @@ -5349,13 +5404,19 @@ static bool execute_rename_table(THD *thd, TABLE_LIST *first_table, &table->next_local->grant.m_internal, 0, 0)) return 1; + + /* check if these are refering to temporary tables */ + table->table= find_temporary_table_for_rename(thd, first_table, table); + table->next_local->table= table->table; + TABLE_LIST old_list, new_list; /* we do not need initialize old_list and new_list because we will - come table[0] and table->next[0] there + copy table[0] and table->next[0] there */ old_list= table[0]; new_list= table->next_local[0]; + if (check_grant(thd, ALTER_ACL | DROP_ACL, &old_list, FALSE, 1, FALSE) || (!test_all_bits(table->next_local->grant.privilege, INSERT_ACL | CREATE_ACL) && diff --git a/sql/sql_rename.cc b/sql/sql_rename.cc index 1a9cb842e6a..119f9063724 100644 --- a/sql/sql_rename.cc +++ b/sql/sql_rename.cc @@ -222,7 +222,7 @@ do_rename_temporary(THD *thd, TABLE_LIST *ren_table, TABLE_LIST *new_table, new_alias= (lower_case_table_names == 2) ? new_table->alias : new_table->table_name; - if (is_temporary_table(new_table)) + if (find_temporary_table(thd, new_table)) { my_error(ER_TABLE_EXISTS_ERROR, MYF(0), new_alias); DBUG_RETURN(1); // This can't be skipped From 908676dfd9d981fd0f37a7cf9332abac522f1936 Mon Sep 17 00:00:00 2001 From: Monty Date: Tue, 22 May 2018 23:05:01 +0300 Subject: [PATCH 05/26] MDEV-15308 Assertion `ha_alter_info->alter_info->drop_list.elements Problem was that handle_if_exists_options() didn't correct alter_info->flags when things was removed from the list. --- mysql-test/r/alter_table.result | 61 +++++++++++++++++++++++++++++++++ mysql-test/t/alter_table.test | 32 +++++++++++++++++ sql/sql_table.cc | 29 +++++++++++++--- 3 files changed, 118 insertions(+), 4 deletions(-) diff --git a/mysql-test/r/alter_table.result b/mysql-test/r/alter_table.result index e32037c4be9..bb78df907ad 100644 --- a/mysql-test/r/alter_table.result +++ b/mysql-test/r/alter_table.result @@ -2164,3 +2164,64 @@ t1 CREATE TABLE `t1` ( `b` int(11) DEFAULT NULL ) ENGINE=InnoDB DEFAULT CHARSET=utf8 DROP TABLE t1; +# +# +# MDEV-15308 +# Assertion `ha_alter_info->alter_info->drop_list.elements > 0' failed +# in ha_innodb::prepare_inplace_alter_table +# +CREATE TABLE t1 (a INT, b INT) ENGINE=InnoDB; +ALTER TABLE t1 DROP FOREIGN KEY IF EXISTS fk, DROP COLUMN b; +Warnings: +Note 1091 Can't DROP 'fk'; check that column/key exists +SHOW CREATE TABLE t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `a` int(11) DEFAULT NULL +) ENGINE=InnoDB DEFAULT CHARSET=latin1 +DROP TABLE t1; +CREATE TABLE t1 (a INT, b INT) ENGINE=InnoDB; +ALTER TABLE t1 DROP INDEX IF EXISTS fk, DROP COLUMN b; +Warnings: +Note 1091 Can't DROP 'fk'; check that column/key exists +SHOW CREATE TABLE t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `a` int(11) DEFAULT NULL +) ENGINE=InnoDB DEFAULT CHARSET=latin1 +DROP TABLE t1; +CREATE TABLE t1 (a INT, b INT, c INT, KEY(c)) ENGINE=InnoDB; +ALTER TABLE t1 DROP FOREIGN KEY IF EXISTS fk, DROP COLUMN c; +Warnings: +Note 1091 Can't DROP 'fk'; check that column/key exists +SHOW CREATE TABLE t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `a` int(11) DEFAULT NULL, + `b` int(11) DEFAULT NULL +) ENGINE=InnoDB DEFAULT CHARSET=latin1 +DROP TABLE t1; +CREATE TABLE t1 (a INT, b INT, c INT, KEY c1(c)) ENGINE=InnoDB; +ALTER TABLE t1 DROP FOREIGN KEY IF EXISTS fk, DROP INDEX c1; +Warnings: +Note 1091 Can't DROP 'fk'; check that column/key exists +SHOW CREATE TABLE t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `a` int(11) DEFAULT NULL, + `b` int(11) DEFAULT NULL, + `c` int(11) DEFAULT NULL +) ENGINE=InnoDB DEFAULT CHARSET=latin1 +DROP TABLE t1; +CREATE TABLE t1 (a INT, b INT) ENGINE=InnoDB; +ALTER TABLE t1 DROP INDEX IF EXISTS fk, DROP COLUMN IF EXISTS c; +Warnings: +Note 1091 Can't DROP 'fk'; check that column/key exists +Note 1091 Can't DROP 'c'; check that column/key exists +SHOW CREATE TABLE t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `a` int(11) DEFAULT NULL, + `b` int(11) DEFAULT NULL +) ENGINE=InnoDB DEFAULT CHARSET=latin1 +DROP TABLE t1; diff --git a/mysql-test/t/alter_table.test b/mysql-test/t/alter_table.test index 8fcc7d044e5..585a272de9b 100644 --- a/mysql-test/t/alter_table.test +++ b/mysql-test/t/alter_table.test @@ -1809,3 +1809,35 @@ SHOW CREATE TABLE t1; ALTER TABLE t1 CONVERT TO CHARACTER SET utf8; SHOW CREATE TABLE t1; DROP TABLE t1; + +--echo # +--echo # +--echo # MDEV-15308 +--echo # Assertion `ha_alter_info->alter_info->drop_list.elements > 0' failed +--echo # in ha_innodb::prepare_inplace_alter_table +--echo # + +CREATE TABLE t1 (a INT, b INT) ENGINE=InnoDB; +ALTER TABLE t1 DROP FOREIGN KEY IF EXISTS fk, DROP COLUMN b; +SHOW CREATE TABLE t1; +DROP TABLE t1; + +CREATE TABLE t1 (a INT, b INT) ENGINE=InnoDB; +ALTER TABLE t1 DROP INDEX IF EXISTS fk, DROP COLUMN b; +SHOW CREATE TABLE t1; +DROP TABLE t1; + +CREATE TABLE t1 (a INT, b INT, c INT, KEY(c)) ENGINE=InnoDB; +ALTER TABLE t1 DROP FOREIGN KEY IF EXISTS fk, DROP COLUMN c; +SHOW CREATE TABLE t1; +DROP TABLE t1; + +CREATE TABLE t1 (a INT, b INT, c INT, KEY c1(c)) ENGINE=InnoDB; +ALTER TABLE t1 DROP FOREIGN KEY IF EXISTS fk, DROP INDEX c1; +SHOW CREATE TABLE t1; +DROP TABLE t1; + +CREATE TABLE t1 (a INT, b INT) ENGINE=InnoDB; +ALTER TABLE t1 DROP INDEX IF EXISTS fk, DROP COLUMN IF EXISTS c; +SHOW CREATE TABLE t1; +DROP TABLE t1; diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 27d579a6b19..376c1362cc7 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -5808,10 +5808,28 @@ drop_create_field: List_iterator drop_it(alter_info->drop_list); Alter_drop *drop; bool remove_drop; + ulonglong left_flags= 0; while ((drop= drop_it++)) { + ulonglong cur_flag= 0; + switch (drop->type) { + case Alter_drop::COLUMN: + cur_flag= Alter_info::ALTER_DROP_COLUMN; + break; + case Alter_drop::FOREIGN_KEY: + cur_flag= Alter_info::DROP_FOREIGN_KEY; + break; + case Alter_drop::KEY: + cur_flag= Alter_info::ALTER_DROP_INDEX; + break; + default: + break; + } if (!drop->drop_if_exists) + { + left_flags|= cur_flag; continue; + } remove_drop= TRUE; if (drop->type == Alter_drop::COLUMN) { @@ -5887,12 +5905,15 @@ drop_create_field: ER_CANT_DROP_FIELD_OR_KEY, ER(ER_CANT_DROP_FIELD_OR_KEY), drop->name); drop_it.remove(); - if (alter_info->drop_list.is_empty()) - alter_info->flags&= ~(Alter_info::ALTER_DROP_COLUMN | - Alter_info::ALTER_DROP_INDEX | - Alter_info::DROP_FOREIGN_KEY); } + else + left_flags|= cur_flag; } + /* Reset state to what's left in drop list */ + alter_info->flags&= ~(Alter_info::ALTER_DROP_COLUMN | + Alter_info::ALTER_DROP_INDEX | + Alter_info::DROP_FOREIGN_KEY); + alter_info->flags|= left_flags; } /* ALTER TABLE ADD KEY IF NOT EXISTS */ From a816aa066e5c879a92819d694a93d245e6ec6e47 Mon Sep 17 00:00:00 2001 From: Monty Date: Wed, 23 May 2018 11:26:49 +0300 Subject: [PATCH 06/26] Fixed ASAN heap-use-after-free handler::ha_index_or_rnd_end MDEV-16123 ASAN heap-use-after-free handler::ha_index_or_rnd_end MDEV-13828 Segmentation fault on RENAME TABLE Problem was that destructor called methods for closed table. Fixed by removing code in destructor. --- mysql-test/r/statistics_close.result | 6 ++++++ mysql-test/t/statistics_close.test | 18 ++++++++++++++++++ sql/sql_statistics.cc | 3 ++- 3 files changed, 26 insertions(+), 1 deletion(-) create mode 100644 mysql-test/r/statistics_close.result create mode 100644 mysql-test/t/statistics_close.test diff --git a/mysql-test/r/statistics_close.result b/mysql-test/r/statistics_close.result new file mode 100644 index 00000000000..348681c3311 --- /dev/null +++ b/mysql-test/r/statistics_close.result @@ -0,0 +1,6 @@ +CREATE TABLE t1 (i int); +RENAME TABLE t1 TO t2; +FLUSH TABLES; +DROP TABLE IF EXISTS t1, t2; +Warnings: +Note 1051 Unknown table 'test.t1' diff --git a/mysql-test/t/statistics_close.test b/mysql-test/t/statistics_close.test new file mode 100644 index 00000000000..de22a5a44fe --- /dev/null +++ b/mysql-test/t/statistics_close.test @@ -0,0 +1,18 @@ +# +# MDEV-16123 ASAN heap-use-after-free handler::ha_index_or_rnd_end +# MDEV-13828 Segmentation fault on RENAME TABLE +# + +CREATE TABLE t1 (i int); +--connect (con1,localhost,root,,test) +--send +RENAME TABLE t1 TO t2; +--connection default +FLUSH TABLES; +--connection con1 +--reap + +# Cleanup +--disconnect con1 +--connection default +DROP TABLE IF EXISTS t1, t2; diff --git a/sql/sql_statistics.cc b/sql/sql_statistics.cc index ce320e87a4f..471749ad346 100644 --- a/sql/sql_statistics.cc +++ b/sql/sql_statistics.cc @@ -1376,7 +1376,8 @@ public: ~Stat_table_write_iter() { - cleanup(); + /* Ensure that cleanup has been run */ + DBUG_ASSERT(rowid_buf == 0); } }; From a61724a3ca187f6eb44fc67948acb76a94efa783 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Thu, 24 May 2018 11:38:34 +0300 Subject: [PATCH 07/26] MDEV-16267 Wrong INFORMATION_SCHEMA.INNODB_BUFFER_PAGE.TABLE_NAME i_s_innodb_buffer_page_fill(), i_s_innodb_buf_page_lru_fill(): Only invoke Field::set_notnull() if the index was found. --- storage/innobase/handler/i_s.cc | 28 +++++++++++++++++++--------- storage/xtradb/handler/i_s.cc | 28 +++++++++++++++++++--------- 2 files changed, 38 insertions(+), 18 deletions(-) diff --git a/storage/innobase/handler/i_s.cc b/storage/innobase/handler/i_s.cc index 41f4114494d..f8aa3bd4637 100644 --- a/storage/innobase/handler/i_s.cc +++ b/storage/innobase/handler/i_s.cc @@ -1,7 +1,7 @@ /***************************************************************************** Copyright (c) 2007, 2016, Oracle and/or its affiliates. All Rights Reserved. -Copyright (c) 2017, MariaDB Corporation. +Copyright (c) 2017, 2018, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software @@ -4955,9 +4955,11 @@ i_s_innodb_buffer_page_fill( mutex_enter(&dict_sys->mutex); - if (const dict_index_t* index = - dict_index_get_if_in_cache_low( - page_info->index_id)) { + const dict_index_t* index = + dict_index_get_if_in_cache_low( + page_info->index_id); + + if (index) { table_name_end = innobase_convert_name( table_name, sizeof(table_name), index->table_name, @@ -4980,7 +4982,10 @@ i_s_innodb_buffer_page_fill( OK(ret); - fields[IDX_BUFFER_PAGE_TABLE_NAME]->set_notnull(); + if (index) { + fields[IDX_BUFFER_PAGE_TABLE_NAME] + ->set_notnull(); + } } OK(fields[IDX_BUFFER_PAGE_NUM_RECS]->store( @@ -5657,9 +5662,11 @@ i_s_innodb_buf_page_lru_fill( mutex_enter(&dict_sys->mutex); - if (const dict_index_t* index = - dict_index_get_if_in_cache_low( - page_info->index_id)) { + const dict_index_t* index = + dict_index_get_if_in_cache_low( + page_info->index_id); + + if (index) { table_name_end = innobase_convert_name( table_name, sizeof(table_name), index->table_name, @@ -5682,7 +5689,10 @@ i_s_innodb_buf_page_lru_fill( OK(ret); - fields[IDX_BUF_LRU_PAGE_TABLE_NAME]->set_notnull(); + if (index) { + fields[IDX_BUF_LRU_PAGE_TABLE_NAME] + ->set_notnull(); + } } OK(fields[IDX_BUF_LRU_PAGE_NUM_RECS]->store( diff --git a/storage/xtradb/handler/i_s.cc b/storage/xtradb/handler/i_s.cc index 011bc77dc3c..834bae399fb 100644 --- a/storage/xtradb/handler/i_s.cc +++ b/storage/xtradb/handler/i_s.cc @@ -1,7 +1,7 @@ /***************************************************************************** Copyright (c) 2007, 2016, Oracle and/or its affiliates. All Rights Reserved. -Copyright (c) 2017, MariaDB Corporation. +Copyright (c) 2017, 2018, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software @@ -4943,9 +4943,11 @@ i_s_innodb_buffer_page_fill( mutex_enter(&dict_sys->mutex); - if (const dict_index_t* index = - dict_index_get_if_in_cache_low( - page_info->index_id)) { + const dict_index_t* index = + dict_index_get_if_in_cache_low( + page_info->index_id); + + if (index) { table_name_end = innobase_convert_name( table_name, sizeof(table_name), index->table_name, @@ -4968,7 +4970,10 @@ i_s_innodb_buffer_page_fill( OK(ret); - fields[IDX_BUFFER_PAGE_TABLE_NAME]->set_notnull(); + if (index) { + fields[IDX_BUFFER_PAGE_TABLE_NAME] + ->set_notnull(); + } } OK(fields[IDX_BUFFER_PAGE_NUM_RECS]->store( @@ -5642,9 +5647,11 @@ i_s_innodb_buf_page_lru_fill( mutex_enter(&dict_sys->mutex); - if (const dict_index_t* index = - dict_index_get_if_in_cache_low( - page_info->index_id)) { + const dict_index_t* index = + dict_index_get_if_in_cache_low( + page_info->index_id); + + if (index) { table_name_end = innobase_convert_name( table_name, sizeof(table_name), index->table_name, @@ -5667,7 +5674,10 @@ i_s_innodb_buf_page_lru_fill( OK(ret); - fields[IDX_BUF_LRU_PAGE_TABLE_NAME]->set_notnull(); + if (index) { + fields[IDX_BUF_LRU_PAGE_TABLE_NAME] + ->set_notnull(); + } } OK(fields[IDX_BUF_LRU_PAGE_NUM_RECS]->store( From bfed1bfe28980d3b1404f99a44712242a5108ef5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Thu, 24 May 2018 11:55:27 +0300 Subject: [PATCH 08/26] Add a missing dependency to a test --- mysql-test/suite/galera/t/galera_var_dirty_reads.test | 1 + 1 file changed, 1 insertion(+) diff --git a/mysql-test/suite/galera/t/galera_var_dirty_reads.test b/mysql-test/suite/galera/t/galera_var_dirty_reads.test index 138b7c1c703..3e2108868af 100644 --- a/mysql-test/suite/galera/t/galera_var_dirty_reads.test +++ b/mysql-test/suite/galera/t/galera_var_dirty_reads.test @@ -4,6 +4,7 @@ --source include/galera_cluster.inc --source include/have_innodb.inc +--source include/have_perfschema.inc # Save original auto_increment_offset values. --let $node_1=node_1 From 4cd2a0eb56010ea1e8d2601151ca0d25834deb7e Mon Sep 17 00:00:00 2001 From: Monty Date: Wed, 23 May 2018 22:42:29 +0300 Subject: [PATCH 09/26] MDEV-15243 Crash with virtual fields and row based binary logging The cause of this was several different bugs: - When using binary logging with binlog_row_image=FULL the all bits in read_set was set, which caused a different (wrong) pattern for marking vcol_set. - TABLE::mark_virtual_columns_for_write() didn't in all cases mark vcol_set with the vcol_field. - TABLE::update_virtual_fields() has to update all vcol fields on REPLACE if binary logging with FULL is used. - VCOL_UPDATE_INDEXED should update all vcol fields part of an index that was not updated by VCOL_UPDATE_FOR_READ - max_row_length() calculated length of NULL and not used fields. This didn't cause any crash, but used more memory than needed. --- mysql-test/suite/vcol/r/binlog.result | 70 ++++ mysql-test/suite/vcol/r/update.result | 12 + mysql-test/suite/vcol/r/update_binlog.result | 361 +++++++++++++++++++ mysql-test/suite/vcol/t/binlog.test | 55 +++ mysql-test/suite/vcol/t/update.test | 2 + mysql-test/suite/vcol/t/update_binlog.test | 14 + sql/sql_class.cc | 17 +- sql/table.cc | 87 ++--- 8 files changed, 569 insertions(+), 49 deletions(-) create mode 100644 mysql-test/suite/vcol/r/binlog.result create mode 100644 mysql-test/suite/vcol/r/update_binlog.result create mode 100644 mysql-test/suite/vcol/t/binlog.test create mode 100644 mysql-test/suite/vcol/t/update_binlog.test diff --git a/mysql-test/suite/vcol/r/binlog.result b/mysql-test/suite/vcol/r/binlog.result new file mode 100644 index 00000000000..35bfb50ba2f --- /dev/null +++ b/mysql-test/suite/vcol/r/binlog.result @@ -0,0 +1,70 @@ +include/master-slave.inc +[connection master] +CREATE TABLE t1 ( +pk SERIAL, +vcol_date DATE AS (col_date) PERSISTENT, +vcol_int INT AS (col_int) VIRTUAL, +vcol_year YEAR AS (col_year) PERSISTENT, +vcol_blob BLOB AS (col_blob) VIRTUAL, +col_date DATE, +col_int INT NULL, +col_blob BLOB NULL, +col_year YEAR, +PRIMARY KEY(pk) +) ENGINE=InnoDB; +INSERT INTO t1 (col_date,col_int,col_blob,col_year) VALUES ('2010-04-24',5,'foo',1981); +SET SQL_MODE=''; +set binlog_row_image="FULL"; +CREATE VIEW v1 AS SELECT * FROM t1; +REPLACE INTO v1 SELECT pk, vcol_date, vcol_int, vcol_year, vcol_blob, col_date, col_int, col_blob, 1982 FROM t1; +Warnings: +Warning 1906 The value specified for generated column 'vcol_date' in table 't1' ignored +Warning 1906 The value specified for generated column 'vcol_int' in table 't1' ignored +Warning 1906 The value specified for generated column 'vcol_year' in table 't1' ignored +Warning 1906 The value specified for generated column 'vcol_blob' in table 't1' ignored +select col_date,col_int,col_blob,col_year from v1; +col_date col_int col_blob col_year +2010-04-24 5 foo 1982 +connection slave; +select col_date,col_int,col_blob,col_year from v1; +col_date col_int col_blob col_year +2010-04-24 5 foo 1982 +connection master; +DROP VIEW v1; +set binlog_row_image="MINIMAL"; +CREATE VIEW v1 AS SELECT * FROM t1; +REPLACE INTO v1 SELECT pk, vcol_date, vcol_int, vcol_year, vcol_blob, col_date, col_int, col_blob, 1983 FROM t1; +Warnings: +Warning 1906 The value specified for generated column 'vcol_date' in table 't1' ignored +Warning 1906 The value specified for generated column 'vcol_int' in table 't1' ignored +Warning 1906 The value specified for generated column 'vcol_year' in table 't1' ignored +Warning 1906 The value specified for generated column 'vcol_blob' in table 't1' ignored +select col_date,col_int,col_blob,col_year from v1; +col_date col_int col_blob col_year +2010-04-24 5 foo 1983 +connection slave; +select col_date,col_int,col_blob,col_year from v1; +col_date col_int col_blob col_year +2010-04-24 5 foo 1983 +connection master; +DROP VIEW v1; +set @@binlog_row_image="NOBLOB"; +CREATE VIEW v1 AS SELECT * FROM t1; +REPLACE INTO v1 SELECT pk, vcol_date, vcol_int, vcol_year, vcol_blob, col_date, col_int, col_blob, 1984 FROM t1; +Warnings: +Warning 1906 The value specified for generated column 'vcol_date' in table 't1' ignored +Warning 1906 The value specified for generated column 'vcol_int' in table 't1' ignored +Warning 1906 The value specified for generated column 'vcol_year' in table 't1' ignored +Warning 1906 The value specified for generated column 'vcol_blob' in table 't1' ignored +select col_date,col_int,col_blob,col_year from v1; +col_date col_int col_blob col_year +2010-04-24 5 foo 1984 +connection slave; +select col_date,col_int,col_blob,col_year from v1; +col_date col_int col_blob col_year +2010-04-24 5 foo 1984 +connection master; +DROP VIEW v1; +set @@binlog_row_image=default; +DROP TABLE t1; +include/rpl_end.inc diff --git a/mysql-test/suite/vcol/r/update.result b/mysql-test/suite/vcol/r/update.result index 5c7905cf547..5a6355e1773 100644 --- a/mysql-test/suite/vcol/r/update.result +++ b/mysql-test/suite/vcol/r/update.result @@ -9,8 +9,20 @@ a b c 2 3 4 drop table t1; create table t1 (a int, c int as(a), p varchar(20) as(y), y char(20), index (p,c)); +show create table t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `a` int(11) DEFAULT NULL, + `c` int(11) GENERATED ALWAYS AS (`a`) VIRTUAL, + `p` varchar(20) GENERATED ALWAYS AS (`y`) VIRTUAL, + `y` char(20) DEFAULT NULL, + KEY `p` (`p`,`c`) +) ENGINE=MyISAM DEFAULT CHARSET=latin1 insert into t1 (a,y) values(1, "yyy"); update t1 set a = 100 where a = 1; +check table t1; +Table Op Msg_type Msg_text +test.t1 check status OK drop table t1; create table t1 ( a varchar(10000), diff --git a/mysql-test/suite/vcol/r/update_binlog.result b/mysql-test/suite/vcol/r/update_binlog.result new file mode 100644 index 00000000000..d4102fc460a --- /dev/null +++ b/mysql-test/suite/vcol/r/update_binlog.result @@ -0,0 +1,361 @@ +set binlog_row_image="FULL"; +set @@default_storage_engine="myisam"; +create table t1 (a int, b int as (a+1), c int as (b+1) stored); +insert t1 set a=1; +select * from t1; +a b c +1 2 3 +update t1 set a=2; +select * from t1; +a b c +2 3 4 +drop table t1; +create table t1 (a int, c int as(a), p varchar(20) as(y), y char(20), index (p,c)); +show create table t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `a` int(11) DEFAULT NULL, + `c` int(11) GENERATED ALWAYS AS (`a`) VIRTUAL, + `p` varchar(20) GENERATED ALWAYS AS (`y`) VIRTUAL, + `y` char(20) DEFAULT NULL, + KEY `p` (`p`,`c`) +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +insert into t1 (a,y) values(1, "yyy"); +update t1 set a = 100 where a = 1; +check table t1; +Table Op Msg_type Msg_text +test.t1 check status OK +drop table t1; +create table t1 ( +a varchar(10000), +b varchar(3000), +c varchar(14000) generated always as (concat(a,b)) virtual, +d varchar(5000) generated always as (b) virtual, +e int(11) generated always as (10) virtual, +h int(11) not null primary key, +index(c(100), d(20))); +insert t1 (a,b,h) values (repeat('g', 10000), repeat('x', 2800), 1); +update t1 set a = repeat(cast(1 as char), 2000); +drop table t1; +create table t1 ( +a varchar(10000), +b varchar(3000), +c varchar(14000) generated always as (concat(a,b)) virtual, +i varchar(5000) generated always as (b) virtual, +d varchar(5000) generated always as (i) virtual, +e int(11) generated always as (10) virtual, +h int(11) not null primary key, +index(c(100), d(20))); +insert t1 (a,b,h) values (repeat('g', 10000), repeat('x', 2800), 1); +update t1 set a = repeat(cast(1 as char), 2000); +drop table t1; +create table t1(a blob not null, b int, c varbinary (10) generated always as (a) virtual, unique (c(9))); +insert t1 (a,b) values ('a', 1); +replace t1 set a = 'a',b =1; +insert t1 (a,b) values ('a', 1) on duplicate key update a='b', b=2; +select * from t1; +a b c +b 2 b +drop table t1; +create table t (a int primary key, b int, c int as (b), index (c)); +insert t (a,b) values (9,0); +create table t2 select * from t; +update t, t2 set t.b=10 where t.a=t2.a; +check table t; +Table Op Msg_type Msg_text +test.t check status OK +select * from t; +a b c +9 10 10 +drop table t, t2; +create table t1 (a int, b int, c int, d int, e int); +insert t1 values (1,2,3,4,5), (1,2,3,4,5); +SET STATEMENT sql_mode = 'NO_ENGINE_SUBSTITUTION' FOR +create table t (a int primary key, +b int, c blob as (b), index (c(57)), +d blob, e blob as (d), index (e(57))) +replace select * from t1; +Warnings: +Warning 1906 The value specified for generated column 'c' in table 't' ignored +Warning 1906 The value specified for generated column 'e' in table 't' ignored +Warning 1906 The value specified for generated column 'c' in table 't' ignored +Warning 1906 The value specified for generated column 'e' in table 't' ignored +check table t; +Table Op Msg_type Msg_text +test.t check status OK +select * from t; +a b c d e +1 2 2 4 4 +update t set a=10, b=1, d=1; +check table t; +Table Op Msg_type Msg_text +test.t check status OK +select * from t; +a b c d e +10 1 1 1 1 +replace t (a,b,d) values (10,2,2); +check table t; +Table Op Msg_type Msg_text +test.t check status OK +select * from t; +a b c d e +10 2 2 2 2 +insert t(a,b,d) values (10) on duplicate key update b=3; +ERROR 21S01: Column count doesn't match value count at row 1 +insert t(a,b,d) values (10,2,2) on duplicate key update b=3, d=3; +check table t; +Table Op Msg_type Msg_text +test.t check status OK +select * from t; +a b c d e +10 3 3 3 3 +replace t (a,b,d) select 10,4,4; +check table t; +Table Op Msg_type Msg_text +test.t check status OK +select * from t; +a b c d e +10 4 4 4 4 +insert t(a,b,d) select 10,4,4 on duplicate key update b=5, d=5; +check table t; +Table Op Msg_type Msg_text +test.t check status OK +select * from t; +a b c d e +10 5 5 5 5 +replace delayed t (a,b,d) values (10,6,6); +flush tables; +check table t; +Table Op Msg_type Msg_text +test.t check status OK +select * from t; +a b c d e +10 6 6 6 6 +insert delayed t(a,b,d) values (10,6,6) on duplicate key update b=7, d=7; +flush tables; +check table t; +Table Op Msg_type Msg_text +test.t check status OK +select * from t; +a b c d e +10 7 7 7 7 +load data infile 'MYSQLTEST_VARDIR/tmp/vblobs.txt' replace into table t; +check table t; +Table Op Msg_type Msg_text +test.t check status OK +select * from t; +a b c d e +10 8 8 8 8 +update t set a=11, b=9, d=9 where a>5; +check table t; +Table Op Msg_type Msg_text +test.t check status OK +select * from t; +a b c d e +11 9 9 9 9 +create table t2 select * from t; +update t, t2 set t.b=10, t.d=10 where t.a=t2.a; +check table t; +Table Op Msg_type Msg_text +test.t check status OK +select * from t; +a b c d e +11 10 10 10 10 +update t, t tt set t.b=11, tt.d=11 where t.a=tt.a; +check table t; +Table Op Msg_type Msg_text +test.t check status OK +select * from t; +a b c d e +11 11 11 11 11 +drop table t, t1, t2; +create table t (f1 int, f2 int, f3 int as (f1*2) virtual, key(f3,f2)); +insert into t (f1,f2) values (1,1),(2,2); +create view v as +select a2.f1, a2.f2, a1.f3 +from t a1, t a2 +where a2.f3 <> 0 +with local check option; +update v set f3 = 52; +drop view v; +drop table t; +set binlog_row_image="MINIMAL"; +create table t1 (a int, b int as (a+1), c int as (b+1) stored); +insert t1 set a=1; +select * from t1; +a b c +1 2 3 +update t1 set a=2; +select * from t1; +a b c +2 3 4 +drop table t1; +create table t1 (a int, c int as(a), p varchar(20) as(y), y char(20), index (p,c)); +show create table t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `a` int(11) DEFAULT NULL, + `c` int(11) GENERATED ALWAYS AS (`a`) VIRTUAL, + `p` varchar(20) GENERATED ALWAYS AS (`y`) VIRTUAL, + `y` char(20) DEFAULT NULL, + KEY `p` (`p`,`c`) +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +insert into t1 (a,y) values(1, "yyy"); +update t1 set a = 100 where a = 1; +check table t1; +Table Op Msg_type Msg_text +test.t1 check status OK +drop table t1; +create table t1 ( +a varchar(10000), +b varchar(3000), +c varchar(14000) generated always as (concat(a,b)) virtual, +d varchar(5000) generated always as (b) virtual, +e int(11) generated always as (10) virtual, +h int(11) not null primary key, +index(c(100), d(20))); +insert t1 (a,b,h) values (repeat('g', 10000), repeat('x', 2800), 1); +update t1 set a = repeat(cast(1 as char), 2000); +drop table t1; +create table t1 ( +a varchar(10000), +b varchar(3000), +c varchar(14000) generated always as (concat(a,b)) virtual, +i varchar(5000) generated always as (b) virtual, +d varchar(5000) generated always as (i) virtual, +e int(11) generated always as (10) virtual, +h int(11) not null primary key, +index(c(100), d(20))); +insert t1 (a,b,h) values (repeat('g', 10000), repeat('x', 2800), 1); +update t1 set a = repeat(cast(1 as char), 2000); +drop table t1; +create table t1(a blob not null, b int, c varbinary (10) generated always as (a) virtual, unique (c(9))); +insert t1 (a,b) values ('a', 1); +replace t1 set a = 'a',b =1; +insert t1 (a,b) values ('a', 1) on duplicate key update a='b', b=2; +select * from t1; +a b c +b 2 b +drop table t1; +create table t (a int primary key, b int, c int as (b), index (c)); +insert t (a,b) values (9,0); +create table t2 select * from t; +update t, t2 set t.b=10 where t.a=t2.a; +check table t; +Table Op Msg_type Msg_text +test.t check status OK +select * from t; +a b c +9 10 10 +drop table t, t2; +create table t1 (a int, b int, c int, d int, e int); +insert t1 values (1,2,3,4,5), (1,2,3,4,5); +SET STATEMENT sql_mode = 'NO_ENGINE_SUBSTITUTION' FOR +create table t (a int primary key, +b int, c blob as (b), index (c(57)), +d blob, e blob as (d), index (e(57))) +replace select * from t1; +Warnings: +Warning 1906 The value specified for generated column 'c' in table 't' ignored +Warning 1906 The value specified for generated column 'e' in table 't' ignored +Warning 1906 The value specified for generated column 'c' in table 't' ignored +Warning 1906 The value specified for generated column 'e' in table 't' ignored +check table t; +Table Op Msg_type Msg_text +test.t check status OK +select * from t; +a b c d e +1 2 2 4 4 +update t set a=10, b=1, d=1; +check table t; +Table Op Msg_type Msg_text +test.t check status OK +select * from t; +a b c d e +10 1 1 1 1 +replace t (a,b,d) values (10,2,2); +check table t; +Table Op Msg_type Msg_text +test.t check status OK +select * from t; +a b c d e +10 2 2 2 2 +insert t(a,b,d) values (10) on duplicate key update b=3; +ERROR 21S01: Column count doesn't match value count at row 1 +insert t(a,b,d) values (10,2,2) on duplicate key update b=3, d=3; +check table t; +Table Op Msg_type Msg_text +test.t check status OK +select * from t; +a b c d e +10 3 3 3 3 +replace t (a,b,d) select 10,4,4; +check table t; +Table Op Msg_type Msg_text +test.t check status OK +select * from t; +a b c d e +10 4 4 4 4 +insert t(a,b,d) select 10,4,4 on duplicate key update b=5, d=5; +check table t; +Table Op Msg_type Msg_text +test.t check status OK +select * from t; +a b c d e +10 5 5 5 5 +replace delayed t (a,b,d) values (10,6,6); +flush tables; +check table t; +Table Op Msg_type Msg_text +test.t check status OK +select * from t; +a b c d e +10 6 6 6 6 +insert delayed t(a,b,d) values (10,6,6) on duplicate key update b=7, d=7; +flush tables; +check table t; +Table Op Msg_type Msg_text +test.t check status OK +select * from t; +a b c d e +10 7 7 7 7 +load data infile 'MYSQLTEST_VARDIR/tmp/vblobs.txt' replace into table t; +check table t; +Table Op Msg_type Msg_text +test.t check status OK +select * from t; +a b c d e +10 8 8 8 8 +update t set a=11, b=9, d=9 where a>5; +check table t; +Table Op Msg_type Msg_text +test.t check status OK +select * from t; +a b c d e +11 9 9 9 9 +create table t2 select * from t; +update t, t2 set t.b=10, t.d=10 where t.a=t2.a; +check table t; +Table Op Msg_type Msg_text +test.t check status OK +select * from t; +a b c d e +11 10 10 10 10 +update t, t tt set t.b=11, tt.d=11 where t.a=tt.a; +check table t; +Table Op Msg_type Msg_text +test.t check status OK +select * from t; +a b c d e +11 11 11 11 11 +drop table t, t1, t2; +create table t (f1 int, f2 int, f3 int as (f1*2) virtual, key(f3,f2)); +insert into t (f1,f2) values (1,1),(2,2); +create view v as +select a2.f1, a2.f2, a1.f3 +from t a1, t a2 +where a2.f3 <> 0 +with local check option; +update v set f3 = 52; +drop view v; +drop table t; diff --git a/mysql-test/suite/vcol/t/binlog.test b/mysql-test/suite/vcol/t/binlog.test new file mode 100644 index 00000000000..95bb4df4cc5 --- /dev/null +++ b/mysql-test/suite/vcol/t/binlog.test @@ -0,0 +1,55 @@ +--source include/have_innodb.inc +--source include/have_binlog_format_row.inc +--source include/master-slave.inc + +# +# MDEV-15243 +# Server crashes in in Field_blob::pack upon REPLACE into view with virtual +# columns with binlog enabled +# + +CREATE TABLE t1 ( + pk SERIAL, + vcol_date DATE AS (col_date) PERSISTENT, + vcol_int INT AS (col_int) VIRTUAL, + vcol_year YEAR AS (col_year) PERSISTENT, + vcol_blob BLOB AS (col_blob) VIRTUAL, + col_date DATE, + col_int INT NULL, + col_blob BLOB NULL, + col_year YEAR, + PRIMARY KEY(pk) +) ENGINE=InnoDB; + +INSERT INTO t1 (col_date,col_int,col_blob,col_year) VALUES ('2010-04-24',5,'foo',1981); +SET SQL_MODE=''; + +set binlog_row_image="FULL"; +CREATE VIEW v1 AS SELECT * FROM t1; +REPLACE INTO v1 SELECT pk, vcol_date, vcol_int, vcol_year, vcol_blob, col_date, col_int, col_blob, 1982 FROM t1; +select col_date,col_int,col_blob,col_year from v1; +sync_slave_with_master; +select col_date,col_int,col_blob,col_year from v1; +connection master; +DROP VIEW v1; +set binlog_row_image="MINIMAL"; +CREATE VIEW v1 AS SELECT * FROM t1; +REPLACE INTO v1 SELECT pk, vcol_date, vcol_int, vcol_year, vcol_blob, col_date, col_int, col_blob, 1983 FROM t1; +select col_date,col_int,col_blob,col_year from v1; +sync_slave_with_master; +select col_date,col_int,col_blob,col_year from v1; +connection master; +DROP VIEW v1; +set @@binlog_row_image="NOBLOB"; +CREATE VIEW v1 AS SELECT * FROM t1; +REPLACE INTO v1 SELECT pk, vcol_date, vcol_int, vcol_year, vcol_blob, col_date, col_int, col_blob, 1984 FROM t1; +select col_date,col_int,col_blob,col_year from v1; +sync_slave_with_master; +select col_date,col_int,col_blob,col_year from v1; +connection master; +DROP VIEW v1; +set @@binlog_row_image=default; + +DROP TABLE t1; + +--source include/rpl_end.inc diff --git a/mysql-test/suite/vcol/t/update.test b/mysql-test/suite/vcol/t/update.test index 1797bdd501e..53189ee3219 100644 --- a/mysql-test/suite/vcol/t/update.test +++ b/mysql-test/suite/vcol/t/update.test @@ -15,8 +15,10 @@ drop table t1; # this tests TABLE::mark_columns_needed_for_update() # create table t1 (a int, c int as(a), p varchar(20) as(y), y char(20), index (p,c)); +show create table t1; insert into t1 (a,y) values(1, "yyy"); update t1 set a = 100 where a = 1; +check table t1; drop table t1; # diff --git a/mysql-test/suite/vcol/t/update_binlog.test b/mysql-test/suite/vcol/t/update_binlog.test new file mode 100644 index 00000000000..458aac480c7 --- /dev/null +++ b/mysql-test/suite/vcol/t/update_binlog.test @@ -0,0 +1,14 @@ +# +# Check that vcol update works with binlog enabled +# + +--source include/have_binlog_format_row.inc + +set binlog_row_image="FULL"; +set @@default_storage_engine="myisam"; + +--source update.test + +set binlog_row_image="MINIMAL"; + +--source update.test diff --git a/sql/sql_class.cc b/sql/sql_class.cc index 23eda5ead77..577007dad38 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -6567,15 +6567,17 @@ int THD::binlog_delete_row(TABLE* table, bool is_trans, } +/** + Remove from read_set spurious columns. The write_set has been + handled before in table->mark_columns_needed_for_update. +*/ + void THD::binlog_prepare_row_images(TABLE *table) { DBUG_ENTER("THD::binlog_prepare_row_images"); - /** - Remove from read_set spurious columns. The write_set has been - handled before in table->mark_columns_needed_for_update. - */ - DBUG_PRINT_BITSET("debug", "table->read_set (before preparing): %s", table->read_set); + DBUG_PRINT_BITSET("debug", "table->read_set (before preparing): %s", + table->read_set); THD *thd= table->in_use; /** @@ -6593,7 +6595,7 @@ void THD::binlog_prepare_row_images(TABLE *table) */ DBUG_ASSERT(table->read_set != &table->tmp_set); - switch(thd->variables.binlog_row_image) + switch (thd->variables.binlog_row_image) { case BINLOG_ROW_IMAGE_MINIMAL: /* MINIMAL: Mark only PK */ @@ -6623,7 +6625,8 @@ void THD::binlog_prepare_row_images(TABLE *table) table->write_set); } - DBUG_PRINT_BITSET("debug", "table->read_set (after preparing): %s", table->read_set); + DBUG_PRINT_BITSET("debug", "table->read_set (after preparing): %s", + table->read_set); DBUG_VOID_RETURN; } diff --git a/sql/table.cc b/sql/table.cc index d7cbf555b72..b0e1f07a881 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -6566,6 +6566,12 @@ void TABLE::mark_columns_per_binlog_row_image() DBUG_ASSERT(FALSE); } } + /* + We have to ensure that all virtual columns that are part of read set + are calculated. + */ + if (vcol_set) + bitmap_union(vcol_set, read_set); file->column_bitmaps_signal(); } @@ -6607,7 +6613,8 @@ bool TABLE::mark_virtual_col(Field *field) /* @brief Mark virtual columns for update/insert commands - @param insert_fl <-> virtual columns are marked for insert command + @param insert_fl true if virtual columns are marked for insert command + For the moment this is not used, may be used in future. @details The function marks virtual columns used in a update/insert commands @@ -6632,7 +6639,8 @@ bool TABLE::mark_virtual_col(Field *field) be added to read_set either. */ -bool TABLE::mark_virtual_columns_for_write(bool insert_fl) +bool TABLE::mark_virtual_columns_for_write(bool insert_fl + __attribute__((unused))) { Field **vfield_ptr, *tmp_vfield; bool bitmap_updated= false; @@ -6642,35 +6650,13 @@ bool TABLE::mark_virtual_columns_for_write(bool insert_fl) { tmp_vfield= *vfield_ptr; if (bitmap_is_set(write_set, tmp_vfield->field_index)) - bitmap_updated= mark_virtual_col(tmp_vfield); + bitmap_updated|= mark_virtual_col(tmp_vfield); else if (tmp_vfield->vcol_info->stored_in_db || (tmp_vfield->flags & (PART_KEY_FLAG | FIELD_IN_PART_FUNC_FLAG))) { - if (insert_fl) - { - bitmap_set_bit(write_set, tmp_vfield->field_index); - mark_virtual_col(tmp_vfield); - bitmap_updated= true; - } - else - { - MY_BITMAP *save_read_set= read_set, *save_vcol_set= vcol_set; - Item *vcol_item= tmp_vfield->vcol_info->expr; - DBUG_ASSERT(vcol_item); - bitmap_clear_all(&tmp_set); - read_set= vcol_set= &tmp_set; - vcol_item->walk(&Item::register_field_in_read_map, 1, 0); - read_set= save_read_set; - vcol_set= save_vcol_set; - if (bitmap_is_overlapping(&tmp_set, write_set)) - { - bitmap_set_bit(write_set, tmp_vfield->field_index); - bitmap_set_bit(vcol_set, tmp_vfield->field_index); - bitmap_union(read_set, &tmp_set); - bitmap_union(vcol_set, &tmp_set); - bitmap_updated= true; - } - } + bitmap_set_bit(write_set, tmp_vfield->field_index); + mark_virtual_col(tmp_vfield); + bitmap_updated= true; } } if (bitmap_updated) @@ -7262,8 +7248,8 @@ bool TABLE_LIST::process_index_hints(TABLE *tbl) } /* - TODO: get rid of tbl->force_index (on if any FORCE INDEX is specified) and - create tbl->force_index_join instead. + TODO: get rid of tbl->force_index (on if any FORCE INDEX is specified) + and create tbl->force_index_join instead. Then use the correct force_index_XX instead of the global one. */ if (!index_join[INDEX_HINT_FORCE].is_clear_all() || @@ -7299,15 +7285,21 @@ size_t max_row_length(TABLE *table, const uchar *data) size_t length= table_s->reclength + 2 * table_s->fields; uint *const beg= table_s->blob_field; uint *const end= beg + table_s->blob_fields; + my_ptrdiff_t const rec_offset= (my_ptrdiff_t) (data - table->record[0]); + DBUG_ENTER("max_row_length"); for (uint *ptr= beg ; ptr != end ; ++ptr) { - Field_blob* const blob= (Field_blob*) table->field[*ptr]; - length+= blob->get_length((const uchar*) - (data + blob->offset(table->record[0]))) + - HA_KEY_BLOB_LENGTH; + Field * const field= table->field[*ptr]; + if (bitmap_is_set(table->read_set, field->field_index) && + !field->is_null(rec_offset)) + { + Field_blob * const blob= (Field_blob*) field; + length+= blob->get_length(rec_offset) + HA_KEY_BLOB_LENGTH; + } } - return length; + DBUG_PRINT("exit", ("length: %lld", (longlong) length)); + DBUG_RETURN(length); } @@ -7422,7 +7414,7 @@ int TABLE::update_virtual_fields(handler *h, enum_vcol_update_mode update_mode) Query_arena backup_arena; Turn_errors_to_warnings_handler Suppress_errors; int error; - bool handler_pushed= 0; + bool handler_pushed= 0, update_all_columns= 1; DBUG_ASSERT(vfield); if (h->keyread_enabled()) @@ -7439,6 +7431,16 @@ int TABLE::update_virtual_fields(handler *h, enum_vcol_update_mode update_mode) in_use->push_internal_handler(&Suppress_errors); handler_pushed= 1; } + else if (update_mode == VCOL_UPDATE_FOR_REPLACE && + in_use->is_current_stmt_binlog_format_row() && + in_use->variables.binlog_row_image != BINLOG_ROW_IMAGE_MINIMAL) + { + /* + If we are doing a replace with not minimal binary logging, we have to + calculate all virtual columns. + */ + update_all_columns= 1; + } /* Iterate over virtual fields in the table */ for (vfield_ptr= vfield; *vfield_ptr; vfield_ptr++) @@ -7451,8 +7453,8 @@ int TABLE::update_virtual_fields(handler *h, enum_vcol_update_mode update_mode) bool update= 0, swap_values= 0; switch (update_mode) { case VCOL_UPDATE_FOR_READ: - update= !vcol_info->stored_in_db - && bitmap_is_set(vcol_set, vf->field_index); + update= (!vcol_info->stored_in_db && + bitmap_is_set(vcol_set, vf->field_index)); swap_values= 1; break; case VCOL_UPDATE_FOR_DELETE: @@ -7460,8 +7462,9 @@ int TABLE::update_virtual_fields(handler *h, enum_vcol_update_mode update_mode) update= bitmap_is_set(vcol_set, vf->field_index); break; case VCOL_UPDATE_FOR_REPLACE: - update= !vcol_info->stored_in_db && (vf->flags & PART_KEY_FLAG) - && bitmap_is_set(vcol_set, vf->field_index); + update= ((!vcol_info->stored_in_db && (vf->flags & PART_KEY_FLAG) && + bitmap_is_set(vcol_set, vf->field_index)) || + update_all_columns); if (update && (vf->flags & BLOB_FLAG)) { /* @@ -7478,8 +7481,8 @@ int TABLE::update_virtual_fields(handler *h, enum_vcol_update_mode update_mode) case VCOL_UPDATE_INDEXED: case VCOL_UPDATE_INDEXED_FOR_UPDATE: /* Read indexed fields that was not updated in VCOL_UPDATE_FOR_READ */ - update= !vcol_info->stored_in_db && (vf->flags & PART_KEY_FLAG) && - bitmap_is_set(vcol_set, vf->field_index); + update= (!vcol_info->stored_in_db && (vf->flags & PART_KEY_FLAG) && + !bitmap_is_set(vcol_set, vf->field_index)); swap_values= 1; break; } From 72a8d29e00d4deec7c836228b54e19fa35ab7ad7 Mon Sep 17 00:00:00 2001 From: Monty Date: Wed, 23 May 2018 22:45:32 +0300 Subject: [PATCH 10/26] Fixed archive to work with record[1] ha_archive::max_row_length() and ha_archive::pack_row() didn't use record parameter properly. Especially testing of null was wrong for record[1] --- storage/archive/ha_archive.cc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/storage/archive/ha_archive.cc b/storage/archive/ha_archive.cc index afa39e6a5f7..2b6079d4a29 100644 --- a/storage/archive/ha_archive.cc +++ b/storage/archive/ha_archive.cc @@ -897,18 +897,19 @@ int ha_archive::real_write_row(uchar *buf, azio_stream *writer) the bytes required for the length in the header. */ -uint32 ha_archive::max_row_length(const uchar *buf) +uint32 ha_archive::max_row_length(const uchar *record) { uint32 length= (uint32)(table->s->reclength + table->s->fields*2); length+= ARCHIVE_ROW_HEADER_SIZE; + my_ptrdiff_t const rec_offset= record - table->record[0]; uint *ptr, *end; for (ptr= table->s->blob_field, end=ptr + table->s->blob_fields ; ptr != end ; ptr++) { - if (!table->field[*ptr]->is_null()) - length += 2 + ((Field_blob*)table->field[*ptr])->get_length(); + if (!table->field[*ptr]->is_null(rec_offset)) + length += 2 + ((Field_blob*)table->field[*ptr])->get_length(rec_offset); } return length; @@ -918,10 +919,9 @@ uint32 ha_archive::max_row_length(const uchar *buf) unsigned int ha_archive::pack_row(uchar *record, azio_stream *writer) { uchar *ptr; - + my_ptrdiff_t const rec_offset= record - table->record[0]; DBUG_ENTER("ha_archive::pack_row"); - if (fix_rec_buff(max_row_length(record))) DBUG_RETURN(HA_ERR_OUT_OF_MEM); /* purecov: inspected */ @@ -935,7 +935,7 @@ unsigned int ha_archive::pack_row(uchar *record, azio_stream *writer) for (Field **field=table->field ; *field ; field++) { - if (!((*field)->is_null())) + if (!((*field)->is_null(rec_offset))) ptr= (*field)->pack(ptr, record + (*field)->offset(record)); } From 7a4f81b4c0d78e51c7b260b6f84e2732fbb99192 Mon Sep 17 00:00:00 2001 From: Monty Date: Thu, 24 May 2018 16:20:31 +0300 Subject: [PATCH 11/26] Extend debug_assert_on_not_freed_memory Don't check global_memory_used if debug_assert_on_not_freed_memory is not set --- sql/mysqld.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sql/mysqld.cc b/sql/mysqld.cc index 99cad2c0423..8dd178108b9 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -2169,7 +2169,8 @@ static void mysqld_exit(int exit_code) if (opt_endinfo && global_status_var.global_memory_used) fprintf(stderr, "Warning: Memory not freed: %ld\n", (long) global_status_var.global_memory_used); - if (!opt_debugging && !my_disable_leak_check && exit_code == 0) + if (!opt_debugging && !my_disable_leak_check && exit_code == 0 && + debug_assert_on_not_freed_memory) { DBUG_ASSERT(global_status_var.global_memory_used == 0); } From b8fdd56a4d64d9bb02f2e3e9609af82e558cbcb5 Mon Sep 17 00:00:00 2001 From: Vladislav Vaintroub Date: Thu, 24 May 2018 20:11:52 +0000 Subject: [PATCH 12/26] Fix conversion warnings/errors. --- sql/field.h | 2 +- unittest/sql/mf_iocache-t.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sql/field.h b/sql/field.h index 7074a636ff9..4bf92faecdd 100644 --- a/sql/field.h +++ b/sql/field.h @@ -3319,7 +3319,7 @@ public: { store_length(ptr, packlength, number); } - inline uint32 get_length(uint row_offset= 0) const + inline uint32 get_length(my_ptrdiff_t row_offset= 0) const { return get_length(ptr+row_offset, this->packlength); } uint32 get_length(const uchar *ptr, uint packlength) const; uint32 get_length(const uchar *ptr_arg) const diff --git a/unittest/sql/mf_iocache-t.cc b/unittest/sql/mf_iocache-t.cc index fb5577a7ab9..9f2e4b1b303 100644 --- a/unittest/sql/mf_iocache-t.cc +++ b/unittest/sql/mf_iocache-t.cc @@ -208,7 +208,7 @@ void mdev10259() res= my_b_flush_io_cache(&info, 1); ok(res == 0, "flush" INFO_TAIL); - ulong saved_pos= my_b_tell(&info); + my_off_t saved_pos= my_b_tell(&info); res= reinit_io_cache(&info, READ_CACHE, 0, 0, 0); ok(res == 0, "reinit READ_CACHE" INFO_TAIL); From 9e22cae1cfcb4fce7a6469e9b136d599efe6b87c Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Wed, 23 May 2018 23:30:14 +0200 Subject: [PATCH 13/26] embedded use-after-free ASAN error Close MYSQL (and destroy THD) in the same thread where it was used, because THD embeds MDL_context, that owns some LF_PINS, that remember a pointer to my_thread_var->stack_ends_here. --- client/mysqltest.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/client/mysqltest.cc b/client/mysqltest.cc index 5ab84323a76..620a3417ac3 100644 --- a/client/mysqltest.cc +++ b/client/mysqltest.cc @@ -903,6 +903,8 @@ pthread_handler_t connection_thread(void *arg) end_thread: cn->query_done= 1; + mysql_close(cn->mysql); + cn->mysql= 0; mysql_thread_end(); pthread_exit(0); return 0; From 29dbb23fb7ac3b10e70e4c5f99dcedab91af85ba Mon Sep 17 00:00:00 2001 From: Monty Date: Thu, 24 May 2018 15:18:09 +0300 Subject: [PATCH 14/26] MDEV-16093 Memory leak with triggers Problem was that blob memory allocated in Table_trigger_list was not properly freed --- mysql-test/r/trigger.result | 13 ++++++++++++- mysql-test/t/trigger.test | 14 +++++++++++++- sql/sql_trigger.cc | 6 ++++++ 3 files changed, 31 insertions(+), 2 deletions(-) diff --git a/mysql-test/r/trigger.result b/mysql-test/r/trigger.result index 8455450e294..8852a622251 100644 --- a/mysql-test/r/trigger.result +++ b/mysql-test/r/trigger.result @@ -2306,4 +2306,15 @@ CREATE TABLE t1 (i INT); insert into t2 value (2); DROP VIEW v1; DROP TABLE t1,t2,t3; -End of 10.1 tests. +# +# MDEV-16093 +# Assertion `global_status_var.global_memory_used == 0' failed or +# bytes lost after inserting into table with non-null blob and trigger +# +CREATE TABLE t1 (b BLOB NOT NULL); +CREATE TRIGGER tr BEFORE UPDATE ON t1 FOR EACH ROW BEGIN END; +INSERT INTO t1 VALUES ('foo'); +DROP TABLE t1; +# +# End of 10.1 tests. +# diff --git a/mysql-test/t/trigger.test b/mysql-test/t/trigger.test index ff6f38b719d..a6d4107e591 100644 --- a/mysql-test/t/trigger.test +++ b/mysql-test/t/trigger.test @@ -2656,5 +2656,17 @@ insert into t2 value (2); DROP VIEW v1; DROP TABLE t1,t2,t3; +--echo # +--echo # MDEV-16093 +--echo # Assertion `global_status_var.global_memory_used == 0' failed or +--echo # bytes lost after inserting into table with non-null blob and trigger +--echo # ---echo End of 10.1 tests. +CREATE TABLE t1 (b BLOB NOT NULL); +CREATE TRIGGER tr BEFORE UPDATE ON t1 FOR EACH ROW BEGIN END; +INSERT INTO t1 VALUES ('foo'); +DROP TABLE t1; + +--echo # +--echo # End of 10.1 tests. +--echo # diff --git a/sql/sql_trigger.cc b/sql/sql_trigger.cc index 293a4c17156..bbcc75718a3 100644 --- a/sql/sql_trigger.cc +++ b/sql/sql_trigger.cc @@ -614,6 +614,7 @@ end: #endif /* WITH_WSREP */ } + /** Build stmt_query to write it in the bin-log and get the trigger definer. @@ -1061,6 +1062,11 @@ Table_triggers_list::~Table_triggers_list() for (int j= 0; j < (int)TRG_ACTION_MAX; j++) delete bodies[i][j]; + /* Free blobs used in insert */ + if (record0_field) + for (Field **fld_ptr= record0_field; *fld_ptr; fld_ptr++) + (*fld_ptr)->free(); + if (record1_field) for (Field **fld_ptr= record1_field; *fld_ptr; fld_ptr++) delete *fld_ptr; From 5a16fe0e6f33f1b123bbe9422126dd3b0fdf5ed1 Mon Sep 17 00:00:00 2001 From: Monty Date: Thu, 24 May 2018 15:19:55 +0300 Subject: [PATCH 15/26] Fixed compiler warnings When merging this with 10.2 and later, one can just use the 10.2 or later code --- storage/connect/ha_connect.cc | 2 +- storage/sphinx/ha_sphinx.cc | 87 +++++++++++++++++------------------ 2 files changed, 43 insertions(+), 46 deletions(-) diff --git a/storage/connect/ha_connect.cc b/storage/connect/ha_connect.cc index e6bfa97f327..af329c0c8df 100644 --- a/storage/connect/ha_connect.cc +++ b/storage/connect/ha_connect.cc @@ -432,7 +432,7 @@ handlerton *connect_hton= NULL; uint GetTraceValue(void) {return (uint)(connect_hton ? THDVAR(current_thd, xtrace) : 0);} bool ExactInfo(void) {return THDVAR(current_thd, exact_info);} -bool CondPushEnabled(void) {return THDVAR(current_thd, cond_push);} +static bool CondPushEnabled(void) {return THDVAR(current_thd, cond_push);} USETEMP UseTemp(void) {return (USETEMP)THDVAR(current_thd, use_tempfile);} int GetConvSize(void) {return THDVAR(current_thd, conv_size);} TYPCONV GetTypeConv(void) {return (TYPCONV)THDVAR(current_thd, type_conv);} diff --git a/storage/sphinx/ha_sphinx.cc b/storage/sphinx/ha_sphinx.cc index 5308f780e0e..3f6770b5d26 100644 --- a/storage/sphinx/ha_sphinx.cc +++ b/storage/sphinx/ha_sphinx.cc @@ -17,7 +17,7 @@ #pragma implementation // gcc: Class implementation #endif -#if _MSC_VER>=1400 +#if defined(_MSC_VER) && _MSC_VER>=1400 #define _CRT_SECURE_NO_DEPRECATE 1 #define _CRT_NONSTDC_NO_DEPRECATE 1 #endif @@ -64,7 +64,7 @@ #define MSG_WAITALL 0 #endif -#if _MSC_VER>=1400 +#if defined(_MSC_VER) && _MSC_VER>=1400 #pragma warning(push,4) #endif @@ -1041,8 +1041,8 @@ static bool ParseUrl ( CSphSEShare * share, TABLE * table, bool bCreate ) bool bOk = true; bool bQL = false; char * sScheme = NULL; - char * sHost = SPHINXAPI_DEFAULT_HOST; - char * sIndex = SPHINXAPI_DEFAULT_INDEX; + char * sHost = (char*) SPHINXAPI_DEFAULT_HOST; + char * sIndex = (char*) SPHINXAPI_DEFAULT_INDEX; int iPort = SPHINXAPI_DEFAULT_PORT; // parse connection string, if any @@ -1068,12 +1068,12 @@ static bool ParseUrl ( CSphSEShare * share, TABLE * table, bool bCreate ) sHost--; // reuse last slash iPort = 0; if (!( sIndex = strrchr ( sHost, ':' ) )) - sIndex = SPHINXAPI_DEFAULT_INDEX; + sIndex = (char*) SPHINXAPI_DEFAULT_INDEX; else { *sIndex++ = '\0'; if ( !*sIndex ) - sIndex = SPHINXAPI_DEFAULT_INDEX; + sIndex = (char*) SPHINXAPI_DEFAULT_INDEX; } bOk = true; break; @@ -1095,7 +1095,7 @@ static bool ParseUrl ( CSphSEShare * share, TABLE * table, bool bCreate ) if ( sIndex ) *sIndex++ = '\0'; else - sIndex = SPHINXAPI_DEFAULT_INDEX; + sIndex = (char*) SPHINXAPI_DEFAULT_INDEX; iPort = atoi(sPort); if ( !iPort ) @@ -1107,7 +1107,7 @@ static bool ParseUrl ( CSphSEShare * share, TABLE * table, bool bCreate ) if ( sIndex ) *sIndex++ = '\0'; else - sIndex = SPHINXAPI_DEFAULT_INDEX; + sIndex = (char*) SPHINXAPI_DEFAULT_INDEX; } bOk = true; break; @@ -1303,8 +1303,8 @@ CSphSEQuery::CSphSEQuery ( const char * sQuery, int iLength, const char * sIndex , m_sGeoLongAttr ( "" ) , m_fGeoLatitude ( 0.0f ) , m_fGeoLongitude ( 0.0f ) - , m_sComment ( "" ) - , m_sSelect ( "*" ) + , m_sComment ( (char*) "" ) + , m_sSelect ( (char*) "*" ) , m_pBuf ( NULL ) , m_pCur ( NULL ) @@ -1738,7 +1738,7 @@ bool CSphSEQuery::ParseField ( char * sField ) } } else if ( !strcmp ( sName, "override" ) ) // name,type,id:value,id:value,... { - char * sName = NULL; + sName = NULL; int iType = 0; CSphSEQuery::Override_t * pOverride = NULL; @@ -1794,7 +1794,7 @@ bool CSphSEQuery::ParseField ( char * sField ) *sRest++ = '\0'; if (!( sRest - sId )) break; - char * sValue = sRest; + sValue = sRest; if ( ( sRest = strchr ( sRest, ',' ) )!=NULL ) *sRest++ = '\0'; if ( !*sValue ) @@ -2213,7 +2213,7 @@ int ha_sphinx::Connect ( const char * sHost, ushort uPort ) } char sError[512]; - int iSocket = socket ( iDomain, SOCK_STREAM, 0 ); + int iSocket = (int) socket ( iDomain, SOCK_STREAM, 0 ); if ( iSocket<0 ) { @@ -2538,12 +2538,6 @@ char * ha_sphinx::UnpackString () } -static inline const char * FixNull ( const char * s ) -{ - return s ? s : "(null)"; -} - - bool ha_sphinx::UnpackSchema () { SPH_ENTER_METHOD(); @@ -2674,7 +2668,7 @@ bool ha_sphinx::UnpackStats ( CSphSEStats * pStats ) assert ( pStats ); char * pCurSave = m_pCur; - for ( uint i=0; istore ( sBuf, pCur-sBuf, &my_charset_bin ); + af->store ( sBuf, uint(pCur-sBuf), &my_charset_bin ); } break; @@ -3386,39 +3380,39 @@ ha_rows ha_sphinx::records_in_range ( uint, key_range *, key_range * ) // currently provided for doing that. // // Called from handle.cc by ha_create_table(). -int ha_sphinx::create ( const char * name, TABLE * table, HA_CREATE_INFO * ) +int ha_sphinx::create ( const char * name, TABLE * table_arg, HA_CREATE_INFO * ) { SPH_ENTER_METHOD(); char sError[256]; CSphSEShare tInfo; - if ( !ParseUrl ( &tInfo, table, true ) ) + if ( !ParseUrl ( &tInfo, table_arg, true ) ) SPH_RET(-1); // check SphinxAPI table for ( ; !tInfo.m_bSphinxQL; ) { // check system fields (count and types) - if ( table->s->fieldss->fieldsfield[0] ) ) + if ( !IsIDField ( table_arg->field[0] ) ) { my_snprintf ( sError, sizeof(sError), "%s: 1st column (docid) MUST be unsigned integer or bigint", name ); break; } - if ( !IsIntegerFieldType ( table->field[1]->type() ) ) + if ( !IsIntegerFieldType ( table_arg->field[1]->type() ) ) { my_snprintf ( sError, sizeof(sError), "%s: 2nd column (weight) MUST be integer or bigint", name ); break; } - enum_field_types f2 = table->field[2]->type(); + enum_field_types f2 = table_arg->field[2]->type(); if ( f2!=MYSQL_TYPE_VARCHAR && f2!=MYSQL_TYPE_BLOB && f2!=MYSQL_TYPE_MEDIUM_BLOB && f2!=MYSQL_TYPE_LONG_BLOB && f2!=MYSQL_TYPE_TINY_BLOB ) { @@ -3428,28 +3422,28 @@ int ha_sphinx::create ( const char * name, TABLE * table, HA_CREATE_INFO * ) // check attributes int i; - for ( i=3; i<(int)table->s->fields; i++ ) + for ( i=3; i<(int)table_arg->s->fields; i++ ) { - enum_field_types eType = table->field[i]->type(); + enum_field_types eType = table_arg->field[i]->type(); if ( eType!=MYSQL_TYPE_TIMESTAMP && !IsIntegerFieldType(eType) && eType!=MYSQL_TYPE_VARCHAR && eType!=MYSQL_TYPE_FLOAT ) { my_snprintf ( sError, sizeof(sError), "%s: %dth column (attribute %s) MUST be integer, bigint, timestamp, varchar, or float", - name, i+1, table->field[i]->field_name ); + name, i+1, table_arg->field[i]->field_name ); break; } } - if ( i!=(int)table->s->fields ) + if ( i!=(int)table_arg->s->fields ) break; // check index if ( - table->s->keys!=1 || - table->key_info[0].user_defined_key_parts!=1 || - strcasecmp ( table->key_info[0].key_part[0].field->field_name, table->field[2]->field_name ) ) + table_arg->s->keys!=1 || + table_arg->key_info[0].user_defined_key_parts!=1 || + strcasecmp ( table_arg->key_info[0].key_part[0].field->field_name, table->field[2]->field_name ) ) { my_snprintf ( sError, sizeof(sError), "%s: there must be an index on '%s' column", - name, table->field[2]->field_name ); + name, table_arg->field[2]->field_name ); break; } @@ -3464,13 +3458,13 @@ int ha_sphinx::create ( const char * name, TABLE * table, HA_CREATE_INFO * ) sError[0] = '\0'; // check that 1st column is id, is of int type, and has an index - if ( strcmp ( table->field[0]->field_name, "id" ) ) + if ( strcmp ( table_arg->field[0]->field_name, "id" ) ) { my_snprintf ( sError, sizeof(sError), "%s: 1st column must be called 'id'", name ); break; } - if ( !IsIDField ( table->field[0] ) ) + if ( !IsIDField ( table_arg->field[0] ) ) { my_snprintf ( sError, sizeof(sError), "%s: 'id' column must be INT UNSIGNED or BIGINT", name ); break; @@ -3478,22 +3472,22 @@ int ha_sphinx::create ( const char * name, TABLE * table, HA_CREATE_INFO * ) // check index if ( - table->s->keys!=1 || - table->key_info[0].user_defined_key_parts!=1 || - strcasecmp ( table->key_info[0].key_part[0].field->field_name, "id" ) ) + table_arg->s->keys!=1 || + table_arg->key_info[0].user_defined_key_parts!=1 || + strcasecmp ( table_arg->key_info[0].key_part[0].field->field_name, "id" ) ) { my_snprintf ( sError, sizeof(sError), "%s: 'id' column must be indexed", name ); break; } // check column types - for ( int i=1; i<(int)table->s->fields; i++ ) + for ( int i=1; i<(int)table_arg->s->fields; i++ ) { - enum_field_types eType = table->field[i]->type(); + enum_field_types eType = table_arg->field[i]->type(); if ( eType!=MYSQL_TYPE_TIMESTAMP && !IsIntegerFieldType(eType) && eType!=MYSQL_TYPE_VARCHAR && eType!=MYSQL_TYPE_FLOAT ) { my_snprintf ( sError, sizeof(sError), "%s: column %d(%s) is of unsupported type (use int/bigint/timestamp/varchar/float)", - name, i+1, table->field[i]->field_name ); + name, i+1, table_arg->field[i]->field_name ); break; } } @@ -3507,8 +3501,11 @@ int ha_sphinx::create ( const char * name, TABLE * table, HA_CREATE_INFO * ) // report and bail if ( sError[0] ) { - my_error ( ER_CANT_CREATE_TABLE, MYF(0), - table->s->db.str, table->s->table_name, sError ); + my_printf_error(ER_CANT_CREATE_TABLE, + "Can\'t create table %s.%s (Error: %s)", + MYF(0), + table_arg->s->db.str, + table_arg->s->table_name.str, sError); SPH_RET(-1); } From 199517f501b5d50daf85d3d5620cb391c03fddfe Mon Sep 17 00:00:00 2001 From: Monty Date: Thu, 24 May 2018 19:09:33 +0300 Subject: [PATCH 16/26] Avoid warnings in String::copy when copying string on itself (ok to do) --- sql/sql_string.cc | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/sql/sql_string.cc b/sql/sql_string.cc index 20772adcb22..2c5be35e887 100644 --- a/sql/sql_string.cc +++ b/sql/sql_string.cc @@ -183,7 +183,16 @@ bool String::copy(const char *str,uint32 arg_length, CHARSET_INFO *cs) { if (alloc(arg_length)) return TRUE; - if ((str_length=arg_length)) + if (Ptr == str && arg_length == str_length) + { + /* + This can happen in some cases. This code is here mainly to avoid + warnings from valgrind, but can also be an indication of error. + */ + DBUG_PRINT("warning", ("Copying string on itself: %p %lu", + str, arg_length)); + } + else if ((str_length=arg_length)) memcpy(Ptr,str,arg_length); Ptr[arg_length]=0; str_charset=cs; From d8da920264a0321e6d03b3cbe3c3b414f622aefa Mon Sep 17 00:00:00 2001 From: Monty Date: Fri, 25 May 2018 11:51:43 +0300 Subject: [PATCH 17/26] MDEV-10679 Crash in performance schema and partitioning with discovery Crash happened because in discover, table->work_part_info was not properly reset before execution. Fixed by resetting before calling execute alter table, create table or mysql_create_frm_image. --- mysql-test/suite/perfschema/r/partition.result | 10 ++++++++++ mysql-test/suite/perfschema/t/partition.test | 15 +++++++++++++++ sql/sql_alter.cc | 5 +++-- sql/sql_parse.cc | 5 +---- sql/sql_table.cc | 3 +++ sql/table.cc | 1 + 6 files changed, 33 insertions(+), 6 deletions(-) create mode 100644 mysql-test/suite/perfschema/r/partition.result create mode 100644 mysql-test/suite/perfschema/t/partition.test diff --git a/mysql-test/suite/perfschema/r/partition.result b/mysql-test/suite/perfschema/r/partition.result new file mode 100644 index 00000000000..9bc624268bb --- /dev/null +++ b/mysql-test/suite/perfschema/r/partition.result @@ -0,0 +1,10 @@ +# +# MDEV-10679 +# Server crashes in in mysql_create_frm_image upon query from +# performance schema in ps-protocol mode +# +CREATE TABLE t1 (i INT); +ALTER TABLE t1 ADD PARTITION (PARTITION p VALUES LESS THAN (1)); +ERROR HY000: Partition management on a not partitioned table is not possible +SELECT * FROM performance_schema.events_stages_summary_by_user_by_event_name; +DROP TABLE t1; diff --git a/mysql-test/suite/perfschema/t/partition.test b/mysql-test/suite/perfschema/t/partition.test new file mode 100644 index 00000000000..073a41e9252 --- /dev/null +++ b/mysql-test/suite/perfschema/t/partition.test @@ -0,0 +1,15 @@ +--source include/have_perfschema.inc + +--echo # +--echo # MDEV-10679 +--echo # Server crashes in in mysql_create_frm_image upon query from +--echo # performance schema in ps-protocol mode +--echo # + +CREATE TABLE t1 (i INT); +--error ER_PARTITION_MGMT_ON_NONPARTITIONED +ALTER TABLE t1 ADD PARTITION (PARTITION p VALUES LESS THAN (1)); +--disable_result_log +SELECT * FROM performance_schema.events_stages_summary_by_user_by_event_name; +--enable_result_log +DROP TABLE t1; diff --git a/sql/sql_alter.cc b/sql/sql_alter.cc index bff45e089a4..6f21fb4b931 100644 --- a/sql/sql_alter.cc +++ b/sql/sql_alter.cc @@ -262,8 +262,8 @@ bool Sql_cmd_alter_table::execute(THD *thd) - For temporary MERGE tables we do not track if their child tables are base or temporary. As result we can't guarantee that privilege check - which was done in presence of temporary child will stay relevant later - as this temporary table might be removed. + which was done in presence of temporary child will stay relevant + later as this temporary table might be removed. If SELECT_ACL | UPDATE_ACL | DELETE_ACL privileges were not checked for the underlying *base* tables, it would create a security breach as in @@ -303,6 +303,7 @@ bool Sql_cmd_alter_table::execute(THD *thd) create_info.data_file_name= create_info.index_file_name= NULL; thd->enable_slow_log= opt_log_slow_admin_statements; + thd->work_part_info= 0; #ifdef WITH_WSREP TABLE *find_temporary_table(THD *thd, const TABLE_LIST *tl); diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 55673ba2713..be7408af77d 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -2475,10 +2475,6 @@ mysql_execute_command(THD *thd) #endif DBUG_ENTER("mysql_execute_command"); -#ifdef WITH_PARTITION_STORAGE_ENGINE - thd->work_part_info= 0; -#endif - DBUG_ASSERT(thd->transaction.stmt.is_empty() || thd->in_sub_stmt); /* Each statement or replication event which might produce deadlock @@ -3318,6 +3314,7 @@ mysql_execute_command(THD *thd) create_info.add(DDL_options_st::OPT_OR_REPLACE_SLAVE_GENERATED); } + thd->work_part_info= 0; #ifdef WITH_PARTITION_STORAGE_ENGINE { partition_info *part_info= thd->lex->part_info; diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 31da585443a..a68f9e626e0 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -6057,6 +6057,7 @@ remove_key: } } + DBUG_ASSERT(thd->work_part_info == 0); #ifdef WITH_PARTITION_STORAGE_ENGINE partition_info *tab_part_info= table->part_info; thd->work_part_info= thd->lex->part_info; @@ -8411,6 +8412,8 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name, { DBUG_ENTER("mysql_alter_table"); + thd->work_part_info= 0; // Used by partitioning + /* Check if we attempt to alter mysql.slow_log or mysql.general_log table and return an error if diff --git a/sql/table.cc b/sql/table.cc index 6770ebdfd4a..d053e9b5670 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -2233,6 +2233,7 @@ int TABLE_SHARE::init_from_sql_statement_string(THD *thd, bool write, goto ret; thd->lex->create_info.db_type= hton; + thd->work_part_info= 0; // For partitioning if (tabledef_version.str) thd->lex->create_info.tabledef_version= tabledef_version; From 2d62a4cb2fc116c084d366619808cc4419d9dced Mon Sep 17 00:00:00 2001 From: Monty Date: Sat, 26 May 2018 17:01:42 +0300 Subject: [PATCH 18/26] Updated results for galera_encrypt_tmp_files --- mysql-test/suite/galera/r/galera_encrypt_tmp_files.result | 2 ++ 1 file changed, 2 insertions(+) diff --git a/mysql-test/suite/galera/r/galera_encrypt_tmp_files.result b/mysql-test/suite/galera/r/galera_encrypt_tmp_files.result index 63f6c281af1..38480d186ba 100644 --- a/mysql-test/suite/galera/r/galera_encrypt_tmp_files.result +++ b/mysql-test/suite/galera/r/galera_encrypt_tmp_files.result @@ -6,6 +6,7 @@ VARIABLE_VALUE = 2 1 CREATE TABLE t1 (f1 INTEGER PRIMARY KEY) Engine=InnoDB; INSERT INTO t1 VALUES (1); +connection node_2; SELECT VARIABLE_VALUE = 'Synced' FROM INFORMATION_SCHEMA.GLOBAL_STATUS WHERE VARIABLE_NAME = 'wsrep_local_state_comment'; VARIABLE_VALUE = 'Synced' 1 @@ -16,6 +17,7 @@ SELECT COUNT(*) = 1 FROM t1; COUNT(*) = 1 1 DROP TABLE t1; +connection node_1; CREATE TABLE `t1` ( `col1` int(11) NOT NULL, `col2` varchar(64) NOT NULL DEFAULT '', From 13c241c64f4609776169fd8a807270ad99241ca3 Mon Sep 17 00:00:00 2001 From: Monty Date: Sat, 26 May 2018 17:03:00 +0300 Subject: [PATCH 19/26] Fixed memory overrun in binlog_encryption.encrypted_master Problem was that max_row_lengt() used different bitmap than pack_row() --- sql/sql_class.cc | 12 ++++++++---- sql/table.cc | 6 +++--- sql/table.h | 2 +- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/sql/sql_class.cc b/sql/sql_class.cc index 577007dad38..7e15eff4dd4 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -6414,7 +6414,8 @@ int THD::binlog_write_row(TABLE* table, bool is_trans, Pack records into format for transfer. We are allocating more memory than needed, but that doesn't matter. */ - Row_data_memory memory(table, max_row_length(table, record)); + Row_data_memory memory(table, max_row_length(table, table->rpl_write_set, + record)); if (!memory.has_memory()) return HA_ERR_OUT_OF_MEM; @@ -6451,8 +6452,10 @@ int THD::binlog_update_row(TABLE* table, bool is_trans, DBUG_ASSERT(is_current_stmt_binlog_format_row() && ((WSREP(this) && wsrep_emulate_bin_log) || mysql_bin_log.is_open())); - size_t const before_maxlen = max_row_length(table, before_record); - size_t const after_maxlen = max_row_length(table, after_record); + size_t const before_maxlen= max_row_length(table, table->read_set, + before_record); + size_t const after_maxlen= max_row_length(table, table->rpl_write_set, + after_record); Row_data_memory row_data(table, before_maxlen, after_maxlen); if (!row_data.has_memory()) @@ -6528,7 +6531,8 @@ int THD::binlog_delete_row(TABLE* table, bool is_trans, Pack records into format for transfer. We are allocating more memory than needed, but that doesn't matter. */ - Row_data_memory memory(table, max_row_length(table, record)); + Row_data_memory memory(table, max_row_length(table, table->read_set, + record)); if (unlikely(!memory.has_memory())) return HA_ERR_OUT_OF_MEM; diff --git a/sql/table.cc b/sql/table.cc index b0e1f07a881..040eec0d2e8 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -7279,7 +7279,7 @@ bool TABLE_LIST::process_index_hints(TABLE *tbl) } -size_t max_row_length(TABLE *table, const uchar *data) +size_t max_row_length(TABLE *table, MY_BITMAP const *cols, const uchar *data) { TABLE_SHARE *table_s= table->s; size_t length= table_s->reclength + 2 * table_s->fields; @@ -7291,11 +7291,11 @@ size_t max_row_length(TABLE *table, const uchar *data) for (uint *ptr= beg ; ptr != end ; ++ptr) { Field * const field= table->field[*ptr]; - if (bitmap_is_set(table->read_set, field->field_index) && + if (bitmap_is_set(cols, field->field_index) && !field->is_null(rec_offset)) { Field_blob * const blob= (Field_blob*) field; - length+= blob->get_length(rec_offset) + HA_KEY_BLOB_LENGTH; + length+= blob->get_length(rec_offset) + 8; /* max blob store length */ } } DBUG_PRINT("exit", ("length: %lld", (longlong) length)); diff --git a/sql/table.h b/sql/table.h index 61ac6c06e38..6febeb555f9 100644 --- a/sql/table.h +++ b/sql/table.h @@ -2672,7 +2672,7 @@ enum get_table_share_flags { GTS_FORCE_DISCOVERY = 16 }; -size_t max_row_length(TABLE *table, const uchar *data); +size_t max_row_length(TABLE *table, MY_BITMAP const *cols, const uchar *data); void init_mdl_requests(TABLE_LIST *table_list); From 1a8afb48859c099417594752ae77bb536e6348dc Mon Sep 17 00:00:00 2001 From: Sergei Petrunia Date: Mon, 28 May 2018 13:01:27 +0300 Subject: [PATCH 20/26] MDEV-16310: rocksdb.check_ignore_unknown_options fails on OS X Use a compatible xargs command-line arguments. --- .../mysql-test/rocksdb/t/check_ignore_unknown_options.test | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/storage/rocksdb/mysql-test/rocksdb/t/check_ignore_unknown_options.test b/storage/rocksdb/mysql-test/rocksdb/t/check_ignore_unknown_options.test index 3316e7ea987..9e7c816ae6e 100644 --- a/storage/rocksdb/mysql-test/rocksdb/t/check_ignore_unknown_options.test +++ b/storage/rocksdb/mysql-test/rocksdb/t/check_ignore_unknown_options.test @@ -22,5 +22,5 @@ let SEARCH_PATTERN= RocksDB: Compatibility check against existing database optio --enable_reconnect --exec echo "restart" > $restart_file --source include/wait_until_connected_again.inc ---exec find $MYSQLD_DATADIR/#rocksdb/OPTIONS* | sort -n | tail -1 | xargs -0 -I {} -t sh -c "sed -i '/hello=world/d' {}" +--exec find $MYSQLD_DATADIR/#rocksdb/OPTIONS* | sort -n | tail -1 | xargs -0 -I {} -t sh -c "sed -i'' -e '/hello=world/d' {}" select variable_name, variable_value from information_schema.global_variables where variable_name="rocksdb_ignore_unknown_options"; From 8a42ad7a5d3168b7ce92a38652d77424336a32b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Mon, 28 May 2018 14:20:44 +0300 Subject: [PATCH 21/26] MDEV-13834 10.2 wrongly recognizes 10.1.10 innodb_encrypt_log=ON data as after-crash and refuses to start infos[]: Allocate enough entries to accommodate all keys from both checkpoint pages. infos_used: The size of infos[]. get_crypt_info(): Merge to the only caller, log_crypt_101_read_block(). log_crypt_101_read_block(): Do not validate the log block checksum, because it will not be valid when upgrading from MariaDB 10.1.10. Instead, check that the encryption key exists. log_crypt_101_read_checkpoint(): Append to infos[] instead of overwriting. --- storage/innobase/log/log0crypt.cc | 67 ++++++++++++++----------------- 1 file changed, 31 insertions(+), 36 deletions(-) diff --git a/storage/innobase/log/log0crypt.cc b/storage/innobase/log/log0crypt.cc index f527dfeb272..0358057612d 100644 --- a/storage/innobase/log/log0crypt.cc +++ b/storage/innobase/log/log0crypt.cc @@ -61,7 +61,9 @@ struct crypt_info_t { static crypt_info_t info; /** Crypt info when upgrading from 10.1 */ -static crypt_info_t infos[5]; +static crypt_info_t infos[5 * 2]; +/** First unused slot in infos[] */ +static size_t infos_used; /*********************************************************************//** Get a log block's start lsn. @@ -79,28 +81,6 @@ log_block_get_start_lsn( return start_lsn; } -/*********************************************************************//** -Get crypt info from checkpoint. -@return a crypt info or NULL if not present. */ -static -const crypt_info_t* -get_crypt_info(ulint checkpoint_no) -{ - /* a log block only stores 4-bytes of checkpoint no */ - checkpoint_no &= 0xFFFFFFFF; - for (unsigned i = 0; i < 5; i++) { - const crypt_info_t* it = &infos[i]; - - if (it->key_version && it->checkpoint_no == checkpoint_no) { - return it; - } - } - - /* If checkpoint contains more than one key and we did not - find the correct one use the first one. */ - return infos; -} - /** Encrypt or decrypt log blocks. @param[in,out] buf log blocks to encrypt or decrypt @param[in] lsn log sequence number of the start of the buffer @@ -165,9 +145,7 @@ log_crypt(byte* buf, lsn_t lsn, ulint size, bool decrypt) @param[in,out] info encryption key @param[in] upgrade whether to use the key in MariaDB 10.1 format @return whether the operation was successful */ -static -bool -init_crypt_key(crypt_info_t* info, bool upgrade = false) +static bool init_crypt_key(crypt_info_t* info, bool upgrade = false) { byte mysqld_key[MY_AES_MAX_KEY_LENGTH]; uint keylen = sizeof mysqld_key; @@ -252,8 +230,20 @@ log_crypt_101_read_checkpoint(const byte* buf) const size_t n = *buf++ == 2 ? std::min(unsigned(*buf++), 5U) : 0; for (size_t i = 0; i < n; i++) { - struct crypt_info_t& info = infos[i]; - info.checkpoint_no = mach_read_from_4(buf); + struct crypt_info_t& info = infos[infos_used]; + unsigned checkpoint_no = mach_read_from_4(buf); + for (size_t j = 0; j < infos_used; j++) { + if (infos[j].checkpoint_no == checkpoint_no) { + /* Do not overwrite an existing slot. */ + goto next_slot; + } + } + if (infos_used >= UT_ARR_SIZE(infos)) { + ut_ad(!"too many checkpoint pages"); + goto next_slot; + } + infos_used++; + info.checkpoint_no = checkpoint_no; info.key_version = mach_read_from_4(buf + 4); memcpy(info.crypt_msg.bytes, buf + 8, sizeof info.crypt_msg); memcpy(info.crypt_nonce.bytes, buf + 24, @@ -262,6 +252,7 @@ log_crypt_101_read_checkpoint(const byte* buf) if (!init_crypt_key(&info, true)) { return false; } +next_slot: buf += 4 + 4 + 2 * MY_AES_BLOCK_SIZE; } @@ -277,13 +268,19 @@ log_crypt_101_read_block(byte* buf) { ut_ad(log_block_calc_checksum_format_0(buf) != log_block_get_checksum(buf)); - const crypt_info_t* info = get_crypt_info( - log_block_get_checkpoint_no(buf)); - - if (!info || info->key_version == 0) { - return false; + const uint32_t checkpoint_no + = uint32_t(log_block_get_checkpoint_no(buf)); + const crypt_info_t* info = infos; + for (const crypt_info_t* const end = info + infos_used; info < end; + info++) { + if (info->key_version + && info->checkpoint_no == checkpoint_no) { + goto found; + } } + return false; +found: byte dst[OS_FILE_LOG_BLOCK_SIZE]; uint dst_len; byte aes_ctr_iv[MY_AES_BLOCK_SIZE]; @@ -312,9 +309,7 @@ log_crypt_101_read_block(byte* buf) LOG_DEFAULT_ENCRYPTION_KEY, info->key_version); - if (rc != MY_AES_OK || dst_len != src_len - || log_block_calc_checksum_format_0(dst) - != log_block_get_checksum(dst)) { + if (rc != MY_AES_OK || dst_len != src_len) { return false; } From 35a9c90fff5e2b8ffc5acc7d7ed051f04c013213 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 29 May 2018 08:54:33 +0300 Subject: [PATCH 22/26] MDEV-14589 InnoDB should not lock a delete-marked record When the transaction isolation level is SERIALIZABLE, or when a locking read is performed in the REPEATABLE READ isolation level, InnoDB must lock delete-marked records in order to prevent another transaction from inserting something. However, at READ UNCOMMITTED or READ COMMITTED isolation level or when the parameter innodb_locks_unsafe_for_binlog is set, the repeatability of the reads does not matter, and there is no need to lock any records. row_search_for_mysql(): Skip locks on delete-marked committed records upfront, instead of invoking row_unlock_for_mysql() afterwards. The unlocking never worked for secondary index records. --- mysql-test/suite/innodb/r/lock_deleted.result | 40 +++++++++++ mysql-test/suite/innodb/t/lock_deleted.test | 72 +++++++++++++++++++ storage/innobase/row/row0sel.cc | 35 +++++---- storage/xtradb/row/row0sel.cc | 35 +++++---- 4 files changed, 154 insertions(+), 28 deletions(-) create mode 100644 mysql-test/suite/innodb/r/lock_deleted.result create mode 100644 mysql-test/suite/innodb/t/lock_deleted.test diff --git a/mysql-test/suite/innodb/r/lock_deleted.result b/mysql-test/suite/innodb/r/lock_deleted.result new file mode 100644 index 00000000000..d949ac945de --- /dev/null +++ b/mysql-test/suite/innodb/r/lock_deleted.result @@ -0,0 +1,40 @@ +START TRANSACTION WITH CONSISTENT SNAPSHOT; +CREATE TABLE t1(a INT PRIMARY KEY, b INT UNIQUE) ENGINE=InnoDB; +INSERT INTO t1 VALUES(1,1); +DELETE FROM t1; +SET DEBUG_SYNC='row_ins_sec_index_unique SIGNAL inserted WAIT_FOR locked'; +BEGIN; +INSERT INTO t1 VALUES(1,1); +SET DEBUG_SYNC='now WAIT_FOR inserted'; +SET DEBUG_SYNC='innodb_row_search_for_mysql_exit SIGNAL locked'; +SET SESSION TRANSACTION ISOLATION LEVEL READ COMMITTED; +BEGIN; +DELETE FROM t1 WHERE b=1; +COMMIT; +SET DEBUG_SYNC='RESET'; +ROLLBACK; +SET DEBUG_SYNC='row_ins_sec_index_unique SIGNAL inserted WAIT_FOR locked'; +BEGIN; +INSERT INTO t1 VALUES(1,1); +SET DEBUG_SYNC='now WAIT_FOR inserted'; +SET DEBUG_SYNC='innodb_row_search_for_mysql_exit SIGNAL locked'; +SET SESSION TRANSACTION ISOLATION LEVEL READ UNCOMMITTED; +BEGIN; +DELETE FROM t1 WHERE b=1; +COMMIT; +SET DEBUG_SYNC='RESET'; +ROLLBACK; +SET DEBUG_SYNC='row_ins_sec_index_unique SIGNAL inserted WAIT_FOR locked'; +BEGIN; +SET innodb_lock_wait_timeout=1; +INSERT INTO t1 VALUES(1,1); +SET DEBUG_SYNC='now WAIT_FOR inserted'; +SET DEBUG_SYNC='innodb_row_search_for_mysql_exit SIGNAL locked'; +SET SESSION TRANSACTION ISOLATION LEVEL REPEATABLE READ; +BEGIN; +DELETE FROM t1 WHERE b=1; +ERROR HY000: Lock wait timeout exceeded; try restarting transaction +COMMIT; +SET DEBUG_SYNC='RESET'; +COMMIT; +DROP TABLE t1; diff --git a/mysql-test/suite/innodb/t/lock_deleted.test b/mysql-test/suite/innodb/t/lock_deleted.test new file mode 100644 index 00000000000..8dbad90d354 --- /dev/null +++ b/mysql-test/suite/innodb/t/lock_deleted.test @@ -0,0 +1,72 @@ +--source include/have_innodb.inc +--source include/have_debug.inc +--source include/have_debug_sync.inc + +--source include/count_sessions.inc + +connect(stop_purge, localhost, root,,); +START TRANSACTION WITH CONSISTENT SNAPSHOT; +connect(delete, localhost, root,,); +connection default; + +CREATE TABLE t1(a INT PRIMARY KEY, b INT UNIQUE) ENGINE=InnoDB; +INSERT INTO t1 VALUES(1,1); +DELETE FROM t1; + +let $i=2; +while ($i) { +let $iso= `SELECT CASE $i WHEN 1 THEN 'UNCOMMITTED' ELSE 'COMMITTED' END`; + +SET DEBUG_SYNC='row_ins_sec_index_unique SIGNAL inserted WAIT_FOR locked'; +BEGIN; +send INSERT INTO t1 VALUES(1,1); + +connection delete; +SET DEBUG_SYNC='now WAIT_FOR inserted'; +SET DEBUG_SYNC='innodb_row_search_for_mysql_exit SIGNAL locked'; +eval SET SESSION TRANSACTION ISOLATION LEVEL READ $iso; +BEGIN; +send DELETE FROM t1 WHERE b=1; + +connection default; +reap; +connection delete; +reap; +COMMIT; + +connection default; +SET DEBUG_SYNC='RESET'; +ROLLBACK; + +dec $i; +} + +SET DEBUG_SYNC='row_ins_sec_index_unique SIGNAL inserted WAIT_FOR locked'; +BEGIN; +SET innodb_lock_wait_timeout=1; +send INSERT INTO t1 VALUES(1,1); + +connection delete; +SET DEBUG_SYNC='now WAIT_FOR inserted'; +SET DEBUG_SYNC='innodb_row_search_for_mysql_exit SIGNAL locked'; +SET SESSION TRANSACTION ISOLATION LEVEL REPEATABLE READ; +BEGIN; +send DELETE FROM t1 WHERE b=1; + +connection default; +--error ER_LOCK_WAIT_TIMEOUT +reap; +COMMIT; +SET DEBUG_SYNC='RESET'; + +connection delete; +reap; +COMMIT; + +disconnect delete; +disconnect stop_purge; + +connection default; +DROP TABLE t1; + +--source include/wait_until_count_sessions.inc diff --git a/storage/innobase/row/row0sel.cc b/storage/innobase/row/row0sel.cc index 7af788973f2..06bf4cc30c0 100644 --- a/storage/innobase/row/row0sel.cc +++ b/storage/innobase/row/row0sel.cc @@ -4656,9 +4656,27 @@ wrong_offs: ulint lock_type; + if (srv_locks_unsafe_for_binlog + || trx->isolation_level <= TRX_ISO_READ_COMMITTED) { + /* At READ COMMITTED or READ UNCOMMITTED + isolation levels, do not lock committed + delete-marked records. */ + if (!rec_get_deleted_flag(rec, comp)) { + goto no_gap_lock; + } + if (trx_id_t trx_id = index == clust_index + ? row_get_rec_trx_id(rec, index, offsets) + : row_vers_impl_x_locked(rec, index, offsets)) { + if (trx_rw_is_active(trx_id, NULL)) { + /* The record belongs to an active + transaction. We must acquire a lock. */ + goto no_gap_lock; + } + } + goto locks_ok_del_marked; + } + if (!set_also_gap_locks - || srv_locks_unsafe_for_binlog - || trx->isolation_level <= TRX_ISO_READ_COMMITTED || (unique_search && !rec_get_deleted_flag(rec, comp))) { goto no_gap_lock; @@ -4849,20 +4867,9 @@ locks_ok: page_rec_is_comp() cannot be used! */ if (rec_get_deleted_flag(rec, comp)) { - +locks_ok_del_marked: /* The record is delete-marked: we can skip it */ - if ((srv_locks_unsafe_for_binlog - || trx->isolation_level <= TRX_ISO_READ_COMMITTED) - && prebuilt->select_lock_type != LOCK_NONE - && !did_semi_consistent_read) { - - /* No need to keep a lock on a delete-marked record - if we do not want to use next-key locking. */ - - row_unlock_for_mysql(prebuilt, TRUE); - } - /* This is an optimization to skip setting the next key lock on the record that follows this delete-marked record. This optimization works because of the unique search criteria diff --git a/storage/xtradb/row/row0sel.cc b/storage/xtradb/row/row0sel.cc index 97007c1107c..b6b5d107885 100644 --- a/storage/xtradb/row/row0sel.cc +++ b/storage/xtradb/row/row0sel.cc @@ -4665,9 +4665,27 @@ wrong_offs: ulint lock_type; + if (srv_locks_unsafe_for_binlog + || trx->isolation_level <= TRX_ISO_READ_COMMITTED) { + /* At READ COMMITTED or READ UNCOMMITTED + isolation levels, do not lock committed + delete-marked records. */ + if (!rec_get_deleted_flag(rec, comp)) { + goto no_gap_lock; + } + if (trx_id_t trx_id = index == clust_index + ? row_get_rec_trx_id(rec, index, offsets) + : row_vers_impl_x_locked(rec, index, offsets)) { + if (trx_rw_is_active(trx_id, NULL)) { + /* The record belongs to an active + transaction. We must acquire a lock. */ + goto no_gap_lock; + } + } + goto locks_ok_del_marked; + } + if (!set_also_gap_locks - || srv_locks_unsafe_for_binlog - || trx->isolation_level <= TRX_ISO_READ_COMMITTED || (unique_search && !rec_get_deleted_flag(rec, comp))) { goto no_gap_lock; @@ -4862,20 +4880,9 @@ locks_ok: page_rec_is_comp() cannot be used! */ if (rec_get_deleted_flag(rec, comp)) { - +locks_ok_del_marked: /* The record is delete-marked: we can skip it */ - if ((srv_locks_unsafe_for_binlog - || trx->isolation_level <= TRX_ISO_READ_COMMITTED) - && prebuilt->select_lock_type != LOCK_NONE - && !did_semi_consistent_read) { - - /* No need to keep a lock on a delete-marked record - if we do not want to use next-key locking. */ - - row_unlock_for_mysql(prebuilt, TRUE); - } - /* This is an optimization to skip setting the next key lock on the record that follows this delete-marked record. This optimization works because of the unique search criteria From b7985a45a67f5e3537b1d7b5a6830a0ad4267554 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 29 May 2018 08:55:07 +0300 Subject: [PATCH 23/26] Fix type mismatch --- sql/sql_string.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sql/sql_string.cc b/sql/sql_string.cc index 2c5be35e887..c22e33182c6 100644 --- a/sql/sql_string.cc +++ b/sql/sql_string.cc @@ -189,7 +189,7 @@ bool String::copy(const char *str,uint32 arg_length, CHARSET_INFO *cs) This can happen in some cases. This code is here mainly to avoid warnings from valgrind, but can also be an indication of error. */ - DBUG_PRINT("warning", ("Copying string on itself: %p %lu", + DBUG_PRINT("warning", ("Copying string on itself: %p %u", str, arg_length)); } else if ((str_length=arg_length)) From 6aa50bad3947a0eab24fb029cd58f5945439e365 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 29 May 2018 13:52:43 +0300 Subject: [PATCH 24/26] MDEV-16283 ALTER TABLE...DISCARD TABLESPACE still takes long on a large buffer pool Also fixes MDEV-14727, MDEV-14491 InnoDB: Error: Waited for 5 secs for hash index ref_count (1) to drop to 0 by replacing the flawed wait logic in dict_index_remove_from_cache_low(). On DISCARD TABLESPACE, there is no need to drop the adaptive hash index. We must drop it on IMPORT TABLESPACE, and eventually on DROP TABLE or DROP INDEX. As long as the dict_index_t object remains in the cache and the table remains inaccessible, the adaptive hash index entries to orphaned pages would not do any harm. They would be dropped when buffer pool pages are reused for something else. btr_search_drop_page_hash_when_freed(), buf_LRU_drop_page_hash_batch(): Remove the parameter zip_size, and pass 0 to buf_page_get_gen(). buf_page_get_gen(): Ignore zip_size if mode==BUF_PEEK_IF_IN_POOL. buf_LRU_drop_page_hash_for_tablespace(): Drop the adaptive hash index even if the tablespace is inaccessible. buf_LRU_drop_page_hash_for_tablespace(): New global function, to drop the adaptive hash index. buf_LRU_flush_or_remove_pages(), fil_delete_tablespace(): Remove the parameter drop_ahi. dict_index_remove_from_cache_low(): Actively drop the adaptive hash index if entries exist. This should prevent InnoDB hangs on DROP TABLE or DROP INDEX. row_import_for_mysql(): Drop any adaptive hash index entries for the table. row_drop_table_for_mysql(): Drop any adaptive hash index for the table, except if the table resides in the system tablespace. (DISCARD TABLESPACE does not apply to the system tablespace, and we do no want to drop the adaptive hash index for other tables than the one that is being dropped.) row_truncate_table_for_mysql(): Drop any adaptive hash index entries for the table, except if the table resides in the system tablespace. --- storage/innobase/btr/btr0sea.cc | 15 +++------ storage/innobase/buf/buf0buf.cc | 8 +++-- storage/innobase/buf/buf0lru.cc | 54 ++++++++++++++++-------------- storage/innobase/dict/dict0dict.cc | 32 +++--------------- storage/innobase/fil/fil0fil.cc | 4 +-- storage/innobase/fsp/fsp0fsp.cc | 6 ++-- storage/innobase/include/btr0sea.h | 13 +++---- storage/innobase/include/buf0lru.h | 14 ++++---- storage/innobase/row/row0import.cc | 12 +++++++ storage/innobase/row/row0mysql.cc | 17 ++++++++++ storage/xtradb/btr/btr0sea.cc | 15 +++------ storage/xtradb/buf/buf0buf.cc | 8 +++-- storage/xtradb/buf/buf0lru.cc | 54 ++++++++++++++++-------------- storage/xtradb/dict/dict0dict.cc | 34 +++---------------- storage/xtradb/fil/fil0fil.cc | 4 +-- storage/xtradb/fsp/fsp0fsp.cc | 6 ++-- storage/xtradb/include/btr0sea.h | 13 +++---- storage/xtradb/include/buf0lru.h | 14 ++++---- storage/xtradb/row/row0import.cc | 12 +++++++ storage/xtradb/row/row0mysql.cc | 17 ++++++++++ 20 files changed, 178 insertions(+), 174 deletions(-) diff --git a/storage/innobase/btr/btr0sea.cc b/storage/innobase/btr/btr0sea.cc index e36e6d6194c..bd5cd02aa75 100644 --- a/storage/innobase/btr/btr0sea.cc +++ b/storage/innobase/btr/btr0sea.cc @@ -2,6 +2,7 @@ Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved. Copyright (c) 2008, Google Inc. +Copyright (c) 2018, MariaDB Corporation. Portions of this file contain modifications contributed and copyrighted by Google, Inc. Those modifications are gratefully acknowledged and are described @@ -1250,17 +1251,11 @@ cleanup: mem_free(folds); } -/********************************************************************//** -Drops a possible page hash index when a page is evicted from the buffer pool -or freed in a file segment. */ +/** Drop possible adaptive hash index entries when a page is evicted +from the buffer pool or freed in a file, or the index is being dropped. */ UNIV_INTERN void -btr_search_drop_page_hash_when_freed( -/*=================================*/ - ulint space, /*!< in: space id */ - ulint zip_size, /*!< in: compressed page size in bytes - or 0 for uncompressed pages */ - ulint page_no) /*!< in: page number */ +btr_search_drop_page_hash_when_freed(ulint space, ulint page_no) { buf_block_t* block; mtr_t mtr; @@ -1273,7 +1268,7 @@ btr_search_drop_page_hash_when_freed( are possibly holding, we cannot s-latch the page, but must (recursively) x-latch it, even though we are only reading. */ - block = buf_page_get_gen(space, zip_size, page_no, RW_X_LATCH, NULL, + block = buf_page_get_gen(space, 0, page_no, RW_X_LATCH, NULL, BUF_PEEK_IF_IN_POOL, __FILE__, __LINE__, &mtr); diff --git a/storage/innobase/buf/buf0buf.cc b/storage/innobase/buf/buf0buf.cc index 3b160209cf0..50c052fc690 100644 --- a/storage/innobase/buf/buf0buf.cc +++ b/storage/innobase/buf/buf0buf.cc @@ -3075,17 +3075,18 @@ buf_page_get_gen( #ifdef UNIV_DEBUG switch (mode) { case BUF_EVICT_IF_IN_POOL: + case BUF_PEEK_IF_IN_POOL: /* After DISCARD TABLESPACE, the tablespace would not exist, but in IMPORT TABLESPACE, PageConverter::operator() must replace any old pages, which were not evicted during DISCARD. - Skip the assertion on zip_size. */ + Similarly, btr_search_drop_page_hash_when_freed() must + remove any old pages. Skip the assertion on zip_size. */ break; case BUF_GET_NO_LATCH: ut_ad(rw_latch == RW_NO_LATCH); /* fall through */ case BUF_GET: case BUF_GET_IF_IN_POOL: - case BUF_PEEK_IF_IN_POOL: case BUF_GET_IF_IN_POOL_OR_WATCH: case BUF_GET_POSSIBLY_FREED: ut_ad(zip_size == fil_space_get_zip_size(space)); @@ -3257,7 +3258,8 @@ got_block: fix_mutex = buf_page_get_mutex(&fix_block->page); - ut_ad(page_zip_get_size(&block->page.zip) == zip_size); + ut_ad(page_zip_get_size(&block->page.zip) == zip_size + || mode == BUF_PEEK_IF_IN_POOL); switch (mode) { case BUF_GET_IF_IN_POOL: diff --git a/storage/innobase/buf/buf0lru.cc b/storage/innobase/buf/buf0lru.cc index 9e89a291c80..7039ecdf4a6 100644 --- a/storage/innobase/buf/buf0lru.cc +++ b/storage/innobase/buf/buf0lru.cc @@ -241,8 +241,6 @@ void buf_LRU_drop_page_hash_batch( /*=========================*/ ulint space_id, /*!< in: space id */ - ulint zip_size, /*!< in: compressed page size in bytes - or 0 for uncompressed pages */ const ulint* arr, /*!< in: array of page_no */ ulint count) /*!< in: number of entries in array */ { @@ -252,8 +250,7 @@ buf_LRU_drop_page_hash_batch( ut_ad(count <= BUF_LRU_DROP_SEARCH_SIZE); for (i = 0; i < count; ++i) { - btr_search_drop_page_hash_when_freed(space_id, zip_size, - arr[i]); + btr_search_drop_page_hash_when_freed(space_id, arr[i]); } } @@ -272,15 +269,6 @@ buf_LRU_drop_page_hash_for_tablespace( buf_page_t* bpage; ulint* page_arr; ulint num_entries; - ulint zip_size; - - zip_size = fil_space_get_zip_size(id); - - if (UNIV_UNLIKELY(zip_size == ULINT_UNDEFINED)) { - /* Somehow, the tablespace does not exist. Nothing to drop. */ - ut_ad(0); - return; - } page_arr = static_cast(ut_malloc( sizeof(ulint) * BUF_LRU_DROP_SEARCH_SIZE)); @@ -333,8 +321,7 @@ next_page: the latching order. */ buf_pool_mutex_exit(buf_pool); - buf_LRU_drop_page_hash_batch( - id, zip_size, page_arr, num_entries); + buf_LRU_drop_page_hash_batch(id, page_arr, num_entries); num_entries = 0; @@ -365,10 +352,32 @@ next_page: buf_pool_mutex_exit(buf_pool); /* Drop any remaining batch of search hashed pages. */ - buf_LRU_drop_page_hash_batch(id, zip_size, page_arr, num_entries); + buf_LRU_drop_page_hash_batch(id, page_arr, num_entries); ut_free(page_arr); } +/** Drop the adaptive hash index for a tablespace. +@param[in,out] table table */ +UNIV_INTERN void buf_LRU_drop_page_hash_for_tablespace(dict_table_t* table) +{ + for (dict_index_t* index = dict_table_get_first_index(table); + index != NULL; + index = dict_table_get_next_index(index)) { + if (btr_search_info_get_ref_count( + btr_search_get_info(index))) { + goto drop_ahi; + } + } + + return; +drop_ahi: + ulint id = table->space; + for (ulint i = 0; i < srv_buf_pool_instances; i++) { + buf_LRU_drop_page_hash_for_tablespace(buf_pool_from_array(i), + id); + } +} + /******************************************************************//** While flushing (or removing dirty) pages from a tablespace we don't want to hog the CPU and resources. Release the buffer pool and block @@ -675,18 +684,11 @@ buf_flush_dirty_pages(buf_pool_t* buf_pool, ulint id, const trx_t* trx) /** Empty the flush list for all pages belonging to a tablespace. @param[in] id tablespace identifier @param[in] trx transaction, for checking for user interrupt; - or NULL if nothing is to be written -@param[in] drop_ahi whether to drop the adaptive hash index */ -UNIV_INTERN -void -buf_LRU_flush_or_remove_pages(ulint id, const trx_t* trx, bool drop_ahi) + or NULL if nothing is to be written */ +UNIV_INTERN void buf_LRU_flush_or_remove_pages(ulint id, const trx_t* trx) { for (ulint i = 0; i < srv_buf_pool_instances; i++) { - buf_pool_t* buf_pool = buf_pool_from_array(i); - if (drop_ahi) { - buf_LRU_drop_page_hash_for_tablespace(buf_pool, id); - } - buf_flush_dirty_pages(buf_pool, id, trx); + buf_flush_dirty_pages(buf_pool_from_array(i), id, trx); } if (trx && !trx_is_interrupted(trx)) { diff --git a/storage/innobase/dict/dict0dict.cc b/storage/innobase/dict/dict0dict.cc index 623657ef9fe..9609ef96343 100644 --- a/storage/innobase/dict/dict0dict.cc +++ b/storage/innobase/dict/dict0dict.cc @@ -2,7 +2,7 @@ Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved. Copyright (c) 2012, Facebook Inc. -Copyright (c) 2013, 2017, MariaDB Corporation. +Copyright (c) 2013, 2018, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software @@ -1674,7 +1674,7 @@ dict_table_rename_in_cache( filepath = fil_make_ibd_name(table->name, false); } - fil_delete_tablespace(table->space, true); + fil_delete_tablespace(table->space); /* Delete any temp file hanging around. */ if (os_file_status(filepath, &exists, &ftype) @@ -2719,35 +2719,13 @@ dict_index_remove_from_cache_low( zero. See also: dict_table_can_be_evicted() */ do { - ulint ref_count = btr_search_info_get_ref_count(info); - - if (ref_count == 0) { + if (!btr_search_info_get_ref_count(info)) { break; } - /* Sleep for 10ms before trying again. */ - os_thread_sleep(10000); - ++retries; + buf_LRU_drop_page_hash_for_tablespace(table); - if (retries % 500 == 0) { - /* No luck after 5 seconds of wait. */ - fprintf(stderr, "InnoDB: Error: Waited for" - " %lu secs for hash index" - " ref_count (%lu) to drop" - " to 0.\n" - "index: \"%s\"" - " table: \"%s\"\n", - retries/100, - ref_count, - index->name, - table->name); - } - - /* To avoid a hang here we commit suicide if the - ref_count doesn't drop to zero in 600 seconds. */ - if (retries >= 60000) { - ut_error; - } + ut_a(++retries < 10000); } while (srv_shutdown_state == SRV_SHUTDOWN_NONE || !lru_evict); rw_lock_free(&index->lock); diff --git a/storage/innobase/fil/fil0fil.cc b/storage/innobase/fil/fil0fil.cc index 738afe4ab86..2ba1d90d347 100644 --- a/storage/innobase/fil/fil0fil.cc +++ b/storage/innobase/fil/fil0fil.cc @@ -2891,7 +2891,7 @@ fil_delete_tablespace(ulint id, bool drop_ahi) To deal with potential read requests by checking the ::stop_new_ops flag in fil_io() */ - buf_LRU_flush_or_remove_pages(id, NULL, drop_ahi); + buf_LRU_flush_or_remove_pages(id, NULL); #endif /* !UNIV_HOTBACKUP */ @@ -3002,7 +3002,7 @@ fil_discard_tablespace( { dberr_t err; - switch (err = fil_delete_tablespace(id, true)) { + switch (err = fil_delete_tablespace(id)) { case DB_SUCCESS: break; diff --git a/storage/innobase/fsp/fsp0fsp.cc b/storage/innobase/fsp/fsp0fsp.cc index 1cf37f366d7..b20c59c4d8c 100644 --- a/storage/innobase/fsp/fsp0fsp.cc +++ b/storage/innobase/fsp/fsp0fsp.cc @@ -1,7 +1,7 @@ /***************************************************************************** Copyright (c) 1995, 2016, Oracle and/or its affiliates. All Rights Reserved. -Copyright (c) 2017, MariaDB Corporation. +Copyright (c) 2017, 2018, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software @@ -3027,7 +3027,7 @@ fseg_free_page_low( /* Drop search system page hash index if the page is found in the pool and is hashed */ - btr_search_drop_page_hash_when_freed(space, zip_size, page); + btr_search_drop_page_hash_when_freed(space, page); descr = xdes_get_descriptor(space, zip_size, page, mtr); @@ -3247,7 +3247,7 @@ fseg_free_extent( found in the pool and is hashed */ btr_search_drop_page_hash_when_freed( - space, zip_size, first_page_in_extent + i); + space, first_page_in_extent + i); } } diff --git a/storage/innobase/include/btr0sea.h b/storage/innobase/include/btr0sea.h index c95ca28057a..4e1df7066d1 100644 --- a/storage/innobase/include/btr0sea.h +++ b/storage/innobase/include/btr0sea.h @@ -1,6 +1,7 @@ /***************************************************************************** Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved. +Copyright (c) 2018, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software @@ -141,17 +142,11 @@ btr_search_drop_page_hash_index( s- or x-latched, or an index page for which we know that block->buf_fix_count == 0 */ -/********************************************************************//** -Drops a possible page hash index when a page is evicted from the buffer pool -or freed in a file segment. */ +/** Drop possible adaptive hash index entries when a page is evicted +from the buffer pool or freed in a file, or the index is being dropped. */ UNIV_INTERN void -btr_search_drop_page_hash_when_freed( -/*=================================*/ - ulint space, /*!< in: space id */ - ulint zip_size, /*!< in: compressed page size in bytes - or 0 for uncompressed pages */ - ulint page_no); /*!< in: page number */ +btr_search_drop_page_hash_when_freed(ulint space, ulint page_no); /********************************************************************//** Updates the page hash index when a single record is inserted on a page. */ UNIV_INTERN diff --git a/storage/innobase/include/buf0lru.h b/storage/innobase/include/buf0lru.h index 308bda20c7b..623883433c2 100644 --- a/storage/innobase/include/buf0lru.h +++ b/storage/innobase/include/buf0lru.h @@ -1,7 +1,7 @@ /***************************************************************************** Copyright (c) 1995, 2016, Oracle and/or its affiliates. All Rights Reserved. -Copyright (c) 2017, MariaDB Corporation. +Copyright (c) 2017, 2018, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software @@ -34,6 +34,7 @@ Created 11/5/1995 Heikki Tuuri // Forward declaration struct trx_t; +struct dict_table_t; /******************************************************************//** Returns TRUE if less than 25 % of the buffer pool is available. This can be @@ -52,14 +53,15 @@ These are low-level functions /** Minimum LRU list length for which the LRU_old pointer is defined */ #define BUF_LRU_OLD_MIN_LEN 512 /* 8 megabytes of 16k pages */ +/** Drop the adaptive hash index for a tablespace. +@param[in,out] table table */ +UNIV_INTERN void buf_LRU_drop_page_hash_for_tablespace(dict_table_t* table); + /** Empty the flush list for all pages belonging to a tablespace. @param[in] id tablespace identifier @param[in] trx transaction, for checking for user interrupt; - or NULL if nothing is to be written -@param[in] drop_ahi whether to drop the adaptive hash index */ -UNIV_INTERN -void -buf_LRU_flush_or_remove_pages(ulint id, const trx_t* trx, bool drop_ahi=false); + or NULL if nothing is to be written */ +UNIV_INTERN void buf_LRU_flush_or_remove_pages(ulint id, const trx_t* trx); #if defined UNIV_DEBUG || defined UNIV_BUF_DEBUG /********************************************************************//** diff --git a/storage/innobase/row/row0import.cc b/storage/innobase/row/row0import.cc index a9c24a0f8cc..dfd6b4bfbea 100644 --- a/storage/innobase/row/row0import.cc +++ b/storage/innobase/row/row0import.cc @@ -31,6 +31,7 @@ Created 2012-02-08 by Sunny Bains. #endif #include "btr0pcur.h" +#include "btr0sea.h" #include "que0que.h" #include "dict0boot.h" #include "ibuf0ibuf.h" @@ -3983,6 +3984,17 @@ row_import_for_mysql( return(row_import_cleanup(prebuilt, trx, err)); } + /* On DISCARD TABLESPACE, we did not drop any adaptive hash + index entries. If we replaced the discarded tablespace with a + smaller one here, there could still be some adaptive hash + index entries that point to cached garbage pages in the buffer + pool, because PageConverter::operator() only evicted those + pages that were replaced by the imported pages. We must + discard all remaining adaptive hash index entries, because the + adaptive hash index must be a subset of the table contents; + false positives are not tolerated. */ + buf_LRU_drop_page_hash_for_tablespace(table); + row_mysql_lock_data_dictionary(trx); /* If the table is stored in a remote tablespace, we need to diff --git a/storage/innobase/row/row0mysql.cc b/storage/innobase/row/row0mysql.cc index e34e4fa94ff..be24ae885a2 100644 --- a/storage/innobase/row/row0mysql.cc +++ b/storage/innobase/row/row0mysql.cc @@ -3516,6 +3516,8 @@ row_truncate_table_for_mysql( fil_space_release(space); } + buf_LRU_drop_page_hash_for_tablespace(table); + if (flags != ULINT_UNDEFINED && fil_discard_tablespace(space_id) == DB_SUCCESS) { @@ -4209,6 +4211,21 @@ row_drop_table_for_mysql( rw_lock_x_unlock(dict_index_get_lock(index)); } + if (table->space != TRX_SYS_SPACE) { + /* On DISCARD TABLESPACE, we would not drop the + adaptive hash index entries. If the tablespace is + missing here, delete-marking the record in SYS_INDEXES + would not free any pages in the buffer pool. Thus, + dict_index_remove_from_cache() would hang due to + adaptive hash index entries existing in the buffer + pool. To prevent this hang, and also to guarantee + that btr_search_drop_page_hash_when_freed() will avoid + calling btr_search_drop_page_hash_index() while we + hold the InnoDB dictionary lock, we will drop any + adaptive hash index entries upfront. */ + buf_LRU_drop_page_hash_for_tablespace(table); + } + /* We use the private SQL parser of Innobase to generate the query graphs needed in deleting the dictionary data from system tables in Innobase. Deleting a row from SYS_INDEXES table also diff --git a/storage/xtradb/btr/btr0sea.cc b/storage/xtradb/btr/btr0sea.cc index 12c99246f16..713a584ee7e 100644 --- a/storage/xtradb/btr/btr0sea.cc +++ b/storage/xtradb/btr/btr0sea.cc @@ -2,6 +2,7 @@ Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved. Copyright (c) 2008, Google Inc. +Copyright (c) 2018, MariaDB Corporation. Portions of this file contain modifications contributed and copyrighted by Google, Inc. Those modifications are gratefully acknowledged and are described @@ -1299,17 +1300,11 @@ cleanup: mem_free(folds); } -/********************************************************************//** -Drops a possible page hash index when a page is evicted from the buffer pool -or freed in a file segment. */ +/** Drop possible adaptive hash index entries when a page is evicted +from the buffer pool or freed in a file, or the index is being dropped. */ UNIV_INTERN void -btr_search_drop_page_hash_when_freed( -/*=================================*/ - ulint space, /*!< in: space id */ - ulint zip_size, /*!< in: compressed page size in bytes - or 0 for uncompressed pages */ - ulint page_no) /*!< in: page number */ +btr_search_drop_page_hash_when_freed(ulint space, ulint page_no) { buf_block_t* block; mtr_t mtr; @@ -1322,7 +1317,7 @@ btr_search_drop_page_hash_when_freed( are possibly holding, we cannot s-latch the page, but must (recursively) x-latch it, even though we are only reading. */ - block = buf_page_get_gen(space, zip_size, page_no, RW_X_LATCH, NULL, + block = buf_page_get_gen(space, 0, page_no, RW_X_LATCH, NULL, BUF_PEEK_IF_IN_POOL, __FILE__, __LINE__, &mtr); diff --git a/storage/xtradb/buf/buf0buf.cc b/storage/xtradb/buf/buf0buf.cc index 12c5c6efb29..3814d08f19a 100644 --- a/storage/xtradb/buf/buf0buf.cc +++ b/storage/xtradb/buf/buf0buf.cc @@ -2969,17 +2969,18 @@ buf_page_get_gen( #ifdef UNIV_DEBUG switch (mode) { case BUF_EVICT_IF_IN_POOL: + case BUF_PEEK_IF_IN_POOL: /* After DISCARD TABLESPACE, the tablespace would not exist, but in IMPORT TABLESPACE, PageConverter::operator() must replace any old pages, which were not evicted during DISCARD. - Skip the assertion on zip_size. */ + Similarly, btr_search_drop_page_hash_when_freed() must + remove any old pages. Skip the assertion on zip_size. */ break; case BUF_GET_NO_LATCH: ut_ad(rw_latch == RW_NO_LATCH); /* fall through */ case BUF_GET: case BUF_GET_IF_IN_POOL: - case BUF_PEEK_IF_IN_POOL: case BUF_GET_IF_IN_POOL_OR_WATCH: case BUF_GET_POSSIBLY_FREED: ut_ad(zip_size == fil_space_get_zip_size(space)); @@ -3156,7 +3157,8 @@ got_block: fix_mutex = buf_page_get_mutex(&fix_block->page); - ut_ad(page_zip_get_size(&block->page.zip) == zip_size); + ut_ad(page_zip_get_size(&block->page.zip) == zip_size + || mode == BUF_PEEK_IF_IN_POOL); switch (mode) { case BUF_GET_IF_IN_POOL: diff --git a/storage/xtradb/buf/buf0lru.cc b/storage/xtradb/buf/buf0lru.cc index 7bf423ed740..2c4a4049de6 100644 --- a/storage/xtradb/buf/buf0lru.cc +++ b/storage/xtradb/buf/buf0lru.cc @@ -238,8 +238,6 @@ void buf_LRU_drop_page_hash_batch( /*=========================*/ ulint space_id, /*!< in: space id */ - ulint zip_size, /*!< in: compressed page size in bytes - or 0 for uncompressed pages */ const ulint* arr, /*!< in: array of page_no */ ulint count) /*!< in: number of entries in array */ { @@ -249,8 +247,7 @@ buf_LRU_drop_page_hash_batch( ut_ad(count <= BUF_LRU_DROP_SEARCH_SIZE); for (i = 0; i < count; ++i) { - btr_search_drop_page_hash_when_freed(space_id, zip_size, - arr[i]); + btr_search_drop_page_hash_when_freed(space_id, arr[i]); } } @@ -269,15 +266,6 @@ buf_LRU_drop_page_hash_for_tablespace( buf_page_t* bpage; ulint* page_arr; ulint num_entries; - ulint zip_size; - - zip_size = fil_space_get_zip_size(id); - - if (UNIV_UNLIKELY(zip_size == ULINT_UNDEFINED)) { - /* Somehow, the tablespace does not exist. Nothing to drop. */ - ut_ad(0); - return; - } page_arr = static_cast(ut_malloc( sizeof(ulint) * BUF_LRU_DROP_SEARCH_SIZE)); @@ -331,8 +319,7 @@ next_page: the latching order. */ mutex_exit(&buf_pool->LRU_list_mutex); - buf_LRU_drop_page_hash_batch( - id, zip_size, page_arr, num_entries); + buf_LRU_drop_page_hash_batch(id, page_arr, num_entries); num_entries = 0; @@ -363,10 +350,32 @@ next_page: mutex_exit(&buf_pool->LRU_list_mutex); /* Drop any remaining batch of search hashed pages. */ - buf_LRU_drop_page_hash_batch(id, zip_size, page_arr, num_entries); + buf_LRU_drop_page_hash_batch(id, page_arr, num_entries); ut_free(page_arr); } +/** Drop the adaptive hash index for a tablespace. +@param[in,out] table table */ +UNIV_INTERN void buf_LRU_drop_page_hash_for_tablespace(dict_table_t* table) +{ + for (dict_index_t* index = dict_table_get_first_index(table); + index != NULL; + index = dict_table_get_next_index(index)) { + if (btr_search_info_get_ref_count(btr_search_get_info(index), + index)) { + goto drop_ahi; + } + } + + return; +drop_ahi: + ulint id = table->space; + for (ulint i = 0; i < srv_buf_pool_instances; i++) { + buf_LRU_drop_page_hash_for_tablespace(buf_pool_from_array(i), + id); + } +} + /******************************************************************//** While flushing (or removing dirty) pages from a tablespace we don't want to hog the CPU and resources. Release the buffer pool and block @@ -733,18 +742,11 @@ buf_flush_dirty_pages(buf_pool_t* buf_pool, ulint id, const trx_t* trx) /** Empty the flush list for all pages belonging to a tablespace. @param[in] id tablespace identifier @param[in] trx transaction, for checking for user interrupt; - or NULL if nothing is to be written -@param[in] drop_ahi whether to drop the adaptive hash index */ -UNIV_INTERN -void -buf_LRU_flush_or_remove_pages(ulint id, const trx_t* trx, bool drop_ahi) + or NULL if nothing is to be written */ +UNIV_INTERN void buf_LRU_flush_or_remove_pages(ulint id, const trx_t* trx) { for (ulint i = 0; i < srv_buf_pool_instances; i++) { - buf_pool_t* buf_pool = buf_pool_from_array(i); - if (drop_ahi) { - buf_LRU_drop_page_hash_for_tablespace(buf_pool, id); - } - buf_flush_dirty_pages(buf_pool, id, trx); + buf_flush_dirty_pages(buf_pool_from_array(i), id, trx); } if (trx && !trx_is_interrupted(trx)) { diff --git a/storage/xtradb/dict/dict0dict.cc b/storage/xtradb/dict/dict0dict.cc index e361b73ab77..7ade6d79adf 100644 --- a/storage/xtradb/dict/dict0dict.cc +++ b/storage/xtradb/dict/dict0dict.cc @@ -2,7 +2,7 @@ Copyright (c) 1996, 2017, Oracle and/or its affiliates. All Rights Reserved. Copyright (c) 2012, Facebook Inc. -Copyright (c) 2013, 2017, MariaDB Corporation. +Copyright (c) 2013, 2018, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software @@ -1680,7 +1680,7 @@ dict_table_rename_in_cache( filepath = fil_make_ibd_name(table->name, false); } - fil_delete_tablespace(table->space, true); + fil_delete_tablespace(table->space); /* Delete any temp file hanging around. */ if (os_file_status(filepath, &exists, &ftype) @@ -2729,36 +2729,12 @@ dict_index_remove_from_cache_low( zero. See also: dict_table_can_be_evicted() */ do { - ulint ref_count = btr_search_info_get_ref_count(info, - index); - - if (ref_count == 0) { + if (!btr_search_info_get_ref_count(info, index)) { break; } - /* Sleep for 10ms before trying again. */ - os_thread_sleep(10000); - ++retries; - - if (retries % 500 == 0) { - /* No luck after 5 seconds of wait. */ - fprintf(stderr, "InnoDB: Error: Waited for" - " %lu secs for hash index" - " ref_count (%lu) to drop" - " to 0.\n" - "index: \"%s\"" - " table: \"%s\"\n", - retries/100, - ref_count, - index->name, - table->name); - } - - /* To avoid a hang here we commit suicide if the - ref_count doesn't drop to zero in 600 seconds. */ - if (retries >= 60000) { - ut_error; - } + buf_LRU_drop_page_hash_for_tablespace(table); + ut_a(++retries < 10000); } while (srv_shutdown_state == SRV_SHUTDOWN_NONE || !lru_evict); rw_lock_free(&index->lock); diff --git a/storage/xtradb/fil/fil0fil.cc b/storage/xtradb/fil/fil0fil.cc index a24b319fda6..9bd26fcf35b 100644 --- a/storage/xtradb/fil/fil0fil.cc +++ b/storage/xtradb/fil/fil0fil.cc @@ -2936,7 +2936,7 @@ fil_delete_tablespace(ulint id, bool drop_ahi) To deal with potential read requests by checking the ::stop_new_ops flag in fil_io() */ - buf_LRU_flush_or_remove_pages(id, NULL, drop_ahi); + buf_LRU_flush_or_remove_pages(id, NULL); #endif /* !UNIV_HOTBACKUP */ @@ -3047,7 +3047,7 @@ fil_discard_tablespace( { dberr_t err; - switch (err = fil_delete_tablespace(id, true)) { + switch (err = fil_delete_tablespace(id)) { case DB_SUCCESS: break; diff --git a/storage/xtradb/fsp/fsp0fsp.cc b/storage/xtradb/fsp/fsp0fsp.cc index ffed8a6edd3..f97e0c1331b 100644 --- a/storage/xtradb/fsp/fsp0fsp.cc +++ b/storage/xtradb/fsp/fsp0fsp.cc @@ -1,7 +1,7 @@ /***************************************************************************** Copyright (c) 1995, 2016, Oracle and/or its affiliates. All Rights Reserved. -Copyright (c) 2017, MariaDB Corporation. +Copyright (c) 2017, 2018, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software @@ -3035,7 +3035,7 @@ fseg_free_page_low( /* Drop search system page hash index if the page is found in the pool and is hashed */ - btr_search_drop_page_hash_when_freed(space, zip_size, page); + btr_search_drop_page_hash_when_freed(space, page); descr = xdes_get_descriptor(space, zip_size, page, mtr); @@ -3261,7 +3261,7 @@ fseg_free_extent( found in the pool and is hashed */ btr_search_drop_page_hash_when_freed( - space, zip_size, first_page_in_extent + i); + space, first_page_in_extent + i); } } diff --git a/storage/xtradb/include/btr0sea.h b/storage/xtradb/include/btr0sea.h index bfe2c43defb..55366d3c0d3 100644 --- a/storage/xtradb/include/btr0sea.h +++ b/storage/xtradb/include/btr0sea.h @@ -1,6 +1,7 @@ /***************************************************************************** Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved. +Copyright (c) 2018, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software @@ -142,17 +143,11 @@ btr_search_drop_page_hash_index( s- or x-latched, or an index page for which we know that block->buf_fix_count == 0 */ -/********************************************************************//** -Drops a possible page hash index when a page is evicted from the buffer pool -or freed in a file segment. */ +/** Drop possible adaptive hash index entries when a page is evicted +from the buffer pool or freed in a file, or the index is being dropped. */ UNIV_INTERN void -btr_search_drop_page_hash_when_freed( -/*=================================*/ - ulint space, /*!< in: space id */ - ulint zip_size, /*!< in: compressed page size in bytes - or 0 for uncompressed pages */ - ulint page_no); /*!< in: page number */ +btr_search_drop_page_hash_when_freed(ulint space, ulint page_no); /********************************************************************//** Updates the page hash index when a single record is inserted on a page. */ UNIV_INTERN diff --git a/storage/xtradb/include/buf0lru.h b/storage/xtradb/include/buf0lru.h index 1bc11937fa1..f0ba1bb227d 100644 --- a/storage/xtradb/include/buf0lru.h +++ b/storage/xtradb/include/buf0lru.h @@ -1,7 +1,7 @@ /***************************************************************************** Copyright (c) 1995, 2016, Oracle and/or its affiliates. All Rights Reserved. -Copyright (c) 2017, MariaDB Corporation. +Copyright (c) 2017, 2018, MariaDB Corporation. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by the Free Software @@ -36,6 +36,7 @@ Created 11/5/1995 Heikki Tuuri // Forward declaration struct trx_t; +struct dict_table_t; /******************************************************************//** Returns TRUE if less than 25 % of the buffer pool is available. This can be @@ -54,14 +55,15 @@ These are low-level functions /** Minimum LRU list length for which the LRU_old pointer is defined */ #define BUF_LRU_OLD_MIN_LEN 512 /* 8 megabytes of 16k pages */ +/** Drop the adaptive hash index for a tablespace. +@param[in,out] table table */ +UNIV_INTERN void buf_LRU_drop_page_hash_for_tablespace(dict_table_t* table); + /** Empty the flush list for all pages belonging to a tablespace. @param[in] id tablespace identifier @param[in] trx transaction, for checking for user interrupt; - or NULL if nothing is to be written -@param[in] drop_ahi whether to drop the adaptive hash index */ -UNIV_INTERN -void -buf_LRU_flush_or_remove_pages(ulint id, const trx_t* trx, bool drop_ahi=false); + or NULL if nothing is to be written */ +UNIV_INTERN void buf_LRU_flush_or_remove_pages(ulint id, const trx_t* trx); #if defined UNIV_DEBUG || defined UNIV_BUF_DEBUG /********************************************************************//** diff --git a/storage/xtradb/row/row0import.cc b/storage/xtradb/row/row0import.cc index 36ffba68291..a3478dc2fc4 100644 --- a/storage/xtradb/row/row0import.cc +++ b/storage/xtradb/row/row0import.cc @@ -31,6 +31,7 @@ Created 2012-02-08 by Sunny Bains. #endif #include "btr0pcur.h" +#include "btr0sea.h" #include "que0que.h" #include "dict0boot.h" #include "ibuf0ibuf.h" @@ -3982,6 +3983,17 @@ row_import_for_mysql( return(row_import_cleanup(prebuilt, trx, err)); } + /* On DISCARD TABLESPACE, we did not drop any adaptive hash + index entries. If we replaced the discarded tablespace with a + smaller one here, there could still be some adaptive hash + index entries that point to cached garbage pages in the buffer + pool, because PageConverter::operator() only evicted those + pages that were replaced by the imported pages. We must + discard all remaining adaptive hash index entries, because the + adaptive hash index must be a subset of the table contents; + false positives are not tolerated. */ + buf_LRU_drop_page_hash_for_tablespace(table); + row_mysql_lock_data_dictionary(trx); /* If the table is stored in a remote tablespace, we need to diff --git a/storage/xtradb/row/row0mysql.cc b/storage/xtradb/row/row0mysql.cc index 1acdfe53e0c..2b6f38ba2af 100644 --- a/storage/xtradb/row/row0mysql.cc +++ b/storage/xtradb/row/row0mysql.cc @@ -3540,6 +3540,8 @@ row_truncate_table_for_mysql( fil_space_release(space); } + buf_LRU_drop_page_hash_for_tablespace(table); + if (flags != ULINT_UNDEFINED && fil_discard_tablespace(space_id) == DB_SUCCESS) { @@ -4239,6 +4241,21 @@ row_drop_table_for_mysql( rw_lock_x_unlock(dict_index_get_lock(index)); } + if (table->space != TRX_SYS_SPACE) { + /* On DISCARD TABLESPACE, we would not drop the + adaptive hash index entries. If the tablespace is + missing here, delete-marking the record in SYS_INDEXES + would not free any pages in the buffer pool. Thus, + dict_index_remove_from_cache() would hang due to + adaptive hash index entries existing in the buffer + pool. To prevent this hang, and also to guarantee + that btr_search_drop_page_hash_when_freed() will avoid + calling btr_search_drop_page_hash_index() while we + hold the InnoDB dictionary lock, we will drop any + adaptive hash index entries upfront. */ + buf_LRU_drop_page_hash_for_tablespace(table); + } + /* We use the private SQL parser of Innobase to generate the query graphs needed in deleting the dictionary data from system tables in Innobase. Deleting a row from SYS_INDEXES table also From 7269c70821fb6999683f184bf8dad22d35a7ad96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 29 May 2018 16:52:59 +0300 Subject: [PATCH 25/26] Add an end-marker to ease future merges --- mysql-test/suite/maria/lock.result | 1 + mysql-test/suite/maria/lock.test | 2 ++ 2 files changed, 3 insertions(+) diff --git a/mysql-test/suite/maria/lock.result b/mysql-test/suite/maria/lock.result index 1c9d6b18100..b7d5a9c220d 100644 --- a/mysql-test/suite/maria/lock.result +++ b/mysql-test/suite/maria/lock.result @@ -109,3 +109,4 @@ ALTER TABLE t1 ADD UNIQUE KEY (f1); ERROR 23000: Duplicate entry 'foo' for key 'f1' ALTER TABLE t1 ADD KEY (f2); DROP TABLE t1; +# End of 10.2 tests diff --git a/mysql-test/suite/maria/lock.test b/mysql-test/suite/maria/lock.test index 4f3d4e8065e..b22c9cc70d7 100644 --- a/mysql-test/suite/maria/lock.test +++ b/mysql-test/suite/maria/lock.test @@ -117,3 +117,5 @@ LOCK TABLE t1 WRITE; ALTER TABLE t1 ADD UNIQUE KEY (f1); ALTER TABLE t1 ADD KEY (f2); DROP TABLE t1; + +--echo # End of 10.2 tests From 6f96ff7268dd20d6d3b61931c972e7a43c1efdff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Tue, 29 May 2018 17:14:34 +0300 Subject: [PATCH 26/26] Allow tests to work with cmake -DPLUGIN_PARTITION=NO --- mysql-test/suite/binlog_encryption/encrypted_master.test | 1 + mysql-test/suite/binlog_encryption/encrypted_slave.test | 1 + mysql-test/suite/binlog_encryption/testdata.opt | 1 - mysql-test/suite/mariabackup/lock_ddl_per_table.opt | 1 + mysql-test/suite/mariabackup/lock_ddl_per_table.test | 1 + mysql-test/suite/mariabackup/partition_datadir.opt | 1 - mysql-test/suite/mariabackup/partition_datadir.test | 1 + mysql-test/suite/mariabackup/suite.opt | 2 +- 8 files changed, 6 insertions(+), 3 deletions(-) delete mode 100644 mysql-test/suite/binlog_encryption/testdata.opt create mode 100644 mysql-test/suite/mariabackup/lock_ddl_per_table.opt delete mode 100644 mysql-test/suite/mariabackup/partition_datadir.opt diff --git a/mysql-test/suite/binlog_encryption/encrypted_master.test b/mysql-test/suite/binlog_encryption/encrypted_master.test index 503a40443d2..f67e93ce815 100644 --- a/mysql-test/suite/binlog_encryption/encrypted_master.test +++ b/mysql-test/suite/binlog_encryption/encrypted_master.test @@ -18,6 +18,7 @@ # - with annotated events, default checksums and minimal binlog row image # +--source include/have_partition.inc --source encryption_algorithms.inc --source include/have_innodb.inc --enable_connect_log diff --git a/mysql-test/suite/binlog_encryption/encrypted_slave.test b/mysql-test/suite/binlog_encryption/encrypted_slave.test index f5697d91779..94777957b92 100644 --- a/mysql-test/suite/binlog_encryption/encrypted_slave.test +++ b/mysql-test/suite/binlog_encryption/encrypted_slave.test @@ -9,6 +9,7 @@ # relay logs and binary logs are encrypted on slave. # +--source include/have_partition.inc --source encryption_algorithms.inc --source include/have_innodb.inc diff --git a/mysql-test/suite/binlog_encryption/testdata.opt b/mysql-test/suite/binlog_encryption/testdata.opt deleted file mode 100644 index b0c5b9c8188..00000000000 --- a/mysql-test/suite/binlog_encryption/testdata.opt +++ /dev/null @@ -1 +0,0 @@ ---partition diff --git a/mysql-test/suite/mariabackup/lock_ddl_per_table.opt b/mysql-test/suite/mariabackup/lock_ddl_per_table.opt new file mode 100644 index 00000000000..bbb6d7f9ff4 --- /dev/null +++ b/mysql-test/suite/mariabackup/lock_ddl_per_table.opt @@ -0,0 +1 @@ +--loose-partition diff --git a/mysql-test/suite/mariabackup/lock_ddl_per_table.test b/mysql-test/suite/mariabackup/lock_ddl_per_table.test index 04edb9e05e9..2689508e554 100644 --- a/mysql-test/suite/mariabackup/lock_ddl_per_table.test +++ b/mysql-test/suite/mariabackup/lock_ddl_per_table.test @@ -1,4 +1,5 @@ --source include/have_debug.inc +--source include/have_partition.inc CREATE TABLE t(i INT) ENGINE INNODB; INSERT INTO t VALUES(1); diff --git a/mysql-test/suite/mariabackup/partition_datadir.opt b/mysql-test/suite/mariabackup/partition_datadir.opt deleted file mode 100644 index 8a3240370eb..00000000000 --- a/mysql-test/suite/mariabackup/partition_datadir.opt +++ /dev/null @@ -1 +0,0 @@ ---partition \ No newline at end of file diff --git a/mysql-test/suite/mariabackup/partition_datadir.test b/mysql-test/suite/mariabackup/partition_datadir.test index 882b0111267..c525d34a02c 100644 --- a/mysql-test/suite/mariabackup/partition_datadir.test +++ b/mysql-test/suite/mariabackup/partition_datadir.test @@ -1,3 +1,4 @@ +--source include/have_partition.inc let $targetdir=$MYSQLTEST_VARDIR/backup; mkdir $targetdir; mkdir $MYSQLTEST_VARDIR/partitdata; diff --git a/mysql-test/suite/mariabackup/suite.opt b/mysql-test/suite/mariabackup/suite.opt index 8b1e9c7bcb2..3b5cc4f4c45 100644 --- a/mysql-test/suite/mariabackup/suite.opt +++ b/mysql-test/suite/mariabackup/suite.opt @@ -1 +1 @@ ---innodb --loose-changed_page_bitmaps --innodb-file-format=Barracuda --innodb-sys-tables --partition \ No newline at end of file +--innodb --loose-changed_page_bitmaps --innodb-sys-tables