Hi Andrei!
Patch looks good. OK to push.
Just my few cents,
Mats Kindahl
Andrei Elkin wrote:
> Below is the list of changes that have just been committed into a local
> 5.0 repository of aelkin. When aelkin 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, 2008-02-13 14:09:41+02:00, aelkin@stripped +4 -0
> Bug #33931 assertion at write_ignored_events_info_to_relay_log if
> init_slave_thread() fails
> and
> bug#33932 assertion at handle_slave_sql if init_slave_thread() fails
>
> the asserts were caused by
> bug33931: having thd deleted at time of executing err: code plus
> a missed initialization;
> bug33932: initialization of slave_is_running member was missed;
>
> fixed with relocating mi members initialization and removing delete thd
> It is safe to do as deletion happens later explicitly in the caller of
> init_slave_thread().
>
> Todo: at merging the test is better to be moved into suite/bugs for 5.x (when
> x>0).
>
> mysql-test/r/rpl_bug33931.result@stripped, 2008-02-13 14:09:40+02:00,
> aelkin@stripped +38 -0
> the new result file
>
> mysql-test/r/rpl_bug33931.result@stripped, 2008-02-13 14:09:40+02:00,
> aelkin@stripped +0 -0
>
> mysql-test/t/rpl_bug33931-slave.opt@stripped, 2008-02-13 14:09:40+02:00,
> aelkin@stripped +1 -0
> option to spark the simulation code
>
> mysql-test/t/rpl_bug33931-slave.opt@stripped, 2008-02-13 14:09:40+02:00,
> aelkin@stripped +0 -0
>
> mysql-test/t/rpl_bug33931.test@stripped, 2008-02-13 14:09:40+02:00,
> aelkin@stripped +37 -0
> tests check that slave does not crash as before.
> Slave threads must be in NO running state in the end.
>
>
> mysql-test/t/rpl_bug33931.test@stripped, 2008-02-13 14:09:40+02:00,
> aelkin@stripped +0 -0
>
> sql/slave.cc@stripped, 2008-02-13 14:09:40+02:00, aelkin@stripped +14 -4
> adding the bugs simulating code;
> relocating some assignments to satisfy the asserts;
>
> diff -Nrup a/mysql-test/r/rpl_bug33931.result b/mysql-test/r/rpl_bug33931.result
> --- /dev/null Wed Dec 31 16:00:00 196900
> +++ b/mysql-test/r/rpl_bug33931.result 2008-02-13 14:09:40 +02:00
> @@ -0,0 +1,38 @@
> +reset master;
> +stop slave;
> +reset slave;
> +start slave;
> +show slave status;
> +Slave_IO_State #
> +Master_Host 127.0.0.1
> +Master_User root
> +Master_Port MASTER_PORT
> +Connect_Retry 1
> +Master_Log_File
> +Read_Master_Log_Pos 4
> +Relay_Log_File #
> +Relay_Log_Pos #
> +Relay_Master_Log_File
> +Slave_IO_Running No
> +Slave_SQL_Running No
> +Replicate_Do_DB
> +Replicate_Ignore_DB
> +Replicate_Do_Table
> +Replicate_Ignore_Table
> +Replicate_Wild_Do_Table
> +Replicate_Wild_Ignore_Table
> +Last_Errno 0
> +Last_Error
> +Skip_Counter 0
> +Exec_Master_Log_Pos 0
> +Relay_Log_Space #
> +Until_Condition None
> +Until_Log_File
> +Until_Log_Pos 0
> +Master_SSL_Allowed No
> +Master_SSL_CA_File
> +Master_SSL_CA_Path
> +Master_SSL_Cert
> +Master_SSL_Cipher
> +Master_SSL_Key
> +Seconds_Behind_Master #
> diff -Nrup a/mysql-test/t/rpl_bug33931-slave.opt
> b/mysql-test/t/rpl_bug33931-slave.opt
> --- /dev/null Wed Dec 31 16:00:00 196900
> +++ b/mysql-test/t/rpl_bug33931-slave.opt 2008-02-13 14:09:40 +02:00
> @@ -0,0 +1 @@
> +--loose-debug=d,simulate_io_slave_error_on_init,simulate_sql_slave_error_on_init
> diff -Nrup a/mysql-test/t/rpl_bug33931.test b/mysql-test/t/rpl_bug33931.test
> --- /dev/null Wed Dec 31 16:00:00 196900
> +++ b/mysql-test/t/rpl_bug33931.test 2008-02-13 14:09:40 +02:00
> @@ -0,0 +1,37 @@
> +# Test for
> +# Bug #33931 assertion at write_ignored_events_info_to_relay_log if
> init_slave_thread() fails
> +# Bug #33932 assertion at handle_slave_sql if init_slave_thread() fails
> +
> +source include/have_debug.inc;
> +source include/have_log_bin.inc;
> +
> +connect (master,127.0.0.1,root,,test,$MASTER_MYPORT,);
> +connect (slave,127.0.0.1,root,,test,$SLAVE_MYPORT,);
> +
> +
> +connection master;
> +reset master;
> +
> +connection slave;
> +--disable_warnings
> +stop slave;
> +--enable_warnings
> +reset slave;
> +start slave;
> +
> +connection master;
> +save_master_pos;
> +connection slave;
> +
> +#
> +# slave is going to stop because of emulated failures
> +# but there won't be any crashes nor asserts hit.
> +#
> +source include/wait_for_slave_to_stop.inc;
> +
> +--replace_result $MASTER_MYPORT MASTER_PORT
> +--replace_column 1 # 8 # 9 # 23 # 33 #
> +query_vertical show slave status;
> +
> +# no clean-up is needed
> +
> diff -Nrup a/sql/slave.cc b/sql/slave.cc
> --- a/sql/slave.cc 2007-12-20 17:07:52 +02:00
> +++ b/sql/slave.cc 2008-02-13 14:09:40 +02:00
> @@ -2895,6 +2895,9 @@ void set_slave_thread_default_charset(TH
> static int init_slave_thread(THD* thd, SLAVE_THD_TYPE thd_type)
> {
> DBUG_ENTER("init_slave_thread");
> +#if !defined(DBUG_OFF)
> + int simulate_error= 0;
> +#endif
> thd->system_thread = (thd_type == SLAVE_THD_SQL) ?
> SYSTEM_THREAD_SLAVE_SQL : SYSTEM_THREAD_SLAVE_IO;
> thd->security_ctx->skip_grants();
> @@ -2914,10 +2917,17 @@ static int init_slave_thread(THD* thd, S
> thd->thread_id = thread_id++;
> pthread_mutex_unlock(&LOCK_thread_count);
>
> + DBUG_EXECUTE_IF("simulate_io_slave_error_on_init",
> + simulate_error|= (1 << SLAVE_THD_IO););
> + DBUG_EXECUTE_IF("simulate_sql_slave_error_on_init",
> + simulate_error|= (1 << SLAVE_THD_SQL););
> +#if !defined(DBUG_OFF)
> + if (init_thr_lock() || thd->store_globals() || simulate_error & (1<<
> thd_type))
> +#else
> if (init_thr_lock() || thd->store_globals())
> +#endif
> {
> thd->cleanup();
> - delete thd;
> DBUG_RETURN(-1);
> }
>
> @@ -3515,6 +3525,7 @@ slave_begin:
>
> thd= new THD; // note that contructor of THD uses DBUG_ !
> THD_CHECK_SENTRY(thd);
> + mi->io_thd = thd;
>
> pthread_detach_this_thread();
> thd->thread_stack= (char*) &thd; // remember where our stack is
> @@ -3525,7 +3536,6 @@ slave_begin:
> sql_print_error("Failed during slave I/O thread initialization");
> goto err;
> }
> - mi->io_thd = thd;
> pthread_mutex_lock(&LOCK_thread_count);
> threads.append(thd);
> pthread_mutex_unlock(&LOCK_thread_count);
> @@ -3865,9 +3875,11 @@ slave_begin:
>
> thd = new THD; // note that contructor of THD uses DBUG_ !
> thd->thread_stack = (char*)&thd; // remember where our stack is
> + rli->sql_thd= thd;
>
> /* Inform waiting threads that slave has started */
> rli->slave_run_id++;
> + rli->slave_running = 1;
>
> pthread_detach_this_thread();
> if (init_slave_thread(thd, SLAVE_THD_SQL))
> @@ -3882,7 +3894,6 @@ slave_begin:
> goto err;
> }
> thd->init_for_queries();
> - rli->sql_thd= thd;
> thd->temporary_tables = rli->save_temporary_tables; // restore temp tables
> pthread_mutex_lock(&LOCK_thread_count);
> threads.append(thd);
> @@ -3895,7 +3906,6 @@ slave_begin:
> start receiving data so we realize we are not caught up and
> Seconds_Behind_Master grows. No big deal.
> */
> - rli->slave_running = 1;
> rli->abort_slave = 0;
> pthread_mutex_unlock(&rli->run_lock);
> pthread_cond_broadcast(&rli->start_cond);
>
>
--
Mats Kindahl
Lead Software Developer
Replication Team
MySQL AB, www.mysql.com
