From 8acbf9c1f961aa1008ef509e059e1a09943f5ed3 Mon Sep 17 00:00:00 2001 From: Michael Widenius Date: Wed, 19 Jun 2019 00:35:44 +0300 Subject: [PATCH] MDEV-19595 fixed The test cases for the MDEV found several independent bugs in MariaDB server and Aria: - If a temporary table was marked as crashed, it could never be deleted. - Opening of a crashed temporary table gave an error message but the error was never forwarded to the caller which caused an assert() in my_ok() - init_read_record() did mmap of all temporary tables, which is probably not a good idea as this area can potentially be very big. Changed code to only mmap internal temporary tables. - mmap-ed tables where not unmapped in case of repair/optimize which caused bad data in table and crashes if the original table files where replaced with new ones (as the old mmap was still in place). Fixed by removing the mmap in case of repair. - Cleaned up usage of code that disabled mmap in Aria --- mysql-test/suite/maria/temporary.result | 23 +++++++++++++++++++++++ mysql-test/suite/maria/temporary.test | 20 ++++++++++++++++++++ sql/records.cc | 3 +-- sql/sql_parse.cc | 8 +++++++- sql/temporary_tables.cc | 5 ++++- storage/maria/ha_maria.cc | 4 +++- storage/maria/ma_close.c | 5 +---- storage/maria/ma_delete_all.c | 7 +++++-- storage/maria/ma_packrec.c | 9 +++++++-- storage/maria/maria_def.h | 8 ++++++++ 10 files changed, 79 insertions(+), 13 deletions(-) create mode 100644 mysql-test/suite/maria/temporary.result create mode 100644 mysql-test/suite/maria/temporary.test diff --git a/mysql-test/suite/maria/temporary.result b/mysql-test/suite/maria/temporary.result new file mode 100644 index 00000000000..2592e04be2b --- /dev/null +++ b/mysql-test/suite/maria/temporary.result @@ -0,0 +1,23 @@ +CREATE TABLE t1 (b INT); +INSERT INTO t1 VALUES (5); +CREATE TEMPORARY TABLE t1 (a INT) ENGINE=Aria ROW_FORMAT=DYNAMIC; +INSERT INTO t1 VALUES (1); +DELETE FROM t1 LIMIT 2; +OPTIMIZE TABLE t1; +Table Op Msg_type Msg_text +test.t1 optimize status OK +CHECK TABLE t1; +Table Op Msg_type Msg_text +test.t1 check status OK +SELECT * FROM t1; +a +INSERT INTO t1 VALUES (1),(2); +CHECK TABLE t1; +Table Op Msg_type Msg_text +test.t1 check status OK +ALTER TABLE t1 CHANGE COLUMN IF EXISTS x x INT; +Warnings: +Note 1054 Unknown column 'x' in 't1' +ALTER TABLE t1; +DROP TEMPORARY TABLE t1; +DROP TABLE t1; diff --git a/mysql-test/suite/maria/temporary.test b/mysql-test/suite/maria/temporary.test new file mode 100644 index 00000000000..76f8e2526a4 --- /dev/null +++ b/mysql-test/suite/maria/temporary.test @@ -0,0 +1,20 @@ +# +# MDEV-19595 +# ER_CRASHED_ON_USAGE and Assertion `!is_set() || (m_status == DA_OK_BULK && is_bulk_op())' +# failed upon actions on temporary Aria table with ROW_FORMAT DYNAMIC +# + +CREATE TABLE t1 (b INT); +INSERT INTO t1 VALUES (5); +CREATE TEMPORARY TABLE t1 (a INT) ENGINE=Aria ROW_FORMAT=DYNAMIC; +INSERT INTO t1 VALUES (1); +DELETE FROM t1 LIMIT 2; +OPTIMIZE TABLE t1; +CHECK TABLE t1; +SELECT * FROM t1; +INSERT INTO t1 VALUES (1),(2); +CHECK TABLE t1; +ALTER TABLE t1 CHANGE COLUMN IF EXISTS x x INT; +ALTER TABLE t1; +DROP TEMPORARY TABLE t1; +DROP TABLE t1; diff --git a/sql/records.cc b/sql/records.cc index 3db34425e29..6a611d46ca4 100644 --- a/sql/records.cc +++ b/sql/records.cc @@ -197,8 +197,7 @@ bool init_read_record(READ_RECORD *info,THD *thd, TABLE *table, info->forms= &info->table; /* Only one table */ info->addon_field= addon_field; - if ((table->s->tmp_table == INTERNAL_TMP_TABLE || - table->s->tmp_table == NON_TRANSACTIONAL_TMP_TABLE) && + if ((table->s->tmp_table == INTERNAL_TMP_TABLE) && !addon_field) (void) table->file->extra(HA_EXTRA_MMAP); diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 976cfe3df03..6e2c38ad053 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -779,7 +779,6 @@ void init_update_queries(void) 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; sql_command_flags[SQLCOM_CREATE_INDEX]|= CF_PREOPEN_TMP_TABLES; sql_command_flags[SQLCOM_ALTER_TABLE]|= CF_PREOPEN_TMP_TABLES; sql_command_flags[SQLCOM_TRUNCATE]|= CF_PREOPEN_TMP_TABLES; @@ -4459,7 +4458,14 @@ mysql_execute_command(THD *thd) } case SQLCOM_DROP_TABLE: { + int result; DBUG_ASSERT(first_table == all_tables && first_table != 0); + + thd->open_options|= HA_OPEN_FOR_REPAIR; + result= thd->open_temporary_tables(all_tables); + thd->open_options&= ~HA_OPEN_FOR_REPAIR; + if (result) + goto error; if (!lex->tmp_table()) { if (check_table_access(thd, DROP_ACL, all_tables, FALSE, UINT_MAX, FALSE)) diff --git a/sql/temporary_tables.cc b/sql/temporary_tables.cc index 15a81f4b375..ae7cb274843 100644 --- a/sql/temporary_tables.cc +++ b/sql/temporary_tables.cc @@ -382,6 +382,9 @@ bool THD::open_temporary_table(TABLE_LIST *tl) rgi_slave->is_parallel_exec && wait_for_prior_commit()) DBUG_RETURN(true); + + if (!table && is_error()) + DBUG_RETURN(true); // Error when opening table } if (!table) @@ -1103,7 +1106,7 @@ TABLE *THD::open_temporary_table(TMP_TABLE_SHARE *share, if (open_table_from_share(this, share, alias, open_in_engine ? (uint)HA_OPEN_KEYFILE : 0, - EXTRA_RECORD, ha_open_options, table, + EXTRA_RECORD, open_options | ha_open_options, table, open_in_engine ? false : true)) { my_free(table); diff --git a/storage/maria/ha_maria.cc b/storage/maria/ha_maria.cc index 5f0d023f42b..887065ff825 100644 --- a/storage/maria/ha_maria.cc +++ b/storage/maria/ha_maria.cc @@ -1301,6 +1301,7 @@ int ha_maria::check(THD * thd, HA_CHECK_OPT * check_opt) if (!file || !param) return HA_ADMIN_INTERNAL_ERROR; + unmap_file(file); maria_chk_init(param); param->thd= thd; param->op_name= "check"; @@ -1521,6 +1522,7 @@ int ha_maria::zerofill(THD * thd, HA_CHECK_OPT *check_opt) if (!file || !param) return HA_ADMIN_INTERNAL_ERROR; + unmap_file(file); old_trn= file->trn; maria_chk_init(param); param->thd= thd; @@ -1619,6 +1621,7 @@ int ha_maria::repair(THD *thd, HA_CHECK *param, bool do_optimize) param->out_flag= 0; share->state.dupp_key= MI_MAX_KEY; strmov(fixed_name, share->open_file_name.str); + unmap_file(file); /* Don't lock tables if we have used LOCK TABLE or if we come from @@ -1798,7 +1801,6 @@ int ha_maria::assign_to_keycache(THD * thd, HA_CHECK_OPT *check_opt) TABLE_LIST *table_list= table->pos_in_table_list; DBUG_ENTER("ha_maria::assign_to_keycache"); - table->keys_in_use_for_query.clear_all(); if (table_list->process_index_hints(table)) diff --git a/storage/maria/ma_close.c b/storage/maria/ma_close.c index bd0a36c14c8..03501dc49cf 100644 --- a/storage/maria/ma_close.c +++ b/storage/maria/ma_close.c @@ -113,10 +113,7 @@ int maria_close(register MARIA_HA *info) if (flush_pagecache_blocks(share->pagecache, &share->kfile, share->deleting ? FLUSH_IGNORE_CHANGED : FLUSH_RELEASE)) error= my_errno; -#ifdef HAVE_MMAP - if (share->file_map) - _ma_unmap_file(info); -#endif + unmap_file(info); if (((share->changed && share->base.born_transactional) || maria_is_crashed(info) || (share->temporary && !share->deleting))) { diff --git a/storage/maria/ma_delete_all.c b/storage/maria/ma_delete_all.c index 1daf5f3ef80..c1019c01c66 100644 --- a/storage/maria/ma_delete_all.c +++ b/storage/maria/ma_delete_all.c @@ -38,6 +38,9 @@ int maria_delete_all_rows(MARIA_HA *info) MARIA_SHARE *share= info->s; my_bool log_record; LSN lsn; +#ifdef HAVE_MMAP + my_bool mmap_file= share->file_map != 0; +#endif DBUG_ENTER("maria_delete_all_rows"); if (share->options & HA_OPTION_READ_ONLY_DATA) @@ -95,7 +98,7 @@ int maria_delete_all_rows(MARIA_HA *info) */ #ifdef HAVE_MMAP - if (share->file_map) + if (mmap_file) _ma_unmap_file(info); #endif @@ -141,7 +144,7 @@ int maria_delete_all_rows(MARIA_HA *info) _ma_writeinfo(info, WRITEINFO_UPDATE_KEYFILE); #ifdef HAVE_MMAP /* Map again */ - if (share->file_map) + if (mmap_file) _ma_dynmap_file(info, (my_off_t) 0); #endif DBUG_RETURN(0); diff --git a/storage/maria/ma_packrec.c b/storage/maria/ma_packrec.c index 27c5538e51b..e2c1e353616 100644 --- a/storage/maria/ma_packrec.c +++ b/storage/maria/ma_packrec.c @@ -1564,8 +1564,13 @@ my_bool _ma_memmap_file(MARIA_HA *info) void _ma_unmap_file(MARIA_HA *info) { - my_munmap((char*) info->s->file_map, - (size_t) info->s->mmaped_length + MEMMAP_EXTRA_MARGIN); + MARIA_SHARE *share= info->s; + my_munmap((char*) share->file_map, + (size_t) share->mmaped_length + MEMMAP_EXTRA_MARGIN); + share->file_map= 0; + share->file_read= _ma_nommap_pread; + share->file_write= _ma_nommap_pwrite; + info->opt_flag&= ~MEMMAP_USED; } diff --git a/storage/maria/maria_def.h b/storage/maria/maria_def.h index f2c652c9286..a23e00a24a3 100644 --- a/storage/maria/maria_def.h +++ b/storage/maria/maria_def.h @@ -1431,3 +1431,11 @@ extern my_bool ma_yield_and_check_if_killed(MARIA_HA *info, int inx); extern my_bool ma_killed_standalone(MARIA_HA *); extern uint _ma_file_callback_to_id(void *callback_data); + +static inline void unmap_file(MARIA_HA *info __attribute__((unused))) +{ +#ifdef HAVE_MMAP + if (info->s->file_map) + _ma_unmap_file(info); +#endif +}