From 4462578a1cde5b772a253a532cad3b9113eaf7f8 Mon Sep 17 00:00:00 2001 From: unknown Date: Fri, 27 Jul 2007 00:31:06 -0600 Subject: WL#3984 (Revise locking of mysql.general_log and mysql.slow_log) Bug#25422 (Hang with log tables) Bug 17876 (Truncating mysql.slow_log in a SP after using cursor locks the thread) Bug 23044 (Warnings on flush of a log table) Bug 29129 (Resetting general_log while the GLOBAL READ LOCK is set causes a deadlock) Prior to this fix, the server would hang when performing concurrent ALTER TABLE or TRUNCATE TABLE statements against the LOG TABLES, which are mysql.general_log and mysql.slow_log. The root cause traces to the following code: in sql_base.cc, open_table() if (table->in_use != thd) { /* wait_for_condition will unlock LOCK_open for us */ wait_for_condition(thd, &LOCK_open, &COND_refresh); } The problem with this code is that the current implementation of the LOGGER creates 'fake' THD objects, like - Log_to_csv_event_handler::general_log_thd - Log_to_csv_event_handler::slow_log_thd which are not associated to a real thread running in the server, so that waiting for these non-existing threads to release table locks cause the dead lock. In general, the design of Log_to_csv_event_handler does not fit into the general architecture of the server, so that the concept of general_log_thd and slow_log_thd has to be abandoned: - this implementation does not work with table locking - it will not work with commands like SHOW PROCESSLIST - having the log tables always opened does not integrate well with DDL operations / FLUSH TABLES / SET GLOBAL READ_ONLY With this patch, the fundamental design of the LOGGER has been changed to: - always open and close a log table when writing a log - remove totally the usage of fake THD objects - clarify how locking of log tables is implemented in general. See WL#3984 for details related to the new locking design. Additional changes (misc bugs exposed and fixed): 1) mysqldump which would ignore some tables in dump_all_tables_in_db(), but forget to ignore the same in dump_all_views_in_db(). 2) mysqldump would also issue an empty "LOCK TABLE" command when all the tables to lock are to be ignored (numrows == 0), instead of not issuing the query. 3) Internal errors handlers could intercept errors but not warnings (see sql_error.cc). 4) Implementing a nested call to open tables, for the performance schema tables, exposed an existing bug in remove_table_from_cache(), which would perform: in_use->some_tables_deleted=1; against another thread, without any consideration about thread locking. This call inside remove_table_from_cache() was not required anyway, since calling mysql_lock_abort() takes care of aborting -- cleanly -- threads that might hold a lock on a table. This line (in_use->some_tables_deleted=1) has been removed. sql/handler.cc: Moved logic for system / log tables in the SQL layer. sql/handler.h: Moved logic for system / log tables in the SQL layer. sql/lock.cc: Revised locking of log tables sql/log.cc: Major cleanup: changed how log tables are locked / written to. sql/log.h: Major cleanup: changed how log tables are locked / written to. sql/mysql_priv.h: performance schema helpers sql/slave.cc: open_ltable() lock flags sql/sp.cc: open_ltable() lock flags sql/sql_acl.cc: open_ltable() lock flags sql/sql_class.h: performance schema helpers sql/sql_delete.cc: log tables cleanup in TRUNCATE sql/sql_error.cc: Internal handlers can also intercept warnings sql/sql_insert.cc: open_ltable() lock flags sql/sql_parse.cc: performance schema helpers sql/sql_plugin.cc: open_ltable() lock flags sql/sql_rename.cc: log tables cleanup in RENAME sql/sql_servers.cc: open_ltable() lock flags sql/sql_show.cc: Move INFORMATION_SCHEMA_NAME to table.cc sql/sql_table.cc: log tables cleanup (admin operations, ALTER TABLE) sql/sql_udf.cc: open_ltable() lock flags sql/table.cc: Implemented TABLE_CATEGORY. sql/share/errmsg.txt: Changed the wording and name of ER_CANT_READ_LOCK_LOG_TABLE sql/table.h: Implemented TABLE_CATEGORY. storage/csv/ha_tina.cc: Moved logic for system / log tables in the SQL layer. storage/csv/ha_tina.h: Moved logic for system / log tables in the SQL layer. storage/myisam/ha_myisam.cc: Moved logic for system / log tables in the SQL layer. storage/myisam/ha_myisam.h: Moved logic for system / log tables in the SQL layer. client/mysqldump.c: Don't lock tables in the ignore list. Don't issue empty LOCK TABLES queries. sql/sql_base.cc: log tables cleanup performance schema helpers mysql-test/r/ps.result: Adjust test results mysql-test/r/show_check.result: Adjust test results mysql-test/r/status.result: Adjust test results mysql-test/t/log_state.test: Added tests for Bug#29129 mysql-test/t/ps.test: Make the test output deterministic mysql-test/t/show_check.test: Make the test output deterministic mysql-test/r/log_state.result: Changed the default location of the log output to LOG_FILE, for backward compatibility with MySQL 5.0 --- Adjust test results mysql-test/r/log_tables.result: cleanup for -ps-protocol mysql-test/t/log_tables.test: cleanup for -ps-protocol sql/set_var.cc: Changed the default location of the log output to LOG_FILE, for backward compatibility with MySQL 5.0 --- log tables cleanup --- sql/sql_servers.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'sql/sql_servers.cc') diff --git a/sql/sql_servers.cc b/sql/sql_servers.cc index ac5ea6f4ac4..911372d5f4e 100644 --- a/sql/sql_servers.cc +++ b/sql/sql_servers.cc @@ -366,7 +366,7 @@ insert_server(THD *thd, FOREIGN_SERVER *server) tables.alias= tables.table_name= (char*) "servers"; /* need to open before acquiring THR_LOCK_plugin or it will deadlock */ - if (! (table= open_ltable(thd, &tables, TL_WRITE))) + if (! (table= open_ltable(thd, &tables, TL_WRITE, 0))) goto end; /* insert the server into the table */ @@ -588,7 +588,7 @@ int drop_server(THD *thd, LEX_SERVER_OPTIONS *server_options) if ((error= delete_server_record_in_cache(server_options))) goto end; - if (! (table= open_ltable(thd, &tables, TL_WRITE))) + if (! (table= open_ltable(thd, &tables, TL_WRITE, 0))) { error= my_errno; goto end; @@ -705,7 +705,7 @@ int update_server(THD *thd, FOREIGN_SERVER *existing, FOREIGN_SERVER *altered) tables.db= (char*)"mysql"; tables.alias= tables.table_name= (char*)"servers"; - if (!(table= open_ltable(thd, &tables, TL_WRITE))) + if (!(table= open_ltable(thd, &tables, TL_WRITE, 0))) { error= my_errno; goto end; -- cgit v1.2.1