From 4b8db713aaa617ee089eedab42d3ab4598355e3f Mon Sep 17 00:00:00 2001 From: Jon Olav Hauglid Date: Wed, 9 Dec 2009 12:51:45 +0100 Subject: [PATCH 1/9] Backport of revno: 2617.69.34 Bug #45949 Assertion `!tables->table' in open_tables() on ALTER + INSERT DELAYED The assertion was caused by improperly closing tables when INSERT DELAYED needed to reopen tables. This patch replaces the call to close_thread_tables with close_tables_for_reopen which fixes the problem. The only way I was able to trigger the reopen code path and thus the assertion, was if ALTER TABLE killed the delayed insert thread and the delayed insert thread was able to enter the reopen code path before it noticed that thd->killed had been set. Note that in these cases reopen will always fail since open_table() will check thd->killed and return. This patch therefore adds two more thd->killed checks to minimize the chance of entering the reopen code path without hope for success. The patch also changes it so that if the delayed insert is killed using KILL_CONNECTION, the error message that is copied to the connection thread is ER_QUERY_INTERRUPTED rather than ER_SERVER_SHUTDOWN. This means that if INSERT DELAYED fails, the user will now see "Query execution was interrupted" rather than the misleading "Server shutdown in progress". No test case is supplied. This is for two reasons: 1) Unable to reproduce the error without having the delayed insert thread in a killed state which means that reopen is futile and was not supposed to be attempted. 2) Difficulty of using sync points in other threads than the connection thread. The patch has been successfully tested with the RQG and the grammar supplied in the bug description. --- sql/sql_insert.cc | 42 +++++++++++++++++++++++++----------------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index e0537c75e07..666d6400d32 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -1974,6 +1974,11 @@ bool delayed_get_table(THD *thd, TABLE_LIST *table_list) } pthread_mutex_unlock(&di->mutex); thd_proc_info(thd, "got old table"); + if (thd->killed) + { + di->unlock(); + goto end_create; + } if (di->thd.killed) { if (di->thd.is_error()) @@ -1981,20 +1986,19 @@ bool delayed_get_table(THD *thd, TABLE_LIST *table_list) /* Copy the error message. Note that we don't treat fatal errors in the delayed thread as fatal errors in the - main thread. Use of my_message will enable stored - procedures continue handlers. + main thread. If delayed thread was killed, we don't + want to send "Server shutdown in progress" in the + INSERT THREAD. */ - my_message(di->thd.stmt_da->sql_errno(), di->thd.stmt_da->message(), - MYF(0)); - } - di->unlock(); + if (di->thd.stmt_da->sql_errno() == ER_SERVER_SHUTDOWN) + my_message(ER_QUERY_INTERRUPTED, ER(ER_QUERY_INTERRUPTED), MYF(0)); + else + my_message(di->thd.stmt_da->sql_errno(), di->thd.stmt_da->message(), + MYF(0)); + } + di->unlock(); goto end_create; } - if (thd->killed) - { - di->unlock(); - goto end_create; - } pthread_mutex_lock(&LOCK_delayed_insert); delayed_threads.append(di); pthread_mutex_unlock(&LOCK_delayed_insert); @@ -2061,8 +2065,11 @@ TABLE *Delayed_insert::get_local_table(THD* client_thd) goto error; if (dead) { - my_message(thd.stmt_da->sql_errno(), thd.stmt_da->message(), MYF(0)); - goto error; + /* Don't copy over "Server shutdown in progress". */ + if (thd.stmt_da->sql_errno() == ER_SERVER_SHUTDOWN) + my_message(ER_QUERY_INTERRUPTED, ER(ER_QUERY_INTERRUPTED), MYF(0)); + else + my_message(thd.stmt_da->sql_errno(), thd.stmt_da->message(), MYF(0)); } } share= table->s; @@ -2412,7 +2419,7 @@ pthread_handler_t handle_delayed_insert(void *arg) for (;;) { - if (thd->killed == THD::KILL_CONNECTION) + if (thd->killed) { uint lock_count; /* @@ -2474,7 +2481,7 @@ pthread_handler_t handle_delayed_insert(void *arg) } thd_proc_info(&(di->thd), 0); - if (di->tables_in_use && ! thd->lock) + if (di->tables_in_use && ! thd->lock && !thd->killed) { bool need_reopen; /* @@ -2491,14 +2498,15 @@ pthread_handler_t handle_delayed_insert(void *arg) MYSQL_LOCK_IGNORE_GLOBAL_READ_LOCK, &need_reopen))) { - if (need_reopen) + if (need_reopen && !thd->killed) { /* We were waiting to obtain TL_WRITE_DELAYED (probably due to someone having or requesting TL_WRITE_ALLOW_READ) and got aborted. Try to reopen table and if it fails die. */ - close_thread_tables(thd); + TABLE_LIST *tl_ptr = &di->table_list; + close_tables_for_reopen(thd, &tl_ptr); di->table= 0; if (di->open_and_lock_table()) { From ee8184355d85070a1bc272ecfe9863424fbad26d Mon Sep 17 00:00:00 2001 From: Jon Olav Hauglid Date: Wed, 9 Dec 2009 12:58:52 +0100 Subject: [PATCH 2/9] Backport of revno: 3711.1.1 Bug #48725 Assert !thd->is_error() in delayed_get_table() This bug is a regression introduced by the patch for Bug #45949. If the handler thread for INSERT DELAYED was killed by e.g. FLUSH TABLES, the error message is copied from the handler thread to the connection thread. But the error was not reacted on, so the connection thread continued as normal, leading to an eventual assert. No test case added as it would have required sync points to work for handler threads. The plan is to add this in the scope of Bug #48725 / Bug #48541. The patch has been tested with the non-deterministic test case given in the bug description. --- sql/sql_insert.cc | 1 + 1 file changed, 1 insertion(+) diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index 666d6400d32..171c5e2cee0 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -2070,6 +2070,7 @@ TABLE *Delayed_insert::get_local_table(THD* client_thd) my_message(ER_QUERY_INTERRUPTED, ER(ER_QUERY_INTERRUPTED), MYF(0)); else my_message(thd.stmt_da->sql_errno(), thd.stmt_da->message(), MYF(0)); + goto error; } } share= table->s; From 20f19d147f8c7b6a825b7d85541d110ecb265421 Mon Sep 17 00:00:00 2001 From: Jon Olav Hauglid Date: Wed, 9 Dec 2009 13:15:35 +0100 Subject: [PATCH 3/9] Backport of revno: 2617.76.2 Bug #47107 assert in notify_shared_lock on incorrect CREATE TABLE , HANDLER Attempts to create a table (using CREATE TABLE, CREATE TABLE LIKE or CREATE TABLE SELECT statements) which already existed and was opened by the same connection through HANDLER statement, led to a stalled connection (for production builds of the server) or to the server being aborted due to an assertion failure (for debug builds of the server). This problem was introduced by the new implementation of a metadata locking subsystem and didn't affect earlier versions of the server. The cause of the problem was that the HANDLER was not closed by CREATE TABLE before CREATE tried to open and lock the table. Acquiring an exclusive MDL lock on the table to be created would therefore fail since HANDLER already had a shared MDL lock. This triggered an assert as the HANDLER and CREATE statements came from the same thread (self-deadlock). This patch resolves the issue by closing any open HANDLERs on tables to be created by CREATE TABLE, similar to what is already done for DROP and ALTER TABLE. Test case added to create.test. --- mysql-test/r/create.result | 18 ++++++++++++++++++ mysql-test/t/create.test | 30 ++++++++++++++++++++++++++++++ sql/sql_parse.cc | 6 ++++++ 3 files changed, 54 insertions(+) diff --git a/mysql-test/r/create.result b/mysql-test/r/create.result index cf424b8b058..90502f94f44 100644 --- a/mysql-test/r/create.result +++ b/mysql-test/r/create.result @@ -1961,3 +1961,21 @@ END ;| ERROR 42000: This version of MySQL doesn't yet support 'multiple triggers with the same action time and event for one table' DROP TABLE t1; DROP TABLE B; +# +# Bug #47107 assert in notify_shared_lock on incorrect +# CREATE TABLE , HANDLER +DROP TABLE IF EXISTS t1; +CREATE TABLE t1(f1 integer); +# The following CREATE TABLEs before gave an assert. +HANDLER t1 OPEN AS A; +CREATE TABLE t1 SELECT 1 AS f2; +ERROR 42S01: Table 't1' already exists +HANDLER t1 OPEN AS A; +CREATE TABLE t1(f1 integer); +ERROR 42S01: Table 't1' already exists +CREATE TABLE t2(f1 integer); +HANDLER t1 OPEN AS A; +CREATE TABLE t1 LIKE t2; +ERROR 42S01: Table 't1' already exists +DROP TABLE t2; +DROP TABLE t1; diff --git a/mysql-test/t/create.test b/mysql-test/t/create.test index c07014bfc19..21778e00ab9 100644 --- a/mysql-test/t/create.test +++ b/mysql-test/t/create.test @@ -1639,3 +1639,33 @@ END ;| DROP TABLE t1; DROP TABLE B; + + +--echo # +--echo # Bug #47107 assert in notify_shared_lock on incorrect +--echo # CREATE TABLE , HANDLER +--echo # + +--disable_warnings +DROP TABLE IF EXISTS t1; +--enable_warnings + +CREATE TABLE t1(f1 integer); + +--echo # The following CREATE TABLEs before gave an assert. + +HANDLER t1 OPEN AS A; +--error ER_TABLE_EXISTS_ERROR +CREATE TABLE t1 SELECT 1 AS f2; + +HANDLER t1 OPEN AS A; +--error ER_TABLE_EXISTS_ERROR +CREATE TABLE t1(f1 integer); + +CREATE TABLE t2(f1 integer); +HANDLER t1 OPEN AS A; +--error ER_TABLE_EXISTS_ERROR +CREATE TABLE t1 LIKE t2; + +DROP TABLE t2; +DROP TABLE t1; diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 2c5f72c8cf4..056a947f3fa 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -2311,6 +2311,12 @@ case SQLCOM_PREPARE: thd->work_part_info= part_info; } #endif + + /* + Close any open handlers for the table + */ + mysql_ha_rm_tables(thd, create_table); + if (select_lex->item_list.elements) // With select { select_result *result; From 81813b1d72f9ab5c2a4355ff2aa01f99d374e1fb Mon Sep 17 00:00:00 2001 From: Jon Olav Hauglid Date: Wed, 9 Dec 2009 13:27:24 +0100 Subject: [PATCH 4/9] Backport of revno: 2617.76.3 Bug#47107 Add missing line in previous change set. --- mysql-test/r/create.result | 1 + 1 file changed, 1 insertion(+) diff --git a/mysql-test/r/create.result b/mysql-test/r/create.result index 90502f94f44..66439593c7c 100644 --- a/mysql-test/r/create.result +++ b/mysql-test/r/create.result @@ -1964,6 +1964,7 @@ DROP TABLE B; # # Bug #47107 assert in notify_shared_lock on incorrect # CREATE TABLE , HANDLER +# DROP TABLE IF EXISTS t1; CREATE TABLE t1(f1 integer); # The following CREATE TABLEs before gave an assert. From cf1110cd4a807de8e5b4aa7c8655447358f6457c Mon Sep 17 00:00:00 2001 From: Jon Olav Hauglid Date: Wed, 9 Dec 2009 14:03:37 +0100 Subject: [PATCH 5/9] Backport of revno: 3702 Bug #48248 assert in MDL_ticket::upgrade_shared_lock_to_exclusive The assert would happen if REPAIR TABLE was used on a table already locked by LOCK TABLES READ. REPAIR mistakenly tried to upgrade the read-lock to exclusive, thereby triggering the assert. The cause of the problem was that REPAIR TABLE ignored errors from opening and locking tables. This is by design, as REPAIR can be used to broken tables that cannot be opened. However, repair also ignored logical errors such as the inability to exclusivly lock a table due to conflicting LOCK TABLES. This patch fixes the problem by not ignoring errors from opening and locking tables if inside LOCK TABLES mode. In LOCK TABLES we already know that the table can be opened, so that the failure to open must be a logical error. Test added to repair.test. --- mysql-test/r/lock.result | 2 +- mysql-test/r/repair.result | 12 ++++++++++++ mysql-test/r/view.result | 6 +++--- mysql-test/t/repair.test | 15 +++++++++++++++ sql/sql_table.cc | 15 +++++++++++++-- 5 files changed, 44 insertions(+), 6 deletions(-) diff --git a/mysql-test/r/lock.result b/mysql-test/r/lock.result index a542d70c5b9..8f680858fdc 100644 --- a/mysql-test/r/lock.result +++ b/mysql-test/r/lock.result @@ -41,7 +41,7 @@ lock tables t1 write; check table t2; Table Op Msg_type Msg_text test.t2 check Error Table 't2' was not locked with LOCK TABLES -test.t2 check error Corrupt +test.t2 check status Operation failed insert into t1 select index1,nr from t1; ERROR HY000: Table 't1' was not locked with LOCK TABLES unlock tables; diff --git a/mysql-test/r/repair.result b/mysql-test/r/repair.result index 5bb3dd76fed..77eb927a21f 100644 --- a/mysql-test/r/repair.result +++ b/mysql-test/r/repair.result @@ -157,3 +157,15 @@ REPAIR TABLE tt1 USE_FRM; Table Op Msg_type Msg_text tt1 repair error Cannot repair temporary table from .frm file DROP TABLE tt1; +# +# Bug #48248 assert in MDL_ticket::upgrade_shared_lock_to_exclusive +# +DROP TABLE IF EXISTS t1; +CREATE TABLE t1(a INT); +LOCK TABLES t1 READ; +REPAIR TABLE t1; +Table Op Msg_type Msg_text +test.t1 repair Error Table 't1' was locked with a READ lock and can't be updated +test.t1 repair status Operation failed +UNLOCK TABLES; +DROP TABLE t1; diff --git a/mysql-test/r/view.result b/mysql-test/r/view.result index 5f16d88a0dc..7e9739173df 100644 --- a/mysql-test/r/view.result +++ b/mysql-test/r/view.result @@ -1955,15 +1955,15 @@ CHECK TABLE v1, v2, v3, v4, v5, v6; Table Op Msg_type Msg_text test.v1 check Error FUNCTION test.f1 does not exist test.v1 check Error View 'test.v1' references invalid table(s) or column(s) or function(s) or definer/invoker of view lack rights to use them -test.v1 check error Corrupt +test.v1 check status Operation failed test.v2 check status OK test.v3 check Error FUNCTION test.f1 does not exist test.v3 check Error View 'test.v3' references invalid table(s) or column(s) or function(s) or definer/invoker of view lack rights to use them -test.v3 check error Corrupt +test.v3 check status Operation failed test.v4 check status OK test.v5 check Error FUNCTION test.f1 does not exist test.v5 check Error View 'test.v5' references invalid table(s) or column(s) or function(s) or definer/invoker of view lack rights to use them -test.v5 check error Corrupt +test.v5 check status Operation failed test.v6 check status OK create function f1 () returns int return (select max(col1) from t1); DROP TABLE t1; diff --git a/mysql-test/t/repair.test b/mysql-test/t/repair.test index eb2ca7992a6..ec4c9b3cae8 100644 --- a/mysql-test/t/repair.test +++ b/mysql-test/t/repair.test @@ -158,3 +158,18 @@ CREATE TEMPORARY TABLE tt1 (c1 INT); REPAIR TABLE tt1 USE_FRM; DROP TABLE tt1; + +--echo # +--echo # Bug #48248 assert in MDL_ticket::upgrade_shared_lock_to_exclusive +--echo # + +--disable_warnings +DROP TABLE IF EXISTS t1; +--enable_warnings + +CREATE TABLE t1(a INT); +LOCK TABLES t1 READ; +REPAIR TABLE t1; + +UNLOCK TABLES; +DROP TABLE t1; diff --git a/sql/sql_table.cc b/sql/sql_table.cc index ddb53dd3754..341d2d16757 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -4548,6 +4548,7 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables, char table_name[NAME_LEN*2+2]; char* db = table->db; bool fatal_error=0; + bool open_error; DBUG_PRINT("admin", ("table: '%s'.'%s'", table->db, table->table_name)); DBUG_PRINT("admin", ("extra_open_options: %u", extra_open_options)); @@ -4575,12 +4576,22 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables, if (view_operator_func == NULL) table->required_type=FRMTYPE_TABLE; - open_and_lock_tables_derived(thd, table, TRUE, - MYSQL_OPEN_TAKE_UPGRADABLE_MDL); + open_error= open_and_lock_tables_derived(thd, table, TRUE, + MYSQL_OPEN_TAKE_UPGRADABLE_MDL); thd->no_warnings_for_error= 0; table->next_global= save_next_global; table->next_local= save_next_local; thd->open_options&= ~extra_open_options; + /* + Under locked tables, we know that the table can be opened, + so any errors opening the table are logical errors. + In these cases it does not make sense to try to repair. + */ + if (open_error && thd->locked_tables_mode) + { + result_code= HA_ADMIN_FAILED; + goto send_result; + } #ifdef WITH_PARTITION_STORAGE_ENGINE if (table->table) { From fb545cf9d8f2cc38afe35640534f51e6281dee38 Mon Sep 17 00:00:00 2001 From: Jon Olav Hauglid Date: Wed, 9 Dec 2009 14:23:31 +0100 Subject: [PATCH 6/9] Backport of revno: 2617.31.30 Bug #21793 Missing CF_CHANGES_DATA and CF_STATUS_COMMAND for handful of commands CF_CHANGES_DATA and CF_STATUS_COMMAND flags added to the commands mentioned in the bug description. With the following two exceptions: 1) 4 commands do not exist: SQLCOM_RENAME_DB SQLCOM_LOAD_MASTER_DATA SQLCOM_LOAD_MASTER_TABLE SQLCOM_SHOW_COLUMN_TYPES 2) All SQLCOM_SHOW_* commands already had CF_STATUS_COMMAND, leaving only SQLCOM_BINLOG_BASE64_EVENT. Further, check_prepared_statement() in sql_prepare.cc has been simplified by taking advantage of the CF_STATUS_COMMAND flag. Note that no test case has been added. --- sql/sql_parse.cc | 91 ++++++++++++++++++++++++++++-------------------- 1 file changed, 54 insertions(+), 37 deletions(-) diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 056a947f3fa..06f29cd6039 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -197,6 +197,8 @@ void init_update_queries(void) sql_command_flags[SQLCOM_CREATE_VIEW]= CF_CHANGES_DATA | CF_REEXECUTION_FRAGILE | CF_AUTO_COMMIT_TRANS; sql_command_flags[SQLCOM_DROP_VIEW]= CF_CHANGES_DATA | CF_AUTO_COMMIT_TRANS; + sql_command_flags[SQLCOM_CREATE_TRIGGER]= CF_CHANGES_DATA | CF_AUTO_COMMIT_TRANS; + sql_command_flags[SQLCOM_DROP_TRIGGER]= CF_CHANGES_DATA | CF_AUTO_COMMIT_TRANS; sql_command_flags[SQLCOM_CREATE_EVENT]= CF_CHANGES_DATA | CF_AUTO_COMMIT_TRANS; sql_command_flags[SQLCOM_ALTER_EVENT]= CF_CHANGES_DATA | CF_AUTO_COMMIT_TRANS; sql_command_flags[SQLCOM_DROP_EVENT]= CF_CHANGES_DATA | CF_AUTO_COMMIT_TRANS; @@ -221,8 +223,7 @@ void init_update_queries(void) sql_command_flags[SQLCOM_REPLACE_SELECT]= CF_CHANGES_DATA | CF_HAS_ROW_COUNT | CF_REEXECUTION_FRAGILE; sql_command_flags[SQLCOM_SELECT]= CF_REEXECUTION_FRAGILE; - sql_command_flags[SQLCOM_SET_OPTION]= CF_REEXECUTION_FRAGILE | - CF_AUTO_COMMIT_TRANS; + sql_command_flags[SQLCOM_SET_OPTION]= CF_REEXECUTION_FRAGILE | CF_AUTO_COMMIT_TRANS; sql_command_flags[SQLCOM_DO]= CF_REEXECUTION_FRAGILE; sql_command_flags[SQLCOM_SHOW_STATUS_PROC]= CF_STATUS_COMMAND | CF_REEXECUTION_FRAGILE; @@ -237,34 +238,35 @@ void init_update_queries(void) sql_command_flags[SQLCOM_SHOW_VARIABLES]= CF_STATUS_COMMAND | CF_REEXECUTION_FRAGILE; sql_command_flags[SQLCOM_SHOW_CHARSETS]= CF_STATUS_COMMAND | CF_REEXECUTION_FRAGILE; sql_command_flags[SQLCOM_SHOW_COLLATIONS]= CF_STATUS_COMMAND | CF_REEXECUTION_FRAGILE; - sql_command_flags[SQLCOM_SHOW_NEW_MASTER]= CF_STATUS_COMMAND; - sql_command_flags[SQLCOM_SHOW_BINLOGS]= CF_STATUS_COMMAND; + sql_command_flags[SQLCOM_SHOW_NEW_MASTER]= CF_STATUS_COMMAND; + sql_command_flags[SQLCOM_SHOW_BINLOGS]= CF_STATUS_COMMAND; sql_command_flags[SQLCOM_SHOW_SLAVE_HOSTS]= CF_STATUS_COMMAND; sql_command_flags[SQLCOM_SHOW_BINLOG_EVENTS]= CF_STATUS_COMMAND; sql_command_flags[SQLCOM_SHOW_STORAGE_ENGINES]= CF_STATUS_COMMAND; - sql_command_flags[SQLCOM_SHOW_AUTHORS]= CF_STATUS_COMMAND; + sql_command_flags[SQLCOM_SHOW_AUTHORS]= CF_STATUS_COMMAND; sql_command_flags[SQLCOM_SHOW_CONTRIBUTORS]= CF_STATUS_COMMAND; - sql_command_flags[SQLCOM_SHOW_PRIVILEGES]= CF_STATUS_COMMAND; - sql_command_flags[SQLCOM_SHOW_WARNS]= CF_STATUS_COMMAND | CF_DIAGNOSTIC_STMT; - sql_command_flags[SQLCOM_SHOW_ERRORS]= CF_STATUS_COMMAND | CF_DIAGNOSTIC_STMT; + sql_command_flags[SQLCOM_SHOW_PRIVILEGES]= CF_STATUS_COMMAND; + sql_command_flags[SQLCOM_SHOW_WARNS]= CF_STATUS_COMMAND | CF_DIAGNOSTIC_STMT; + sql_command_flags[SQLCOM_SHOW_ERRORS]= CF_STATUS_COMMAND | CF_DIAGNOSTIC_STMT; sql_command_flags[SQLCOM_SHOW_ENGINE_STATUS]= CF_STATUS_COMMAND; sql_command_flags[SQLCOM_SHOW_ENGINE_MUTEX]= CF_STATUS_COMMAND; sql_command_flags[SQLCOM_SHOW_ENGINE_LOGS]= CF_STATUS_COMMAND; sql_command_flags[SQLCOM_SHOW_PROCESSLIST]= CF_STATUS_COMMAND; - sql_command_flags[SQLCOM_SHOW_GRANTS]= CF_STATUS_COMMAND; - sql_command_flags[SQLCOM_SHOW_CREATE_DB]= CF_STATUS_COMMAND; + sql_command_flags[SQLCOM_SHOW_GRANTS]= CF_STATUS_COMMAND; + sql_command_flags[SQLCOM_SHOW_CREATE_DB]= CF_STATUS_COMMAND; sql_command_flags[SQLCOM_SHOW_CREATE]= CF_STATUS_COMMAND; - sql_command_flags[SQLCOM_SHOW_MASTER_STAT]= CF_STATUS_COMMAND; + sql_command_flags[SQLCOM_SHOW_MASTER_STAT]= CF_STATUS_COMMAND; sql_command_flags[SQLCOM_SHOW_SLAVE_STAT]= CF_STATUS_COMMAND; - sql_command_flags[SQLCOM_SHOW_CREATE_PROC]= CF_STATUS_COMMAND; - sql_command_flags[SQLCOM_SHOW_CREATE_FUNC]= CF_STATUS_COMMAND; + sql_command_flags[SQLCOM_SHOW_CREATE_PROC]= CF_STATUS_COMMAND; + sql_command_flags[SQLCOM_SHOW_CREATE_FUNC]= CF_STATUS_COMMAND; sql_command_flags[SQLCOM_SHOW_CREATE_TRIGGER]= CF_STATUS_COMMAND; - sql_command_flags[SQLCOM_SHOW_STATUS_FUNC]= CF_STATUS_COMMAND | CF_REEXECUTION_FRAGILE; - sql_command_flags[SQLCOM_SHOW_PROC_CODE]= CF_STATUS_COMMAND; - sql_command_flags[SQLCOM_SHOW_FUNC_CODE]= CF_STATUS_COMMAND; - sql_command_flags[SQLCOM_SHOW_CREATE_EVENT]= CF_STATUS_COMMAND; - sql_command_flags[SQLCOM_SHOW_PROFILES]= CF_STATUS_COMMAND; - sql_command_flags[SQLCOM_SHOW_PROFILE]= CF_STATUS_COMMAND; + sql_command_flags[SQLCOM_SHOW_STATUS_FUNC]= CF_STATUS_COMMAND | CF_REEXECUTION_FRAGILE; + sql_command_flags[SQLCOM_SHOW_PROC_CODE]= CF_STATUS_COMMAND; + sql_command_flags[SQLCOM_SHOW_FUNC_CODE]= CF_STATUS_COMMAND; + sql_command_flags[SQLCOM_SHOW_CREATE_EVENT]= CF_STATUS_COMMAND; + sql_command_flags[SQLCOM_SHOW_PROFILES]= CF_STATUS_COMMAND; + sql_command_flags[SQLCOM_SHOW_PROFILE]= CF_STATUS_COMMAND; + sql_command_flags[SQLCOM_BINLOG_BASE64_EVENT]= CF_STATUS_COMMAND; sql_command_flags[SQLCOM_SHOW_TABLES]= (CF_STATUS_COMMAND | CF_SHOW_TABLE_COMMAND | @@ -273,37 +275,52 @@ void init_update_queries(void) CF_SHOW_TABLE_COMMAND | CF_REEXECUTION_FRAGILE); + + sql_command_flags[SQLCOM_CREATE_USER]= CF_CHANGES_DATA; + sql_command_flags[SQLCOM_RENAME_USER]= CF_CHANGES_DATA; + sql_command_flags[SQLCOM_DROP_USER]= CF_CHANGES_DATA; + sql_command_flags[SQLCOM_GRANT]= CF_CHANGES_DATA; + sql_command_flags[SQLCOM_REVOKE]= CF_CHANGES_DATA; + sql_command_flags[SQLCOM_ALTER_DB]= CF_CHANGES_DATA; + sql_command_flags[SQLCOM_CREATE_FUNCTION]= CF_CHANGES_DATA; + sql_command_flags[SQLCOM_DROP_FUNCTION]= CF_CHANGES_DATA; + sql_command_flags[SQLCOM_OPTIMIZE]= CF_CHANGES_DATA; + sql_command_flags[SQLCOM_CREATE_PROCEDURE]= CF_CHANGES_DATA; + sql_command_flags[SQLCOM_CREATE_SPFUNCTION]= CF_CHANGES_DATA; + sql_command_flags[SQLCOM_DROP_PROCEDURE]= CF_CHANGES_DATA; + sql_command_flags[SQLCOM_ALTER_PROCEDURE]= CF_CHANGES_DATA; + sql_command_flags[SQLCOM_ALTER_FUNCTION]= CF_CHANGES_DATA; + sql_command_flags[SQLCOM_INSTALL_PLUGIN]= CF_CHANGES_DATA; + sql_command_flags[SQLCOM_UNINSTALL_PLUGIN]= CF_CHANGES_DATA; + /* The following is used to preserver CF_ROW_COUNT during the a CALL or EXECUTE statement, so the value generated by the last called (or executed) statement is preserved. See mysql_execute_command() for how CF_ROW_COUNT is used. */ - sql_command_flags[SQLCOM_CALL]= CF_HAS_ROW_COUNT | CF_REEXECUTION_FRAGILE; - sql_command_flags[SQLCOM_EXECUTE]= CF_HAS_ROW_COUNT; + sql_command_flags[SQLCOM_CALL]= CF_HAS_ROW_COUNT | CF_REEXECUTION_FRAGILE; + sql_command_flags[SQLCOM_EXECUTE]= CF_HAS_ROW_COUNT; /* The following admin table operations are allowed on log tables. */ - sql_command_flags[SQLCOM_REPAIR]= CF_WRITE_LOGS_COMMAND | - CF_AUTO_COMMIT_TRANS; - sql_command_flags[SQLCOM_OPTIMIZE]= CF_WRITE_LOGS_COMMAND | - CF_AUTO_COMMIT_TRANS; - sql_command_flags[SQLCOM_ANALYZE]= CF_WRITE_LOGS_COMMAND | - CF_AUTO_COMMIT_TRANS; + sql_command_flags[SQLCOM_REPAIR]= CF_WRITE_LOGS_COMMAND | CF_AUTO_COMMIT_TRANS; + sql_command_flags[SQLCOM_OPTIMIZE]|= CF_WRITE_LOGS_COMMAND | CF_AUTO_COMMIT_TRANS; + sql_command_flags[SQLCOM_ANALYZE]= CF_WRITE_LOGS_COMMAND | CF_AUTO_COMMIT_TRANS; - sql_command_flags[SQLCOM_CREATE_USER]= CF_AUTO_COMMIT_TRANS; - sql_command_flags[SQLCOM_DROP_USER]= CF_AUTO_COMMIT_TRANS; - sql_command_flags[SQLCOM_RENAME_USER]= CF_AUTO_COMMIT_TRANS; - sql_command_flags[SQLCOM_REVOKE_ALL]= CF_AUTO_COMMIT_TRANS; - sql_command_flags[SQLCOM_REVOKE]= CF_AUTO_COMMIT_TRANS; - sql_command_flags[SQLCOM_GRANT]= CF_AUTO_COMMIT_TRANS; + sql_command_flags[SQLCOM_CREATE_USER]|= CF_AUTO_COMMIT_TRANS; + sql_command_flags[SQLCOM_DROP_USER]|= CF_AUTO_COMMIT_TRANS; + sql_command_flags[SQLCOM_RENAME_USER]|= CF_AUTO_COMMIT_TRANS; + sql_command_flags[SQLCOM_REVOKE_ALL]= CF_AUTO_COMMIT_TRANS; + sql_command_flags[SQLCOM_REVOKE]|= CF_AUTO_COMMIT_TRANS; + sql_command_flags[SQLCOM_GRANT]|= CF_AUTO_COMMIT_TRANS; - sql_command_flags[SQLCOM_CREATE_PROCEDURE]= CF_AUTO_COMMIT_TRANS; - sql_command_flags[SQLCOM_CREATE_SPFUNCTION]= CF_AUTO_COMMIT_TRANS; - sql_command_flags[SQLCOM_ALTER_PROCEDURE]= CF_AUTO_COMMIT_TRANS; - sql_command_flags[SQLCOM_ALTER_FUNCTION]= CF_AUTO_COMMIT_TRANS; + sql_command_flags[SQLCOM_CREATE_PROCEDURE]|= CF_AUTO_COMMIT_TRANS; + sql_command_flags[SQLCOM_CREATE_SPFUNCTION]|= CF_AUTO_COMMIT_TRANS; + sql_command_flags[SQLCOM_ALTER_PROCEDURE]|= CF_AUTO_COMMIT_TRANS; + sql_command_flags[SQLCOM_ALTER_FUNCTION]|= CF_AUTO_COMMIT_TRANS; sql_command_flags[SQLCOM_ASSIGN_TO_KEYCACHE]= CF_AUTO_COMMIT_TRANS; sql_command_flags[SQLCOM_PRELOAD_KEYS]= CF_AUTO_COMMIT_TRANS; From 1642f67b4019c1191ecd8fa5e53302171ef6aba4 Mon Sep 17 00:00:00 2001 From: "lars-erik.bjork@sun.com" <> Date: Wed, 9 Dec 2009 14:41:56 +0100 Subject: [PATCH 7/9] Backport of revno: 2617.68.36 --------------------------------------------- This is a patch for bug#47098 assert in MDL_context::destroy on HANDLER OPEN. The assert occurs in MDL_context::destroy when the connection is terminated, because all mdl_tickets have not been released. MERGE tables do not support being opened using the HANDLER ... OPEN command, and trying to do so will result in an error. In the event of an error, all tables that are opened, should be closed again. The fix for bug#45781 made sure that this also works for MERGE tables, which causes multiple tables to be opened. This fix extends the fix for bug#45781, by ensuring that also all locks are released, when MERGE tables are involved. --- mysql-test/r/merge.result | 30 ++++++++++++++++++++++++++++++ mysql-test/t/merge.test | 39 ++++++++++++++++++++++++++++++++++++++- sql/sql_handler.cc | 1 + 3 files changed, 69 insertions(+), 1 deletion(-) diff --git a/mysql-test/r/merge.result b/mysql-test/r/merge.result index 479645744ec..3a6cd4d3f5a 100644 --- a/mysql-test/r/merge.result +++ b/mysql-test/r/merge.result @@ -2502,4 +2502,34 @@ c1 DROP TRIGGER t2_au; DROP FUNCTION f1; DROP TABLE tm1, t1, t2, t3, t4, t5; +# +# Bug47098 assert in MDL_context::destroy on HANDLER +# OPEN +# +# Test that merge tables are closed correctly when opened using +# HANDLER ... OPEN. +# The general case. +DROP TABLE IF EXISTS t1, t2, t3; +# Connection con1. +CREATE TABLE t1 (c1 int); +CREATE TABLE t2 (c1 int); +CREATE TABLE t3 (c1 int) ENGINE = MERGE UNION (t1,t2); +START TRANSACTION; +HANDLER t3 OPEN; +ERROR HY000: Table storage engine for 't3' doesn't have this option +DROP TABLE t1, t2, t3; +# Connection default. +# Disconnecting con1, all mdl_tickets must have been released. +# The bug-specific case. +# Connection con1. +CREATE TABLE t1 (c1 int); +CREATE TABLE t2 (c1 int); +CREATE TABLE t3 (c1 int) ENGINE = MERGE UNION (t1,t2); +DROP TABLE t2; +START TRANSACTION; +HANDLER t3 OPEN; +ERROR HY000: Unable to open underlying table which is differently defined or of non-MyISAM type or doesn't exist +DROP TABLE t1, t3; +# Connection default. +# Disconnecting con1, all mdl_tickets must have been released. End of 6.0 tests diff --git a/mysql-test/t/merge.test b/mysql-test/t/merge.test index f7ce6ba700b..2738f79247f 100644 --- a/mysql-test/t/merge.test +++ b/mysql-test/t/merge.test @@ -1985,6 +1985,43 @@ DROP TRIGGER t2_au; DROP FUNCTION f1; DROP TABLE tm1, t1, t2, t3, t4, t5; - +--echo # +--echo # Bug47098 assert in MDL_context::destroy on HANDLER +--echo # OPEN +--echo # +--echo # Test that merge tables are closed correctly when opened using +--echo # HANDLER ... OPEN. +--echo # The general case. +--disable_warnings +DROP TABLE IF EXISTS t1, t2, t3; +--enable_warnings +--echo # Connection con1. +connect (con1,localhost,root,,); +CREATE TABLE t1 (c1 int); +CREATE TABLE t2 (c1 int); +CREATE TABLE t3 (c1 int) ENGINE = MERGE UNION (t1,t2); +START TRANSACTION; +--error ER_ILLEGAL_HA +HANDLER t3 OPEN; +DROP TABLE t1, t2, t3; +--echo # Connection default. +connection default; +--echo # Disconnecting con1, all mdl_tickets must have been released. +disconnect con1; +--echo # The bug-specific case. +--echo # Connection con1. +connect (con1,localhost,root,,); +CREATE TABLE t1 (c1 int); +CREATE TABLE t2 (c1 int); +CREATE TABLE t3 (c1 int) ENGINE = MERGE UNION (t1,t2); +DROP TABLE t2; +START TRANSACTION; +--error ER_WRONG_MRG_TABLE +HANDLER t3 OPEN; +DROP TABLE t1, t3; +--echo # Connection default. +connection default; +--echo # Disconnecting con1, all mdl_tickets must have been released. +disconnect con1; --echo End of 6.0 tests diff --git a/sql/sql_handler.cc b/sql/sql_handler.cc index 94f6b248e45..cf178342b51 100644 --- a/sql/sql_handler.cc +++ b/sql/sql_handler.cc @@ -321,6 +321,7 @@ bool mysql_ha_open(THD *thd, TABLE_LIST *tables, bool reopen) can close a single table only. */ close_thread_tables(thd); + thd->mdl_context.release_all_locks(); my_error(ER_ILLEGAL_HA, MYF(0), hash_tables->alias); error= TRUE; } From 17252c325827beeb9a74acc6c56d2c7515f36ef0 Mon Sep 17 00:00:00 2001 From: Jon Olav Hauglid Date: Wed, 9 Dec 2009 15:25:48 +0100 Subject: [PATCH 8/9] Backport of revno: 2617.68.13 Introduce a counter for protection against global read lock on thread level. The functions for protection against global read lock sometimes need a local variable to signal when the protection is set, and hence need to be released. It would be better to control this behaviour via a counter on the THD struct, telling how many times the protection has been claimed by the current thread. A side-effect of the fix is that if protection is claimed twice for a thread, only a simple increment is required for the second claim, instead of a mutex-protected increment of the global variable protect_against_global_read_lock. --- sql/lock.cc | 93 ++++++++++++++++++++++++++++++++++++++++++++++ sql/sql_class.cc | 1 + sql/sql_class.h | 1 + sql/sql_parse.cc | 37 +++++++----------- sql/sql_table.cc | 9 ++--- sql/sql_trigger.cc | 6 +-- 6 files changed, 115 insertions(+), 32 deletions(-) diff --git a/sql/lock.cc b/sql/lock.cc index 8d314c4ad19..420868a25c4 100644 --- a/sql/lock.cc +++ b/sql/lock.cc @@ -1098,6 +1098,19 @@ static volatile uint waiting_for_read_lock=0; #define GOT_GLOBAL_READ_LOCK 1 #define MADE_GLOBAL_READ_LOCK_BLOCK_COMMIT 2 +/** + Take global read lock, wait if there is protection against lock. + + If the global read lock is already taken by this thread, then nothing is done. + + See also "Handling of global read locks" above. + + @param thd Reference to thread. + + @retval False Success, global read lock set, commits are NOT blocked. + @retval True Failure, thread was killed. +*/ + bool lock_global_read_lock(THD *thd) { DBUG_ENTER("lock_global_read_lock"); @@ -1164,6 +1177,16 @@ bool lock_global_read_lock(THD *thd) } +/** + Unlock global read lock. + + Commits may or may not be blocked when this function is called. + + See also "Handling of global read locks" above. + + @param thd Reference to thread. +*/ + void unlock_global_read_lock(THD *thd) { uint tmp; @@ -1190,6 +1213,25 @@ void unlock_global_read_lock(THD *thd) DBUG_VOID_RETURN; } +/** + Wait if the global read lock is set, and optionally seek protection against + global read lock. + + See also "Handling of global read locks" above. + + @param thd Reference to thread. + @param abort_on_refresh If True, abort waiting if a refresh occurs, + do NOT seek protection against GRL. + If False, wait until the GRL is released and seek + protection against GRL. + @param is_not_commit If False, called from a commit operation, + wait only if commit blocking is also enabled. + + @retval False Success, protection against global read lock is set + (if !abort_on_refresh) + @retval True Failure, wait was aborted or thread was killed. +*/ + #define must_wait (global_read_lock && \ (is_not_commit || \ global_read_lock_blocks_commit)) @@ -1201,6 +1243,16 @@ bool wait_if_global_read_lock(THD *thd, bool abort_on_refresh, bool result= 0, need_exit_cond; DBUG_ENTER("wait_if_global_read_lock"); + /* + If we already have protection against global read lock, + just increment the counter. + */ + if (unlikely(thd->global_read_lock_protection > 0)) + { + if (!abort_on_refresh) + thd->global_read_lock_protection++; + DBUG_RETURN(FALSE); + } /* Assert that we do not own LOCK_open. If we would own it, other threads could not close their tables. This would make a pretty @@ -1237,7 +1289,12 @@ bool wait_if_global_read_lock(THD *thd, bool abort_on_refresh, result=1; } if (!abort_on_refresh && !result) + { + thd->global_read_lock_protection++; protect_against_global_read_lock++; + DBUG_PRINT("sql_lock", ("protect_against_global_read_lock incr: %u", + protect_against_global_read_lock)); + } /* The following is only true in case of a global read locks (which is rare) and if old_message is set @@ -1250,10 +1307,31 @@ bool wait_if_global_read_lock(THD *thd, bool abort_on_refresh, } +/** + Release protection against global read lock and restart + global read lock waiters. + + Should only be called if we have protection against global read lock. + + See also "Handling of global read locks" above. + + @param thd Reference to thread. +*/ + void start_waiting_global_read_lock(THD *thd) { bool tmp; DBUG_ENTER("start_waiting_global_read_lock"); + /* + Ignore request if we do not have protection against global read lock. + (Note that this is a violation of the interface contract, hence the assert). + */ + DBUG_ASSERT(thd->global_read_lock_protection > 0); + if (unlikely(thd->global_read_lock_protection == 0)) + DBUG_VOID_RETURN; + /* Decrement local read lock protection counter, return if we still have it */ + if (unlikely(--thd->global_read_lock_protection > 0)) + DBUG_VOID_RETURN; if (unlikely(thd->global_read_lock)) DBUG_VOID_RETURN; (void) pthread_mutex_lock(&LOCK_global_read_lock); @@ -1267,6 +1345,21 @@ void start_waiting_global_read_lock(THD *thd) } +/** + Make global read lock also block commits. + + The scenario is: + - This thread has the global read lock. + - Global read lock blocking of commits is not set. + + See also "Handling of global read locks" above. + + @param thd Reference to thread. + + @retval False Success, global read lock set, commits are blocked. + @retval True Failure, thread was killed. +*/ + bool make_global_read_lock_block_commit(THD *thd) { bool error; diff --git a/sql/sql_class.cc b/sql/sql_class.cc index d83b60810ab..d42a06b3811 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -452,6 +452,7 @@ THD::THD() examined_row_count(0), warning_info(&main_warning_info), stmt_da(&main_da), + global_read_lock_protection(0), global_read_lock(0), is_fatal_error(0), transaction_rollback_request(0), diff --git a/sql/sql_class.h b/sql/sql_class.h index f4504bfed0d..889d7c5472b 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -1874,6 +1874,7 @@ public: ulong rand_saved_seed1, rand_saved_seed2; pthread_t real_id; /* For debugging */ my_thread_id thread_id; + uint global_read_lock_protection;// GRL protection count uint tmp_table, global_read_lock; uint server_status,open_options; enum enum_thread_type system_thread; diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index 06f29cd6039..211788768f9 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -1762,7 +1762,6 @@ int mysql_execute_command(THD *thd) { int res= FALSE; - bool need_start_waiting= FALSE; // have protection against global read lock int up_result= 0; LEX *lex= thd->lex; /* first SELECT_LEX (have special meaning for many of non-SELECTcommands) */ @@ -2039,7 +2038,7 @@ mysql_execute_command(THD *thd) break; if (!thd->locked_tables_mode && lex->protect_against_global_read_lock && - !(need_start_waiting= !wait_if_global_read_lock(thd, 0, 1))) + wait_if_global_read_lock(thd, 0, 1)) break; res= execute_sqlcom_select(thd, all_tables); @@ -2309,10 +2308,9 @@ case SQLCOM_PREPARE: read lock when it succeeds. This needs to be released by start_waiting_global_read_lock(). We protect the normal CREATE TABLE in the same way. That way we avoid that a new table is - created during a gobal read lock. + created during a global read lock. */ - if (!thd->locked_tables_mode && - !(need_start_waiting= !wait_if_global_read_lock(thd, 0, 1))) + if (!thd->locked_tables_mode && wait_if_global_read_lock(thd, 0, 1)) { res= 1; goto end_with_restore_list; @@ -2617,8 +2615,7 @@ end_with_restore_list: "INDEX DIRECTORY"); create_info.data_file_name= create_info.index_file_name= NULL; - if (!thd->locked_tables_mode && - !(need_start_waiting= !wait_if_global_read_lock(thd, 0, 1))) + if (!thd->locked_tables_mode && wait_if_global_read_lock(thd, 0, 1)) { res= 1; break; @@ -2852,8 +2849,7 @@ end_with_restore_list: DBUG_ASSERT(first_table == all_tables && first_table != 0); if (update_precheck(thd, all_tables)) break; - if (!thd->locked_tables_mode && - !(need_start_waiting= !wait_if_global_read_lock(thd, 0, 1))) + if (!thd->locked_tables_mode && wait_if_global_read_lock(thd, 0, 1)) goto error; DBUG_ASSERT(select_lex->offset_limit == 0); unit->set_limit(select_lex); @@ -2891,7 +2887,7 @@ end_with_restore_list: */ if (!thd->locked_tables_mode && lex->sql_command == SQLCOM_UPDATE_MULTI && - !(need_start_waiting= !wait_if_global_read_lock(thd, 0, 1))) + wait_if_global_read_lock(thd, 0, 1)) goto error; res= mysql_multi_update_prepare(thd); @@ -2993,8 +2989,7 @@ end_with_restore_list: if ((res= insert_precheck(thd, all_tables))) break; - if (!thd->locked_tables_mode && - !(need_start_waiting= !wait_if_global_read_lock(thd, 0, 1))) + if (!thd->locked_tables_mode && wait_if_global_read_lock(thd, 0, 1)) { res= 1; break; @@ -3033,8 +3028,7 @@ end_with_restore_list: unit->set_limit(select_lex); - if (! thd->locked_tables_mode && - ! (need_start_waiting= ! wait_if_global_read_lock(thd, 0, 1))) + if (!thd->locked_tables_mode && wait_if_global_read_lock(thd, 0, 1)) { res= 1; break; @@ -3104,7 +3098,7 @@ end_with_restore_list: ER(ER_LOCK_OR_ACTIVE_TRANSACTION), MYF(0)); goto error; } - if (!(need_start_waiting= !wait_if_global_read_lock(thd, 0, 1))) + if (wait_if_global_read_lock(thd, 0, 1)) goto error; res= mysql_truncate(thd, first_table, 0); break; @@ -3116,8 +3110,7 @@ end_with_restore_list: DBUG_ASSERT(select_lex->offset_limit == 0); unit->set_limit(select_lex); - if (!thd->locked_tables_mode && - !(need_start_waiting= !wait_if_global_read_lock(thd, 0, 1))) + if (!thd->locked_tables_mode && wait_if_global_read_lock(thd, 0, 1)) { res= 1; break; @@ -3137,8 +3130,7 @@ end_with_restore_list: (TABLE_LIST *)thd->lex->auxiliary_table_list.first; multi_delete *del_result; - if (!thd->locked_tables_mode && - !(need_start_waiting= !wait_if_global_read_lock(thd, 0, 1))) + if (!thd->locked_tables_mode && wait_if_global_read_lock(thd, 0, 1)) { res= 1; break; @@ -3282,8 +3274,7 @@ end_with_restore_list: if (check_one_table_access(thd, privilege, all_tables)) goto error; - if (!thd->locked_tables_mode && - !(need_start_waiting= !wait_if_global_read_lock(thd, 0, 1))) + if (!thd->locked_tables_mode && wait_if_global_read_lock(thd, 0, 1)) goto error; res= mysql_load(thd, lex->exchange, first_table, lex->field_list, @@ -3357,7 +3348,7 @@ end_with_restore_list: FALSE, UINT_MAX, FALSE)) goto error; if (lex->protect_against_global_read_lock && - !(need_start_waiting= !wait_if_global_read_lock(thd, 0, 1))) + wait_if_global_read_lock(thd, 0, 1)) goto error; init_mdl_requests(all_tables); @@ -4575,7 +4566,7 @@ error: res= TRUE; finish: - if (need_start_waiting) + if (thd->global_read_lock_protection > 0) { /* Release the protection against the global read lock and wake diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 341d2d16757..5500bc286b8 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -1780,16 +1780,16 @@ void write_bin_log(THD *thd, bool clear_error, bool mysql_rm_table(THD *thd,TABLE_LIST *tables, my_bool if_exists, my_bool drop_temporary) { - bool error= FALSE, need_start_waiting= FALSE; + bool error; Drop_table_error_handler err_handler(thd->get_internal_handler()); + DBUG_ENTER("mysql_rm_table"); /* mark for close and remove all cached entries */ if (!drop_temporary) { - if (!thd->locked_tables_mode && - !(need_start_waiting= !wait_if_global_read_lock(thd, 0, 1))) + if (!thd->locked_tables_mode && wait_if_global_read_lock(thd, 0, 1)) DBUG_RETURN(TRUE); } @@ -1797,8 +1797,7 @@ bool mysql_rm_table(THD *thd,TABLE_LIST *tables, my_bool if_exists, error= mysql_rm_table_part2(thd, tables, if_exists, drop_temporary, 0, 0); thd->pop_internal_handler(); - - if (need_start_waiting) + if (thd->global_read_lock_protection > 0) start_waiting_global_read_lock(thd); if (error) diff --git a/sql/sql_trigger.cc b/sql/sql_trigger.cc index 015e0d4daa1..b3108cae3d9 100644 --- a/sql/sql_trigger.cc +++ b/sql/sql_trigger.cc @@ -328,7 +328,6 @@ bool mysql_create_or_drop_trigger(THD *thd, TABLE_LIST *tables, bool create) TABLE *table; bool result= TRUE; String stmt_query; - bool need_start_waiting= FALSE; bool lock_upgrade_done= FALSE; MDL_ticket *mdl_ticket= NULL; @@ -386,8 +385,7 @@ bool mysql_create_or_drop_trigger(THD *thd, TABLE_LIST *tables, bool create) LOCK_open is not enough because global read lock is held without holding LOCK_open). */ - if (!thd->locked_tables_mode && - !(need_start_waiting= !wait_if_global_read_lock(thd, 0, 1))) + if (!thd->locked_tables_mode && wait_if_global_read_lock(thd, 0, 1)) DBUG_RETURN(TRUE); if (!create) @@ -521,7 +519,7 @@ end: if (thd->locked_tables_mode && tables && lock_upgrade_done) mdl_ticket->downgrade_exclusive_lock(); - if (need_start_waiting) + if (thd->global_read_lock_protection > 0) start_waiting_global_read_lock(thd); if (!result) From 1b5f2b9030e0dc19eaac22de7db429b6a7ba31ae Mon Sep 17 00:00:00 2001 From: Jon Olav Hauglid Date: Wed, 9 Dec 2009 16:13:00 +0100 Subject: [PATCH 9/9] Backport of revno: 2617.68.18 Bug #42147 Concurrent DML and LOCK TABLE ... READ for InnoDB table cause warnings in errlog Concurrent execution of LOCK TABLES ... READ statement and DML statements affecting the same InnoDB table on debug builds of MySQL server might lead to "Found lock of type 6 that is write and read locked" warnings appearing in error log. The problem is that the table-level locking code allows a thread to acquire TL_READ_NO_INSERT lock on a table even if there is another thread which holds TL_WRITE_ALLOW_WRITE lock on the same table. At the same time, the locking code assumes that that such locks are incompatible (for example, see check_locks()). This doesn't lead to any problems other than warnings in error log for debug builds of server since for InnoDB tables TL_READ_NO_INSERT type of lock is only used for LOCK TABLES and for this statement InnoDB also performs its own table-level locking. Unfortunately, the table lock compatibility matrix cannot be updated to disallow TL_READ_NO_INSERT when another thread holds TL_WRITE_ALLOW_WRITE without causing starvation of LOCK TABLE READ in InnoDB under high write load. This patch therefore contains no code changes. The issue will be fixed later when LOCK TABLE READ has been updated to not use table locks. This bug will therefore be marked as "To be fixed later". Code comment in thr_lock.c expanded to clarify the issue and a test case based on the bug description added to innodb_mysql_lock.test. Note that a global suppression rule has been added to both MTR v1 and v2 for the "Found lock of type 6 that is write and read locked" warning. These suppression rules must be removed once this bug is properly fixed. --- mysql-test/include/mtr_warnings.sql | 8 ++++ mysql-test/lib/v1/mtr_report.pl | 3 ++ mysql-test/r/innodb_mysql_lock.result | 32 ++++++++++++++ mysql-test/t/innodb_mysql_lock.test | 63 +++++++++++++++++++++++++++ mysys/thr_lock.c | 32 +++++++++++--- 5 files changed, 131 insertions(+), 7 deletions(-) diff --git a/mysql-test/include/mtr_warnings.sql b/mysql-test/include/mtr_warnings.sql index e03e83efac2..14f1dd97830 100644 --- a/mysql-test/include/mtr_warnings.sql +++ b/mysql-test/include/mtr_warnings.sql @@ -186,6 +186,14 @@ INSERT INTO global_suppressions VALUES (": The MySQL server is running with the --secure-backup-file-priv option so it cannot execute this statement"), ("Slave: Unknown table 't1' Error_code: 1051"), + /* + BUG#42147 - Concurrent DML and LOCK TABLE ... READ for InnoDB + table cause warnings in errlog + Note: This is a temporary suppression until Bug#42147 can be + fixed properly. See bug page for more information. + */ + ("Found lock of type 6 that is write and read locked"), + ("THE_LAST_SUPPRESSION")|| diff --git a/mysql-test/lib/v1/mtr_report.pl b/mysql-test/lib/v1/mtr_report.pl index 3c78c3ca064..36aba983c34 100644 --- a/mysql-test/lib/v1/mtr_report.pl +++ b/mysql-test/lib/v1/mtr_report.pl @@ -376,6 +376,9 @@ sub mtr_report_stats ($) { /Slave: Can't DROP 'c7'.* 1091/ or /Slave: Key column 'c6'.* 1072/ or + # Warnings generated until bug#42147 is properly resolved + /Found lock of type 6 that is write and read locked/ or + # rpl_idempotency.test produces warnings for the slave. ($testname eq 'rpl.rpl_idempotency' and (/Slave: Can\'t find record in \'t1\' Error_code: 1032/ or diff --git a/mysql-test/r/innodb_mysql_lock.result b/mysql-test/r/innodb_mysql_lock.result index 147267d5550..374f67358eb 100644 --- a/mysql-test/r/innodb_mysql_lock.result +++ b/mysql-test/r/innodb_mysql_lock.result @@ -21,4 +21,36 @@ INSERT INTO t1 VALUES (2); ERROR 40001: Deadlock found when trying to get lock; try restarting transaction # Cleanup commit; +set @@autocommit=1; commit; +set @@autocommit=1; +set @@autocommit=1; +# +# Bug #42147 Concurrent DML and LOCK TABLE ... READ for InnoDB +# table cause warnings in errlog +# +# +# Note that this test for now relies on a global suppression of +# the warning "Found lock of type 6 that is write and read locked" +# This suppression rule can be removed once Bug#42147 is properly +# fixed. See bug page for more info. +# +DROP TABLE IF EXISTS t1; +CREATE TABLE t1 (i INT) engine= innodb; +# Connection 2 +# Get user-level lock +SELECT get_lock('bug42147_lock', 60); +get_lock('bug42147_lock', 60) +1 +# Connection 1 +INSERT INTO t1 SELECT get_lock('bug42147_lock', 60); +# Connection 2 +LOCK TABLES t1 READ; +SELECT release_lock('bug42147_lock'); +release_lock('bug42147_lock') +1 +# Connection 1 +# Connection 2 +UNLOCK TABLES; +# Connection 1 +DROP TABLE t1; diff --git a/mysql-test/t/innodb_mysql_lock.test b/mysql-test/t/innodb_mysql_lock.test index daee94bedb5..c8c38cd1ab1 100644 --- a/mysql-test/t/innodb_mysql_lock.test +++ b/mysql-test/t/innodb_mysql_lock.test @@ -1,5 +1,8 @@ -- source include/have_innodb.inc +# Save the initial number of concurrent sessions. +--source include/count_sessions.inc + --echo # --echo # Bug #22876 Four-way deadlock --echo # @@ -51,8 +54,68 @@ INSERT INTO t1 VALUES (2); connection con2; --reap commit; +set @@autocommit=1; connection con1; commit; +set @@autocommit=1; connection con3; --reap +set @@autocommit=1; connection default; + +disconnect con1; +disconnect con3; + +--echo # +--echo # Bug #42147 Concurrent DML and LOCK TABLE ... READ for InnoDB +--echo # table cause warnings in errlog +--echo # + +--echo # +--echo # Note that this test for now relies on a global suppression of +--echo # the warning "Found lock of type 6 that is write and read locked" +--echo # This suppression rule can be removed once Bug#42147 is properly +--echo # fixed. See bug page for more info. +--echo # + +--disable_warnings +DROP TABLE IF EXISTS t1; +--enable_warnings + +CREATE TABLE t1 (i INT) engine= innodb; + +--echo # Connection 2 +--echo # Get user-level lock +connection con2; +SELECT get_lock('bug42147_lock', 60); + +--echo # Connection 1 +connection default; +--send INSERT INTO t1 SELECT get_lock('bug42147_lock', 60) + +--echo # Connection 2 +connection con2; +let $wait_condition= + SELECT COUNT(*) > 0 FROM information_schema.processlist + WHERE state = 'User lock' + AND info = 'INSERT INTO t1 SELECT get_lock(\'bug42147_lock\', 60)'; +--source include/wait_condition.inc +LOCK TABLES t1 READ; +SELECT release_lock('bug42147_lock'); + +--echo # Connection 1 +connection default; +--reap + +--echo # Connection 2 +connection con2; +UNLOCK TABLES; + +--echo # Connection 1 +connection default; +disconnect con2; +DROP TABLE t1; + +# Check that all connections opened by test cases in this file are really +# gone so execution of other tests won't be affected by their presence. +--source include/wait_until_count_sessions.inc diff --git a/mysys/thr_lock.c b/mysys/thr_lock.c index 6433c04a96f..a3f3e9ee080 100644 --- a/mysys/thr_lock.c +++ b/mysys/thr_lock.c @@ -540,13 +540,31 @@ thr_lock(THR_LOCK_DATA *data, THR_LOCK_OWNER *owner, /* Request for READ lock */ if (lock->write.data) { - /* We can allow a read lock even if there is already a write lock - on the table in one the following cases: - - This thread alread have a write lock on the table - - The write lock is TL_WRITE_ALLOW_READ or TL_WRITE_DELAYED - and the read lock is TL_READ_HIGH_PRIORITY or TL_READ - - The write lock is TL_WRITE_CONCURRENT_INSERT or TL_WRITE_ALLOW_WRITE - and the read lock is not TL_READ_NO_INSERT + /* + We can allow a read lock even if there is already a + write lock on the table if they are owned by the same + thread or if they satisfy the following lock + compatibility matrix: + + Request + /------- + H|++++ WRITE_ALLOW_WRITE + e|+++- WRITE_ALLOW_READ + l|+++- WRITE_CONCURRENT_INSERT + d|++++ WRITE_DELAYED + |||| + |||\= READ_NO_INSERT + ||\ = READ_HIGH_PRIORITY + |\ = READ_WITH_SHARED_LOCKS + \ = READ + + + = Request can be satisified. + - = Request cannot be satisified. + + READ_NO_INSERT and WRITE_ALLOW_WRITE should in principle + be incompatible. However this will cause starvation of + LOCK TABLE READ in InnoDB under high write load. + See Bug#42147 for more information. */ DBUG_PRINT("lock",("write locked 1 by thread: 0x%lx",