Skip to content
Permalink
Browse files

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
  • Loading branch information...
montywi committed Jun 18, 2019
1 parent b23c82f commit 8acbf9c1f961aa1008ef509e059e1a09943f5ed3
@@ -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;
@@ -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;
@@ -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);

@@ -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))
@@ -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);
@@ -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))
@@ -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)))
{
@@ -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);
@@ -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;
}


@@ -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
}

0 comments on commit 8acbf9c

Please sign in to comment.
You can’t perform that action at this time.