RESET SLAVE and CHANGE MASTER will now give an error if slave is running.

This fixes a problem in replication where RESET SLAVE could crash a running slave.


mysql-test/r/rpl_failsafe.result:
  Added missing drop
mysql-test/r/rpl_log_pos.result:
  Added slave stop before change master
mysql-test/t/rpl_log_pos.test:
  Added slave stop before change master
sql/sql_parse.cc:
  reset_slave() sends it's own errors
sql/sql_repl.cc:
  reset_slave() sends it own error.
  RESET SLAVE and CHANGE MASTER will now give an error if slave is running.
  This fixes a problem in replication where RESET SLAVE could crash a running slave.

mysql-test/r/rpl_failsafe.result

@@ -1,4 +1,5 @@
 slave stop;
+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;

mysql-test/r/rpl_log_pos.result

@@ -10,7 +10,9 @@ master-bin.001	79
 show slave status;
 Master_Host	Master_User	Master_Port	Connect_retry	Master_Log_File	Read_Master_Log_Pos	Relay_Log_File	Relay_Log_Pos	Relay_Master_Log_File	Slave_IO_Running	Slave_SQL_Running	Replicate_do_db	Replicate_ignore_db	Last_errno	Last_error	Skip_counter	Exec_master_log_pos	Relay_log_space
 127.0.0.1	root	MASTER_PORT	1	master-bin.001	79	slave-relay-bin.002	120	master-bin.001	Yes	Yes			0		0	79	124
+slave stop;
 change master to master_log_pos=73;
+slave start;
 slave stop;
 change master to master_log_pos=73;
 show slave status;
@@ -20,6 +22,7 @@ slave start;
 show slave status;
 Master_Host	Master_User	Master_Port	Connect_retry	Master_Log_File	Read_Master_Log_Pos	Relay_Log_File	Relay_Log_Pos	Relay_Master_Log_File	Slave_IO_Running	Slave_SQL_Running	Replicate_do_db	Replicate_ignore_db	Last_errno	Last_error	Skip_counter	Exec_master_log_pos	Relay_log_space
 127.0.0.1	root	MASTER_PORT	1	master-bin.001	73	slave-relay-bin.001	4	master-bin.001	No	Yes			0		0	73	4
+slave stop;
 change master to master_log_pos=173;
 slave start;
 show slave status;
@@ -32,6 +35,7 @@ create table if not exists t1 (n int);
 drop table if exists t1;
 create table t1 (n int);
 insert into t1 values (1),(2),(3);
+slave stop;
 change master to master_log_pos=79;
 slave start;
 select * from t1;

mysql-test/t/rpl_log_pos.test

@@ -8,7 +8,9 @@ connection slave;
 sync_with_master;
 --replace_result 3306 MASTER_PORT 9306 MASTER_PORT 3334 MASTER_PORT 3336 MASTER_PORT
 show slave status;
+slave stop;
 change master to master_log_pos=73;
+slave start;
 sleep 5;
 slave stop;
 
@@ -19,6 +21,7 @@ slave start;
 sleep 5;
 --replace_result 3306 MASTER_PORT 9306 MASTER_PORT 3334 MASTER_PORT 3336 MASTER_PORT
 show slave status;
+slave stop;
 change master to master_log_pos=173;
 --replace_result 3306 MASTER_PORT 9306 MASTER_PORT 3334 MASTER_PORT 3336 MASTER_PORT
 slave start;
@@ -32,6 +35,7 @@ create table t1 (n int);
 insert into t1 values (1),(2),(3);
 save_master_pos;
 connection slave;
+slave stop;
 change master to master_log_pos=79;
 slave start;
 sync_with_master;

sql/sql_parse.cc

@@ -1136,10 +1136,8 @@ bool dispatch_command(enum enum_server_command command, THD *thd,
       if (check_global_access(thd,RELOAD_ACL))
 	break;
       mysql_log.write(thd,command,NullS);
-      if (reload_acl_and_cache(thd, options, (TABLE_LIST*) 0))
+      /* error sending is deferred to reload_acl_and_cache */
-	send_error(net,0);
+      reload_acl_and_cache(thd, options, (TABLE_LIST*) 0) ;
-      else
-	send_eof(net);
       break;
     }
   case COM_SHUTDOWN:
@@ -2347,10 +2345,8 @@ mysql_execute_command(void)
   case SQLCOM_RESET:
     if (check_global_access(thd,RELOAD_ACL) || check_db_used(thd, tables))
       goto error;
-    if (reload_acl_and_cache(thd, lex->type, tables))
+    /* error sending is deferred to reload_acl_and_cache */
-      send_error(&thd->net,0);
+    reload_acl_and_cache(thd, lex->type, tables) ;
-    else
-      send_ok(&thd->net);
     break;
   case SQLCOM_KILL:
     kill_one_thread(thd,lex->thread_id);
@@ -3332,10 +3328,15 @@ static bool check_dup(const char *db, const char *name, TABLE_LIST *tables)
   return 0;
 }
 
+
+/*
+  Reload/resets privileges and the different caches
+*/
+
 bool reload_acl_and_cache(THD *thd, ulong options, TABLE_LIST *tables)
 {
   bool result=0;
-
+  bool error_already_sent=0;
   select_errors=0;				/* Write if more errors */
   if (options & REFRESH_GRANT)
   {
@@ -3393,11 +3394,29 @@ bool reload_acl_and_cache(THD *thd, ulong options, TABLE_LIST *tables)
  {
    LOCK_ACTIVE_MI;
    if (reset_slave(thd, active_mi))
+   {
      result=1;
+     /*
+       reset_slave() sends error itself.
+       If it didn't, one would either change reset_slave()'s prototype, to
+       pass *errorcode and *errmsg to it when it's called or
+       change reset_slave to use my_error() to register the error.
+     */
+     error_already_sent=1;
+   }
    UNLOCK_ACTIVE_MI;
  }
  if (options & REFRESH_USER_RESOURCES)
    reset_mqh(thd,(LEX_USER *) NULL);
+
+ if (thd && !error_already_sent)
+ {
+   if (result)
+     send_error(&thd->net,0);
+   else
+     send_ok(&thd->net);
+ }
+
  return result;
 }
 

sql/sql_repl.cc

@@ -695,20 +695,48 @@ int stop_slave(THD* thd, MASTER_INFO* mi, bool net_report )
   return 0;
 }
 
+
+/*
+  Remove all relay logs and start replication from the start
+
+  SYNOPSIS
+    reset_slave()
+    thd			Thread handler
+    mi			Master info for the slave
+
+
+  NOTES
+    We don't send ok in this functions as this is called from
+    reload_acl_and_cache() which may have done other tasks, which may
+    have failed for which we want to send and error.
+
+  RETURN
+    0	ok
+    1	error
+	In this case error is sent to the client with send_error()
+*/
+
+
 int reset_slave(THD *thd, MASTER_INFO* mi)
 {
   MY_STAT stat_area;
   char fname[FN_REFLEN];
-  int restart_thread_mask = 0,error=0;
+  int thread_mask= 0, error= 0;
+  uint sql_errno=0;
   const char* errmsg=0;
   DBUG_ENTER("reset_slave");
 
   lock_slave_threads(mi);
-  init_thread_mask(&restart_thread_mask,mi,0 /* not inverse */);
+  init_thread_mask(&thread_mask,mi,0 /* not inverse */);
-  if ((error=terminate_slave_threads(mi,restart_thread_mask,1 /*skip lock*/))
+  if (thread_mask) // We refuse if any slave thread is running
-      || (error=purge_relay_logs(&mi->rli, thd,
+  {
-				 1 /* just reset */,
+    sql_errno= ER_SLAVE_MUST_STOP;
-				 &errmsg)))
+    error=1;
+    goto err;
+  }
+  if ((error= purge_relay_logs(&mi->rli, thd,
+			       1 /* just reset */,
+			       &errmsg)))
     goto err;
   
   end_master_info(mi);
@@ -724,17 +752,15 @@ int reset_slave(THD *thd, MASTER_INFO* mi)
     error=1;
     goto err;
   }
-  if (restart_thread_mask)
+
-    error=start_slave_threads(0 /* mutex not needed */,
-			      1 /* wait for start*/,
-			      mi,master_info_file,relay_log_info_file,
-			      restart_thread_mask);
-  // TODO: fix error messages so they get to the client
 err:
   unlock_slave_threads(mi);
+  if (thd && error) 
+    send_error(&thd->net, sql_errno, errmsg);
   DBUG_RETURN(error);
 }
 
+
 void kill_zombie_dump_threads(uint32 slave_server_id)
 {
   pthread_mutex_lock(&LOCK_thread_count);
@@ -766,23 +792,20 @@ void kill_zombie_dump_threads(uint32 slave_server_id)
 
 int change_master(THD* thd, MASTER_INFO* mi)
 {
-  int error=0,restart_thread_mask;
+  int thread_mask;
   const char* errmsg=0;
   bool need_relay_log_purge=1;
   DBUG_ENTER("change_master");
 
-  // kill slave thread
   lock_slave_threads(mi);
-  init_thread_mask(&restart_thread_mask,mi,0 /*not inverse*/);
+  init_thread_mask(&thread_mask,mi,0 /*not inverse*/);
-  if (restart_thread_mask &&
+  if (thread_mask) // We refuse if any slave thread is running
-      (error=terminate_slave_threads(mi,
-				     restart_thread_mask,
-				     1 /*skip lock*/)))
   {
-    send_error(&thd->net,error);
+    net_printf(&thd->net,ER_SLAVE_MUST_STOP);
     unlock_slave_threads(mi);
     DBUG_RETURN(1);
   }
+
   thd->proc_info = "changing master";
   LEX_MASTER_INFO* lex_mi = &thd->lex.mi;
   // TODO: see if needs re-write
@@ -851,6 +874,7 @@ int change_master(THD* thd, MASTER_INFO* mi)
 			 &errmsg))
     {
       net_printf(&thd->net, 0, "Failed purging old relay logs: %s",errmsg);
+      unlock_slave_threads(mi);
       DBUG_RETURN(1);
     }
   }
@@ -881,18 +905,9 @@ int change_master(THD* thd, MASTER_INFO* mi)
   pthread_cond_broadcast(&mi->data_cond);
   pthread_mutex_unlock(&mi->rli.data_lock);
 
-  thd->proc_info = "starting slave";
-  if (restart_thread_mask) 
-      error=start_slave_threads(0 /* mutex not needed*/,
-			        1 /* wait for start*/,
-			        mi,master_info_file,relay_log_info_file,
-				restart_thread_mask);
   unlock_slave_threads(mi);
   thd->proc_info = 0;
-  if (error)
+  send_ok(&thd->net);
-    send_error(&thd->net,error);
-  else
-    send_ok(&thd->net);
   DBUG_RETURN(0);
 }