mirror of
https://github.com/MariaDB/server.git
synced 2025-12-24 11:21:21 +03:00
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.
This commit is contained in:
@@ -3726,11 +3726,12 @@ static int dump_all_tables_in_db(char *database)
|
||||
{
|
||||
DYNAMIC_STRING query;
|
||||
init_dynamic_string_checked(&query, "LOCK TABLES ", 256, 1024);
|
||||
for (numrows= 0 ; (table= getTableName(1)) ; numrows++)
|
||||
for (numrows= 0 ; (table= getTableName(1)) ; )
|
||||
{
|
||||
char *end= strmov(afterdot, table);
|
||||
if (include_table(hash_key,end - hash_key))
|
||||
{
|
||||
numrows++;
|
||||
dynstr_append_checked(&query, quote_name(table, table_buff, 1));
|
||||
dynstr_append_checked(&query, " READ /*!32311 LOCAL */,");
|
||||
}
|
||||
@@ -3804,6 +3805,11 @@ static my_bool dump_all_views_in_db(char *database)
|
||||
char *table;
|
||||
uint numrows;
|
||||
char table_buff[NAME_LEN*2+3];
|
||||
char hash_key[2*NAME_LEN+2]; /* "db.tablename" */
|
||||
char *afterdot;
|
||||
|
||||
afterdot= strmov(hash_key, database);
|
||||
*afterdot++= '.';
|
||||
|
||||
if (init_dumping(database, init_dumping_views))
|
||||
return 1;
|
||||
@@ -3813,10 +3819,15 @@ static my_bool dump_all_views_in_db(char *database)
|
||||
{
|
||||
DYNAMIC_STRING query;
|
||||
init_dynamic_string_checked(&query, "LOCK TABLES ", 256, 1024);
|
||||
for (numrows= 0 ; (table= getTableName(1)); numrows++)
|
||||
for (numrows= 0 ; (table= getTableName(1)); )
|
||||
{
|
||||
dynstr_append_checked(&query, quote_name(table, table_buff, 1));
|
||||
dynstr_append_checked(&query, " READ /*!32311 LOCAL */,");
|
||||
char *end= strmov(afterdot, table);
|
||||
if (include_table(hash_key,end - hash_key))
|
||||
{
|
||||
numrows++;
|
||||
dynstr_append_checked(&query, quote_name(table, table_buff, 1));
|
||||
dynstr_append_checked(&query, " READ /*!32311 LOCAL */,");
|
||||
}
|
||||
}
|
||||
if (numrows && mysql_real_query(mysql, query.str, query.length-1))
|
||||
DB_error(mysql, "when using LOCK TABLES");
|
||||
@@ -3830,7 +3841,11 @@ static my_bool dump_all_views_in_db(char *database)
|
||||
/* We shall continue here, if --force was given */
|
||||
}
|
||||
while ((table= getTableName(0)))
|
||||
get_view_structure(table, database);
|
||||
{
|
||||
char *end= strmov(afterdot, table);
|
||||
if (include_table(hash_key, end - hash_key))
|
||||
get_view_structure(table, database);
|
||||
}
|
||||
if (opt_xml)
|
||||
{
|
||||
fputs("</database>\n", md_result_file);
|
||||
|
||||
Reference in New Issue
Block a user