bk commit into 5.0 tree (petr:1.2000)

[Petr Chardin]

Below is the list of changes that have just been committed into a local
5.0 repository of cps. When cps 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.2000 06/02/21 01:30:54 petr@stripped +4 -0
  Fix race condition: instance map wasn't locked for the
  duration of the whole 'flush instances'. As a consequence,
  it was possible to query instance map, while it is in the
  inconsistent state. The patch was reworked after review.

  server-tools/instance-manager/manager.cc
    1.31 06/02/21 01:30:52 petr@stripped +7 -15
    use instance_map.flush_instances instead of instance_map.load() and
guardian_thread.init()

  server-tools/instance-manager/instance_map.h
    1.18 06/02/21 01:30:52 petr@stripped +10 -7
    add new method. cleanup interface.

  server-tools/instance-manager/instance_map.cc
    1.25 06/02/21 01:30:52 petr@stripped +22 -15
    Eliminate race condition: lock instance map and guardian
    for the duration of the whole "FLUSH INSTANCES" execution.

  server-tools/instance-manager/guardian.cc
    1.20 06/02/21 01:30:52 petr@stripped +0 -2
    do not lock instance map in Guardian_thread::init()

# 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:	petr
# Host:	outpost.site
# Root:	/home/cps/mysql/devel/im/5.0-im-fix-race

--- 1.30/server-tools/instance-manager/manager.cc	2005-10-28 14:29:37 +04:00
+++ 1.31/server-tools/instance-manager/manager.cc	2006-02-21 01:30:52 +03:00
@@ -135,15 +135,6 @@
   if (instance_map.init() || user_map.init())
     return;
 
-
-  if (instance_map.load())
-  {
-    log_error("Cannot init instances repository. This might be caused by "
-               "the wrong config file options. For instance, missing mysqld "
-               "binary. Aborting.");
-    return;
-  }
-
   if (user_map.load(options.password_file_name))
     return;
 
@@ -207,12 +198,13 @@
 
   shutdown_complete= FALSE;
 
-  /* init list of guarded instances */
-  guardian_thread.lock();
-
-  guardian_thread.init();
-
-  guardian_thread.unlock();
+  if (instance_map.flush_instances())
+  {
+    log_error("Cannot init instances repository. This might be caused by "
+               "the wrong config file options. For instance, missing mysqld "
+               "binary. Aborting.");
+    return;
+  }
 
   /*
     After the list of guarded instances have been initialized,

--- 1.19/server-tools/instance-manager/guardian.cc	2005-10-08 18:39:39 +04:00
+++ 1.20/server-tools/instance-manager/guardian.cc	2006-02-21 01:30:52 +03:00
@@ -256,7 +256,6 @@
   Instance *instance;
   Instance_map::Iterator iterator(instance_map);
 
-  instance_map->lock();
   /* clear the list of guarded instances */
   free_root(&alloc, MYF(0));
   init_alloc_root(&alloc, MEM_ROOT_BLOCK_SIZE, 0);
@@ -272,7 +271,6 @@
       }
   }
 
-  instance_map->unlock();
   return 0;
 }
 

--- 1.24/server-tools/instance-manager/instance_map.cc	2005-09-23 22:28:46 +04:00
+++ 1.25/server-tools/instance-manager/instance_map.cc	2006-02-21 01:30:52 +03:00
@@ -80,19 +80,29 @@
 static int process_option(void *ctx, const char *group, const char *option)
 {
   Instance_map *map= NULL;
+
+  map = (Instance_map*) ctx;
+  return map->process_one_option(group, option);
+}
+
+C_MODE_END
+
+
+int Instance_map::process_one_option(const char *group, const char *option)
+{
   Instance *instance= NULL;
   static const char prefix[]= { 'm', 'y', 's', 'q', 'l', 'd' };
 
-  map = (Instance_map*) ctx;
   if (strncmp(group, prefix, sizeof prefix) == 0 &&
       ((my_isdigit(default_charset_info, group[sizeof prefix]))
        || group[sizeof(prefix)] == '\0'))
     {
-      if ((instance= map->find(group, strlen(group))) == NULL)
+      if (!(instance= (Instance *) hash_search(&hash, (byte *) group,
+                                               strlen(group))))
       {
-        if ((instance= new Instance) == 0)
+        if (!(instance= new Instance))
           goto err;
-        if (instance->init(group) || map->add_instance(instance))
+        if (instance->init(group) || my_hash_insert(&hash, (byte *) instance))
           goto err_instance;
       }
 
@@ -108,8 +118,6 @@
   return 1;
 }
 
-C_MODE_END
-
 
 Instance_map::Instance_map(const char *default_mysqld_path_arg):
 mysqld_path(default_mysqld_path_arg)
@@ -149,25 +157,25 @@
 {
   int rc;
 
+  /*
+    Guardian thread relies on the instance map repository for guarding
+    instances. This is why refreshing instance map, we need (1) to stop
+    guardian (2) reload the instance map (3) reinitialize the guardian
+    with new instances.
+  */
   guardian->lock();
   pthread_mutex_lock(&LOCK_instance_map);
   hash_free(&hash);
   hash_init(&hash, default_charset_info, START_HASH_SIZE, 0, 0,
             get_instance_key, delete_instance, 0);
-  pthread_mutex_unlock(&LOCK_instance_map);
   rc= load();
   guardian->init();
+  pthread_mutex_unlock(&LOCK_instance_map);
   guardian->unlock();
   return rc;
 }
 
 
-int Instance_map::add_instance(Instance *instance)
-{
-  return my_hash_insert(&hash, (byte *) instance);
-}
-
-
 Instance *
 Instance_map::find(const char *name, uint name_len)
 {
@@ -190,9 +198,8 @@
     if ((instance= new Instance) == 0)
       goto err;
 
-    if (instance->init("mysqld") || add_instance(instance))
+    if (instance->init("mysqld") || my_hash_insert(&hash, (byte *) instance))
       goto err_instance;
-
 
     /*
       After an instance have been added to the instance_map,

--- 1.17/server-tools/instance-manager/instance_map.h	2005-09-23 22:28:47 +04:00
+++ 1.18/server-tools/instance-manager/instance_map.h	2006-02-21 01:30:52 +03:00
@@ -63,21 +63,24 @@
   void lock();
   void unlock();
   int init();
+  /*
+    Process a given option and assign it to appropricate instance. This is
+    required for the option handler, passed to my_search_option_files().
+  */
+  int process_one_option(const char *group, const char *option);
 
   Instance_map(const char *default_mysqld_path_arg);
   ~Instance_map();
 
-  /* loads options from config files */
-  int load();
-  /* adds instance to internal hash */
-  int add_instance(Instance *instance);
-  /* inits instances argv's after all options have been loaded */
-  int complete_initialization();
-
 public:
   const char *mysqld_path;
   Guardian_thread *guardian;
 
+private:
+  /* loads options from config files */
+  int load();
+  /* inits instances argv's after all options have been loaded */
+  int complete_initialization();
 private:
   enum { START_HASH_SIZE = 16 };
   pthread_mutex_t LOCK_instance_map;