List:Commits« Previous MessageNext Message »
From:ahristov Date:February 28 2006 3:14pm
Subject:bk commit into 5.1 tree (andrey:1.2184) BUG#17619
View as plain text  
Below is the list of changes that have just been committed into a local
5.1 repository of andrey. When andrey 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
  1.2184 06/02/28 16:14:05 andrey@lmy004. +7 -0
  cleanup part 3, as fix for bug #17619 (Scheduler race conditions)

  sql/event_timed.cc
    1.42 06/02/28 16:13:55 andrey@lmy004. +8 -4
    rename ret2 to ret.
    don't spawn a thread if we see that we have been marked killed.

  sql/event_priv.h
    1.22 06/02/28 16:13:55 andrey@lmy004. +3 -0
    export db_find_event

  sql/event_manager.h
    1.3 06/02/28 16:13:55 andrey@lmy004. +19 -9
    - add thd as parameter
    - make run() protected

  sql/event_manager.cc
    1.4 06/02/28 16:13:55 andrey@lmy004. +143 -23
    - add THD as param. 
    - finish add_event code (as feature, has to be tested)
    - finish drop_event code (as feature, has to be tested)
    - copied and modified relevant code from event.cc and event_executor.cc which has
      to be in Event_queue_manager

  sql/event_executor.cc
    1.37 06/02/28 16:13:55 andrey@lmy004. +0 -4
    remove test code

  sql/event.h
    1.28 06/02/28 16:13:54 andrey@lmy004. +12 -4
    - protect the destruction of sphead
    - implement synchronous kill of an event

  sql/event.cc
    1.36 06/02/28 16:13:54 andrey@lmy004. +1 -1
    export db_find_event

# This is a BitKeeper patch.  What follows are the unified diffs for the
# set of deltas contained in the patch.  The rest of the patch, the part
# that BitKeeper cares about, is below these diffs.
# User:	andrey
# Host:	lmy004.
# Root:	/work/mysql-5.1-bug17619

--- 1.35/sql/event.cc	2006-02-28 01:17:43 +01:00
+++ 1.36/sql/event.cc	2006-02-28 16:13:54 +01:00
@@ -947,7 +947,7 @@ err:
      2) tbl is not closed at exit
 */
 
-static int
+int
 db_find_event(THD *thd, Event_timed_name *etn, Event_timed **ett, MEM_ROOT *root)
 {
   TABLE *table;

--- 1.27/sql/event.h	2006-02-28 01:17:43 +01:00
+++ 1.28/sql/event.h	2006-02-28 16:13:54 +01:00
@@ -221,9 +221,11 @@ public:
 
   ~Event_timed()
   {
-    pthread_mutex_destroy(&this->LOCK_running);
+    pthread_mutex_lock(&this->LOCK_running);
     if (free_sphead_on_delete)
       free_sp();
+    pthread_mutex_unlock(&this->LOCK_running);
+    pthread_mutex_destroy(&this->LOCK_running);
   }
 
 
@@ -341,13 +343,19 @@ public:
     {
       /* If zero thread probably shuts down */
       if (event_thread_id)
+      {
         kill_one_thread(current_thd, event_thread_id, false);
+        if (sync)
+        {
+          pthread_mutex_lock(&LOCK_running);
+          pthread_mutex_unlock(&LOCK_running);
+        }
+      }
     }
     else
-    {
       pthread_mutex_unlock(&LOCK_running);
-      return FALSE;
-    }
+
+    return FALSE;
   }
 
   Event_timed_name *

--- 1.36/sql/event_executor.cc	2006-02-28 01:17:43 +01:00
+++ 1.37/sql/event_executor.cc	2006-02-28 16:13:55 +01:00
@@ -203,10 +203,6 @@ init_events()
 
   evex_init_mutexes();
 
-#ifdef DEBUG_EVENT_MANAGER
-  evex_manager_tester();
-#endif
-
   evex_check_system_tables();
 
   VOID(pthread_mutex_lock(&LOCK_evex_running));

--- 1.3/sql/event_manager.cc	2006-02-28 01:17:43 +01:00
+++ 1.4/sql/event_manager.cc	2006-02-28 16:13:55 +01:00
@@ -19,6 +19,7 @@
 #define CHECK_SHUTDOWN(__ret_code) \
 if (mode != MANAGER_RUNNING) \
 { \
+  DBUG_PRINT("info", ("manager not running but %d. doing nothing", mode)); \
   pthread_mutex_unlock(&LOCK_mutex); \
   DBUG_RETURN(__ret_code); \
 }
@@ -41,17 +42,27 @@ Event_scheduler_manager::LOCK_manager;
     TRUE  Failure
 */
 
-
 Event_scheduler_manager*
 Event_scheduler_manager::get_instance(event_worker_func w_func)
 {
   Event_scheduler_manager *tmp= NULL;
   DBUG_ENTER("Event_scheduler_manager::get_instance");
+
+  if (sizeof(my_time_t) != sizeof(time_t))
+  {
+    sql_print_error("SCHEDULER: sizeof(my_time_t) != sizeof(time_t) ."
+                    "The scheduler will not work correctly. Stopping.");
+    DBUG_ASSERT(0);
+    DBUG_RETURN(NULL);
+  }
   
   pthread_mutex_lock(&LOCK_manager);
   if (ston)
     goto skip_init;
-    
+  
+  /* Check it rarely (when creating instance) but do check it */  
+  check_system_tables();
+
   tmp= new Event_scheduler_manager;
   if (init_queue_ex(&tmp->queue, 30 /*num_el*/, 0 /*offset*/,
                     0 /*smallest_on_top*/, event_timed_compare_q,
@@ -72,9 +83,12 @@ Event_scheduler_manager::get_instance(ev
     sql_print_error("SCHEDULER: Unable to initialize COND_new_work");
     goto err;
   }
+  /* init memory root */
+  init_alloc_root(&tmp->manager_root, MEM_ROOT_BLOCK_SIZE, MEM_ROOT_PREALLOC);
+
   tmp->worker_func= w_func;
   tmp->mode= MANAGER_RUNNING;
-
+  
   ston= tmp;
 
 skip_init:
@@ -100,11 +114,13 @@ err:
 */
 
 bool
-Event_scheduler_manager::add_event(Event_timed *et, bool check_existance)
+Event_scheduler_manager::add_event(THD *thd, Event_timed *et,
+                                   bool check_existance)
 {
+  Event_timed *et_new;
   DBUG_ENTER("Event_scheduler_manager::add_event");
   pthread_mutex_lock(&LOCK_mutex);
-  CHECK_SHUTDOWN(true);
+  CHECK_SHUTDOWN(false);
   if (check_existance)
   {
     if (find_event(et->get_ident(), false))
@@ -113,11 +129,14 @@ Event_scheduler_manager::add_event(Event
       DBUG_RETURN(true);
     }
   }
-  queue_insert_safe(&queue, (byte *) et);
-  pthread_cond_signal(&COND_new_work);
-
+  /* We need to load the event on manager_root */
+  if ((et_new= load_and_compile_event(thd, et->get_ident())))
+  {
+    queue_insert_safe(&queue, (byte *) et_new);
+    pthread_cond_signal(&COND_new_work);
+  }
   pthread_mutex_unlock(&LOCK_mutex);
-  DBUG_RETURN(false);
+  DBUG_RETURN(!et_new);
 }
 
 
@@ -135,22 +154,23 @@ Event_scheduler_manager::add_event(Event
 */
 
 bool
-Event_scheduler_manager::drop_event(Event_timed_name *etn,
+Event_scheduler_manager::drop_event(THD *thd, Event_timed_name *etn,
                                     enum event_drop_mode drop_mode)
 {
   uint i;
   Event_timed *et;
   DBUG_ENTER("Event_scheduler_manager::drop_event");
   pthread_mutex_lock(&LOCK_mutex);
-  CHECK_SHUTDOWN(true);
-  
+  CHECK_SHUTDOWN(false);
+
+  /*
+    We can get better parallelism if low granularity lock on the queue.
+    However still has to be aware that the manager is running so we
+    don't kill the queue, as part of manager shutdown process in one thread,
+    just before we add/delete from it in ahother.
+  */  
   if ((et= find_event(etn, true)))
-  {
-    if (et->kill_thread(true))
-    {
-    
-    }
-  }
+    et->kill_thread(true);
   else
     DBUG_PRINT("info", ("no such event found"));
 
@@ -173,12 +193,12 @@ Event_scheduler_manager::drop_event(Even
 */
 
 bool
-Event_scheduler_manager::replace_event(Event_timed *et,
+Event_scheduler_manager::replace_event(THD *thd, Event_timed *et,
                                        enum event_drop_mode replace_mode)
 {
   DBUG_ENTER("Event_scheduler_manager::replace_event");
   pthread_mutex_lock(&LOCK_mutex);
-  CHECK_SHUTDOWN(true);
+  CHECK_SHUTDOWN(false);
 
   pthread_mutex_unlock(&LOCK_mutex);
   DBUG_RETURN(false);
@@ -302,6 +322,106 @@ Event_scheduler_manager::events_count()
 }
 
 
+Event_timed *
+Event_scheduler_manager::load_and_compile_event(THD * thd, Event_timed_name *etn)
+{
+  int ret= 0;
+  MEM_ROOT *tmp_mem_root;
+  Event_timed *et_new;
+  Open_tables_state backup;
+
+  DBUG_ENTER("Event_scheduler_manager::load_and_compile_event");
+  DBUG_PRINT("enter", ("name: %s", etn->name.str));
+
+  thd->reset_n_backup_open_tables_state(&backup);
+  /*
+    no need to use my_error() here because db_find_event() has done it
+    Use th
+  */
+  ret= db_find_event(thd, etn, &et_new, &evex_mem_root);
+  thd->restore_backup_open_tables_state(&backup);
+  if (ret)
+    DBUG_RETURN(NULL);
+
+  tmp_mem_root= thd->mem_root;
+  thd->mem_root= &manager_root;
+  /*
+    allocate on evex_mem_root. if you call without evex_mem_root
+    then sphead will not be cleared!
+  */
+  if (!(ret= et_new->compile(thd, &manager_root)))
+    et_new->compute_next_execution_time();
+  
+  thd->mem_root= tmp_mem_root;
+  if (ret)
+  {
+    delete et_new;
+    et_new= NULL;
+  }
+    
+  return et_new;
+}
+
+
+/*
+  Opens mysql.db and mysql.user and checks whether
+  1. mysql.db has column Event_priv at column 20 (0 based);
+  2. mysql.user has column Event_priv at column 29 (0 based);
+  
+  Synopsis
+    Event_scheduler_manager::evex_check_system_tables()
+*/
+
+void
+Event_scheduler_manager::check_system_tables()
+{
+  THD *thd= current_thd;
+  TABLE_LIST tables;
+  bool not_used;
+  Open_tables_state backup;
+
+  /* thd is 0x0 during boot of the server. Later it's !=0x0 */
+  if (!thd)
+    return;
+
+  thd->reset_n_backup_open_tables_state(&backup);
+  
+  bzero((char*) &tables, sizeof(tables));
+  tables.db= (char*) "mysql";
+  tables.table_name= tables.alias= (char*) "db";
+  tables.lock_type= TL_READ;
+
+  if (simple_open_n_lock_tables(thd, &tables))
+    sql_print_error("Cannot open mysql.db");
+  else
+  {
+    table_check_intact(tables.table, MYSQL_DB_FIELD_COUNT, mysql_db_table_fields,
+                       &mysql_db_table_last_check,ER_CANNOT_LOAD_FROM_TABLE);                           
+    close_thread_tables(thd);
+  }
+
+  bzero((char*) &tables, sizeof(tables));
+  tables.db= (char*) "mysql";
+  tables.table_name= tables.alias= (char*) "user";
+  tables.lock_type= TL_READ;
+
+  if (simple_open_n_lock_tables(thd, &tables))
+    sql_print_error("Cannot open mysql.db");
+  else
+  {
+    if (tables.table->s->fields < 29 ||
+      strncmp(tables.table->field[29]->field_name,
+                STRING_WITH_LEN("Event_priv")))
+      sql_print_error("mysql.user has no `Event_priv` column at position 29");
+
+    close_thread_tables(thd);
+  }
+
+  thd->restore_backup_open_tables_state(&backup);
+}
+
+
+
 #define ERROR_W_MESSAGE(cond, msg) \
 if (!(cond)){ \
   sql_print_error("[ERR] %s", msg); \
@@ -347,15 +467,15 @@ evex_manager_tester()
   et.ident.definer.str= (char *)"definer@host";
   et.ident.definer.length= strlen("definer@host");
 
-  mgr->add_event(&et, false);
+  mgr->add_event(thd, &et, false);
 
   ERROR_W_MESSAGE(mgr->events_count() == 1, "Test count");
   
-  mgr->add_event(&et, true);
+  mgr->add_event(thd, &et, true);
 
   ERROR_W_MESSAGE(mgr->events_count() == 1, "Test count with check");
 
-  mgr->add_event(&et, false);
+  mgr->add_event(thd, &et, false);
 
   ERROR_W_MESSAGE(mgr->events_count() == 2, "Test count with check");
 

--- 1.2/sql/event_manager.h	2006-02-27 18:34:31 +01:00
+++ 1.3/sql/event_manager.h	2006-02-28 16:13:55 +01:00
@@ -59,13 +59,13 @@ public:
   };
 
   bool
-  add_event(Event_timed *et, bool check_existance);
+  add_event(THD *thd, Event_timed *et, bool check_existance);
   
   bool
-  drop_event(Event_timed_name *etn, enum event_drop_mode drop_mode);
+  drop_event(THD *thd, Event_timed_name *etn, enum event_drop_mode drop_mode);
   
   bool
-  replace_event(Event_timed *et, enum event_drop_mode replace_mode);
+  replace_event(THD *thd, Event_timed *et, enum event_drop_mode replace_mode);
   
   /*
     LOCKING PROTOCOL: Does not do any locking. You are responsible to lock
@@ -74,7 +74,7 @@ public:
   find_event(Event_timed_name *etn, bool remove_from_q);
 
   bool
-  trigger_event(Event_timed_name *etn) { DBUG_ASSERT(0);}
+  trigger_event(THD *thd, Event_timed_name *etn) { DBUG_ASSERT(0);}
   
   void
   register_worker_function(event_worker_func func)
@@ -84,7 +84,7 @@ public:
 
   int
   events_count();
-    
+
   bool
   start();
   
@@ -94,8 +94,20 @@ public:
 
   static Event_scheduler_manager*
   get_instance(event_worker_func w_func);
-  
+ 
+protected:
+  bool
+  run();
+
+  /* helper functions */
+  static void
+  check_system_tables();
+ 
+  Event_timed *
+  load_and_compile_event(THD *thd, Event_timed_name *etn);
+
 #ifdef DEBUG_EVENT_MANAGER
+public:
   /*
     This method is pretty dangerous and we use it only for
     testing purposes. 
@@ -116,9 +128,6 @@ private:
 #else
 public:
 #endif
-  bool
-  run();
-
   static Event_scheduler_manager *ston;
 
   pthread_mutex_t LOCK_mutex;
@@ -127,6 +136,7 @@ public:
   QUEUE queue;
   pthread_cond_t    COND_new_work;
   event_worker_func worker_func;
+  MEM_ROOT          manager_root;
 };
 
 #endif /* _EVENT_MANAGER_H_ */

--- 1.21/sql/event_priv.h	2006-02-28 01:17:43 +01:00
+++ 1.22/sql/event_priv.h	2006-02-28 16:13:55 +01:00
@@ -42,6 +42,9 @@ evex_db_find_event_by_name(THD *thd, con
                           const LEX_STRING ev_name,
                           const LEX_STRING user_name,
                           TABLE *table);
+int
+db_find_event(THD *thd, Event_timed_name *etn, Event_timed **ett,MEM_ROOT *root);
+
 
 int
 event_timed_compare_q(void *vptr, byte* a, byte *b);

--- 1.41/sql/event_timed.cc	2006-02-28 01:17:43 +01:00
+++ 1.42/sql/event_timed.cc	2006-02-28 16:13:55 +01:00
@@ -1384,13 +1384,17 @@ done:
 bool
 Event_timed::can_spawn_now_n_lock(THD *thd)
 {
-  int ret2;
+  int ret;
   DBUG_ENTER("Event_timed::can_spawn_now_n_lock");
   DBUG_PRINT("info", ("trylock LOCK_RUNNING"));
-  ret2= pthread_mutex_trylock(&this->LOCK_running);
-  if (ret2 == EBUSY)
+  ret= pthread_mutex_trylock(&this->LOCK_running);
+  if (ret || ret == EBUSY)
     DBUG_RETURN(false);
-
+  if (killed)
+  {
+    pthread_mutex_unlock(&this->LOCK_running);
+    DBUG_RETURN(false);
+  }
   DBUG_PRINT("info", ("setting thread_id"));
   locked_by_thread_id= thd->thread_id;
   DBUG_RETURN(true);  
Thread
bk commit into 5.1 tree (andrey:1.2184) BUG#17619ahristov28 Feb