Re: bzr commit into mysql-5.1 branch (aelkin:2656) Bug#33029, Bug#36443

[Mats Kindahl]

Hi Andrei!

I have a few comments, that you can see below.

Just my few cents,
Mats Kindahl

Andrei Elkin wrote:
> #At file:///home/andrei/MySQL/BZR/FIXES/bug36443-slave_crash_insert_trigger/
> 
>  2656 Andrei Elkin	2008-06-17
>       Bug #36443 Server crashes when executing insert when insert trigger on table
>       
>       The crash appeared to be a result of allocating an instance of
> Discrete_interval 
>       automatically whereas it meant to be dynamical.
>       
>       Fixed with correcting allocation; added simulation code that forces those fixes
> of bug@33029
>       that targeted at master-and-slave having incompatible bug33029-prone versions.
> 

There is no explanation what BUG#33029 has to do with this. Is this bug
a regression or is it just related to that bug in some other way. Could
you please add a sentence saying how they are related.

> added:
>   mysql-test/suite/bugs/r/rpl_bug33029.result
>   mysql-test/suite/bugs/t/rpl_bug33029.test
> modified:
>   sql/slave.cc
>   sql/structs.h
>   support-files/build-tags
> 
> per-file messages:
>   mysql-test/suite/bugs/r/rpl_bug33029.result
>     new result file
>   mysql-test/suite/bugs/t/rpl_bug33029.test
>     test merely checks no crash happens on slave.
>   sql/slave.cc
>     forcing to execute special logics implemented for bug#33029 if
>     simulate_bug33029 the debug option is set.
>   sql/structs.h
>     converting automatic allocation of a being appended item to Discrete_interval to
> dynamic.
> === added file 'mysql-test/suite/bugs/r/rpl_bug33029.result'
> --- a/mysql-test/suite/bugs/r/rpl_bug33029.result	1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/bugs/r/rpl_bug33029.result	2008-06-17 13:45:05 +0000
> @@ -0,0 +1,15 @@
> +stop slave;
> +drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
> +reset master;
> +reset slave;
> +drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
> +start slave;
> +create table `t1` (`id` int not null auto_increment primary key);
> +create trigger `trg` before insert on `t1` for each row begin end;
> +set @@global.debug="+d,simulate_bug33029";
> +stop slave;
> +start slave;
> +insert into `t1` values ();
> +select * from t1;
> +id
> +1
> 
> === added file 'mysql-test/suite/bugs/t/rpl_bug33029.test'
> --- a/mysql-test/suite/bugs/t/rpl_bug33029.test	1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/bugs/t/rpl_bug33029.test	2008-06-17 13:45:05 +0000
> @@ -0,0 +1,25 @@
> +#
> +# Bug #36443 Server crashes when executing insert when insert trigger on table
> +#
> +# Emulating the former bug#33029 situation to see that there is no crash anymore.
> +# 
> +
> +
> +source include/master-slave.inc;
> +
> +create table `t1` (`id` int not null auto_increment primary key);
> +create trigger `trg` before insert on `t1` for each row begin end;
> +
> +sync_slave_with_master;
> +set @@global.debug="+d,simulate_bug33029";
> +
> +stop slave;
> +start slave;
> +
> +connection master;
> +
> +insert into `t1` values ();
> +
> +sync_slave_with_master;
> +select * from t1;
> +
> 
> === modified file 'sql/slave.cc'
> --- a/sql/slave.cc	2008-03-31 08:57:18 +0000
> +++ b/sql/slave.cc	2008-06-17 13:45:05 +0000
> @@ -4136,6 +4136,7 @@ bool rpl_master_erroneous_autoinc(THD *t
>    if (active_mi && active_mi->rli.sql_thd == thd)
>    {
>      Relay_log_info *rli= &active_mi->rli;
> +    DBUG_EXECUTE_IF("simulate_bug33029", return TRUE;);
>      return rpl_master_has_bug(rli, 33029, FALSE);
>    }
>    return FALSE;
> 
> === modified file 'sql/structs.h'
> --- a/sql/structs.h	2008-03-14 03:35:41 +0000
> +++ b/sql/structs.h	2008-06-17 13:45:05 +0000
> @@ -320,8 +320,8 @@ private:
>    {
>      for (Discrete_interval *i= from.head; i; i= i->next)
>      {
> -      Discrete_interval j= *i;
> -      append(&j);
> +      Discrete_interval *j= new Discrete_interval(*i);
> +      append(j);

Here new can return NULL, causing a partial list to be constructed
without any indication that is the case. A better option would be to
release the memory that was allocated in the copying and mark the list
as "bad". That way, it is at least possible for the caller to check if
the list was copied successfully.

>      }
>    }
>  public:
> 
> === modified file 'support-files/build-tags'
> --- a/support-files/build-tags	2002-01-20 02:16:52 +0000
> +++ b/support-files/build-tags	2008-06-17 13:45:05 +0000
> @@ -2,7 +2,7 @@
>  
>  rm -f TAGS
>  filter='\.cc$\|\.c$\|\.h$\|\.yy$'
> -files=`bk -r sfiles -gU | grep $filter `
> +files=`bzr ls | grep $filter `
>  for f in $files ;
>  do
>  	 etags -o TAGS --append $f
> 
> 


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