Merge lp:~sergei.glushchenko/percona-server/ST-28169-Bug1127008 into lp:percona-server/5.5

[Sergei Glushchenko]

Bug 1127008 CREATE TABLE for a table that already exists does
not fail immediately
Upstream bug http://bugs.mysql.com/bug.php?id=63144
CREATE TABLE IF NOT EXISTS metadata lock is too restrictive.
Fix is just a port of https://mariadb.atlassian.net/browse/MDEV-3941
Original commit comment:
- Added option to check_if_table_exists() to quickly check if table exists (either SHARE or .FRM)
- Extended lock_table_names() to not wait for meta data locks if CREATE IF NOT EXISTS is used.
Also ported fix for https://mariadb.atlassian.net/browse/MDEV-4009
Fix for MDEV-4009: main.delayed sporadically fails with "query 'REPLACE DELAYED t1 VALUES (5)' failed: 1317: Query execution was interrupted"
- Fixed broadcast without a proper mutex
- Don't break existing locks if we are just testing if we can get the lock

[Sergei Glushchenko (sergei.glushchenko)]

http://jenkins.percona.com/view/PS%205.5/job/percona-server-5.5-param/679/

[Alexey Kopytov (akopytov)]

Looks good (and thanks for taking care of all the follow-up fixes). One
minor comment: I would follow our practice of creating testcases in
separate files (i.e. percona_bug#.test) instead of modifying existing
tests to reduce merge conflicts.

review: Needs Fixing

[Sergei Glushchenko (sergei.glushchenko)]

Change from create.test as been moved to separate testcase percona_bug1127008

[Sergei Glushchenko (sergei.glushchenko)]

http://jenkins.percona.com/view/PS%205.5/job/percona-server-5.5-param/702/

[Alexey Kopytov (akopytov)]

review: Approve

=== added file 'Percona-Server/mysql-test/r/create_delayed.result'
--- Percona-Server/mysql-test/r/create_delayed.result	1970-01-01 00:00:00 +0000
+++ Percona-Server/mysql-test/r/create_delayed.result	2013-03-18 11:52:26 +0000
@@ -0,0 +1,3 @@
+drop table if exists t1;
+Starting test
+# All done
 
=== added file 'Percona-Server/mysql-test/r/percona_bug1127008.result'
--- Percona-Server/mysql-test/r/percona_bug1127008.result	1970-01-01 00:00:00 +0000
+++ Percona-Server/mysql-test/r/percona_bug1127008.result	2013-03-18 11:52:26 +0000
@@ -0,0 +1,28 @@
+#
+# Checking that CREATE IF NOT EXISTS is not blocked by running SELECT
+#
+create table t1 (a int, b int) engine=myisam;
+create table t2 (a int, b int) engine=myisam;
+insert into t1 values (1,1);
+lock tables t1 read;
+set @@lock_wait_timeout=5;
+create table if not exists t1 (a int, b int);
+Warnings:
+Note	1050	Table 't1' already exists
+create table if not exists t1 (a int, b int) select 2,2;
+Warnings:
+Note	1050	Table 't1' already exists
+create table if not exists t1 like t2;
+Warnings:
+Note	1050	Table 't1' already exists
+create table t1 (a int, b int);
+ERROR 42S01: Table 't1' already exists
+create table t1 (a int, b int) select 2,2;
+ERROR 42S01: Table 't1' already exists
+create table t1 like t2;
+ERROR 42S01: Table 't1' already exists
+select * from t1;
+a	b
+1	1
+unlock tables;
+drop table t1,t2;
 
=== added file 'Percona-Server/mysql-test/t/create_delayed.test'
--- Percona-Server/mysql-test/t/create_delayed.test	1970-01-01 00:00:00 +0000
+++ Percona-Server/mysql-test/t/create_delayed.test	2013-03-18 11:52:26 +0000
@@ -0,0 +1,34 @@
+#
+# Ensure that INSERT DELAYED works with CREATE TABLE on existing table
+#
+
+-- source include/big_test.inc
+
+--disable_warnings
+drop table if exists t1;
+--enable_warnings
+
+--disable_query_log
+--disable_result_log
+
+--let $run=1000
+
+--echo Starting test
+
+while ($run)
+{
+# --echo # $run attempts left...
+  CREATE TABLE t1 ( f1 INTEGER AUTO_INCREMENT, PRIMARY KEY (f1)) ENGINE=MyISAM;
+  INSERT DELAYED t1 VALUES (4);
+--error ER_TABLE_EXISTS_ERROR
+  CREATE TABLE t1 AS SELECT 1 AS f1;
+
+  REPLACE DELAYED t1 VALUES (5);
+  DROP TABLE t1;
+--dec $run
+}
+
+--enable_query_log
+--enable_result_log
+
+--echo # All done
 
=== added file 'Percona-Server/mysql-test/t/percona_bug1127008.test'
--- Percona-Server/mysql-test/t/percona_bug1127008.test	1970-01-01 00:00:00 +0000
+++ Percona-Server/mysql-test/t/percona_bug1127008.test	2013-03-18 11:52:26 +0000
@@ -0,0 +1,28 @@
+######################################################################
+# Bug #1127008: CREATE TABLE for a table that already exists does not
+#               fail immediately 
+######################################################################
+
+--echo #
+--echo # Checking that CREATE IF NOT EXISTS is not blocked by running SELECT
+--echo #
+create table t1 (a int, b int) engine=myisam;
+create table t2 (a int, b int) engine=myisam;
+insert into t1 values (1,1);
+lock tables t1 read;
+connect (user1,localhost,root,,test);
+set @@lock_wait_timeout=5;
+create table if not exists t1 (a int, b int);
+create table if not exists t1 (a int, b int) select 2,2;
+create table if not exists t1 like t2;
+--error ER_TABLE_EXISTS_ERROR
+create table t1 (a int, b int);
+--error ER_TABLE_EXISTS_ERROR
+create table t1 (a int, b int) select 2,2;
+--error ER_TABLE_EXISTS_ERROR
+create table t1 like t2;
+disconnect user1;
+connection default;
+select * from t1;
+unlock tables;
+drop table t1,t2;
 
=== modified file 'Percona-Server/sql/mdl.cc'
--- Percona-Server/sql/mdl.cc	2012-08-07 06:18:36 +0000
+++ Percona-Server/sql/mdl.cc	2013-03-18 11:52:26 +0000
@@ -2075,7 +2075,11 @@
   */
   m_wait.reset_status();
 
-  if (lock->needs_notification(ticket))
+  /*
+    Don't break conflicting locks if timeout is 0 as 0 is used
+    To check if there is any conflicting locks...
+  */
+  if (lock->needs_notification(ticket) && lock_wait_timeout)
     lock->notify_conflicting_locks(this);
 
   mysql_prlock_unlock(&lock->m_rwlock);
 
=== modified file 'Percona-Server/sql/sql_base.cc'
--- Percona-Server/sql/sql_base.cc	2013-02-18 04:48:10 +0000
+++ Percona-Server/sql/sql_base.cc	2013-03-18 11:52:26 +0000
@@ -2358,10 +2358,11 @@
     Check that table exists in table definition cache, on disk
     or in some storage engine.
 
-    @param       thd     Thread context
-    @param       table   Table list element
-    @param[out]  exists  Out parameter which is set to TRUE if table
-                         exists and to FALSE otherwise.
+    @param       thd        Thread context
+    @param       table      Table list element
+    @param       fast_check Check only if share or .frm file exists 
+    @param[out]  exists     Out parameter which is set to TRUE if table
+                            exists and to FALSE otherwise.
 
     @note This function acquires LOCK_open internally.
 
@@ -2373,7 +2374,8 @@
     @retval  FALSE  No error. 'exists' out parameter set accordingly.
 */
 
-bool check_if_table_exists(THD *thd, TABLE_LIST *table, bool *exists)
+bool check_if_table_exists(THD *thd, TABLE_LIST *table, bool fast_check,
+                           bool *exists)
 {
   char path[FN_REFLEN + 1];
   TABLE_SHARE *share;
@@ -2381,7 +2383,8 @@
 
   *exists= TRUE;
 
-  DBUG_ASSERT(thd->mdl_context.
+  DBUG_ASSERT(fast_check ||
+              thd->mdl_context.
               is_lock_owner(MDL_key::TABLE, table->db,
                             table->table_name, MDL_SHARED));
 
@@ -2398,6 +2401,12 @@
   if (!access(path, F_OK))
     goto end;
 
+  if (fast_check)
+  {
+    *exists= FALSE;
+    goto end;
+  }
+
   /* .FRM file doesn't exist. Check if some engine can provide it. */
   if (ha_check_if_table_exists(thd, table->db, table->table_name, exists))
   {
@@ -2947,7 +2956,7 @@
   {
     bool exists;
 
-    if (check_if_table_exists(thd, table_list, &exists))
+    if (check_if_table_exists(thd, table_list, 0, &exists))
       DBUG_RETURN(TRUE);
 
     if (!exists)
@@ -4653,7 +4662,18 @@
                            open, see open_table() description for details.
 
   @retval FALSE  Success.
-  @retval TRUE   Failure (e.g. connection was killed)
+  @retval TRUE   Failure (e.g. connection was killed) or table existed
+	         for a CREATE TABLE.
+
+  @notes
+  In case of CREATE TABLE we avoid a wait for tables that are in use
+  by first trying to do a meta data lock with timeout == 0.  If we get a
+  timeout we will check if table exists (it should) and retry with
+  normal timeout if it didn't exists.
+  Note that for CREATE TABLE IF EXISTS we only generate a warning
+  but still return TRUE (to abort the calling open_table() function).
+  On must check THD->is_error() if one wants to distinguish between warning
+  and error.
 */
 
 bool
@@ -4665,6 +4685,10 @@
   TABLE_LIST *table;
   MDL_request global_request;
   Hash_set<TABLE_LIST, schema_set_get_key> schema_set;
+  ulong org_lock_wait_timeout= lock_wait_timeout;
+  /* Check if we are using CREATE TABLE ... IF NOT EXISTS */
+  bool create_table;
+  Dummy_error_handler error_handler;
 
   DBUG_ASSERT(!thd->locked_tables_mode);
 
@@ -4685,8 +4709,14 @@
     }
   }
 
-  if (! (flags & MYSQL_OPEN_SKIP_SCOPED_MDL_LOCK) &&
-      ! mdl_requests.is_empty())
+  if (mdl_requests.is_empty())
+    return FALSE;
+
+  /* Check if CREATE TABLE IF NOT EXISTS was used */
+  create_table= (tables_start && tables_start->open_strategy ==
+                 TABLE_LIST::OPEN_IF_EXISTS);
+
+  if (!(flags & MYSQL_OPEN_SKIP_SCOPED_MDL_LOCK))
   {
     /*
       Scoped locks: Take intention exclusive locks on all involved
@@ -4714,12 +4744,58 @@
     global_request.init(MDL_key::GLOBAL, "", "", MDL_INTENTION_EXCLUSIVE,
                         MDL_STATEMENT);
     mdl_requests.push_front(&global_request);
-  }
-
-  if (thd->mdl_context.acquire_locks(&mdl_requests, lock_wait_timeout))
-    return TRUE;
-
-  return FALSE;
+
+    if (create_table)
+      lock_wait_timeout= 0;                     // Don't wait for timeout
+  }
+
+  for (;;)
+  {
+    bool exists= TRUE;
+    bool res;
+
+    if (create_table)
+      thd->push_internal_handler(&error_handler);  // Avoid warnings & errors
+    res= thd->mdl_context.acquire_locks(&mdl_requests, lock_wait_timeout);
+    if (create_table)
+      thd->pop_internal_handler();
+    if (!res)
+      return FALSE;                             // Got locks
+
+    if (!create_table)
+      return TRUE;                              // Return original error
+
+    /*
+      We come here in the case of lock timeout when executing
+      CREATE TABLE IF NOT EXISTS.
+      Verify that table really exists (it should as we got a lock conflict)
+    */
+    if (check_if_table_exists(thd, tables_start, 1, &exists))
+      return TRUE;                              // Should never happen
+    if (exists)
+    {
+      if (thd->lex->create_info.options & HA_LEX_CREATE_IF_NOT_EXISTS)
+      {
+        push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_NOTE,
+                            ER_TABLE_EXISTS_ERROR, ER(ER_TABLE_EXISTS_ERROR),
+                            tables_start->table_name);
+      }
+      else
+        my_error(ER_TABLE_EXISTS_ERROR, MYF(0), tables_start->table_name);
+      return TRUE;
+    }
+    /* purecov: begin inspected */
+    /*
+      We got error from acquire_locks but table didn't exists.
+      In theory this should never happen, except maybe in
+      CREATE or DROP DATABASE scenario.
+      We play safe and restart the original acquire_locks with the
+      original timeout
+    */
+    create_table= 0;
+    lock_wait_timeout= org_lock_wait_timeout;
+    /* purecov: end */
+  }
 }
 
 
@@ -8912,7 +8988,11 @@
     in_use->killed= THD::KILL_CONNECTION;
     mysql_mutex_lock(&in_use->mysys_var->mutex);
     if (in_use->mysys_var->current_cond)
+    {
+      mysql_mutex_lock(in_use->mysys_var->current_mutex);
       mysql_cond_broadcast(in_use->mysys_var->current_cond);
+      mysql_mutex_unlock(in_use->mysys_var->current_mutex);
+    }
     mysql_mutex_unlock(&in_use->mysys_var->mutex);
     signalled= TRUE;
   }
 
=== modified file 'Percona-Server/sql/sql_base.h'
--- Percona-Server/sql/sql_base.h	2012-12-11 18:04:30 +0000
+++ Percona-Server/sql/sql_base.h	2013-03-18 11:52:26 +0000
@@ -319,7 +319,8 @@
                                   const char *table_name,
                                   bool no_error);
 void mark_tmp_table_for_reuse(TABLE *table);
-bool check_if_table_exists(THD *thd, TABLE_LIST *table, bool *exists);
+bool check_if_table_exists(THD *thd, TABLE_LIST *table, bool fast_check,
+                           bool *exists);
 
 extern TABLE *unused_tables;
 extern Item **not_found_item;
 
=== modified file 'Percona-Server/sql/sql_db.cc'
--- Percona-Server/sql/sql_db.cc	2013-01-09 23:45:25 +0000
+++ Percona-Server/sql/sql_db.cc	2013-03-18 11:52:26 +0000
@@ -924,7 +924,7 @@
       bool exists;
 
       // Only write drop table to the binlog for tables that no longer exist.
-      if (check_if_table_exists(thd, tbl, &exists))
+      if (check_if_table_exists(thd, tbl, 0, &exists))
       {
         error= true;
         goto exit;
 
=== modified file 'Percona-Server/sql/sql_insert.cc'
--- Percona-Server/sql/sql_insert.cc	2013-01-09 23:45:25 +0000
+++ Percona-Server/sql/sql_insert.cc	2013-03-18 11:52:26 +0000
@@ -2748,8 +2748,12 @@
         set_timespec(abstime, delayed_insert_timeout);
 
         /* Information for pthread_kill */
+        mysql_mutex_unlock(&di->mutex);
+        mysql_mutex_lock(&di->thd.mysys_var->mutex);
         di->thd.mysys_var->current_mutex= &di->mutex;
         di->thd.mysys_var->current_cond= &di->cond;
+        mysql_mutex_unlock(&di->thd.mysys_var->mutex);
+        mysql_mutex_lock(&di->mutex);
         thd_proc_info(&(di->thd), "Waiting for INSERT");
 
         DBUG_PRINT("info",("Waiting for someone to insert rows"));
 
=== modified file 'Percona-Server/sql/sql_parse.cc'
--- Percona-Server/sql/sql_parse.cc	2013-03-05 12:16:18 +0000
+++ Percona-Server/sql/sql_parse.cc	2013-03-18 11:52:26 +0000
@@ -2677,7 +2677,14 @@
         goto end_with_restore_list;
       }
 
-      if (!(res= open_and_lock_tables(thd, lex->query_tables, TRUE, 0)))
+      res= open_and_lock_tables(thd, lex->query_tables, TRUE, 0);
+      if (res)
+      {
+        /* Got error or warning. Set res to 1 if error */
+        if (!(res= thd->is_error()))
+          my_ok(thd);                           // CREATE ... IF NOT EXISTS
+      }
+      else
       {
         /* The table already exists */
         if (create_table->table)
 
=== modified file 'Percona-Server/sql/sql_table.cc'
--- Percona-Server/sql/sql_table.cc	2013-02-18 04:48:10 +0000
+++ Percona-Server/sql/sql_table.cc	2013-03-18 11:52:26 +0000
@@ -4428,7 +4428,8 @@
   */
   if (open_and_lock_tables(thd, thd->lex->query_tables, FALSE, 0))
   {
-    result= TRUE;
+    /* is_error() may be 0 if table existed and we generated a warning */
+    result= thd->is_error();
     goto end;
   }
 
@@ -4623,7 +4624,10 @@
     properly isolated from all concurrent operations which matter.
   */
   if (open_tables(thd, &thd->lex->query_tables, &not_used, 0))
+  {
+    res= thd->is_error();
     goto err;
+  }
   src_table->table->use_all_columns();
 
   DEBUG_SYNC(thd, "create_table_like_after_open");