bk commit into 5.0 tree (antony:1.2390) BUG#25679

[antony]

Below is the list of changes that have just been committed into a local
5.0 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-01-31 16:07:55-08:00, antony@stripped +10 -0
  Bug#25679
    "Deadlock entire server with self-referencing FEDERATED"
    Deadlock caused by LOCK_open being held by federated table creation.
    Ensure that federated table is not accessible until after storage
    engine create_table() method completes with new HTON_SLOW_CREATE flag.
    Includes test for bug.

  mysql-test/r/federated.result@stripped, 2007-01-31 16:07:51-08:00, antony@stripped +26 -0
    test for deadlock bog 25679

  mysql-test/t/federated.test@stripped, 2007-01-31 16:07:51-08:00, antony@stripped +27 -0
    test for deadlock bog 25679

  sql/ha_federated.cc@stripped, 2007-01-31 16:07:52-08:00, antony@stripped +8 -1
    Set HTON_SLOW_CREATE for federated.
    Add some comment for explanation.

  sql/handler.cc@stripped, 2007-01-31 16:07:52-08:00, antony@stripped +1 -1
    pass real frm path

  sql/handler.h@stripped, 2007-01-31 16:07:52-08:00, antony@stripped +2 -0
    New handlerton flag
      HTON_SLOW_CREATE for storage engines who are slow at creating tables
    New HA_CREATE_INFO field
      frm_path - contains path to real frm file

  sql/mysql_priv.h@stripped, 2007-01-31 16:07:52-08:00, antony@stripped +2 -1
    New mutex:
      LOCK_create
    New optional frm_path arg for openfrm()

  sql/mysqld.cc@stripped, 2007-01-31 16:07:52-08:00, antony@stripped +3 -0
    Initialize new mutex LOCK_create

  sql/sql_table.cc@stripped, 2007-01-31 16:07:52-08:00, antony@stripped +5 -2
    lock LOCK_create during table creation operation, so that slow storage
    engines can optionally unlock LOCK_open (iff they have HTON_SLOW_CREATE)
    This prevents table creation from causing the server to a standstill or
    deadlock in case of self-referencing federated access.

  sql/table.cc@stripped, 2007-01-31 16:07:53-08:00, antony@stripped +4 -2
    optional frm_path arg for openfrm
    Use frm_path if supplied to open frm file

  sql/unireg.cc@stripped, 2007-01-31 16:07:53-08:00, antony@stripped +29 -3
    If a storage engine has HTON_SLOW_CREATE, we create the frm file with
    a $ suffix so that other connections cannot yet open the table. We also
    unlock LOCK_open for the duration of the storage engine create table.
    After table creation is successful, we rename the frm file.

# 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:	antony
# Host:	ppcg5.local
# Root:	/Users/antony/Work/p2-bug25679.1

--- 1.225/sql/handler.cc	2007-01-31 16:08:03 -08:00
+++ 1.226/sql/handler.cc	2007-01-31 16:08:03 -08:00
@@ -2215,7 +2215,7 @@ int ha_create_table(const char *name, HA
   char name_buff[FN_REFLEN];
   DBUG_ENTER("ha_create_table");
 
-  if (openfrm(current_thd, name,"",0,(uint) READ_ALL, 0, &table))
+  if (openfrm(current_thd, name,"",0,(uint) READ_ALL, 0, &table,
create_info->frm_path))
     DBUG_RETURN(1);
   if (update_create_info)
   {

--- 1.182/sql/handler.h	2007-01-31 16:08:03 -08:00
+++ 1.183/sql/handler.h	2007-01-31 16:08:03 -08:00
@@ -409,6 +409,7 @@ struct show_table_alias_st {
 #define HTON_ALTER_NOT_SUPPORTED     (1 << 1) //Engine does not support alter
 #define HTON_CAN_RECREATE            (1 << 2) //Delete all is used fro truncate
 #define HTON_HIDDEN                  (1 << 3) //Engine does not appear in lists
+#define HTON_SLOW_CREATE             (1 << 4) //Slow table creation, use tmp frm
 
 typedef struct st_thd_trans
 {
@@ -431,6 +432,7 @@ typedef struct st_ha_create_information
   const char *password;
   const char *data_file_name, *index_file_name;
   const char *alias;
+  const char *frm_path;                 /* path to FRM file */
   ulonglong max_rows,min_rows;
   ulonglong auto_increment_value;
   ulong table_options;

--- 1.431/sql/mysql_priv.h	2007-01-31 16:08:03 -08:00
+++ 1.432/sql/mysql_priv.h	2007-01-31 16:08:03 -08:00
@@ -1279,6 +1279,7 @@ extern int bootstrap_error;
 extern FILE *stderror_file;
 extern pthread_key(MEM_ROOT**,THR_MALLOC);
 extern pthread_mutex_t LOCK_mysql_create_db,LOCK_Acl,LOCK_open,
+       LOCK_create,
        LOCK_thread_count,LOCK_mapped_file,LOCK_user_locks, LOCK_status,
        LOCK_error_log, LOCK_delayed_insert, LOCK_uuid_generator,
        LOCK_delayed_status, LOCK_delayed_create, LOCK_crypt, LOCK_timezone,
@@ -1439,7 +1440,7 @@ int rea_create_table(THD *thd, my_string
 int format_number(uint inputflag,uint max_length,my_string pos,uint length,
 		  my_string *errpos);
 int openfrm(THD *thd, const char *name,const char *alias,uint filestat,
-            uint prgflag, uint ha_open_flags, TABLE *outparam);
+            uint prgflag, uint ha_open_flags, TABLE *outparam, const char *frm_path=0);
 int readfrm(const char *name, const void** data, uint* length);
 int writefrm(const char* name, const void* data, uint len);
 int closefrm(TABLE *table);

--- 1.591/sql/mysqld.cc	2007-01-31 16:08:03 -08:00
+++ 1.592/sql/mysqld.cc	2007-01-31 16:08:03 -08:00
@@ -502,6 +502,7 @@ SHOW_COMP_OPTION have_crypt, have_compre
 pthread_key(MEM_ROOT**,THR_MALLOC);
 pthread_key(THD*, THR_THD);
 pthread_mutex_t LOCK_mysql_create_db, LOCK_Acl, LOCK_open, LOCK_thread_count,
+                LOCK_create,
 		LOCK_mapped_file, LOCK_status, LOCK_global_read_lock,
 		LOCK_error_log, LOCK_uuid_generator,
 		LOCK_delayed_insert, LOCK_delayed_status, LOCK_delayed_create,
@@ -1225,6 +1226,7 @@ static void clean_up_mutexes()
   (void) pthread_mutex_destroy(&LOCK_Acl);
   (void) rwlock_destroy(&LOCK_grant);
   (void) pthread_mutex_destroy(&LOCK_open);
+  (void) pthread_mutex_destroy(&LOCK_create);
   (void) pthread_mutex_destroy(&LOCK_thread_count);
   (void) pthread_mutex_destroy(&LOCK_mapped_file);
   (void) pthread_mutex_destroy(&LOCK_status);
@@ -2834,6 +2836,7 @@ static int init_thread_environment()
   (void) pthread_mutex_init(&LOCK_mysql_create_db,MY_MUTEX_INIT_SLOW);
   (void) pthread_mutex_init(&LOCK_Acl,MY_MUTEX_INIT_SLOW);
   (void) pthread_mutex_init(&LOCK_open,MY_MUTEX_INIT_FAST);
+  (void) pthread_mutex_init(&LOCK_create,MY_MUTEX_INIT_FAST);
   (void) pthread_mutex_init(&LOCK_thread_count,MY_MUTEX_INIT_FAST);
   (void) pthread_mutex_init(&LOCK_mapped_file,MY_MUTEX_INIT_SLOW);
   (void) pthread_mutex_init(&LOCK_status,MY_MUTEX_INIT_FAST);

--- 1.333/sql/sql_table.cc	2007-01-31 16:08:03 -08:00
+++ 1.334/sql/sql_table.cc	2007-01-31 16:08:03 -08:00
@@ -1698,7 +1698,7 @@ bool mysql_create_table(THD *thd,const c
     my_error(ER_TABLE_EXISTS_ERROR, MYF(0), alias);
     DBUG_RETURN(TRUE);
   }
-  VOID(pthread_mutex_lock(&LOCK_open));
+  VOID(pthread_mutex_lock(&LOCK_create));
   if (!internal_tmp_table && !(create_info->options &
HA_LEX_CREATE_TMP_TABLE))
   {
     if (!access(path,F_OK))
@@ -1706,7 +1706,7 @@ bool mysql_create_table(THD *thd,const c
       if (create_info->options & HA_LEX_CREATE_IF_NOT_EXISTS)
         goto warn;
       my_error(ER_TABLE_EXISTS_ERROR,MYF(0),table_name);
-      goto end;
+      goto fail;
     }
   }
 
@@ -1719,6 +1719,7 @@ bool mysql_create_table(THD *thd,const c
     one else is attempting to discover the table. Since
     it's not on disk as a frm file, no one could be using it!
   */
+  VOID(pthread_mutex_lock(&LOCK_open));
   if (!(create_info->options & HA_LEX_CREATE_TMP_TABLE))
   {
     bool create_if_not_exists =
@@ -1765,6 +1766,8 @@ bool mysql_create_table(THD *thd,const c
 
 end:
   VOID(pthread_mutex_unlock(&LOCK_open));
+fail:
+  VOID(pthread_mutex_unlock(&LOCK_create));
   thd->proc_info="After create";
   DBUG_RETURN(error);
 

--- 1.241/sql/table.cc	2007-01-31 16:08:03 -08:00
+++ 1.242/sql/table.cc	2007-01-31 16:08:03 -08:00
@@ -50,6 +50,7 @@ static byte* get_field_name(Field **buff
     prgflag        READ_ALL etc..
     ha_open_flags  HA_OPEN_ABORT_IF_LOCKED etc..
     outparam       result table
+    frm_path       real path to frm file
 
   RETURN VALUES
    0	ok
@@ -62,7 +63,7 @@ static byte* get_field_name(Field **buff
 */
 
 int openfrm(THD *thd, const char *name, const char *alias, uint db_stat,
-            uint prgflag, uint ha_open_flags, TABLE *outparam)
+            uint prgflag, uint ha_open_flags, TABLE *outparam, const char *frm_path)
 {
   reg1 uint i;
   reg2 uchar *strpos;
@@ -98,7 +99,8 @@ int openfrm(THD *thd, const char *name, 
   outparam->in_use= thd;
   outparam->s= share= &outparam->share_not_to_be_used;
 
-  if ((file=my_open(fn_format(index_file, name, "", reg_ext,
+  if ((file=my_open(frm_path ? frm_path :
+                    fn_format(index_file, name, "", reg_ext,
 			      MY_UNPACK_FILENAME),
 		    O_RDONLY | O_SHARE,
 		    MYF(0)))

--- 1.80/sql/unireg.cc	2007-01-31 16:08:03 -08:00
+++ 1.81/sql/unireg.cc	2007-01-31 16:08:03 -08:00
@@ -281,16 +281,42 @@ int rea_create_table(THD *thd, my_string
 		     List<create_field> &create_fields,
 		     uint keys, KEY *key_info)
 {
+  char tmppath[FN_REFLEN], *frm_path= file_name;
+  bool slow, fail;
   DBUG_ENTER("rea_create_table");
 
-  if (mysql_create_frm(thd, file_name, db, table, create_info,
+  if ((slow= !create_info->frm_only && 
+       ha_check_storage_engine_flag(create_info->db_type,HTON_SLOW_CREATE)))
+  {
+    /* append a $ to the name to make table inaccessible for now */
+    my_snprintf(tmppath, sizeof(tmppath), "%s$", file_name);
+    frm_path= tmppath;
+  }
+  create_info->frm_path= frm_path;
+
+  if (mysql_create_frm(thd, frm_path, db, table, create_info,
                        create_fields, keys, key_info, NULL))
     DBUG_RETURN(1);
-  if (!create_info->frm_only && ha_create_table(file_name,create_info,0))
+
+  /*
+    The following operation may be slow: We don't want to prevent other
+    threads from opening tables.
+  */ 
+  if (slow)
+    VOID(pthread_mutex_unlock(&LOCK_open));
+
+  fail= !create_info->frm_only && ha_create_table(file_name,create_info,0);
+
+  if (slow)
+    VOID(pthread_mutex_lock(&LOCK_open));
+
+  /* Now we atomically rename file as if neccessary */
+  if (fail || (slow && my_rename(frm_path, file_name, MYF(MY_WME))))
   {
-    my_delete(file_name,MYF(0));
+    my_delete(frm_path,MYF(0));
     DBUG_RETURN(1);
   }
+  
   DBUG_RETURN(0);
 } /* rea_create_table */
 

--- 1.40/mysql-test/r/federated.result	2007-01-31 16:08:03 -08:00
+++ 1.41/mysql-test/r/federated.result	2007-01-31 16:08:03 -08:00
@@ -1843,6 +1843,32 @@ C3A4C3B6C3BCC39F
 D18DD184D184D0B5D0BAD182D0B8D0B2D0BDD183D18E
 drop table federated.t1;
 drop table federated.t1;
+create table federated.t1 (a int not null, b varchar(64), primary key (a))
+ENGINE=MYISAM DEFAULT CHARSET=utf8;
+create table federated.t2 (a int not null, b varchar(64), primary key (a))
+ENGINE=FEDERATED
+connection='mysql://root@stripped:12000/federated/t1'
+DEFAULT CHARSET=utf8;
+insert into federated.t1 (a,b) values (1, "foo");
+insert into federated.t1 (a,b) values (2, "bar");
+insert into federated.t2 (a,b) values (3, "hello");
+flush tables;
+select * from federated.t2 where a = 1;
+a	b
+1	foo
+select * from federated.t2 where a = 2;
+a	b
+2	bar
+select * from federated.t1 where a = 3;
+a	b
+3	hello
+create table federated.t3 (a int not null, b varchar(64), primary key (a))
+ENGINE=FEDERATED
+connection='mysql://root@stripped:12000/federated/t3'
+DEFAULT CHARSET=utf8;
+ERROR HY000: Can't create federated table. Foreign data src error:  error: 1146  'Table
'federated.t3' doesn't exist'
+drop table federated.t2;
+drop table federated.t1;
 DROP TABLE IF EXISTS federated.t1;
 DROP DATABASE IF EXISTS federated;
 DROP TABLE IF EXISTS federated.t1;

--- 1.35/mysql-test/t/federated.test	2007-01-31 16:08:03 -08:00
+++ 1.36/mysql-test/t/federated.test	2007-01-31 16:08:03 -08:00
@@ -1575,5 +1575,32 @@ drop table federated.t1;
 connection slave;
 drop table federated.t1;
 
+#
+#  BUG#25679 Deadlock when connecting to self
+#
+connection master;
+create table federated.t1 (a int not null, b varchar(64), primary key (a))
+ENGINE=MYISAM DEFAULT CHARSET=utf8;
+eval create table federated.t2 (a int not null, b varchar(64), primary key (a))
+ENGINE=FEDERATED
+connection='mysql://root@stripped:$MASTER_MYPORT/federated/t1'
+DEFAULT CHARSET=utf8;
+# check that we are looking at the same table
+insert into federated.t1 (a,b) values (1, "foo");
+insert into federated.t1 (a,b) values (2, "bar");
+insert into federated.t2 (a,b) values (3, "hello");
+flush tables;
+select * from federated.t2 where a = 1;
+select * from federated.t2 where a = 2;
+select * from federated.t1 where a = 3;
+# Check infinite recursion
+--error ER_CANT_CREATE_FEDERATED_TABLE
+eval create table federated.t3 (a int not null, b varchar(64), primary key (a))
+ENGINE=FEDERATED
+connection='mysql://root@stripped:$MASTER_MYPORT/federated/t3'
+DEFAULT CHARSET=utf8;
+drop table federated.t2;
+drop table federated.t1;
+
 
 source include/federated_cleanup.inc;

--- 1.73/sql/ha_federated.cc	2007-01-31 16:08:03 -08:00
+++ 1.74/sql/ha_federated.cc	2007-01-31 16:08:03 -08:00
@@ -372,7 +372,7 @@ handlerton federated_hton= {
   NULL,    /* create_cursor_read_view */
   NULL,    /* set_cursor_read_view */
   NULL,    /* close_cursor_read_view */
-  HTON_ALTER_NOT_SUPPORTED
+  HTON_ALTER_NOT_SUPPORTED | HTON_SLOW_CREATE
 };
 
 
@@ -2603,7 +2603,14 @@ int ha_federated::create(const char *nam
   DBUG_ENTER("ha_federated::create");
 
   if (!(retval= parse_url(&tmp_share, table_arg, 1)))
+  {
+    /*
+      The following operation may be slow: 
+      We must have HTON_SLOW_CREATE flagged so that other threads cannot
+      open the table until we are done but they are able to open other tables
+    */ 
     retval= check_foreign_data_source(&tmp_share, 1);
+  }
 
   my_free((gptr) tmp_share.scheme, MYF(MY_ALLOW_ZERO_PTR));
   DBUG_RETURN(retval);