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.

2 files changed, 73 insertions, 39 deletions

diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
index 8f1e9c37eae..bb26826b620 100644
--- a/sql/sql_parse.cc
+++ b/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))
-	send_error(net,0);
-      else
-	send_eof(net);
+      /* error sending is deferred to reload_acl_and_cache */
+      reload_acl_and_cache(thd, options, (TABLE_LIST*) 0) ;
       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))
-      send_error(&thd->net,0);
-    else
-      send_ok(&thd->net);
+    /* error sending is deferred to reload_acl_and_cache */
+    reload_acl_and_cache(thd, lex->type, tables) ;
     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;
 }
 
diff --git a/sql/sql_repl.cc b/sql/sql_repl.cc
index ee16b84f87e..02440f511e1 100644
--- a/sql/sql_repl.cc
+++ b/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 */);
-  if ((error=terminate_slave_threads(mi,restart_thread_mask,1 /*skip lock*/))
-      || (error=purge_relay_logs(&mi->rli, thd,
-				 1 /* just reset */,
-				 &errmsg)))
+  init_thread_mask(&thread_mask,mi,0 /* not inverse */);
+  if (thread_mask) // We refuse if any slave thread is running
+  {
+    sql_errno= ER_SLAVE_MUST_STOP;
+    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*/);
-  if (restart_thread_mask &&
-      (error=terminate_slave_threads(mi,
-				     restart_thread_mask,
-				     1 /*skip lock*/)))
+  init_thread_mask(&thread_mask,mi,0 /*not inverse*/);
+  if (thread_mask) // We refuse if any slave thread is running
   {
-    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_error(&thd->net,error);
-  else
-    send_ok(&thd->net);
+  send_ok(&thd->net);
   DBUG_RETURN(0);
 }