MySQL Lists are EOL. Please join:

List:Commits« Previous MessageNext Message »
From:Alfranio Correia Date:November 13 2009 4:16pm
Subject:Re: bzr commit into mysql-5.1-rep+2 branch (luis.soares:3139) Bug#46166
View as plain text  
Hi Luis,

Great work. However, there are still work to be done.

Cheers.


STATUS
------

Not approved.

REQUIRED CHANGES
----------------

  R1. Please, avoid the duplicate code presented below.
      See suggestion in R2.

    if((thd && (thd->options & OPTION_BIN_LOG)) && invalid_flags)
    {
      my_error(ER_BINLOG_LOGGING_IMPOSSIBLE, MYF(0), "Binlog is in invalid "
               "state. Check your binary log files.");
      VOID(pthread_mutex_unlock(&LOCK_log));
      DBUG_RETURN(1);
    }


  R2. Try to merge the code above with the MYSQL_BIN_LOG::check_write_error


  R3. In fact, it would be better to also re-use/extend the flag write_error,
  instead of creating an additional flag. So, please try to eliminate the
  invalid_flags.


  R4. Feel free to change the name of methods, functions and variables in order
  to reflect the actual use.


  R5. Please, encapsulate the access to the attributes. In this case, the access
  to the invalid_flags or whatever you decide to use.

  R6. When a non-transactional statement fails while rotating or writing to the binary
  log, we need to try to write an incident event.


REQUESTS
--------
  R1. Although I understand the changes in the handler.c, I was wondering if it would
  be possible to avoid them and check the error in a higher level since you are setting
the
  diagnostic are. I am asking this because it is quite strange to have such check in
  this file. Something similar to the changes done in sql/sql_table.cc. What do you
  think? Is this possible?


SUGGESTIONS
-----------
  S1. Why don't you use a macro to check what follows?

  +    error= (thd->is_error() &&
  +            thd->main_da.sql_errno() == ER_NO_UNIQUE_LOGFILE);



DETAILS
-------
  D1. See some comments in-line.


Cheers.


Luis Soares wrote:
> #At file:///home/lsoares/Workspace/bzr/work/bugfixing/46166/mysql-5.1-rep%2B2-commit/
> based on revid:zhenxing.he@stripped
> 
>  3139 Luis Soares	2009-11-12
>       BUG#46166: MYSQL_BIN_LOG::new_file_impl is not propagating error 
>       when generating new name.
>       
>       If find_uniq_filename returns an error, then this error is not
>       being propagated upwards, and execution does not report error 
>       to the user (although a entry in the error log is generated).
>       
>       This patch addresses this by propagating the error up in the execution
>       stack. Additionally, when rotation fails, mysqld stops accepting new
>       commands by raising BINLOGGING IMPOSSIBLE error. Then the super user
>       is only allowed to disable binary log and proceed to do some recovery
>       or forensics operations. In detail:
>       
>         - Ongoing statements
>            1. first statement causing rotation failure sets an invalid flag
>               and raises error (NO UNIQ FILENAME).
>       
>               For transactional statements, at commit time a commit is
>               performed (ie, change data) and events are written to the
>               binlog. Then error is reported back regarding failure to
>               rotate.
>       
>               If statement is non-transactional, then it changes the
>               database and writes event to the binlog. 
>       
>            2. Subsequent stmts notice that invalid flag was set but only
>               when writing to the binlog. At that time, they fail and return
>               error (BINLOGGING IMPOSSIBLE) without writing to the binlog.
>                  
>         - New statements (after rotation failed)
>       
>             1. Fail immediately on mysql_execute_command with BINLOGGING
>                IMPOSSIBLE error, thence statements are not even executed.
>                 
>                One exception to the rule: if user is super then we allow one
>                command (SET ...) so that he can turn off binlogging and
>                proceed with forensics analysis/repair.
>       
>       This patch is even more important after BUG 40611 is completely backported.
>       It also requires BUG 37148 patch.
>      @ mysql-test/suite/binlog/r/binlog_uniq_filename.result
>         Result file.
>      @ mysql-test/suite/binlog/t/binlog_uniq_filename-master.opt
>         Limit binlog size to 4096.
>      @ mysql-test/suite/binlog/t/binlog_uniq_filename.test
>         Test case.
>      @ sql/handler.cc
>         Added error propagation when rotate_and_purge fail inside unlog
>         method.
>      @ sql/log.cc
>         Added invalid_flags (re)setting.
>         Added check point for evaluating invalid_flags when trying to write
>         to the binlog. 
>         Added error propagation for rotate_and_purge.
>      @ sql/log.h
>         Added invalig_flags and changed signature of rotate_and_purge so
>         that it returns an error.
>      @ sql/rpl_injector.cc
>         Added propagation of rotate errors.
>      @ sql/slave.cc
>         Added propagation of rotate errors.
>      @ sql/slave.h
>         Changed signature of rotate_relay_log so that it returns error.
>      @ sql/sql_load.cc
>         Added error propagation for write_execute_load_query_log_event (should
>         be redundant after BUG 37148).
>      @ sql/sql_parse.cc
>         Added blocking point in mysql_execute_command.
>         Added error propagation for rotate_and_purge and rotate_relay_log.
>      @ sql/sql_table.cc
>         Added error propagation when writing to the binlog (should be redundant
>         after BUG 37148).
> 
>     added:
>       mysql-test/suite/binlog/r/binlog_uniq_filename.result
>       mysql-test/suite/binlog/t/binlog_uniq_filename-master.opt
>       mysql-test/suite/binlog/t/binlog_uniq_filename.test
>     modified:
>       sql/handler.cc
>       sql/log.cc
>       sql/log.h
>       sql/rpl_injector.cc
>       sql/slave.cc
>       sql/slave.h
>       sql/sql_load.cc
>       sql/sql_parse.cc
>       sql/sql_table.cc
> === added file 'mysql-test/suite/binlog/r/binlog_uniq_filename.result'
> --- a/mysql-test/suite/binlog/r/binlog_uniq_filename.result	1970-01-01 00:00:00
> +0000
> +++ b/mysql-test/suite/binlog/r/binlog_uniq_filename.result	2009-11-12 17:32:40
> +0000
> @@ -0,0 +1,171 @@
> +call mtr.add_suppression("Can't generate a unique log-filename");
> +call mtr.add_suppression("Binlog is in invalid state.*");
> +call mtr.add_suppression("Writing one row to the row-based binary log failed.*");
> +call mtr.add_suppression("Error writing file .*");
> +SELECT repeat('x',8192) INTO OUTFILE 'MYSQLTEST_VARDIR/tmp/bug_46166.data';
> +RESET MASTER;
> +######################################################################
> +### assertion: no problem flushing logs (should show two binlogs)
> +FLUSH LOGS;
> +show binary logs;
> +Log_name	File_size
> +master-bin.000001	#
> +master-bin.000002	#
> +######################################################################
> +### assertion: check that FLUSH LOGS actually fails and reports failure
> +### back to the user if find_uniq_filename fails (should show
> +### just one binlog)
> +RESET MASTER;
> +SET GLOBAL debug="+d,error_unique_log_filename";
> +FLUSH LOGS;
> +ERROR HY000: Can't generate a unique log-filename master-bin.(1-999)
> +
> +SET SQL_LOG_BIN=0;
> +show binary logs;
> +Log_name	File_size
> +master-bin.000001	#
> +SET SQL_LOG_BIN=1;
> +######################################################################
> +### assertion: since the problem got resolved (find_uniq_filename does
> +### not return failure anymore), we can flush logs again and 
> +### rotation will be done accordingly (invalid flag is removed from 
> +### binlog).
> +SET GLOBAL debug="";
> +SET SQL_LOG_BIN=0;
> +FLUSH LOGS;
> +SET SQL_LOG_BIN=1;
> +RESET MASTER;
> +CREATE TABLE t1 (a int);
> +INSERT INTO t1 VALUES (1);
> +CREATE TABLE t2 (a TEXT) Engine=InnoDB;
> +CREATE TABLE t4 (a TEXT);
> +######################################################################
> +### assertion: check that even if one is using a transactional table
> +### if rotation fails we get the error but, transaction is not rolled 
> +### back, ie, rotation happens only after the commit is done and the 
> +### event was already written to the binlog 
> +###    (check handler.cc:ha_commit_trans()).
> +SET GLOBAL debug="+d,error_unique_log_filename";
> +SELECT count(*) FROM t2;
> +count(*)
> +0
> +SET AUTOCOMMIT=0;
> +INSERT INTO t2 VALUES ('casa');
> +LOAD DATA INFILE 'MYSQLTEST_VARDIR/tmp/bug_46166.data' INTO TABLE t2;
> +INSERT INTO t2 VALUES ('casa');
> +COMMIT;
> +ERROR HY000: Can't generate a unique log-filename master-bin.(1-999)
> +
> +SET AUTOCOMMIT= 1;
> +SET SQL_LOG_BIN=0;
> +SELECT count(*) FROM t2;
> +count(*)
> +3
> +### simulate that we have fixed the problem and reset logs
> +SET GLOBAL debug="";
> +FLUSH LOGS;
> +SET SQL_LOG_BIN=1;
> +RESET MASTER;
> +######################################################################
> +### assertion: check that on a non-transactional table, if rotation
> +### fails then table contains inserted data.
> +SET GLOBAL debug="+d,error_unique_log_filename";
> +SELECT count(*) FROM t4;
> +count(*)
> +0
> +LOAD DATA INFILE 'MYSQLTEST_VARDIR/tmp/bug_46166.data' INTO TABLE t4;
> +ERROR HY000: Can't generate a unique log-filename master-bin.(1-999)
> +
> +SET SQL_LOG_BIN=0;
> +SELECT count(*) FROM t4;
> +count(*)
> +1
> +### simulate that we have fixed the problem and reset logs
> +SET GLOBAL debug="";
> +FLUSH LOGS;
> +SET SQL_LOG_BIN=1;
> +RESET MASTER;
> +######################################################################
> +### assertion: we force binlog to rotate (should show two binlogs)
> +LOAD DATA INFILE 'MYSQLTEST_VARDIR/tmp/bug_46166.data' INTO TABLE t2;
> +show binary logs;
> +Log_name	File_size
> +master-bin.000001	#
> +master-bin.000002	#
> +######################################################################
> +### we create a user without super previlidges
> +create user 'test'@'localhost%';
> +GRANT INSERT, CREATE, DROP ON test.* TO 'test'@'localhost%';
> +######################################################################
> +### assertion: assert that mysql enters "protected" mode - All
> +### statements are refused except the "SET ... " command for super
> +### user. This provides the DBA a mean to inspect current db state, do
> +### some forensics and/or repairing.
> +SELECT count(*) FROM t4;
> +count(*)
> +1
> +INSERT INTO t2 VALUES ('aaa'), ('bbb'), ('ccc'), ('ddd');
> +SET GLOBAL debug="+d,error_unique_log_filename";
> +LOAD DATA INFILE 'MYSQLTEST_VARDIR/tmp/bug_46166.data' INTO TABLE t4;
> +ERROR HY000: Can't generate a unique log-filename master-bin.(1-999)
> +
> +######################################################################
> +### assertion: check that statements end up in error
> +DELETE FROM t4;
> +ERROR HY000: Error when executing command DELETE FROM t4: Binlog is in invalid
> state! Refusing commands until binary logging is fixed or it is disabled.
> +DELETE FROM t2;
> +ERROR HY000: Error when executing command DELETE FROM t2: Binlog is in invalid
> state! Refusing commands until binary logging is fixed or it is disabled.
> +SELECT count(*) FROM t2;
> +ERROR HY000: Error when executing command SELECT count(*) FROM t2: Binlog is in
> invalid state! Refusing commands until binary logging is fixed or it is disabled.
> +######################################################################
> +### assertion: check that if we disable binlogging, then statements
> +### succeed.
> +SET SQL_LOG_BIN=0;
> +SELECT count(*) FROM t2;
> +count(*)
> +8
> +DELETE FROM t2;
> +SELECT count(*) FROM t2;
> +count(*)
> +0
> +SELECT count(*) FROM t4;
> +count(*)
> +2
> +DELETE FROM t4;
> +SELECT count(*) FROM t4;
> +count(*)
> +0
> +SET SQL_LOG_BIN=1;
> +### connection for a regular user (not super)
> +######################################################################
> +### assertion: assert that mysql is in "protected" mode (commands 
> +### must fail because binlog is in invalid state).
> +CREATE TABLE t3 (a TEXT) engine=InnoDB;
> +ERROR HY000: Error when executing command CREATE TABLE t3 (a TEXT) engine=InnoDB:
> Binlog is in invalid state! Refusing commands until binary logging is fixed or it is
> disabled.
> +LOAD DATA INFILE 'MYSQLTEST_VARDIR/tmp/bug_46166.data' INTO TABLE t3;
> +ERROR HY000: Error when executing command LOAD DATA INFILE
> 'MYSQLTEST_VARDIR/tmp/bug_46166.data' INTO TABLE t3: Binlog is in invalid state! Refusing
> commands until binary logging is fixed or it is disabled.
> +DROP TABLE t3;
> +ERROR HY000: Error when executing command DROP TABLE t3: Binlog is in invalid state!
> Refusing commands until binary logging is fixed or it is disabled.
> +### returning to super user connection
> +### simulate that we have fixed the problem.
> +SET GLOBAL debug="";
> +SET SQL_LOG_BIN=0;
> +### flush the logs so that invalid flag is updated
> +FLUSH LOGS;
> +SET SQL_LOG_BIN=1;
> +RESET MASTER;
> +### returning to regular user connection
> +######################################################################
> +### assertion: assert that mysql comes out of "protected" mode even
> +### for regular user when problem is fixed. (statements should execute
> +### successfully).
> +CREATE TABLE t3 (a TEXT);
> +DROP TABLE t1, t2, t3, t4;
> +### returning to super user connection
> +######################################################################
> +### assertion: assert that regular user statements were binlogged.
> +show binlog events from <binlog_start>;
> +Log_name	Pos	Event_type	Server_id	End_log_pos	Info
> +master-bin.000001	#	Query	#	#	use `test`; CREATE TABLE t3 (a TEXT)
> +master-bin.000001	#	Query	#	#	use `test`; DROP TABLE t1, t2, t3, t4
> +DROP USER 'test'@'localhost%';
> 
> === added file 'mysql-test/suite/binlog/t/binlog_uniq_filename-master.opt'
> --- a/mysql-test/suite/binlog/t/binlog_uniq_filename-master.opt	1970-01-01 00:00:00
> +0000
> +++ b/mysql-test/suite/binlog/t/binlog_uniq_filename-master.opt	2009-11-12 17:32:40
> +0000
> @@ -0,0 +1 @@
> +--max_binlog_size=4096
> 
> === added file 'mysql-test/suite/binlog/t/binlog_uniq_filename.test'
> --- a/mysql-test/suite/binlog/t/binlog_uniq_filename.test	1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/binlog/t/binlog_uniq_filename.test	2009-11-12 17:32:40 +0000
> @@ -0,0 +1,203 @@
> +# BUG#46166: MYSQL_BIN_LOG::new_file_impl is not propagating error
> +#            when generating new name.
> +#  
> +# WHY
> +# ===
> +#
> +# We want to check whether error is reported or not when
> +# find_uniq_filename fails (this may happen when rotation is not
> +# possible because there is some problem finding an unique filename).
> +#
> +# Additionally, we want to check that mysql goes into some kind of
> +# safe/protected mode in which only super user can execute statements
> +# (useful for forensics and recovery).
> +#
> +# HOW
> +# ===
> +# 
> +# Test cases are documented inline.
> +
> +call mtr.add_suppression("Can't generate a unique log-filename");
> +call mtr.add_suppression("Binlog is in invalid state.*");
> +call mtr.add_suppression("Writing one row to the row-based binary log failed.*");
> +call mtr.add_suppression("Error writing file .*");
> +
> +-- source include/have_log_bin.inc
> +-- source include/have_innodb.inc
> +
> +-- let $load_file= $MYSQLTEST_VARDIR/tmp/bug_46166.data
> +-- let $MYSQLD_DATADIR= `select @@datadir`
> +-- replace_result $MYSQLTEST_VARDIR MYSQLTEST_VARDIR
> +-- eval SELECT repeat('x',8192) INTO OUTFILE '$load_file'
> +
> +RESET MASTER;
> +
> +-- echo ######################################################################
> +-- echo ### assertion: no problem flushing logs (should show two binlogs)
> +FLUSH LOGS;
> +-- source include/show_binary_logs.inc
> +
> +-- echo ######################################################################
> +-- echo ### assertion: check that FLUSH LOGS actually fails and reports failure
> +-- echo ### back to the user if find_uniq_filename fails (should show
> +-- echo ### just one binlog)
> +RESET MASTER;
> +SET GLOBAL debug="+d,error_unique_log_filename";
> +-- error ER_NO_UNIQUE_LOGFILE
> +FLUSH LOGS;



> +SET SQL_LOG_BIN=0;
> +-- source include/show_binary_logs.inc
> +SET SQL_LOG_BIN=1;


Why SQL_LOG_BIN=0?


> +
> +-- echo ######################################################################
> +-- echo ### assertion: since the problem got resolved (find_uniq_filename does
> +-- echo ### not return failure anymore), we can flush logs again and 
> +-- echo ### rotation will be done accordingly (invalid flag is removed from 
> +-- echo ### binlog).
> +
> +# simulate that the problem is fixed
> +SET GLOBAL debug="";
> +SET SQL_LOG_BIN=0; # we need to disable binlog to run flush logs command.
> +FLUSH LOGS; 
> +SET SQL_LOG_BIN=1;

> +RESET MASTER;

Why do you need to reset the master?
I am ok with it but I don't see why you need it.

> +
> +# issue some statements
> +CREATE TABLE t1 (a int);
> +INSERT INTO t1 VALUES (1);
> +CREATE TABLE t2 (a TEXT) Engine=InnoDB;
> +CREATE TABLE t4 (a TEXT);
> +
> +-- echo ######################################################################
> +-- echo ### assertion: check that even if one is using a transactional table
> +-- echo ### if rotation fails we get the error but, transaction is not rolled 
> +-- echo ### back, ie, rotation happens only after the commit is done and the 
> +-- echo ### event was already written to the binlog 
> +-- echo ###    (check handler.cc:ha_commit_trans()).
> +SET GLOBAL debug="+d,error_unique_log_filename";
> +SELECT count(*) FROM t2;
> +SET AUTOCOMMIT=0;
> +INSERT INTO t2 VALUES ('casa');
> +-- replace_result $MYSQLTEST_VARDIR MYSQLTEST_VARDIR
> +-- eval LOAD DATA INFILE '$load_file' INTO TABLE t2
> +INSERT INTO t2 VALUES ('casa');
> +-- error ER_NO_UNIQUE_LOGFILE
> +COMMIT;
> +SET AUTOCOMMIT= 1;

> +SET SQL_LOG_BIN=0;
> +SELECT count(*) FROM t2;
> +-- echo ### simulate that we have fixed the problem and reset logs
> +SET GLOBAL debug="";
> +FLUSH LOGS;
> +SET SQL_LOG_BIN=1;
> +RESET MASTER;


Please, keep the same pattern to clean up the state.
Put the SET outside the SQL_LOG_BIN.


> +
> +
> +-- echo ######################################################################
> +-- echo ### assertion: check that on a non-transactional table, if rotation
> +-- echo ### fails then table contains inserted data.
> +SET GLOBAL debug="+d,error_unique_log_filename";
> +SELECT count(*) FROM t4;
> +-- replace_result $MYSQLTEST_VARDIR MYSQLTEST_VARDIR
> +-- error ER_NO_UNIQUE_LOGFILE
> +-- eval LOAD DATA INFILE '$load_file' INTO TABLE t4
> +SET SQL_LOG_BIN=0;
> +SELECT count(*) FROM t4;
> +-- echo ### simulate that we have fixed the problem and reset logs
> +SET GLOBAL debug="";
> +FLUSH LOGS;
> +SET SQL_LOG_BIN=1;
> +RESET MASTER;


> +
> +-- echo ######################################################################
> +-- echo ### assertion: we force binlog to rotate (should show two binlogs)

Where do you force the binlog to rotate?

> +-- replace_result $MYSQLTEST_VARDIR MYSQLTEST_VARDIR
> +-- eval LOAD DATA INFILE '$load_file' INTO TABLE t2
> +-- source include/show_binary_logs.inc


> +
> +-- echo ######################################################################
> +-- echo ### we create a user without super previlidges
> +create user 'test'@'localhost%';
> +GRANT INSERT, CREATE, DROP ON test.* TO 'test'@'localhost%';
> +
> +-- echo ######################################################################
> +-- echo ### assertion: assert that mysql enters "protected" mode - All
> +-- echo ### statements are refused except the "SET ... " command for super
> +-- echo ### user. This provides the DBA a mean to inspect current db state, do
> +-- echo ### some forensics and/or repairing.
> +SELECT count(*) FROM t4;
> +INSERT INTO t2 VALUES ('aaa'), ('bbb'), ('ccc'), ('ddd');
> +SET GLOBAL debug="+d,error_unique_log_filename";
> +-- replace_result $MYSQLTEST_VARDIR MYSQLTEST_VARDIR
> +-- error ER_NO_UNIQUE_LOGFILE
> +-- eval LOAD DATA INFILE '$load_file' INTO TABLE t4
> +
> +-- echo ######################################################################
> +-- echo ### assertion: check that statements end up in error
> +-- error ER_ERROR_WHEN_EXECUTING_COMMAND
> +DELETE FROM t4;
> +-- error ER_ERROR_WHEN_EXECUTING_COMMAND
> +DELETE FROM t2;
> +-- error ER_ERROR_WHEN_EXECUTING_COMMAND
> +SELECT count(*) FROM t2;


Please, try to call SET SQL_LOG_BIN=0 with this regular user.

> +
> +-- echo ######################################################################
> +-- echo ### assertion: check that if we disable binlogging, then statements
> +-- echo ### succeed.
> +SET SQL_LOG_BIN=0;
> +SELECT count(*) FROM t2;
> +DELETE FROM t2;
> +SELECT count(*) FROM t2;
> +SELECT count(*) FROM t4;
> +DELETE FROM t4;
> +SELECT count(*) FROM t4;
> +SET SQL_LOG_BIN=1;
> +
> +connect (conn2,localhost,test,,test,$MASTER_MYPORT,$MASTER_MYSOCK);
> +
> +-- echo ### connection for a regular user (not super)
> +-- connection conn2
> +
> +-- echo ######################################################################
> +-- echo ### assertion: assert that mysql is in "protected" mode (commands 
> +-- echo ### must fail because binlog is in invalid state).
> +-- error ER_ERROR_WHEN_EXECUTING_COMMAND
> +CREATE TABLE t3 (a TEXT) engine=InnoDB;
> +-- replace_result $MYSQLTEST_VARDIR MYSQLTEST_VARDIR
> +-- error ER_ERROR_WHEN_EXECUTING_COMMAND
> +-- eval LOAD DATA INFILE '$load_file' INTO TABLE t3
> +-- error ER_ERROR_WHEN_EXECUTING_COMMAND
> +DROP TABLE t3; 
> +
> +-- echo ### returning to super user connection
> +-- connection default
> +
> +-- echo ### simulate that we have fixed the problem.
> +SET GLOBAL debug="";
> +SET SQL_LOG_BIN=0;
> +-- echo ### flush the logs so that invalid flag is updated
> +FLUSH LOGS;
> +SET SQL_LOG_BIN=1;
> +RESET MASTER;
> +
> +-- echo ### returning to regular user connection
> +-- connection conn2
> +
> +-- echo ######################################################################
> +-- echo ### assertion: assert that mysql comes out of "protected" mode even
> +-- echo ### for regular user when problem is fixed. (statements should execute
> +-- echo ### successfully).
> +CREATE TABLE t3 (a TEXT);
> +DROP TABLE t1, t2, t3, t4; 
> +
> +-- echo ### returning to super user connection
> +-- connection default
> +-- echo ######################################################################
> +-- echo ### assertion: assert that regular user statements were binlogged.
> +-- source include/show_binlog_events.inc
> +
> +DROP USER 'test'@'localhost%';
> +-- disconnect conn2
> +
> +-- exit
> +
> 
> === modified file 'sql/handler.cc'
> --- a/sql/handler.cc	2009-10-03 10:50:25 +0000
> +++ b/sql/handler.cc	2009-11-12 17:32:40 +0000
> @@ -1207,7 +1207,17 @@ int ha_commit_trans(THD *thd, bool all)
>      error=ha_commit_one_phase(thd, all) ? (cookie ? 2 : 1) : 0;
>      DBUG_EXECUTE_IF("crash_commit_before_unlog", abort(););
>      if (cookie)
> +    {
>        tc_log->unlog(cookie, xid);
> +      /** 
> +        check if unlog call resulted in error in DA, if it
> +        has an error - rotate_and_puge could not create a
> +        new log file - then propagate error upwards. 
> +       */
> +      if (!error && thd->is_error() && 
> +          thd->main_da.sql_errno() == ER_NO_UNIQUE_LOGFILE)
> +        error= 2;
> +    }
>      DBUG_EXECUTE_IF("crash_commit_after", abort(););
>      RUN_HOOK(transaction, after_commit, (thd, FALSE));
>  end:
> 
> === modified file 'sql/log.cc'
> --- a/sql/log.cc	2009-10-07 21:13:07 +0000
> +++ b/sql/log.cc	2009-11-12 17:32:40 +0000
> @@ -1853,6 +1853,8 @@ static int find_uniq_filename(char *name
>    char			*start, *end;
>    DBUG_ENTER("find_uniq_filename");
>  
> +  DBUG_EXECUTE_IF("error_unique_log_filename", DBUG_RETURN(1););
> +
>    length= dirname_part(buff, name, &buf_length);
>    start=  name + length;
>    end=    strend(start);
> @@ -2077,6 +2079,8 @@ int MYSQL_LOG::generate_new_name(char *n
>      {
>        if (find_uniq_filename(new_name))
>        {
> +        my_printf_error(ER_NO_UNIQUE_LOGFILE, ER(ER_NO_UNIQUE_LOGFILE),
> +                        MYF(0), log_name); 
>  	sql_print_error(ER(ER_NO_UNIQUE_LOGFILE), log_name);
>  	return 1;
>        }
> @@ -3574,7 +3578,12 @@ void MYSQL_BIN_LOG::new_file_impl(bool n
>      new file name in the current binary log file.
>    */
>    if (generate_new_name(new_name, name))
> +  {
> +    invalid_flags |= BINLOG_CANT_ROTATE_INVALID_F;
>      goto end;
> +  }
> +  else
> +    invalid_flags &= ~BINLOG_CANT_ROTATE_INVALID_F;
>    new_name_ptr=new_name;
>  
>    if (log_type == LOG_BIN)
> @@ -3650,6 +3659,9 @@ bool MYSQL_BIN_LOG::append(Log_event* ev
>    if ((uint) my_b_append_tell(&log_file) > max_size)
>      new_file_without_locking();
>  
> +  error= current_thd->is_error() && 
> +         current_thd->main_da.sql_errno() == ER_NO_UNIQUE_LOGFILE;
> +
>  err:
>    pthread_mutex_unlock(&LOCK_log);
>    signal_update();				// Safe as we don't call close
> @@ -3682,6 +3694,8 @@ bool MYSQL_BIN_LOG::appendv(const char* 
>    if ((uint) my_b_append_tell(&log_file) > max_size)
>      new_file_without_locking();
>  
> +  error= current_thd->is_error() && 
> +         current_thd->main_da.sql_errno() == ER_NO_UNIQUE_LOGFILE;
>  err:
>    if (!error)
>      signal_update();
> @@ -3997,7 +4011,7 @@ MYSQL_BIN_LOG::flush_and_set_pending_row
>        if (!error)
>        {
>          signal_update();
> -        rotate_and_purge(RP_LOCK_LOG_IS_ALREADY_LOCKED);
> +        error= rotate_and_purge(RP_LOCK_LOG_IS_ALREADY_LOCKED);
>        }
>      }
>  
> @@ -4069,7 +4083,14 @@ bool MYSQL_BIN_LOG::write(Log_event *eve
>        DBUG_RETURN(0);
>      }
>  #endif /* HAVE_REPLICATION */
> -
> +    if((thd && (thd->options & OPTION_BIN_LOG)) &&
> invalid_flags)
> +    {
> +      error= invalid_flags;
> +      my_error(ER_BINLOG_LOGGING_IMPOSSIBLE, MYF(0), "Binlog is in invalid "
> +               "state. Check your binary log files.");
> +      VOID(pthread_mutex_unlock(&LOCK_log));
> +      DBUG_RETURN(error);
> +    }
>  #if defined(USING_TRANSACTIONS) 
>      /*
>        Should we write to the binlog cache or to the binlog on disk?
> @@ -4190,7 +4211,9 @@ bool MYSQL_BIN_LOG::write(Log_event *eve
>        }
>  
>        signal_update();
> -      rotate_and_purge(RP_LOCK_LOG_IS_ALREADY_LOCKED);
> +      if ((error= rotate_and_purge(RP_LOCK_LOG_IS_ALREADY_LOCKED)))
> +        goto err;
> +      
>      }
>      error=0;
>  
> @@ -4276,15 +4299,22 @@ bool general_log_write(THD *thd, enum en
>    return FALSE;
>  }
>  
> -void MYSQL_BIN_LOG::rotate_and_purge(uint flags)
> +int MYSQL_BIN_LOG::rotate_and_purge(uint flags)
>  {
> +  int error= 0;
> +  DBUG_ENTER("MYSQL_BIN_LOG::rotate_and_purge");
>    if (!(flags & RP_LOCK_LOG_IS_ALREADY_LOCKED))
>      pthread_mutex_lock(&LOCK_log);
>    if ((flags & RP_FORCE_ROTATE) ||
>        (my_b_tell(&log_file) >= (my_off_t) max_size))
>    {
>      new_file_without_locking();
> +    THD *thd= current_thd;
> +    error= (thd->is_error() &&
> +            thd->main_da.sql_errno() == ER_NO_UNIQUE_LOGFILE);
>  #ifdef HAVE_REPLICATION
> +    if(error)
> +      goto end;
>      if (expire_logs_days)
>      {
>        time_t purge_time= my_time(0) - expire_logs_days*24*60*60;
> @@ -4293,8 +4323,10 @@ void MYSQL_BIN_LOG::rotate_and_purge(uin
>      }
>  #endif
>    }
> +end:
>    if (!(flags & RP_LOCK_LOG_IS_ALREADY_LOCKED))
>      pthread_mutex_unlock(&LOCK_log);
> +  DBUG_RETURN(error);
>  }
>  
>  uint MYSQL_BIN_LOG::next_file_id()
> @@ -4493,7 +4525,7 @@ bool MYSQL_BIN_LOG::write_incident(THD *
>      if (!error && !(error= flush_and_sync(0)))
>      {
>        signal_update();
> -      rotate_and_purge(RP_LOCK_LOG_IS_ALREADY_LOCKED);
> +      error= rotate_and_purge(RP_LOCK_LOG_IS_ALREADY_LOCKED);
>      }
>      pthread_mutex_unlock(&LOCK_log);
>    }
> @@ -4536,6 +4568,14 @@ bool MYSQL_BIN_LOG::write(THD *thd, IO_C
>    DBUG_ASSERT(is_open());
>    if (likely(is_open()))                       // Should always be true
>    {
> +    if((thd && (thd->options & OPTION_BIN_LOG)) &&
> invalid_flags)
> +    {
> +      my_error(ER_BINLOG_LOGGING_IMPOSSIBLE, MYF(0), "Binlog is in invalid "
> +               "state. Check your binary log files.");
> +      VOID(pthread_mutex_unlock(&LOCK_log));
> +      DBUG_RETURN(1);
> +    }
> +
>      /*
>        We only bother to write to the binary log if there is anything
>        to write.
> @@ -4615,7 +4655,8 @@ bool MYSQL_BIN_LOG::write(THD *thd, IO_C
>        pthread_mutex_unlock(&LOCK_prep_xids);
>      }
>      else
> -      rotate_and_purge(RP_LOCK_LOG_IS_ALREADY_LOCKED);
> +      if (rotate_and_purge(RP_LOCK_LOG_IS_ALREADY_LOCKED))
> +        goto err;
>    }
>    VOID(pthread_mutex_unlock(&LOCK_log));
>  
> 
> === modified file 'sql/log.h'
> --- a/sql/log.h	2009-10-07 21:13:07 +0000
> +++ b/sql/log.h	2009-11-12 17:32:40 +0000
> @@ -223,6 +223,8 @@ private:
>    time_t last_time;
>  };
>  
> +#define BINLOG_CANT_ROTATE_INVALID_F 1 
> +
>  class MYSQL_BIN_LOG: public TC_LOG, private MYSQL_LOG
>  {
>   private:
> @@ -291,6 +293,7 @@ class MYSQL_BIN_LOG: public TC_LOG, priv
>    void new_file_impl(bool need_lock);
>  
>  public:
> +  bool invalid_flags;
>    MYSQL_LOG::generate_name;
>    MYSQL_LOG::is_open;
>  
> @@ -390,7 +393,7 @@ public:
>    void make_log_name(char* buf, const char* log_ident);
>    bool is_active(const char* log_file_name);
>    int update_log_index(LOG_INFO* linfo, bool need_update_threads);
> -  void rotate_and_purge(uint flags);
> +  int rotate_and_purge(uint flags);
>    /**
>       Flush binlog cache and synchronize to disk.
>  
> 
> === modified file 'sql/rpl_injector.cc'
> --- a/sql/rpl_injector.cc	2008-02-19 11:43:01 +0000
> +++ b/sql/rpl_injector.cc	2009-11-12 17:32:40 +0000
> @@ -222,8 +222,7 @@ int injector::record_incident(THD *thd, 
>    Incident_log_event ev(thd, incident);
>    if (int error= mysql_bin_log.write(&ev))
>      return error;
> -  mysql_bin_log.rotate_and_purge(RP_FORCE_ROTATE);
> -  return 0;
> +  return mysql_bin_log.rotate_and_purge(RP_FORCE_ROTATE);
>  }
>  
>  int injector::record_incident(THD *thd, Incident incident, LEX_STRING const
> message)
> @@ -231,6 +230,5 @@ int injector::record_incident(THD *thd, 
>    Incident_log_event ev(thd, incident, message);
>    if (int error= mysql_bin_log.write(&ev))
>      return error;
> -  mysql_bin_log.rotate_and_purge(RP_FORCE_ROTATE);
> -  return 0;
> +  return mysql_bin_log.rotate_and_purge(RP_FORCE_ROTATE);
>  }
> 
> === modified file 'sql/slave.cc'
> --- a/sql/slave.cc	2009-10-20 06:30:15 +0000
> +++ b/sql/slave.cc	2009-11-12 17:32:40 +0000
> @@ -3659,6 +3659,7 @@ static int process_io_rotate(Master_info
>  {
>    DBUG_ENTER("process_io_rotate");
>    safe_mutex_assert_owner(&mi->data_lock);
> +  int error= 0;
>  
>    if (unlikely(!rev->is_valid()))
>      DBUG_RETURN(1);
> @@ -3695,8 +3696,8 @@ static int process_io_rotate(Master_info
>      Rotate the relay log makes binlog format detection easier (at next slave
>      start or mysqlbinlog)
>    */
> -  rotate_relay_log(mi); /* will take the right mutexes */
> -  DBUG_RETURN(0);
> +  error= rotate_relay_log(mi); /* will take the right mutexes */
> +  DBUG_RETURN(error);
>  }
>  
>  /*
> @@ -4872,10 +4873,11 @@ err:
>    is void).
>  */
>  
> -void rotate_relay_log(Master_info* mi)
> +int rotate_relay_log(Master_info* mi)
>  {
>    DBUG_ENTER("rotate_relay_log");
>    Relay_log_info* rli= &mi->rli;
> +  int error= 0;
>  
>    DBUG_EXECUTE_IF("crash_before_rotate_relaylog", abort(););
>  
> @@ -4894,6 +4896,9 @@ void rotate_relay_log(Master_info* mi)
>  
>    /* If the relay log is closed, new_file() will do nothing. */
>    rli->relay_log.new_file();
> +  if (error= (current_thd->is_error() && 
> +              current_thd->main_da.sql_errno() == ER_NO_UNIQUE_LOGFILE))
> +    goto end;
>  
>    /*
>      We harvest now, because otherwise BIN_LOG_HEADER_SIZE will not immediately
> @@ -4911,7 +4916,7 @@ void rotate_relay_log(Master_info* mi)
>    rli->relay_log.harvest_bytes_written(&rli->log_space_total);
>  end:
>    pthread_mutex_unlock(&mi->run_lock);
> -  DBUG_VOID_RETURN;
> +  DBUG_RETURN(error);
>  }
>  
>  
> 
> === modified file 'sql/slave.h'
> --- a/sql/slave.h	2009-10-01 17:22:44 +0000
> +++ b/sql/slave.h	2009-11-12 17:32:40 +0000
> @@ -200,7 +200,7 @@ int purge_relay_logs(Relay_log_info* rli
>  		     const char** errmsg);
>  void set_slave_thread_options(THD* thd);
>  void set_slave_thread_default_charset(THD *thd, Relay_log_info const *rli);
> -void rotate_relay_log(Master_info* mi);
> +int rotate_relay_log(Master_info* mi);
>  int apply_event_and_update_pos(Log_event* ev, THD* thd, Relay_log_info* rli,
>                                 bool skip);
>  
> 
> === modified file 'sql/sql_load.cc'
> --- a/sql/sql_load.cc	2009-07-31 17:14:52 +0000
> +++ b/sql/sql_load.cc	2009-11-12 17:32:40 +0000
> @@ -542,8 +542,9 @@ int mysql_load(THD *thd,sql_exchange *ex
>        if (lf_info.wrote_create_file)
>        {
>          int errcode= query_error_code(thd, killed_status == THD::NOT_KILLED);
> -        write_execute_load_query_log_event(thd, handle_duplicates, ignore,
> -                                           transactional_table, errcode);
> +        if (write_execute_load_query_log_event(thd, handle_duplicates, ignore,
> +                                               transactional_table, errcode))
> +          goto err;
>        }
>      }
>    }
> 
> === modified file 'sql/sql_parse.cc'
> --- a/sql/sql_parse.cc	2009-10-03 10:50:25 +0000
> +++ b/sql/sql_parse.cc	2009-11-12 17:32:40 +0000
> @@ -2009,6 +2009,36 @@ mysql_execute_command(THD *thd)
>    if ((all_tables || !lex->is_single_level_stmt()) && !thd->spcont)
>      mysql_reset_errors(thd, 0);
>  
> +  /*
> +    Before actually executing any command we need to check whether the
> +    binlog is in use or not, and more importantly, if it is in a valid
> +    state.
> +
> +    If the user is SUPER then we allow the command "SET ... " so that 
> +    DBA is able to turn off binlog for his session. Then he can issue 
> +    regular statements and proceed with forensics and/or repair 
> +    operations.
> +  */
> +  if ((mysql_bin_log.is_open() && (thd->options & OPTION_BIN_LOG)))
> +  {
> +     pthread_mutex_lock(mysql_bin_log.get_log_lock());
> +     int invalid_flags= mysql_bin_log.invalid_flags;
> +     my_bool user_is_super= (((ulong)(thd->security_ctx->master_access &
> +                                        SUPER_ACL) == (ulong)SUPER_ACL));
> +     if (invalid_flags && 
> +         (!user_is_super || ((lex->sql_command != SQLCOM_SET_OPTION))))
> +     {
> +       my_error(ER_ERROR_WHEN_EXECUTING_COMMAND, MYF(0), thd->query, "Binlog "
> +                "is in invalid state! Refusing commands until binary logging is "
> +                "fixed or it is disabled.");
> +       sql_print_error("Binlog is in invalid state (FLAGS: 0x%x). Check your "
> +                       "binary log files.", invalid_flags);
> +       pthread_mutex_unlock(mysql_bin_log.get_log_lock());
> +       DBUG_RETURN(-1);
> +     }
> +     pthread_mutex_unlock(mysql_bin_log.get_log_lock());
> +  }
> +
>  #ifdef HAVE_REPLICATION
>    if (unlikely(thd->slave_thread))
>    {
> @@ -3118,7 +3148,11 @@ end_with_restore_list:
>        {
>          Incident_log_event ev(thd, incident);
>          mysql_bin_log.write(&ev);
> -        mysql_bin_log.rotate_and_purge(RP_FORCE_ROTATE);
> +        if (mysql_bin_log.rotate_and_purge(RP_FORCE_ROTATE))
> +        {
> +          res= 1;
> +          break;
> +        }
>        }
>        DBUG_PRINT("debug", ("Just after generate_incident()"));
>      }
> @@ -6805,12 +6839,16 @@ bool reload_acl_and_cache(THD *thd, ulon
>      tmp_write_to_binlog= 0;
>      if( mysql_bin_log.is_open() )
>      {
> -      mysql_bin_log.rotate_and_purge(RP_FORCE_ROTATE);
> +      if (mysql_bin_log.rotate_and_purge(RP_FORCE_ROTATE))
> +        return 1;
>      }
>  #ifdef HAVE_REPLICATION
> +    int rotate_error= 0;
>      pthread_mutex_lock(&LOCK_active_mi);
> -    rotate_relay_log(active_mi);
> +    rotate_error= rotate_relay_log(active_mi);
>      pthread_mutex_unlock(&LOCK_active_mi);
> +    if (rotate_error)
> +      return 1;
>  #endif
>  
>      /* flush slow and general logs */
> 
> === modified file 'sql/sql_table.cc'
> --- a/sql/sql_table.cc	2009-09-02 10:22:47 +0000
> +++ b/sql/sql_table.cc	2009-11-12 17:32:40 +0000
> @@ -2139,6 +2139,8 @@ int mysql_rm_table_part2(THD *thd, TABLE
>          In both these cases, nothing should be written to the binary
>          log.
>        */
> +
> +      error= error || thd->is_error();
>      }
>    }
>    pthread_mutex_lock(&LOCK_open);
> @@ -3941,7 +3943,7 @@ bool mysql_create_table_no_lock(THD *thd
>    }
>  
>    write_create_table_bin_log(thd, create_info, internal_tmp_table);
> -  error= FALSE;
> +  error= thd->is_error();
>  unlock_and_end:
>    VOID(pthread_mutex_unlock(&LOCK_open));
>  
> 
> 
> ------------------------------------------------------------------------
> 
> This body part will be downloaded on demand.
Thread
bzr commit into mysql-5.1-rep+2 branch (luis.soares:3139) Bug#46166Luis Soares12 Nov
  • Re: bzr commit into mysql-5.1-rep+2 branch (luis.soares:3139) Bug#46166Alfranio Correia13 Nov
  • Re: bzr commit into mysql-5.1-rep+2 branch (luis.soares:3139) Bug#46166He Zhenxing21 Nov