From 0688c086a0739333bf642f77f85f9f36cc0dcbef Mon Sep 17 00:00:00 2001 From: Date: Mon, 30 Aug 2010 14:03:28 +0800 Subject: Bug #54579 Wrong unsafe warning for INSERT DELAYED in SBR The lock_type is upgrade to TL_WRITE from TL_WRITE_DELAYED for INSERT DELAYED when inserting multi values in one statement. It's safe. But it causes an unsafe warning in SBR. Make INSERT DELAYED safe by logging it as INSERT without DELAYED. --- sql/sql_insert.cc | 51 ++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 46 insertions(+), 5 deletions(-) (limited to 'sql/sql_insert.cc') diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index 5f6ab43e2ea..adbdc1ffbc8 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -620,6 +620,32 @@ bool open_and_lock_for_insert_delayed(THD *thd, TABLE_LIST *table_list) } +/** + Create a new query string for removing DELAYED keyword for + multi INSERT DEALAYED statement. + + @param[in] thd Thread handler + @param[in] buf Query string + + @return + 0 ok + 1 error +*/ +static int +create_insert_stmt_from_insert_delayed(THD *thd, String *buf) +{ + /* Append the part of thd->query before "DELAYED" keyword */ + if (buf->append(thd->query(), + thd->lex->keyword_delayed_begin - thd->query())) + return 1; + /* Append the part of thd->query after "DELAYED" keyword */ + if (buf->append(thd->lex->keyword_delayed_begin + 7)) + return 1; + + return 0; +} + + /** INSERT statement implementation @@ -999,13 +1025,28 @@ bool mysql_insert(THD *thd,TABLE_LIST *table_list, such case the flag is ignored for constructing binlog event. */ DBUG_ASSERT(thd->killed != THD::KILL_BAD_DATA || error > 0); - if (thd->binlog_query(THD::ROW_QUERY_TYPE, - thd->query(), thd->query_length(), - transactional_table, FALSE, FALSE, - errcode)) + if (was_insert_delayed && table_list->lock_type == TL_WRITE) { + /* Binlog multi INSERT DELAYED as INSERT without DELAYED. */ + String log_query; + if (create_insert_stmt_from_insert_delayed(thd, &log_query)) + { + sql_print_error("Event Error: An error occurred while creating query string" + "for INSERT DELAYED stmt, before writing it into binary log."); + + error= 1; + } + else if (thd->binlog_query(THD::ROW_QUERY_TYPE, + log_query.c_ptr(), log_query.length(), + transactional_table, FALSE, FALSE, + errcode)) + error= 1; + } + else if (thd->binlog_query(THD::ROW_QUERY_TYPE, + thd->query(), thd->query_length(), + transactional_table, FALSE, FALSE, + errcode)) error= 1; - } } } DBUG_ASSERT(transactional_table || !changed || -- cgit v1.2.1 From 6d5065a9f4b6accf51906f5c5e38325dfb4707e8 Mon Sep 17 00:00:00 2001 From: Dmitry Lenev Date: Wed, 15 Sep 2010 18:15:31 +0400 Subject: Fix for bug #56251 "Deadlock with INSERT DELAYED and MERGE tables". Attempting to issue an INSERT DELAYED statement for a MERGE table might have caused a deadlock if it happened as part of a transaction or under LOCK TABLES, and there was a concurrent DDL or LOCK TABLES ... WRITE statement which tried to lock one of its underlying tables. The problem occurred when a delayed insert handler thread tried to open a MERGE table and discovered that to do this it had also to open all underlying tables and hence acquire metadata locks on them. Since metadata locks on the underlying tables were not pre-acquired by the connection thread executing INSERT DELAYED, attempts to do so might lead to waiting. In this case the connection thread had to wait for the delayed insert thread. If the thread which was preventing the lock on the underlying table from being acquired had to wait for the connection thread (due to this or other metadata locks), a deadlock occurred. This deadlock was not detected by the MDL deadlock detector since waiting for the handler thread by the connection thread is not represented in the wait-for graph. This patch solves the problem by ensuring that the delayed insert handler thread never tries to open underlying tables of a MERGE table. Instead open_tables() is aborted right after the parent table is opened and a ER_DELAYED_NOT_SUPPORTED error is emitted (which is passed to the connection thread and ultimately to the user). --- sql/sql_insert.cc | 77 ++++++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 68 insertions(+), 9 deletions(-) (limited to 'sql/sql_insert.cc') diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index adbdc1ffbc8..ad324fed4fe 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -2495,6 +2495,65 @@ void kill_delayed_threads(void) } +/** + A strategy for the prelocking algorithm which prevents the + delayed insert thread from opening tables with engines which + do not support delayed inserts. + + Particularly it allows to abort open_tables() as soon as we + discover that we have opened a MERGE table, without acquiring + metadata locks on underlying tables. +*/ + +class Delayed_prelocking_strategy : public Prelocking_strategy +{ +public: + virtual bool handle_routine(THD *thd, Query_tables_list *prelocking_ctx, + Sroutine_hash_entry *rt, sp_head *sp, + bool *need_prelocking); + virtual bool handle_table(THD *thd, Query_tables_list *prelocking_ctx, + TABLE_LIST *table_list, bool *need_prelocking); + virtual bool handle_view(THD *thd, Query_tables_list *prelocking_ctx, + TABLE_LIST *table_list, bool *need_prelocking); +}; + + +bool Delayed_prelocking_strategy:: +handle_table(THD *thd, Query_tables_list *prelocking_ctx, + TABLE_LIST *table_list, bool *need_prelocking) +{ + DBUG_ASSERT(table_list->lock_type == TL_WRITE_DELAYED); + + if (!(table_list->table->file->ha_table_flags() & HA_CAN_INSERT_DELAYED)) + { + my_error(ER_DELAYED_NOT_SUPPORTED, MYF(0), table_list->table_name); + return TRUE; + } + return FALSE; +} + + +bool Delayed_prelocking_strategy:: +handle_routine(THD *thd, Query_tables_list *prelocking_ctx, + Sroutine_hash_entry *rt, sp_head *sp, + bool *need_prelocking) +{ + /* LEX used by the delayed insert thread has no routines. */ + DBUG_ASSERT(0); + return FALSE; +} + + +bool Delayed_prelocking_strategy:: +handle_view(THD *thd, Query_tables_list *prelocking_ctx, + TABLE_LIST *table_list, bool *need_prelocking) +{ + /* We don't open views in the delayed insert thread. */ + DBUG_ASSERT(0); + return FALSE; +} + + /** Open and lock table for use by delayed thread and check that this table is suitable for delayed inserts. @@ -2505,21 +2564,21 @@ void kill_delayed_threads(void) bool Delayed_insert::open_and_lock_table() { + Delayed_prelocking_strategy prelocking_strategy; + + /* + Use special prelocking strategy to get ER_DELAYED_NOT_SUPPORTED + error for tables with engines which don't support delayed inserts. + */ if (!(table= open_n_lock_single_table(&thd, &table_list, TL_WRITE_DELAYED, - MYSQL_OPEN_IGNORE_GLOBAL_READ_LOCK))) + MYSQL_OPEN_IGNORE_GLOBAL_READ_LOCK, + &prelocking_strategy))) { thd.fatal_error(); // Abort waiting inserts return TRUE; } - if (!(table->file->ha_table_flags() & HA_CAN_INSERT_DELAYED)) - { - /* To rollback InnoDB statement transaction. */ - trans_rollback_stmt(&thd); - my_error(ER_DELAYED_NOT_SUPPORTED, MYF(ME_FATALERROR), - table_list.table_name); - return TRUE; - } + if (table->triggers) { /* -- cgit v1.2.1 From 71affc142d0b77dcfaa54c7d1d71666adf131b4e Mon Sep 17 00:00:00 2001 From: Jon Olav Hauglid Date: Fri, 24 Sep 2010 10:44:09 +0200 Subject: Bug #56678 Valgrind warnings from binlog.binlog_unsafe After the patch for Bug#54579, multi inserts done with INSERT DELAYED are binlogged as normal INSERT. During processing of the statement, a new query string without the DELAYED keyword is made. The problem was that this new string was incorrectly made when the INSERT DELAYED was part of a prepared statement - data was read outside the allocated buffer. The reason for this bug was that a pointer to the position of the DELAYED keyword inside the query string was stored when parsing the statement. This pointer was then later (at runtime) used (via pointer subtraction) to find the number of characters to skip when making a new query string without DELAYED. But when the statement was re-executed as part of a prepared statement, the original pointer would be invalid and the pointer subtraction would give a wrong/random result. This patch fixes the problem by instead storing the offsets from the beginning of the query string to the start and end of the DELAYED keyword. These values will not depend on the memory position of the query string at runtime and therefore not give wrong results when the statement is executed in a prepared statement. This bug was a regression introduced by the patch for Bug#54579. No test case added as this bug is already covered by the existing binlog.binlog_unsafe test case when running with valgrind. --- sql/sql_insert.cc | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) (limited to 'sql/sql_insert.cc') diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index ad324fed4fe..cccb715bd5e 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -634,14 +634,12 @@ bool open_and_lock_for_insert_delayed(THD *thd, TABLE_LIST *table_list) static int create_insert_stmt_from_insert_delayed(THD *thd, String *buf) { - /* Append the part of thd->query before "DELAYED" keyword */ - if (buf->append(thd->query(), - thd->lex->keyword_delayed_begin - thd->query())) + /* Make a copy of thd->query() and then remove the "DELAYED" keyword */ + if (buf->append(thd->query()) || + buf->replace(thd->lex->keyword_delayed_begin_offset, + thd->lex->keyword_delayed_end_offset - + thd->lex->keyword_delayed_begin_offset, 0)) return 1; - /* Append the part of thd->query after "DELAYED" keyword */ - if (buf->append(thd->lex->keyword_delayed_begin + 7)) - return 1; - return 0; } -- cgit v1.2.1