mirror of
https://github.com/MariaDB/server.git
synced 2025-12-24 11:21:21 +03:00
Fix for:
Bug #20662 "Infinite loop in CREATE TABLE IF NOT EXISTS ... SELECT with locked tables" Bug #20903 "Crash when using CREATE TABLE .. SELECT and triggers" Bug #24738 "CREATE TABLE ... SELECT is not isolated properly" Bug #24508 "Inconsistent results of CREATE TABLE ... SELECT when temporary table exists" Deadlock occured when one tried to execute CREATE TABLE IF NOT EXISTS ... SELECT statement under LOCK TABLES which held read lock on target table. Attempt to execute the same statement for already existing target table with triggers caused server crashes. Also concurrent execution of CREATE TABLE ... SELECT statement and other statements involving target table suffered from various races (some of which might've led to deadlocks). Finally, attempt to execute CREATE TABLE ... SELECT in case when a temporary table with same name was already present led to the insertion of data into this temporary table and creation of empty non-temporary table. All above problems stemmed from the old implementation of CREATE TABLE ... SELECT in which we created, opened and locked target table without any special protection in a separate step and not with the rest of tables used by this statement. This underminded deadlock-avoidance approach used in server and created window for races. It also excluded target table from prelocking causing problems with trigger execution. The patch solves these problems by implementing new approach to handling of CREATE TABLE ... SELECT for base tables. We try to open and lock table to be created at the same time as the rest of tables used by this statement. If such table does not exist at this moment we create and place in the table cache special placeholder for it which prevents its creation or any other usage by other threads. We still use old approach for creation of temporary tables. Also note that we decided to postpone introduction of some tests for concurrent behaviour of CREATE TABLE ... SELECT till 5.1. The main reason for this is absence in 5.0 ability to set @@debug variable at runtime, which can be circumvented only by using several test files with individual .opt files. Since the latter is likely to slowdown test-suite unnecessary we chose not to push this tests into 5.0, but run them manually for this version and later push their optimized version into 5.1 mysql-test/r/create.result: Extended test coverage for CREATE TABLE ... SELECT. In particular added tests for bug #24508 "Inconsistent results of CREATE TABLE ... SELECT when temporary table exists" and bug #20662 "Infinite loop in CREATE TABLE IF NOT EXISTS ... SELECT with locked tables". mysql-test/r/trigger.result: Added test case for bug #20903 "Crash when using CREATE TABLE .. SELECT and triggers" mysql-test/t/create.test: Extended test coverage for CREATE TABLE ... SELECT. In particular added tests for bug #24508 "Inconsistent results of CREATE TABLE ... SELECT when temporary table exists" and bug #20662 "Infinite loop in CREATE TABLE IF NOT EXISTS ... SELECT with locked tables". mysql-test/t/trigger.test: Added test case for bug #20903 "Crash when using CREATE TABLE .. SELECT and triggers" sql/lock.cc: Now for creation of name-lock placeholder in lock_table_name() we use auxiliary function table_cache_insert_placeholder(). sql/mysql_priv.h: Made build_table_path() function available outside of sql_table.cc file. reopen_name_locked_table() now has 3rd argument which controls linking in of table being opened into THD::open_tables (this is useful in cases when placeholder used for name-locking is already linked into this list). Added declaration of auxiliary function table_cache_insert_placeholder() which is used for creation of table placeholders for name-locking. Added declaration of table_cache_has_open_placeholder() function which can be used for checking if table cache contains an open placeholder for the table and if this placeholder was created by another thread. (This function is needed only in 5.0 where we use it in various versions of CREATE TABLE in order to protect it from concurrent CREATE TABLE ... SELECT operations for the table. Starting from 5.1 we use different approach so it is going to be removed there). Made close_old_data_files() static within sql_base.cc file. Added auxiliary drop_open_table() routine. Moved declaration of refresh_version to table.h header to make it accessible from inline methods of TABLE class. MYSQL_OPEN_IGNORE_LOCKED_TABLES flag is no longer used. Instead MYSQL_OPEN_TEMPORARY_ONLY option was added. sql/sql_base.cc: Added support for the new approach to the handling of CREATE TABLE ... SELECT for base tables. Now we try to open and lock table to be created at the same time as the rest of tables used by this statement. If such table does not exist at this moment we create and place in the table cache special placeholder for it which prevents its creation or any other usage by other threads. Note significant distinctions of this placeholder from the placeholder used for normal name-lock: 1) It is treated like open table by other name-locks so it does not allow name-lock taking operations like DROP TABLE or RENAME TABLE to proceed. 2) it is linked into THD::open_tables list and automatically removed during close_thread_tables() call. open_tables(): Implemented logic described above. To do this added auxiliary check_if_table_exists() function. Removed support for MYSQL_OPEN_IGNORE_LOCKED_TABLES option which is no longer used. Added MYSQL_OPEN_TEMPORARY_ONLY which is used to restrict search for temporary tables only. close_cached_tables()/close_thread_table()/reopen_tables()/ close_old_data_files()/table_is_used()/remove_table_from_cache(): Added support for open placeholders (note that we also use them when we need to re-open tables during flush). Added auxiliary drop_open_table() routine. reopen_name_locked_table(): Now has 3rd argument which controls linking in of table being opened into THD::open_tables (this is useful in cases when placeholder used for name-locking is already linked into this list). Added auxiliary table_cache_insert_placeholder() routine which simplifies creation of placeholders used for name-locking. Added table_cache_has_open_placeholder() function which can be used for checking if table cache contains an open placeholder for the table and if this placeholder was created by another thread. (This function is needed only in 5.0 where we use it in various versions of CREATE TABLE in order to protect it from concurrent CREATE TABLE ... SELECT operations for the table. Starting from 5.1 we use different approach so it is going to be removed there). sql/sql_handler.cc: Adjusted mysql_ha_mark_tables_for_reopen() routine to properly handle placeholders which now can be linked into open tables list. sql/sql_insert.cc: Introduced new approach to handling of base tables in CREATE TABLE ... SELECT statement. Now we try to open and lock table to be created at the same time as the rest of tables used by this statement. If such table does not exist at this moment we create and place in the table cache special placeholder for it which prevents its creation or any other usage by other threads. By doing this we avoid races which existed with previous approach in which we created, opened and locked target in separate step without any special protection. This also allows properly calculate prelocking set in cases when target table already exists and has some on insert triggers. Note that we don't employ the same approach for temporary tables (this is okay as such tables are unaffected by other threads). Changed create_table_from_items() and select_create methods to implement this approach. sql/sql_parse.cc: The new approach to handling of CREATE TABLE ... SELECT for base tables assumes that all tables (including table to be created) are opened and (or) locked at the same time. So in cases when we create base table we have to pass to open_and_lock_tables() table list which includes target table. sql/sql_prepare.cc: The new approach to handling of CREATE TABLE ... SELECT for base tables assumes that all tables (including table to be created) are opened and (or) locked at the same time. So in cases when we create base table we have to pass to open_and_lock_tables() table list which includes target table. sql/sql_table.cc: Now mysql_create_table_internal(), mysql_create_like_table() and mysql_alter_table() not only check that destination table doesn't exist on disk but also check that there is no create placeholder in table cache for it (i.e. there is no CREATE TABLE ... SELECT operation in progress for it). Note that starting from 5.1 we use different approach in order to to protect CREATE TABLE ... SELECT from concurrent CREATE TABLE (ALTER TABLE ... RENAME) operations, the latter simply take name-locks on table before its creation (on target table name before renaming). Also made build_table_path() available from other files and asjusted calls to reopen_name_locked_table(), which now takes extra argument, which controls linking of open table into THD::open_tables list. sql/sql_trigger.cc: reopen_name_locked_tables() now has one more argument which controls linking of opened table into the THD::open_tables list. sql/sql_yacc.yy: The new approach to handling of CREATE TABLE ... SELECT statement for base tables assumes that all tables including table to be created are open and (or) locked at the same time. Therefore we need to set correct lock for target table. sql/table.h: Moved declaration of refresh_version variable from mysql_priv.h to make it accessible from inline methods of TABLE class. Renamed TABLE::locked_by_flush member to open_placeholder since now it is also used for taking exclusive name-lock and not only by flush. Introduced TABLE::is_name_opened() helper method which can be used to distinguish TABLE instances corresponding to open tables or placeholders for them from closed instances (e.g. due to their old version). Also introduced TABLE::needs_reopen_or_name_lock() helper which allows to check if TABLE instance corresponds to outdated version of table or to name-lock placeholder. Introduced TABLE_LIST::create member which marks elements of table list corresponds to the table to be created. Adjusted TABLE_LIST::placeholder() method to take into account name-lock placeholders for tables to be created (this, for example, allows to properly handle such placeholders in lock_tables()).
This commit is contained in:
@@ -2179,7 +2179,7 @@ bool delayed_insert::handle_inserts(void)
|
||||
|
||||
thd.proc_info="insert";
|
||||
max_rows= delayed_insert_limit;
|
||||
if (thd.killed || table->s->version != refresh_version)
|
||||
if (thd.killed || table->needs_reopen_or_name_lock())
|
||||
{
|
||||
thd.killed= THD::KILL_CONNECTION;
|
||||
max_rows= ~(ulong)0; // Do as much as possible
|
||||
@@ -2791,8 +2791,8 @@ bool select_insert::send_eof()
|
||||
***************************************************************************/
|
||||
|
||||
/*
|
||||
Create table from lists of fields and items (or open existing table
|
||||
with same name).
|
||||
Create table from lists of fields and items (or just return TABLE
|
||||
object for pre-opened existing table).
|
||||
|
||||
SYNOPSIS
|
||||
create_table_from_items()
|
||||
@@ -2807,18 +2807,24 @@ bool select_insert::send_eof()
|
||||
of fields for the table (corresponding fields will
|
||||
be added to the end of alter_info->create_list)
|
||||
lock out Pointer to the MYSQL_LOCK object for table created
|
||||
(open) will be returned in this parameter. Since
|
||||
this table is not included in THD::lock caller is
|
||||
responsible for explicitly unlocking this table.
|
||||
(or open temporary table) will be returned in this
|
||||
parameter. Since this table is not included in
|
||||
THD::lock caller is responsible for explicitly
|
||||
unlocking this table.
|
||||
|
||||
NOTES
|
||||
If 'create_info->options' bitmask has HA_LEX_CREATE_IF_NOT_EXISTS
|
||||
flag and table with name provided already exists then this function will
|
||||
simply open existing table.
|
||||
Also note that create, open and lock sequence in this function is not
|
||||
atomic and thus contains gap for deadlock and can cause other troubles.
|
||||
Since this function contains some logic specific to CREATE TABLE ... SELECT
|
||||
it should be changed before it can be used in other contexts.
|
||||
This function behaves differently for base and temporary tables:
|
||||
- For base table we assume that either table exists and was pre-opened
|
||||
and locked at open_and_lock_tables() stage (and in this case we just
|
||||
emit error or warning and return pre-opened TABLE object) or special
|
||||
placeholder was put in table cache that guarantees that this table
|
||||
won't be created or opened until the placeholder will be removed
|
||||
(so there is an exclusive lock on this table).
|
||||
- We don't pre-open existing temporary table, instead we either open
|
||||
or create and then open table in this function.
|
||||
|
||||
Since this function contains some logic specific to CREATE TABLE ...
|
||||
SELECT it should be changed before it can be used in other contexts.
|
||||
|
||||
RETURN VALUES
|
||||
non-zero Pointer to TABLE object for table created or opened
|
||||
@@ -2841,6 +2847,25 @@ static TABLE *create_table_from_items(THD *thd, HA_CREATE_INFO *create_info,
|
||||
bool not_used;
|
||||
DBUG_ENTER("create_table_from_items");
|
||||
|
||||
DBUG_EXECUTE_IF("sleep_create_select_before_check_if_exists", my_sleep(6000000););
|
||||
|
||||
if (!(create_info->options & HA_LEX_CREATE_TMP_TABLE) &&
|
||||
create_table->table->db_stat)
|
||||
{
|
||||
/* Table already exists and was open at open_and_lock_tables() stage. */
|
||||
if (create_info->options & HA_LEX_CREATE_IF_NOT_EXISTS)
|
||||
{
|
||||
create_info->table_existed= 1; // Mark that table existed
|
||||
push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_NOTE,
|
||||
ER_TABLE_EXISTS_ERROR, ER(ER_TABLE_EXISTS_ERROR),
|
||||
create_table->table_name);
|
||||
DBUG_RETURN(create_table->table);
|
||||
}
|
||||
|
||||
my_error(ER_TABLE_EXISTS_ERROR, MYF(0), create_table->table_name);
|
||||
DBUG_RETURN(0);
|
||||
}
|
||||
|
||||
tmp_table.alias= 0;
|
||||
tmp_table.timestamp_field= 0;
|
||||
tmp_table.s= &tmp_table.share_not_to_be_used;
|
||||
@@ -2869,8 +2894,15 @@ static TABLE *create_table_from_items(THD *thd, HA_CREATE_INFO *create_info,
|
||||
cr_field->flags &= ~NOT_NULL_FLAG;
|
||||
alter_info->create_list.push_back(cr_field);
|
||||
}
|
||||
|
||||
DBUG_EXECUTE_IF("sleep_create_select_before_create", my_sleep(6000000););
|
||||
|
||||
/*
|
||||
create and lock table
|
||||
Create and lock table.
|
||||
|
||||
Note that we either creating (or opening existing) temporary table or
|
||||
creating base table on which name we have exclusive lock. So code below
|
||||
should not cause deadlocks or races.
|
||||
|
||||
We don't log the statement, it will be logged later.
|
||||
|
||||
@@ -2880,59 +2912,70 @@ static TABLE *create_table_from_items(THD *thd, HA_CREATE_INFO *create_info,
|
||||
don't want to delete from it) 2) it would be written before the CREATE
|
||||
TABLE, which is a wrong order. So we keep binary logging disabled when we
|
||||
open_table().
|
||||
NOTE: By locking table which we just have created (or for which we just have
|
||||
have found that it already exists) separately from other tables used by the
|
||||
statement we create potential window for deadlock.
|
||||
TODO: create and open should be done atomic !
|
||||
*/
|
||||
{
|
||||
tmp_disable_binlog(thd);
|
||||
if (!mysql_create_table(thd, create_table->db, create_table->table_name,
|
||||
create_info, alter_info, 0, select_field_count))
|
||||
{
|
||||
/*
|
||||
If we are here in prelocked mode we either create temporary table
|
||||
or prelocked mode is caused by the SELECT part of this statement.
|
||||
*/
|
||||
DBUG_ASSERT(!thd->prelocked_mode ||
|
||||
create_info->options & HA_LEX_CREATE_TMP_TABLE ||
|
||||
thd->lex->requires_prelocking());
|
||||
|
||||
/*
|
||||
NOTE: We don't want to ignore set of locked tables here if we are
|
||||
under explicit LOCK TABLES since it will open gap for deadlock
|
||||
too wide (and also is not backward compatible).
|
||||
*/
|
||||
if (! (table= open_table(thd, create_table, thd->mem_root, (bool*) 0,
|
||||
(MYSQL_LOCK_IGNORE_FLUSH |
|
||||
((thd->prelocked_mode == PRELOCKED) ?
|
||||
MYSQL_OPEN_IGNORE_LOCKED_TABLES:0)))))
|
||||
quick_rm_table(create_info->db_type, create_table->db,
|
||||
table_case_name(create_info, create_table->table_name));
|
||||
if (create_info->table_existed &&
|
||||
!(create_info->options & HA_LEX_CREATE_TMP_TABLE))
|
||||
{
|
||||
/*
|
||||
This means that someone created table underneath server
|
||||
or it was created via different mysqld front-end to the
|
||||
cluster. We don't have much options but throw an error.
|
||||
*/
|
||||
my_error(ER_TABLE_EXISTS_ERROR, MYF(0), create_table->table_name);
|
||||
DBUG_RETURN(0);
|
||||
}
|
||||
|
||||
DBUG_EXECUTE_IF("sleep_create_select_before_open", my_sleep(6000000););
|
||||
|
||||
if (!(create_info->options & HA_LEX_CREATE_TMP_TABLE))
|
||||
{
|
||||
VOID(pthread_mutex_lock(&LOCK_open));
|
||||
if (reopen_name_locked_table(thd, create_table, FALSE))
|
||||
{
|
||||
quick_rm_table(create_info->db_type, create_table->db,
|
||||
table_case_name(create_info,
|
||||
create_table->table_name));
|
||||
}
|
||||
else
|
||||
table= create_table->table;
|
||||
VOID(pthread_mutex_unlock(&LOCK_open));
|
||||
}
|
||||
else
|
||||
{
|
||||
if (!(table= open_table(thd, create_table, thd->mem_root, (bool*) 0,
|
||||
MYSQL_OPEN_TEMPORARY_ONLY)) &&
|
||||
!create_info->table_existed)
|
||||
{
|
||||
/*
|
||||
This shouldn't happen as creation of temporary table should make
|
||||
it preparable for open. But let us do close_temporary_table() here
|
||||
just in case.
|
||||
*/
|
||||
close_temporary_table(thd, create_table->db, create_table->table_name);
|
||||
}
|
||||
}
|
||||
}
|
||||
reenable_binlog(thd);
|
||||
if (!table) // open failed
|
||||
DBUG_RETURN(0);
|
||||
}
|
||||
|
||||
/*
|
||||
FIXME: What happens if trigger manages to be created while we are
|
||||
obtaining this lock ? May be it is sensible just to disable
|
||||
trigger execution in this case ? Or will MYSQL_LOCK_IGNORE_FLUSH
|
||||
save us from that ?
|
||||
*/
|
||||
DBUG_EXECUTE_IF("sleep_create_select_before_lock", my_sleep(6000000););
|
||||
|
||||
table->reginfo.lock_type=TL_WRITE;
|
||||
if (! ((*lock)= mysql_lock_tables(thd, &table, 1,
|
||||
MYSQL_LOCK_IGNORE_FLUSH, ¬_used)))
|
||||
{
|
||||
VOID(pthread_mutex_lock(&LOCK_open));
|
||||
hash_delete(&open_cache,(byte*) table);
|
||||
VOID(pthread_mutex_unlock(&LOCK_open));
|
||||
quick_rm_table(create_info->db_type, create_table->db,
|
||||
table_case_name(create_info, create_table->table_name));
|
||||
if (!create_info->table_existed)
|
||||
drop_open_table(thd, table, create_table->db, create_table->table_name);
|
||||
DBUG_RETURN(0);
|
||||
}
|
||||
table->file->extra(HA_EXTRA_WRITE_CACHE);
|
||||
DBUG_RETURN(table);
|
||||
}
|
||||
|
||||
@@ -2983,8 +3026,10 @@ select_create::prepare(List<Item> &values, SELECT_LEX_UNIT *u)
|
||||
(thd->variables.sql_mode &
|
||||
(MODE_STRICT_TRANS_TABLES |
|
||||
MODE_STRICT_ALL_TABLES)));
|
||||
DBUG_RETURN(check_that_all_fields_are_given_values(thd, table,
|
||||
table_list));
|
||||
if (check_that_all_fields_are_given_values(thd, table, table_list))
|
||||
DBUG_RETURN(1);
|
||||
table->file->extra(HA_EXTRA_WRITE_CACHE);
|
||||
DBUG_RETURN(0);
|
||||
}
|
||||
|
||||
|
||||
@@ -3016,31 +3061,18 @@ bool select_create::send_eof()
|
||||
{
|
||||
table->file->extra(HA_EXTRA_NO_IGNORE_DUP_KEY);
|
||||
table->file->extra(HA_EXTRA_WRITE_CANNOT_REPLACE);
|
||||
VOID(pthread_mutex_lock(&LOCK_open));
|
||||
mysql_unlock_tables(thd, lock);
|
||||
/*
|
||||
TODO:
|
||||
Check if we can remove the following two rows.
|
||||
We should be able to just keep the table in the table cache.
|
||||
*/
|
||||
if (!table->s->tmp_table)
|
||||
if (lock)
|
||||
{
|
||||
ulong version= table->s->version;
|
||||
hash_delete(&open_cache,(byte*) table);
|
||||
/* Tell threads waiting for refresh that something has happened */
|
||||
if (version != refresh_version)
|
||||
broadcast_refresh();
|
||||
mysql_unlock_tables(thd, lock);
|
||||
lock= 0;
|
||||
}
|
||||
lock=0;
|
||||
table=0;
|
||||
VOID(pthread_mutex_unlock(&LOCK_open));
|
||||
}
|
||||
return tmp;
|
||||
}
|
||||
|
||||
|
||||
void select_create::abort()
|
||||
{
|
||||
VOID(pthread_mutex_lock(&LOCK_open));
|
||||
if (lock)
|
||||
{
|
||||
mysql_unlock_tables(thd, lock);
|
||||
@@ -3050,22 +3082,10 @@ void select_create::abort()
|
||||
{
|
||||
table->file->extra(HA_EXTRA_NO_IGNORE_DUP_KEY);
|
||||
table->file->extra(HA_EXTRA_WRITE_CANNOT_REPLACE);
|
||||
enum db_type table_type=table->s->db_type;
|
||||
if (!table->s->tmp_table)
|
||||
{
|
||||
ulong version= table->s->version;
|
||||
hash_delete(&open_cache,(byte*) table);
|
||||
if (!create_info->table_existed)
|
||||
quick_rm_table(table_type, create_table->db, create_table->table_name);
|
||||
/* Tell threads waiting for refresh that something has happened */
|
||||
if (version != refresh_version)
|
||||
broadcast_refresh();
|
||||
}
|
||||
else if (!create_info->table_existed)
|
||||
close_temporary_table(thd, create_table->db, create_table->table_name);
|
||||
if (!create_info->table_existed)
|
||||
drop_open_table(thd, table, create_table->db, create_table->table_name);
|
||||
table=0;
|
||||
}
|
||||
VOID(pthread_mutex_unlock(&LOCK_open));
|
||||
}
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user