Re: bk commit into 5.0 tree (cbell:1.2499) BUG#26395

[Mats Kindahl]

Hi Sven!

Here are my comments.

cbell@stripped wrote:
> Below is the list of changes that have just been committed into a local
> 5.0 repository of cbell. When cbell does a push these changes will
> be propagated to the main repository and, within 24 hours after the
> push, to the public repository.
> For information on how to access the public repository
> see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html
>
> ChangeSet@stripped, 2007-08-30 15:34:50-04:00, cbell@mysql_cab_desk. +1 -0
>   BUG#26395 : if crash during autocommit update to transactional table on master,
> slave fails
>   
>   This patch ensures that a BEGIN statement is written every time the method
>   MYSQL_LOG::write() there is called with a transactional engine present.
>
>   sql/log.cc@stripped, 2007-08-30 15:34:48-04:00, cbell@mysql_cab_desk. +6 -2
>     BUG#26395 : if crash during autocommit update to transactional table on master,
> slave fails
>     
>     This patch adds a condition to the code to write the "BEGIN" 
>     statement every time the method MYSQL_LOG::write(THD *thd, 
>     IO_CACHE *cache, Log_event *commit_event) is called with a 
>     transactional engine present.
>
> diff -Nrup a/sql/log.cc b/sql/log.cc
> --- a/sql/log.cc	2007-07-30 11:27:30 -04:00
> +++ b/sql/log.cc	2007-08-30 15:34:48 -04:00
> @@ -1842,8 +1842,13 @@ bool MYSQL_LOG::write(THD *thd, IO_CACHE
>      /*
>        Log "BEGIN" at the beginning of the transaction.
>        which may contain more than 1 SQL statement.
> +
> +      We also need to write the "BEGIN" if we are using transactional
> +      engines (nht == 1 whenever binlog_start_trans_and_stmt() is called)
> +      indicated by nht > 1 in the st_thd_trans (THD_TRANS) structure.
>   

binlog_start_trans_and_stmt() does not exist in 5.0, so the comment has 
to be something else.

>      */
> -    if (thd->options & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN))
> +    if ((thd->options & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN)) ||
> +        (thd->transaction.stmt.nht > 1))
>   
Since binlog_start_trans_and_stmt() is a RBR-ism, I suspect that the 
condition should be something else. Also, checking the number of 
statement handlertons registered is bound to fail, since it means you 
write a BEGIN before each statement, regardless of whether you are 
inside a real transaction or not.

Try the following:

# Create two tables with two different transactional storage engines.
# This will cause two handlertons to be registered for statements that
#  use two different engines in the statement.
CREATE TABLE t1 (a INT, b INT) engine=ndb;
CREATE TABLE t2 (a INT, b INT) engine=innodb;
SET AUTOCOMMIT=1;
INSERT INTO t1 VALUES (1,1), (2,2);
BEGIN;
INSERT INTO t2 VALUES (1,2), (2,4);
UPDATE t1, t2 SET t1.b = t2.b WHERE t1.a = t2.a;
INSERT INTO t1 VALUES (1,1), (2,2);
COMMIT;
# See what's in the binary log
SHOW BINLOG EVENTS;

I suspect that you will have extra BEGIN's and COMMIT's of the 
transaction just before or after the UPDATE statement, but am not sure, 
so please check. Check if you get the same result with the new condition 
as before, I suspect that the condition is either unnecessary or wrong.

>      {
>        Query_log_event qinfo(thd, STRING_WITH_LEN("BEGIN"), TRUE, FALSE);
>        /*
> @@ -1871,7 +1876,6 @@ bool MYSQL_LOG::write(THD *thd, IO_CACHE
>        goto err;
>  
>      length= my_b_bytes_in_cache(cache);
> -    DBUG_EXECUTE_IF("half_binlogged_transaction", length-=100;);
>   

The old tests using half_binlogged_transaction still have to work, and 
there is a "crash_before_commit" debug variable in the XA code. Maybe 
you can use that?

Just my few cents,
Mats Kindahl

>  
>      /*
>        The events in the buffer have incorrect end_log_pos data
>
>
>   


-- 
Mats Kindahl
Lead Software Developer
Replication Team
MySQL AB, www.mysql.com