List:Commits« Previous MessageNext Message »
From:antony Date:March 24 2007 12:31am
Subject:bk commit into 5.1 tree (acurtis:1.2498) BUG#25721
View as plain text  
Below is the list of changes that have just been committed into a local
5.1 repository of antony. When antony 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, 2007-03-23 17:31:27-07:00, acurtis@stripped +5 -0
  Bug#25721
    "Concurrent ALTER/CREATE SERVER can lead to deadlock"
    Deadlock caused by inconsistant use of mutexes in sql_server.cc
    One mutex has been removed to resolve deadlock.
    Many functions were made private which should not be exported.
    Unused variables and function removed.

  mysql-test/r/federated_server.result@stripped, 2007-03-23 17:31:21-07:00, acurtis@stripped +25 -0
    Bug 25721 "Concurrent ALTER/CREATE SERVER can lead to deadlock"
    
    New test results

  mysql-test/t/federated_server.test@stripped, 2007-03-23 17:31:22-07:00, acurtis@stripped +35 -0
    Bug 25721 "Concurrent ALTER/CREATE SERVER can lead to deadlock"
    
    Test for bug by using stored procedure. Unpatched server would deadlock frequently.

  sql/sql_parse.cc@stripped, 2007-03-23 17:31:22-07:00, acurtis@stripped +1 -1
    Bug 25721 "Concurrent ALTER/CREATE SERVER can lead to deadlock"
    
    check for correct error code when dropping server

  sql/sql_servers.cc@stripped, 2007-03-23 17:31:22-07:00, acurtis@stripped +158 -197
    Bug 25721 "Concurrent ALTER/CREATE SERVER can lead to deadlock"
    
    Removed unneccessary mutex, only need THR_LOCK_servers rwlock to
    guard data structures against race conditions. Misuse of other mutex
    caused deadlock by inconsistant ordering of mutex lock operations.
    Alter order of some operations to hit memory before disk.
    Removed unused function.
    Removed servers_version and servers_cache_initialised variables.
    Made many internal functions static.

  sql/sql_servers.h@stripped, 2007-03-23 17:31:22-07:00, acurtis@stripped +4 -26
    Bug 25721 "Concurrent ALTER/CREATE SERVER can lead to deadlock"
    
    remove internal functions from being exported.

# 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:	acurtis
# Host:	ltamd64.xiphis.org
# Root:	/home/antony/work2/p1-bug25721.4

--- 1.644/sql/sql_parse.cc	2007-03-23 17:31:39 -07:00
+++ 1.645/sql/sql_parse.cc	2007-03-23 17:31:39 -07:00
@@ -4320,7 +4320,7 @@
 
     if ((err_code= drop_server(thd, &lex->server_options)))
     {
-      if (! lex->drop_if_exists && err_code == ER_FOREIGN_SERVER_EXISTS)
+      if (! lex->drop_if_exists && err_code == ER_FOREIGN_SERVER_DOESNT_EXIST)
       {
         DBUG_PRINT("info", ("problem dropping server %s",
                             lex->server_options.server_name));

--- 1.6/mysql-test/r/federated_server.result	2007-03-23 17:31:39 -07:00
+++ 1.7/mysql-test/r/federated_server.result	2007-03-23 17:31:39 -07:00
@@ -189,6 +189,31 @@
 drop table federated.t1;
 drop server 's1';
 # End of 5.1 tests
+use test;
+create procedure p1 ()
+begin
+DECLARE v INT DEFAULT 0;
+DECLARE e INT DEFAULT 0;
+DECLARE CONTINUE HANDLER FOR SQLEXCEPTION SET e = e + 1;
+WHILE v < 10000 do
+CREATE SERVER s
+FOREIGN DATA WRAPPER mysql
+OPTIONS (USER 'Remote', HOST '192.168.1.106', DATABASE 'test');
+ALTER SERVER s OPTIONS (USER 'Remote');
+DROP SERVER s;
+SET v = v + 1;
+END WHILE;
+SELECT e > 0;
+END//
+use test;
+call p1();
+call p1();
+e > 0
+1
+e > 0
+1
+drop procedure p1;
+drop server if exists s;
 DROP TABLE IF EXISTS federated.t1;
 DROP DATABASE IF EXISTS federated;
 DROP TABLE IF EXISTS federated.t1;

--- 1.5/mysql-test/t/federated_server.test	2007-03-23 17:31:39 -07:00
+++ 1.6/mysql-test/t/federated_server.test	2007-03-23 17:31:39 -07:00
@@ -234,4 +234,39 @@
 
 --echo # End of 5.1 tests
 
+
+#
+# Bug#25721 - deadlock with ALTER/CREATE SERVER
+#
+connect (other,localhost,root,,);
+connection master;
+use test;
+delimiter //;
+create procedure p1 ()
+begin
+  DECLARE v INT DEFAULT 0;
+  DECLARE e INT DEFAULT 0;
+  DECLARE CONTINUE HANDLER FOR SQLEXCEPTION SET e = e + 1;
+  WHILE v < 10000 do
+    CREATE SERVER s
+      FOREIGN DATA WRAPPER mysql
+      OPTIONS (USER 'Remote', HOST '192.168.1.106', DATABASE 'test');
+    ALTER SERVER s OPTIONS (USER 'Remote');
+	DROP SERVER s;
+    SET v = v + 1;
+  END WHILE;
+  SELECT e > 0;
+END//
+delimiter ;//
+connection other;
+use test;
+send call p1();
+connection master;
+call p1();
+connection other;
+reap;
+drop procedure p1;
+drop server if exists s;
+
+
 source include/federated_cleanup.inc;

--- 1.6/sql/sql_servers.cc	2007-03-23 17:31:39 -07:00
+++ 1.7/sql/sql_servers.cc	2007-03-23 17:31:39 -07:00
@@ -25,14 +25,43 @@
 #include "sp_head.h"
 #include "sp.h"
 
-HASH servers_cache;
-pthread_mutex_t servers_cache_mutex;                // To init the hash
-uint servers_cache_initialised=FALSE;
-/* Version of server table. incremented by servers_load */
-static uint servers_version=0;
+/*
+  We only use 1 mutex to guard the data structures - THR_LOCK_servers.
+  Read locked when only reading data and write-locked for all other access.
+*/
+
+static HASH servers_cache;
 static MEM_ROOT mem;
 static rw_lock_t THR_LOCK_servers;
 
+static bool get_server_from_table_to_cache(TABLE *table);
+
+/* insert functions */
+static int insert_server(THD *thd, FOREIGN_SERVER *server_options);
+static int insert_server_record(TABLE *table, FOREIGN_SERVER *server);
+static int insert_server_record_into_cache(FOREIGN_SERVER *server);
+static void prepare_server_struct_for_insert(LEX_SERVER_OPTIONS *server_options,
+                                             FOREIGN_SERVER *server);
+/* drop functions */ 
+static int delete_server_record(TABLE *table,
+                                char *server_name,
+                                int server_name_length);
+static int delete_server_record_in_cache(LEX_SERVER_OPTIONS *server_options);
+
+/* update functions */
+static void prepare_server_struct_for_update(LEX_SERVER_OPTIONS *server_options,
+                                             FOREIGN_SERVER *existing,
+                                             FOREIGN_SERVER *altered);
+static int update_server(THD *thd, FOREIGN_SERVER *existing, 
+					     FOREIGN_SERVER *altered);
+static int update_server_record(TABLE *table, FOREIGN_SERVER *server);
+static int update_server_record_in_cache(FOREIGN_SERVER *existing,
+                                         FOREIGN_SERVER *altered);
+/* utility functions */
+static void merge_server_struct(FOREIGN_SERVER *from, FOREIGN_SERVER *to);
+
+
+
 static byte *servers_cache_get_key(FOREIGN_SERVER *server, uint *length,
 			       my_bool not_used __attribute__((unused)))
 {
@@ -45,6 +74,7 @@
   DBUG_RETURN((byte*) server->server_name);
 }
 
+
 /*
   Initialize structures responsible for servers used in federated
   server scheme information for them from the server
@@ -64,35 +94,27 @@
     1	Could not initialize servers
 */
 
-my_bool servers_init(bool dont_read_servers_table)
+bool servers_init(bool dont_read_servers_table)
 {
   THD  *thd;
-  my_bool return_val= 0;
+  bool return_val= FALSE;
   DBUG_ENTER("servers_init");
 
   /* init the mutex */
-  if (pthread_mutex_init(&servers_cache_mutex, MY_MUTEX_INIT_FAST))
-    DBUG_RETURN(1);
-
   if (my_rwlock_init(&THR_LOCK_servers, NULL))
-    DBUG_RETURN(1);
+    DBUG_RETURN(TRUE);
 
   /* initialise our servers cache */
   if (hash_init(&servers_cache, system_charset_info, 32, 0, 0,
                 (hash_get_key) servers_cache_get_key, 0, 0))
   {
-    return_val= 1; /* we failed, out of memory? */
+    return_val= TRUE; /* we failed, out of memory? */
     goto end;
   }
 
   /* Initialize the mem root for data */
   init_alloc_root(&mem, ACL_ALLOC_BLOCK_SIZE, 0);
 
-  /*
-    at this point, the cache is initialised, let it be known
-  */
-  servers_cache_initialised= TRUE;
-
   if (dont_read_servers_table)
     goto end;
 
@@ -100,7 +122,7 @@
     To be able to run this from boot, we allocate a temporary THD
   */
   if (!(thd=new THD))
-    DBUG_RETURN(1);
+    DBUG_RETURN(TRUE);
   thd->thread_stack= (char*) &thd;
   thd->store_globals();
   /*
@@ -130,19 +152,13 @@
     TRUE   Error
 */
 
-static my_bool servers_load(THD *thd, TABLE_LIST *tables)
+static bool servers_load(THD *thd, TABLE_LIST *tables)
 {
   TABLE *table;
   READ_RECORD read_record_info;
-  my_bool return_val= TRUE;
+  bool return_val= TRUE;
   DBUG_ENTER("servers_load");
 
-  if (!servers_cache_initialised)
-    DBUG_RETURN(0);
-
-  /* need to figure out how to utilise this variable */
-  servers_version++; /* servers updated */
-
   /* first, send all cached rows to sleep with the fishes, oblivion!
      I expect this crappy comment replaced */
   free_root(&mem, MYF(MY_MARK_BLOCKS_FREE));
@@ -156,7 +172,7 @@
       goto end;
   }
 
-  return_val=0;
+  return_val= FALSE;
 
 end:
   end_read_record(&read_record_info);
@@ -183,10 +199,10 @@
     TRUE   Failure
 */
 
-my_bool servers_reload(THD *thd)
+bool servers_reload(THD *thd)
 {
   TABLE_LIST tables[1];
-  my_bool return_val= 1;
+  bool return_val= TRUE;
   DBUG_ENTER("servers_reload");
 
   if (thd->locked_tables)
@@ -196,10 +212,9 @@
     close_thread_tables(thd);
   }
 
-  /*
-    To avoid deadlocks we should obtain table locks before
-    obtaining servers_cache->lock mutex.
-  */
+  DBUG_PRINT("info", ("locking servers_cache"));
+  rw_wrlock(&THR_LOCK_servers);
+
   bzero((char*) tables, sizeof(tables));
   tables[0].alias= tables[0].table_name= (char*) "servers";
   tables[0].db= (char*) "mysql";
@@ -212,12 +227,6 @@
     goto end;
   }
 
-  DBUG_PRINT("info", ("locking servers_cache"));
-  VOID(pthread_mutex_lock(&servers_cache_mutex));
-
-  //old_servers_cache= servers_cache;
-  //old_mem=mem;
-
   if ((return_val= servers_load(thd, tables)))
   {					// Error. Revert to old list
     /* blast, for now, we have no servers, discuss later way to preserve */
@@ -226,14 +235,14 @@
     servers_free();
   }
 
-  DBUG_PRINT("info", ("unlocking servers_cache"));
-  VOID(pthread_mutex_unlock(&servers_cache_mutex));
-
 end:
   close_thread_tables(thd);
+  DBUG_PRINT("info", ("unlocking servers_cache"));
+  rw_unlock(&THR_LOCK_servers);
   DBUG_RETURN(return_val);
 }
 
+
 /*
   Initialize structures responsible for servers used in federated
   server scheme information for them from the server
@@ -260,7 +269,8 @@
     1	could not insert server struct into global servers cache
 */
 
-my_bool get_server_from_table_to_cache(TABLE *table)
+static bool 
+get_server_from_table_to_cache(TABLE *table)
 {
   /* alloc a server struct */
   char *ptr;
@@ -308,69 +318,6 @@
   DBUG_RETURN(FALSE);
 }
 
-/*
-  SYNOPSIS
-    server_exists_in_table()
-      THD   *thd - thread pointer
-      LEX_SERVER_OPTIONS *server_options - pointer to Lex->server_options
-
-  NOTES
-    This function takes a LEX_SERVER_OPTIONS struct, which is very much the
-    same type of structure as a FOREIGN_SERVER, it contains the values parsed
-    in any one of the [CREATE|DELETE|DROP] SERVER statements. Using the
-    member "server_name", index_read_idx either founds the record and returns
-    1, or doesn't find the record, and returns 0
-
-  RETURN VALUES
-    0   record not found
-    1	record found
-*/
-
-my_bool server_exists_in_table(THD *thd, LEX_SERVER_OPTIONS *server_options)
-{
-  int result= 1;
-  int error= 0;
-  TABLE_LIST tables;
-  TABLE *table;
-  DBUG_ENTER("server_exists");
-
-  bzero((char*) &tables, sizeof(tables));
-  tables.db= (char*) "mysql";
-  tables.alias= tables.table_name= (char*) "servers";
-
-  /* need to open before acquiring THR_LOCK_plugin or it will deadlock */
-  if (! (table= open_ltable(thd, &tables, TL_WRITE)))
-    DBUG_RETURN(TRUE);
-
-  table->use_all_columns();
-
-  rw_wrlock(&THR_LOCK_servers);
-  VOID(pthread_mutex_lock(&servers_cache_mutex));
-
-  /* set the field that's the PK to the value we're looking for */
-  table->field[0]->store(server_options->server_name,
-                         server_options->server_name_length,
-                         system_charset_info);
-
-  if ((error= table->file->index_read_idx(table->record[0], 0,
-                                   (byte *)table->field[0]->ptr,
-                                   table->key_info[0].key_length,
-                                   HA_READ_KEY_EXACT)))
-  {
-    if (error != HA_ERR_KEY_NOT_FOUND && error != HA_ERR_END_OF_FILE)
-    {
-      table->file->print_error(error, MYF(0));
-      result= -1;
-    }
-    result= 0;
-    DBUG_PRINT("info",("record for server '%s' not found!",
-                       server_options->server_name));
-  }
-
-  VOID(pthread_mutex_unlock(&servers_cache_mutex));
-  rw_unlock(&THR_LOCK_servers);
-  DBUG_RETURN(result);
-}
 
 /*
   SYNOPSIS
@@ -382,15 +329,18 @@
     This function takes a server object that is has all members properly
     prepared, ready to be inserted both into the mysql.servers table and
     the servers cache.
+	
+    THR_LOCK_servers must be write locked.
 
   RETURN VALUES
     0  - no error
     other - error code
 */
 
-int insert_server(THD *thd, FOREIGN_SERVER *server)
+static int 
+insert_server(THD *thd, FOREIGN_SERVER *server)
 {
-  int error= 0;
+  int error= -1;
   TABLE_LIST tables;
   TABLE *table;
 
@@ -402,13 +352,7 @@
 
   /* need to open before acquiring THR_LOCK_plugin or it will deadlock */
   if (! (table= open_ltable(thd, &tables, TL_WRITE)))
-    DBUG_RETURN(TRUE);
-
-  /* lock mutex to make sure no changes happen */
-  VOID(pthread_mutex_lock(&servers_cache_mutex));
-
-  /* lock table */
-  rw_wrlock(&THR_LOCK_servers);
+    goto end;
 
   /* insert the server into the table */
   if ((error= insert_server_record(table, server)))
@@ -419,12 +363,10 @@
     goto end;
 
 end:
-  /* unlock the table */
-  rw_unlock(&THR_LOCK_servers);
-  VOID(pthread_mutex_unlock(&servers_cache_mutex));
   DBUG_RETURN(error);
 }
 
+
 /*
   SYNOPSIS
     int insert_server_record_into_cache()
@@ -434,13 +376,16 @@
     This function takes a FOREIGN_SERVER pointer to an allocated (root mem)
     and inserts it into the global servers cache
 
+    THR_LOCK_servers must be write locked.
+
   RETURN VALUE
     0   - no error
     >0  - error code
 
 */
 
-int insert_server_record_into_cache(FOREIGN_SERVER *server)
+static int 
+insert_server_record_into_cache(FOREIGN_SERVER *server)
 {
   int error=0;
   DBUG_ENTER("insert_server_record_into_cache");
@@ -461,6 +406,7 @@
   DBUG_RETURN(error);
 }
 
+
 /*
   SYNOPSIS
     store_server_fields()
@@ -478,7 +424,8 @@
 
 */
 
-void store_server_fields(TABLE *table, FOREIGN_SERVER *server)
+static void 
+store_server_fields(TABLE *table, FOREIGN_SERVER *server)
 {
 
   table->use_all_columns();
@@ -539,6 +486,7 @@
 
   */
 
+static
 int insert_server_record(TABLE *table, FOREIGN_SERVER *server)
 {
   int error;
@@ -606,7 +554,7 @@
 
 int drop_server(THD *thd, LEX_SERVER_OPTIONS *server_options)
 {
-  int error= 0;
+  int error;
   TABLE_LIST tables;
   TABLE *table;
 
@@ -618,28 +566,33 @@
   tables.db= (char*) "mysql";
   tables.alias= tables.table_name= (char*) "servers";
 
-  /* need to open before acquiring THR_LOCK_plugin or it will deadlock */
-  if (! (table= open_ltable(thd, &tables, TL_WRITE)))
-    DBUG_RETURN(TRUE);
-
   rw_wrlock(&THR_LOCK_servers);
-  VOID(pthread_mutex_lock(&servers_cache_mutex));
 
+  /* hit the memory hit first */
+  if ((error= delete_server_record_in_cache(server_options)))
+    goto end;
 
-  if ((error= delete_server_record(table,
-                                   server_options->server_name,
-                                   server_options->server_name_length)))
+  if (! (table= open_ltable(thd, &tables, TL_WRITE)))
+  {
+    error= my_errno;
     goto end;
+  }
 
+  error= delete_server_record(table,
+                              server_options->server_name,
+                              server_options->server_name_length);
 
-  if ((error= delete_server_record_in_cache(server_options)))
-    goto end;
+  /*
+	Perform a reload so we don't have a 'hole' in our mem_root
+  */
+  servers_load(thd, &tables);
 
 end:
-  VOID(pthread_mutex_unlock(&servers_cache_mutex));
   rw_unlock(&THR_LOCK_servers);
   DBUG_RETURN(error);
 }
+
+
 /*
 
   SYNOPSIS
@@ -658,10 +611,10 @@
 
 */
 
-int delete_server_record_in_cache(LEX_SERVER_OPTIONS *server_options)
+static int 
+delete_server_record_in_cache(LEX_SERVER_OPTIONS *server_options)
 {
-
-  int error= 0;
+  int error= ER_FOREIGN_SERVER_DOESNT_EXIST;
   FOREIGN_SERVER *server;
   DBUG_ENTER("delete_server_record_in_cache");
 
@@ -677,7 +630,7 @@
     DBUG_PRINT("info", ("server_name %s length %d not found!",
                         server_options->server_name,
                         server_options->server_name_length));
-    // what should be done if not found in the cache?
+    goto end;
   }
   /*
     We succeded in deletion of the server to the table, now delete
@@ -687,14 +640,15 @@
                      server->server_name,
                      server->server_name_length));
 
-  if (server)
-    VOID(hash_delete(&servers_cache, (byte*) server));
-
-  servers_version++; /* servers updated */
+  VOID(hash_delete(&servers_cache, (byte*) server));
+  
+  error= 0;
 
+end:
   DBUG_RETURN(error);
 }
 
+
 /*
 
   SYNOPSIS
@@ -714,6 +668,8 @@
     table for the particular server via the call to update_server_record,
     and in the servers_cache via update_server_record_in_cache. 
 
+    THR_LOCK_servers must be write locked.
+
   RETURN VALUE
     0 - no error
     >0 - error code
@@ -722,7 +678,7 @@
 
 int update_server(THD *thd, FOREIGN_SERVER *existing, FOREIGN_SERVER *altered)
 {
-  int error= 0;
+  int error;
   TABLE *table;
   TABLE_LIST tables;
   DBUG_ENTER("update_server");
@@ -732,19 +688,26 @@
   tables.alias= tables.table_name= (char*)"servers";
 
   if (!(table= open_ltable(thd, &tables, TL_WRITE)))
-    DBUG_RETURN(1);
+  {
+    error= my_errno;
+    goto end;
+  }
 
-  rw_wrlock(&THR_LOCK_servers);
   if ((error= update_server_record(table, altered)))
     goto end;
 
-  update_server_record_in_cache(existing, altered);
+  error= update_server_record_in_cache(existing, altered);
+
+  /*
+	Perform a reload so we don't have a 'hole' in our mem_root
+  */
+  servers_load(thd, &tables);
 
 end:
-  rw_unlock(&THR_LOCK_servers);
   DBUG_RETURN(error);
 }
 
+
 /*
 
   SYNOPSIS
@@ -761,6 +724,8 @@
     HASH, then the updated record inserted, in essence replacing the old
     record.
 
+    THR_LOCK_servers must be write locked.
+
   RETURN VALUE
     0 - no error
     1 - error
@@ -791,13 +756,13 @@
   {
     DBUG_PRINT("info", ("had a problem inserting server %s at %lx",
                         altered->server_name, (long unsigned int) altered));
-    error= 1;
+    error= ER_OUT_OF_RESOURCES;
   }
 
-  servers_version++; /* servers updated */
   DBUG_RETURN(error);
 }
 
+
 /*
 
   SYNOPSIS
@@ -830,9 +795,9 @@
     to->password= strdup_root(&mem, from->password);
   if (to->port == -1)
     to->port= from->port;
-  if (!to->socket)
+  if (!to->socket && from->socket)
     to->socket= strdup_root(&mem, from->socket);
-  if (!to->scheme)
+  if (!to->scheme && from->scheme)
     to->scheme= strdup_root(&mem, from->scheme);
   if (!to->owner)
     to->owner= strdup_root(&mem, from->owner);
@@ -840,6 +805,7 @@
   DBUG_VOID_RETURN;
 }
 
+
 /*
 
   SYNOPSIS
@@ -862,7 +828,9 @@
 
 */
 
-int update_server_record(TABLE *table, FOREIGN_SERVER *server)
+
+static int 
+update_server_record(TABLE *table, FOREIGN_SERVER *server)
 {
   int error=0;
   DBUG_ENTER("update_server_record");
@@ -878,10 +846,7 @@
                                    HA_READ_KEY_EXACT)))
   {
     if (error != HA_ERR_KEY_NOT_FOUND && error != HA_ERR_END_OF_FILE)
-    {
       table->file->print_error(error, MYF(0));
-      error= 1;
-    }
     DBUG_PRINT("info",("server not found!"));
     error= ER_FOREIGN_SERVER_DOESNT_EXIST;
   }
@@ -901,6 +866,7 @@
   DBUG_RETURN(error);
 }
 
+
 /*
 
   SYNOPSIS
@@ -916,11 +882,11 @@
 
 */
 
-int delete_server_record(TABLE *table,
-                         char *server_name,
-                         int server_name_length)
+static int 
+delete_server_record(TABLE *table,
+                     char *server_name, int server_name_length)
 {
-  int error= 0;
+  int error;
   DBUG_ENTER("delete_server_record");
   table->use_all_columns();
 
@@ -933,10 +899,7 @@
                                    HA_READ_KEY_EXACT)))
   {
     if (error != HA_ERR_KEY_NOT_FOUND && error != HA_ERR_END_OF_FILE)
-    {
       table->file->print_error(error, MYF(0));
-      error= 1;
-    }
     DBUG_PRINT("info",("server not found!"));
     error= ER_FOREIGN_SERVER_DOESNT_EXIST;
   }
@@ -965,28 +928,35 @@
 
 int create_server(THD *thd, LEX_SERVER_OPTIONS *server_options)
 {
-  int error;
+  int error= ER_FOREIGN_SERVER_EXISTS;
   FOREIGN_SERVER *server;
 
   DBUG_ENTER("create_server");
   DBUG_PRINT("info", ("server_options->server_name %s",
                       server_options->server_name));
 
+  rw_wrlock(&THR_LOCK_servers);
+
+  /* hit the memory first */
+  if (hash_search(&servers_cache, (byte*) server_options->server_name,
+				   server_options->server_name_length))
+    goto end;
+
   server= (FOREIGN_SERVER *)alloc_root(&mem,
                                        sizeof(FOREIGN_SERVER));
 
-  if ((error= prepare_server_struct_for_insert(server_options, server)))
-    goto end;
+  prepare_server_struct_for_insert(server_options, server);
 
-  if ((error= insert_server(thd, server)))
-    goto end;
+  error= insert_server(thd, server);
 
   DBUG_PRINT("info", ("error returned %d", error));
 
 end:
+  rw_unlock(&THR_LOCK_servers);
   DBUG_RETURN(error);
 }
 
+
 /*
 
   SYNOPSIS
@@ -1003,37 +973,33 @@
 
 int alter_server(THD *thd, LEX_SERVER_OPTIONS *server_options)
 {
-  int error= 0;
+  int error= ER_FOREIGN_SERVER_DOESNT_EXIST;
   FOREIGN_SERVER *altered, *existing;
   DBUG_ENTER("alter_server");
   DBUG_PRINT("info", ("server_options->server_name %s",
                       server_options->server_name));
 
-  altered= (FOREIGN_SERVER *)alloc_root(&mem,
-                                        sizeof(FOREIGN_SERVER));
-
-  VOID(pthread_mutex_lock(&servers_cache_mutex));
+  rw_wrlock(&THR_LOCK_servers);
 
   if (!(existing= (FOREIGN_SERVER *) hash_search(&servers_cache,
                                                  (byte*) server_options->server_name,
                                                server_options->server_name_length)))
-  {
-    error= ER_FOREIGN_SERVER_DOESNT_EXIST;
     goto end;
-  }
 
-  if ((error= prepare_server_struct_for_update(server_options, existing, altered)))
-    goto end;
+  altered= (FOREIGN_SERVER *)alloc_root(&mem,
+                                        sizeof(FOREIGN_SERVER));
 
-  if ((error= update_server(thd, existing, altered)))
-    goto end;
+  prepare_server_struct_for_update(server_options, existing, altered);
+
+  error= update_server(thd, existing, altered);
 
 end:
   DBUG_PRINT("info", ("error returned %d", error));
-  VOID(pthread_mutex_unlock(&servers_cache_mutex));
+  rw_unlock(&THR_LOCK_servers);
   DBUG_RETURN(error);
 }
 
+
 /*
 
   SYNOPSIS
@@ -1044,19 +1010,17 @@
   NOTES
 
   RETURN VALUE
-    0 - no error
+    none
 
 */
 
-int prepare_server_struct_for_insert(LEX_SERVER_OPTIONS *server_options,
-                                     FOREIGN_SERVER *server)
+static void
+prepare_server_struct_for_insert(LEX_SERVER_OPTIONS *server_options,
+                                 FOREIGN_SERVER *server)
 {
-  int error;
   char *unset_ptr= (char*)"";
   DBUG_ENTER("prepare_server_struct");
 
-  error= 0;
-
   /* these two MUST be set */
   server->server_name= strdup_root(&mem, server_options->server_name);
   server->server_name_length= server_options->server_name_length;
@@ -1086,7 +1050,7 @@
   server->owner= server_options->owner ?
     strdup_root(&mem, server_options->owner) : unset_ptr;
 
-  DBUG_RETURN(error);
+  DBUG_VOID_RETURN;
 }
 
 /*
@@ -1102,13 +1066,12 @@
 
 */
 
-int prepare_server_struct_for_update(LEX_SERVER_OPTIONS *server_options,
-                                     FOREIGN_SERVER *existing,
-                                     FOREIGN_SERVER *altered)
+static void
+prepare_server_struct_for_update(LEX_SERVER_OPTIONS *server_options,
+                                 FOREIGN_SERVER *existing,
+                                 FOREIGN_SERVER *altered)
 {
-  int error;
   DBUG_ENTER("prepare_server_struct_for_update");
-  error= 0;
 
   altered->server_name= strdup_root(&mem, server_options->server_name);
   altered->server_name_length= server_options->server_name_length;
@@ -1159,7 +1122,7 @@
     (strcmp(server_options->owner, existing->owner))) ?
       strdup_root(&mem, server_options->owner) : 0;
 
-  DBUG_RETURN(error);
+  DBUG_VOID_RETURN;
 }
 
 /*
@@ -1178,17 +1141,15 @@
 void servers_free(bool end)
 {
   DBUG_ENTER("servers_free");
-  if (!servers_cache_initialised)
+  if (!hash_inited(&servers_cache))
     DBUG_VOID_RETURN;
-  VOID(pthread_mutex_destroy(&servers_cache_mutex));
-  servers_cache_initialised=0;
+  rwlock_destroy(&THR_LOCK_servers);
   free_root(&mem,MYF(0));
   hash_free(&servers_cache);
   DBUG_VOID_RETURN;
 }
 
 
-
 /*
 
   SYNOPSIS
@@ -1220,7 +1181,7 @@
   }
 
   DBUG_PRINT("info", ("locking servers_cache"));
-  VOID(pthread_mutex_lock(&servers_cache_mutex));
+  rw_rdlock(&THR_LOCK_servers);
   if (!(server= (FOREIGN_SERVER *) hash_search(&servers_cache,
                                                (byte*) server_name,
                                                server_name_length)))
@@ -1230,7 +1191,7 @@
     server= (FOREIGN_SERVER *) NULL;
   }
   DBUG_PRINT("info", ("unlocking servers_cache"));
-  VOID(pthread_mutex_unlock(&servers_cache_mutex));
+  rw_unlock(&THR_LOCK_servers);
   DBUG_RETURN(server);
 
 }

--- 1.4/sql/sql_servers.h	2007-03-23 17:31:39 -07:00
+++ 1.5/sql/sql_servers.h	2007-03-23 17:31:39 -07:00
@@ -25,40 +25,18 @@
 } FOREIGN_SERVER;
 
 /* cache handlers */
-my_bool servers_init(bool dont_read_server_table);
-my_bool servers_reload(THD *thd);
-my_bool get_server_from_table_to_cache(TABLE *table);
+bool servers_init(bool dont_read_server_table);
+bool servers_reload(THD *thd);
 void servers_free(bool end=0);
 
 /* insert functions */
 int create_server(THD *thd, LEX_SERVER_OPTIONS *server_options);
-int insert_server(THD *thd, FOREIGN_SERVER *server_options);
-int insert_server_record(TABLE *table, FOREIGN_SERVER *server);
-int insert_server_record_into_cache(FOREIGN_SERVER *server);
-void store_server_fields_for_insert(TABLE *table, FOREIGN_SERVER *server);
-void store_server_fields_for_insert(TABLE *table,
-                                    FOREIGN_SERVER *existing,
-                                    FOREIGN_SERVER *altered);
-int prepare_server_struct_for_insert(LEX_SERVER_OPTIONS *server_options,
-                                     FOREIGN_SERVER *server);
 
 /* drop functions */ 
 int drop_server(THD *thd, LEX_SERVER_OPTIONS *server_options);
-int delete_server_record(TABLE *table,
-                         char *server_name,
-                         int server_name_length);
-int delete_server_record_in_cache(LEX_SERVER_OPTIONS *server_options);
 
 /* update functions */
 int alter_server(THD *thd, LEX_SERVER_OPTIONS *server_options);
-int prepare_server_struct_for_update(LEX_SERVER_OPTIONS *server_options,
-                                     FOREIGN_SERVER *existing,
-                                     FOREIGN_SERVER *altered);
-int update_server(THD *thd, FOREIGN_SERVER *existing, FOREIGN_SERVER *altered);
-int update_server_record(TABLE *table, FOREIGN_SERVER *server);
-int update_server_record_in_cache(FOREIGN_SERVER *existing,
-                                  FOREIGN_SERVER *altered);
-/* utility functions */
-void merge_server_struct(FOREIGN_SERVER *from, FOREIGN_SERVER *to);
+
+/* lookup functions */
 FOREIGN_SERVER *get_server_by_name(const char *server_name);
-my_bool server_exists_in_table(THD *thd, char *server_name);
Thread
bk commit into 5.1 tree (acurtis:1.2498) BUG#25721antony24 Mar