From 9438b9851901c48f8e280c2665f9469c2f998c2f Mon Sep 17 00:00:00 2001 From: unknown Date: Fri, 13 Oct 2006 17:26:46 +0400 Subject: Fix for Bug #17544 "Cannot do atomic log rotate", Bug #21785 "Server crashes after rename of the log table" and Bug #21966 "Strange warnings on create like/repair of the log tables" According to the patch, from now on, one should use RENAME to perform a log table rotation (this should also be reflected in the manual). Here is a sample: use mysql; CREATE TABLE IF NOT EXISTS general_log2 LIKE general_log; RENAME TABLE general_log TO general_log_backup, general_log2 TO general_log; The rules for Rename of the log tables are following: IF 1. Log tables are enabled AND 2. Rename operates on the log table and nothing is being renamed to the log table. DO 3. Throw an error message. ELSE 4. Perform rename. The very RENAME query will go the the old (backup) table. This is consistent with the behavoiur we have with binlog ROTATE LOGS statement. Other problems, which are solved by the patch are: 1) Now REPAIR of the log table is exclusive operation (as it should be), this also eliminates lock-related warnings. and 2) CREATE LIKE TABLE now usese usual read lock on the source table rather then name lock, which is too restrictive. This way we get rid of another log table-related warning, which occured because of the above fact (as a side-effect, name lock resulted in a warning). mysql-test/r/log_tables.result: update result file mysql-test/t/log_tables.test: Add tests for the bugs sql/handler.cc: update comment sql/handler.h: update function to reflect changes in log tables locking logic. sql/lock.cc: Now we allow locking of the log tables for "privileged" threads Privileged thread must explicitly close and lock log tables. This is required for admin operations such as REPAIR. sql/log.cc: Changes to the file: 1) Add checks for table schema. It's more important now, as we allow rename of the log tables. Since we should check for schema when writing to a log table. E.g. if one created a table with one-only comlumn and renamed it to general_log, the server should cope with it. 2) refactor LOGGER::flush(), so that we can now use the same machinery as we use in FLUSH LOGS in other statements: whenever we have to perform a serious operation on the log tables, we have to (a) lock logger, which blocks other concurrent statements (such as selects) (b) close logs. Then perform an exclusive operation, c) reenable logs and d) unlock logger. 3) Add a function to check if a given table is a log table. 4) Add support for "privileged" thread 5) merge is_[general/slow]_log_table_enabled() into one function. 6) Add new function: reopen _log_tables, which reopens the tables, which were enabled (after temporary close, required for admin operation) sql/log.h: 1) add a new call close_n_lock_tables(). Now we use it instead of LOGGER::flush() in FLUSH LOGS implementation. 2) add a prototype for the function to check if a given table is a log table; 3) add privileged table flag to table logger 4) merge is_[general/slow]_log_table_enabled() into one function. sql/mysql_priv.h: move log table defines to log.h sql/sql_delete.cc: use new function check_if_log_table() instead of direct strcmp sql/sql_rename.cc: Traverse the list of tables in mysql_rename_tables to make sure that log tables are processed correctly (that is, according to the rules specified in the main CS comment) sql/sql_table.cc: 1) mysql_admin_table() should disable logs if it performs exclusive admin operation on a log table. This way we also eliminate warning on REPAIR of the log table. 2) mysql_create_like_table should read-lock the source table instead getting name lock on it. Name lock is too restrictive in this case. sql/share/errmsg.txt: Add a new error message for rename of the log tables sql/table.cc: use new function instead of direct strcmp. change my_strcasecmp() -> strcmp(), when comparing system db and table names storage/csv/ha_tina.cc: update function to reflect changes in log tables locking logic. storage/myisam/ha_myisam.cc: update function to reflect changes in log tables locking logic. --- sql/log.cc | 234 ++++++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 153 insertions(+), 81 deletions(-) (limited to 'sql/log.cc') diff --git a/sql/log.cc b/sql/log.cc index b93d36cf630..0d9453cb02a 100644 --- a/sql/log.cc +++ b/sql/log.cc @@ -92,6 +92,31 @@ struct binlog_trx_data { handlerton binlog_hton; +/* Check if a given table is opened log table */ +int check_if_log_table(uint db_len, const char *db, uint table_name_len, + const char *table_name, uint check_if_opened) +{ + if (db_len == 5 && + !(lower_case_table_names ? + my_strcasecmp(system_charset_info, db, "mysql") : + strcmp(db, "mysql"))) + { + if (table_name_len == 11 && !(lower_case_table_names ? + my_strcasecmp(system_charset_info, + table_name, "general_log") : + strcmp(table_name, "general_log")) && + (!check_if_opened || logger.is_log_table_enabled(QUERY_LOG_GENERAL))) + return QUERY_LOG_GENERAL; + else + if (table_name_len == 8 && !(lower_case_table_names ? + my_strcasecmp(system_charset_info, table_name, "slow_log") : + strcmp(table_name, "slow_log")) && + (!check_if_opened ||logger.is_log_table_enabled(QUERY_LOG_SLOW))) + return QUERY_LOG_SLOW; + } + return 0; +} + /* Open log table of a given type (general or slow log) @@ -192,6 +217,12 @@ bool Log_to_csv_event_handler::open_log_table(uint log_table_type) my_pthread_setspecific_ptr(THR_MALLOC, 0); } + /* + After a log table was opened, we should clear privileged thread + flag (which allows locking of a log table by a special thread, usually + the one who closed log tables temporarily). + */ + privileged_thread= 0; DBUG_RETURN(error); } @@ -208,6 +239,8 @@ Log_to_csv_event_handler::Log_to_csv_event_handler() /* logger thread always works with mysql database */ slow_log_thd->db= my_strdup("mysql", MYF(0));; slow_log_thd->db_length= 5; + /* no privileged thread exists at the moment */ + privileged_thread= 0; } @@ -260,6 +293,7 @@ bool Log_to_csv_event_handler::reopen_log_table(uint log_table_type) return open_log_table(log_table_type); } + void Log_to_csv_event_handler::cleanup() { if (opt_log) @@ -314,9 +348,6 @@ bool Log_to_csv_event_handler:: filled by the Logger (=> no need to load default ones). */ - /* log table entries are not replicated at the moment */ - tmp_disable_binlog(current_thd); - /* Set current time. Required for CURRENT_TIMESTAMP to work */ general_log_thd->start_time= event_time; @@ -325,21 +356,36 @@ bool Log_to_csv_event_handler:: default value (which is CURRENT_TIMESTAMP). */ - table->field[1]->store(user_host, user_host_len, client_cs); + /* check that all columns exist */ + if (!table->field[1] || !table->field[2] || !table->field[3] || + !table->field[4] || !table->field[5]) + goto err; + + /* do a write */ + if (table->field[1]->store(user_host, user_host_len, client_cs) || + table->field[2]->store((longlong) thread_id, TRUE) || + table->field[3]->store((longlong) server_id, TRUE) || + table->field[4]->store(command_type, command_type_len, client_cs) || + table->field[5]->store(sql_text, sql_text_len, client_cs)) + goto err; + + /* mark tables as not null */ table->field[1]->set_notnull(); - table->field[2]->store((longlong) thread_id, TRUE); table->field[2]->set_notnull(); - table->field[3]->store((longlong) server_id, TRUE); table->field[3]->set_notnull(); - table->field[4]->store(command_type, command_type_len, client_cs); table->field[4]->set_notnull(); - table->field[5]->store(sql_text, sql_text_len, client_cs); table->field[5]->set_notnull(); + + /* log table entries are not replicated at the moment */ + tmp_disable_binlog(current_thd); + table->file->ha_write_row(table->record[0]); reenable_binlog(current_thd); return FALSE; +err: + return TRUE; } @@ -388,9 +434,6 @@ bool Log_to_csv_event_handler:: if (unlikely(!logger.is_log_tables_initialized)) return FALSE; - /* log table entries are not replicated at the moment */ - tmp_disable_binlog(current_thd); - /* Set start time for CURRENT_TIMESTAMP to the start of the query. This will be default value for the field[0] @@ -403,19 +446,30 @@ bool Log_to_csv_event_handler:: default value. */ + if (!table->field[1] || !table->field[2] || !table->field[3] || + !table->field[4] || !table->field[5] || !table->field[6] || + !table->field[7] || !table->field[8] || !table->field[9] || + !table->field[10]) + goto err; + /* store the value */ - table->field[1]->store(user_host, user_host_len, client_cs); + if (table->field[1]->store(user_host, user_host_len, client_cs)) + goto err; if (query_start_arg) { /* fill in query_time field */ - table->field[2]->store(query_time, TRUE); + if (table->field[2]->store(query_time, TRUE)) + goto err; /* lock_time */ - table->field[3]->store(lock_time, TRUE); + if (table->field[3]->store(lock_time, TRUE)) + goto err; /* rows_sent */ - table->field[4]->store((longlong) thd->sent_row_count, TRUE); + if (table->field[4]->store((longlong) thd->sent_row_count, TRUE)) + goto err; /* rows_examined */ - table->field[5]->store((longlong) thd->examined_row_count, TRUE); + if (table->field[5]->store((longlong) thd->examined_row_count, TRUE)) + goto err; } else { @@ -428,14 +482,18 @@ bool Log_to_csv_event_handler:: /* fill database field */ if (thd->db) { - table->field[6]->store(thd->db, thd->db_length, client_cs); + if (table->field[6]->store(thd->db, thd->db_length, client_cs)) + goto err; table->field[6]->set_notnull(); } if (thd->stmt_depends_on_first_successful_insert_id_in_prev_stmt) { - table->field[7]->store((longlong) - thd->first_successful_insert_id_in_prev_stmt_for_binlog, TRUE); + if (table-> + field[7]->store((longlong) + thd->first_successful_insert_id_in_prev_stmt_for_binlog, + TRUE)) + goto err; table->field[7]->set_notnull(); } @@ -447,16 +505,23 @@ bool Log_to_csv_event_handler:: */ if (thd->auto_inc_intervals_in_cur_stmt_for_binlog.nb_elements() > 0) { - table->field[8]->store((longlong) - thd->auto_inc_intervals_in_cur_stmt_for_binlog.minimum(), TRUE); + if (table-> + field[8]->store((longlong) + thd->auto_inc_intervals_in_cur_stmt_for_binlog.minimum(), TRUE)) + goto err; table->field[8]->set_notnull(); } - table->field[9]->store((longlong) server_id, TRUE); + if (table->field[9]->store((longlong) server_id, TRUE)) + goto err; table->field[9]->set_notnull(); /* sql_text */ - table->field[10]->store(sql_text,sql_text_len, client_cs); + if (table->field[10]->store(sql_text,sql_text_len, client_cs)) + goto err; + + /* log table entries are not replicated at the moment */ + tmp_disable_binlog(current_thd); /* write the row */ table->file->ha_write_row(table->record[0]); @@ -464,6 +529,8 @@ bool Log_to_csv_event_handler:: reenable_binlog(current_thd); DBUG_RETURN(0); +err: + DBUG_RETURN(1); } bool Log_to_csv_event_handler:: @@ -652,61 +719,48 @@ bool LOGGER::reopen_log_table(uint log_table_type) return table_log_handler->reopen_log_table(log_table_type); } - -bool LOGGER::flush_logs(THD *thd) +bool LOGGER::reopen_log_tables() { - TABLE_LIST close_slow_log, close_general_log; + /* + we use | and not || here, to ensure that both reopen_log_table + are called, even if the first one fails + */ + if ((opt_slow_log && logger.reopen_log_table(QUERY_LOG_SLOW)) | + (opt_log && logger.reopen_log_table(QUERY_LOG_GENERAL))) + return TRUE; + return FALSE; +} - /* reopen log tables */ - bzero((char*) &close_slow_log, sizeof(TABLE_LIST)); - close_slow_log.alias= close_slow_log.table_name=(char*) "slow_log"; - close_slow_log.table_name_length= 8; - close_slow_log.db= (char*) "mysql"; - close_slow_log.db_length= 5; - bzero((char*) &close_general_log, sizeof(TABLE_LIST)); - close_general_log.alias= close_general_log.table_name=(char*) "general_log"; - close_general_log.table_name_length= 11; - close_general_log.db= (char*) "mysql"; - close_general_log.db_length= 5; +void LOGGER::tmp_close_log_tables(THD *thd) +{ + table_log_handler->tmp_close_log_tables(thd); +} - /* lock tables, in the case they are enabled */ - if (logger.is_log_tables_initialized) - { - /* - This will lock and wait for all but the logger thread to release the - tables. Then we could reopen log tables. Then release the name locks. - - NOTE: in fact, the first parameter used in lock_and_wait_for_table_name() - and table_log_handler->flush() could be any non-NULL THD, as the - underlying code makes certain assumptions about this. - Here we use one of the logger handler THD's. Simply because it - seems appropriate. - */ - if (opt_slow_log) - lock_and_wait_for_table_name(table_log_handler->general_log_thd, - &close_slow_log); - if (opt_log) - lock_and_wait_for_table_name(table_log_handler->general_log_thd, - &close_general_log); - } +bool LOGGER::flush_logs(THD *thd) +{ + int rc= 0; /* - Deny others from logging to general and slow log, - while reopening tables. + Now we lock logger, as nobody should be able to use logging routines while + log tables are closed */ logger.lock(); + if (logger.is_log_tables_initialized) + table_log_handler->tmp_close_log_tables(thd); // the locking happens here /* reopen log files */ file_log_handler->flush(); - /* flush tables, in the case they are enabled */ + /* reopen tables in the case they were enabled */ if (logger.is_log_tables_initialized) - table_log_handler->flush(table_log_handler->general_log_thd, - &close_slow_log, &close_general_log); + { + if (reopen_log_tables()) + rc= TRUE; + } /* end of log flush */ logger.unlock(); - return FALSE; + return rc; } @@ -1014,31 +1068,50 @@ void LOGGER::deactivate_log_handler(THD *thd, uint log_type) } -bool Log_to_csv_event_handler::flush(THD *thd, TABLE_LIST *close_slow_log, - TABLE_LIST *close_general_log) +/* + Close log tables temporarily. The thread which closed + them this way can lock them in any mode it needs. + NOTE: one should call logger.lock() before entering this + function. +*/ +void Log_to_csv_event_handler::tmp_close_log_tables(THD *thd) { + TABLE_LIST close_slow_log, close_general_log; + + /* fill lists, we will need to perform operations on tables */ + bzero((char*) &close_slow_log, sizeof(TABLE_LIST)); + close_slow_log.alias= close_slow_log.table_name=(char*) "slow_log"; + close_slow_log.table_name_length= 8; + close_slow_log.db= (char*) "mysql"; + close_slow_log.db_length= 5; + + bzero((char*) &close_general_log, sizeof(TABLE_LIST)); + close_general_log.alias= close_general_log.table_name=(char*) "general_log"; + close_general_log.table_name_length= 11; + close_general_log.db= (char*) "mysql"; + close_general_log.db_length= 5; + + privileged_thread= thd; + VOID(pthread_mutex_lock(&LOCK_open)); + /* + NOTE: in fact, the first parameter used in query_cache_invalidate3() + could be any non-NULL THD, as the underlying code makes certain + assumptions about this. + Here we use one of the logger handler THD's. Simply because it + seems appropriate. + */ if (opt_log) { close_log_table(QUERY_LOG_GENERAL, TRUE); - query_cache_invalidate3(thd, close_general_log, 0); - unlock_table_name(thd, close_general_log); + query_cache_invalidate3(general_log_thd, &close_general_log, 0); } if (opt_slow_log) { close_log_table(QUERY_LOG_SLOW, TRUE); - query_cache_invalidate3(thd, close_slow_log, 0); - unlock_table_name(thd, close_slow_log); + query_cache_invalidate3(general_log_thd, &close_slow_log, 0); } VOID(pthread_mutex_unlock(&LOCK_open)); - /* - we use | and not || here, to ensure that both reopen_log_table - are called, even if the first one fails - */ - if ((opt_slow_log && reopen_log_table(QUERY_LOG_SLOW)) | - (opt_log && reopen_log_table(QUERY_LOG_GENERAL))) - return 1; - return 0; } /* the parameters are unused for the log tables */ @@ -1106,16 +1179,15 @@ void Log_to_csv_event_handler:: THD *log_thd, *curr= current_thd; TABLE_LIST *table; + if (!logger.is_log_table_enabled(log_table_type)) + return; /* do nothing */ + switch (log_table_type) { case QUERY_LOG_GENERAL: - if (!logger.is_general_log_table_enabled()) - return; /* do nothing */ log_thd= general_log_thd; table= &general_log; break; case QUERY_LOG_SLOW: - if (!logger.is_slow_log_table_enabled()) - return; /* do nothing */ log_thd= slow_log_thd; table= &slow_log; break; -- cgit v1.2.1 From e4bacda5b25d0d4afaa559974d2e6b576073cb4a Mon Sep 17 00:00:00 2001 From: unknown Date: Wed, 25 Oct 2006 12:54:59 +0400 Subject: fix test failures in the runtime tree sql/log.cc: NULL table pointer during initilization sql/sql_table.cc: don't lock the destination table with table lock, but use name lock instead --- sql/log.cc | 2 ++ 1 file changed, 2 insertions(+) (limited to 'sql/log.cc') diff --git a/sql/log.cc b/sql/log.cc index e64696cbe38..8b666faccff 100644 --- a/sql/log.cc +++ b/sql/log.cc @@ -317,11 +317,13 @@ Log_to_csv_event_handler::Log_to_csv_event_handler() /* logger thread always works with mysql database */ general_log_thd->db= my_strdup("mysql", MYF(0)); general_log_thd->db_length= 5; + general_log.table= 0; slow_log_thd= new THD; /* logger thread always works with mysql database */ slow_log_thd->db= my_strdup("mysql", MYF(0));; slow_log_thd->db_length= 5; + slow_log.table= 0; /* no privileged thread exists at the moment */ privileged_thread= 0; } -- cgit v1.2.1