bzr commit into mysql-5.1 branch (tomas.ulin:2710) Bug#40246

[Tomas Ulin]

#At file:///home/tomas/mysql_src/mysql-5.1-telco-6.2-merge/

 2710 Tomas Ulin	2008-10-29
      Bug#40246 - alter when table is locked does not work
added:
  mysql-test/suite/ndb/r/ndb_dbug_lock.result
  mysql-test/suite/ndb/t/ndb_dbug_lock.test
modified:
  sql/ha_ndbcluster.cc
  sql/ha_ndbcluster_binlog.cc
  sql/ha_ndbcluster_binlog.h
  sql/handler.cc
  sql/handler.h
  sql/sql_table.cc

=== added file 'mysql-test/suite/ndb/r/ndb_dbug_lock.result'
--- a/mysql-test/suite/ndb/r/ndb_dbug_lock.result	1970-01-01 00:00:00 +0000
+++ b/mysql-test/suite/ndb/r/ndb_dbug_lock.result	2008-10-29 13:09:15 +0000
@@ -0,0 +1,44 @@
+drop table if exists t1;
+#
+# Test that alter during lock table aborts with error if
+# global schema lock already exists
+#
+create table t1 (a int key) engine ndb row_format dynamic;
+set session debug="+d,sleep_after_global_schema_lock";
+alter table t1 add column (b int);
+lock tables t1 write;
+alter table t1 add column (c int);
+ERROR HY000: Can't execute the given command because you have active locked tables or an
active transaction
+alter table t1 add column (c int);
+ERROR HY000: Can't execute the given command because you have active locked tables or an
active transaction
+alter table t1 add column (c int);
+ERROR HY000: Can't execute the given command because you have active locked tables or an
active transaction
+unlock tables;
+#
+# Test that alter during lock works without global schema lock
+# and that an alter done in parallell test serialized
+#
+lock tables t1 write;
+# delay is still set after lock
+alter table t1 add column (c int);
+# issue alter in parallell, which should be hanging waiting on
+alter table t1 add column (d int);
+# check thread state which should be:
+# "Waiting for allowed to take ndbcluster global schema lock"
+# _not_ "Waiting for ndbcluster global schema lock"
+select state from information_schema.processlist where info = "alter table t1 add column
(d int)";
+state
+Waiting for allowed to take ndbcluster global schema lock
+unlock tables;
+show create table t1;
+Table	Create Table
+t1	CREATE TABLE `t1` (
+  `a` int(11) NOT NULL,
+  `b` int(11) DEFAULT NULL,
+  `c` int(11) DEFAULT NULL,
+  `d` int(11) DEFAULT NULL,
+  PRIMARY KEY (`a`)
+) ENGINE=ndbcluster DEFAULT CHARSET=latin1 ROW_FORMAT=DYNAMIC
+# Cleanup
+set session debug="-d,sleep_after_global_schema_lock";
+drop table t1;

=== added file 'mysql-test/suite/ndb/t/ndb_dbug_lock.test'
--- a/mysql-test/suite/ndb/t/ndb_dbug_lock.test	1970-01-01 00:00:00 +0000
+++ b/mysql-test/suite/ndb/t/ndb_dbug_lock.test	2008-10-29 13:09:15 +0000
@@ -0,0 +1,70 @@
+-- source include/have_ndb.inc
+# We are using some debug-only features in this test
+--source include/have_debug.inc
+
+--disable_warnings
+drop table if exists t1;
+--enable_warnings
+
+--connect (another_con, localhost, root,,)
+--connect (yet_another_con, localhost, root,,)
+
+--echo #
+--echo # Test that alter during lock table aborts with error if
+--echo # global schema lock already exists
+--echo #
+--connection default
+create table t1 (a int key) engine ndb row_format dynamic;
+# delay after lock
+set session debug="+d,sleep_after_global_schema_lock";
+--send alter table t1 add column (b int)
+--sleep 2
+
+--connection another_con
+lock tables t1 write;
+--error ER_LOCK_OR_ACTIVE_TRANSACTION
+alter table t1 add column (c int);
+# make sure it continues to block
+--error ER_LOCK_OR_ACTIVE_TRANSACTION
+alter table t1 add column (c int);
+--error ER_LOCK_OR_ACTIVE_TRANSACTION
+alter table t1 add column (c int);
+unlock tables;
+
+--connection default
+--reap
+
+--echo #
+--echo # Test that alter during lock works without global schema lock
+--echo # and that an alter done in parallell test serialized
+--echo #
+--connection default
+lock tables t1 write;
+--echo # delay is still set after lock
+--send alter table t1 add column (c int)
+--sleep 2
+
+--echo # issue alter in parallell, which should be hanging waiting on
+--connection another_con
+--send alter table t1 add column (d int)
+
+--echo # check thread state which should be:
+--echo # "Waiting for allowed to take ndbcluster global schema lock"
+--echo # _not_ "Waiting for ndbcluster global schema lock"
+--connection yet_another_con
+select state from information_schema.processlist where info = "alter table t1 add column
(d int)";
+
+# wait for alter to complete
+--connection default
+--reap
+unlock tables;
+
+--connection another_con
+--reap
+
+show create table t1;
+
+--echo # Cleanup
+--connection default
+set session debug="-d,sleep_after_global_schema_lock";
+drop table t1;

=== modified file 'sql/ha_ndbcluster.cc'
--- a/sql/ha_ndbcluster.cc	2008-10-28 14:02:09 +0000
+++ b/sql/ha_ndbcluster.cc	2008-10-29 13:09:15 +0000
@@ -7615,6 +7615,7 @@ static int ndbcluster_init(void *p)
   ndbcluster_terminating= 0;
   ndb_dictionary_is_mysqld= 1;
   ndbcluster_hton= (handlerton *)p;
+  ndbcluster_global_schema_lock_init();
 
   {
     handlerton *h= ndbcluster_hton;
@@ -7667,6 +7668,7 @@ static int ndbcluster_init(void *p)
     pthread_mutex_destroy(&LOCK_ndb_util_thread);
     pthread_cond_destroy(&COND_ndb_util_thread);
     pthread_cond_destroy(&COND_ndb_util_ready);
+    ndbcluster_global_schema_lock_deinit();
     goto ndbcluster_init_error;
   }
 
@@ -7684,6 +7686,7 @@ static int ndbcluster_init(void *p)
     pthread_mutex_destroy(&LOCK_ndb_util_thread);
     pthread_cond_destroy(&COND_ndb_util_thread);
     pthread_cond_destroy(&COND_ndb_util_ready);
+    ndbcluster_global_schema_lock_deinit();
     goto ndbcluster_init_error;
   }
 
@@ -7747,6 +7750,7 @@ static int ndbcluster_end(handlerton *ht
   pthread_mutex_destroy(&LOCK_ndb_util_thread);
   pthread_cond_destroy(&COND_ndb_util_thread);
   pthread_cond_destroy(&COND_ndb_util_ready);
+  ndbcluster_global_schema_lock_deinit();
   DBUG_RETURN(0);
 }
 

=== modified file 'sql/ha_ndbcluster_binlog.cc'
--- a/sql/ha_ndbcluster_binlog.cc	2008-10-09 09:10:50 +0000
+++ b/sql/ha_ndbcluster_binlog.cc	2008-10-29 13:09:15 +0000
@@ -767,7 +767,19 @@ int ndbcluster_no_global_schema_lock_abo
   lock/unlock calls are reference counted, so calls to lock
   must be matched to a call to unlock even if the lock call fails
 */
-static int ndbcluster_global_schema_lock(THD *thd,
+static int ndbcluster_global_schema_lock_is_locked_or_queued= 0;
+static int ndbcluster_global_schema_lock_no_locking_allowed= 0;
+static pthread_mutex_t ndbcluster_global_schema_lock_mutex;
+void ndbcluster_global_schema_lock_init()
+{
+  pthread_mutex_init(&ndbcluster_global_schema_lock_mutex, MY_MUTEX_INIT_FAST);
+}
+void ndbcluster_global_schema_lock_deinit()
+{
+  pthread_mutex_destroy(&ndbcluster_global_schema_lock_mutex);
+}
+
+static int ndbcluster_global_schema_lock(THD *thd, int no_lock_queue,
                                          int report_cluster_disconnected)
 {
   Ndb *ndb= check_ndb_in_thd(thd);
@@ -776,7 +788,8 @@ static int ndbcluster_global_schema_lock
   if (thd_ndb->options & TNO_NO_LOCK_SCHEMA_OP)
     return 0;
   DBUG_ENTER("ndbcluster_global_schema_lock");
-  DBUG_PRINT("enter", ("query: %s", thd->query));
+  DBUG_PRINT("enter", ("query: %s, no_lock_queue: %d",
+                       thd->query, no_lock_queue));
   if (thd_ndb->global_schema_lock_count)
   {
     if (thd_ndb->global_schema_lock_trans)
@@ -794,8 +807,65 @@ static int ndbcluster_global_schema_lock
   DBUG_PRINT("exit", ("global_schema_lock_count: %d",
                       thd_ndb->global_schema_lock_count));
 
-  if ((thd_ndb->global_schema_lock_trans=
-       ndbcluster_global_schema_lock_ext(thd, ndb, ndb_error, -1)) != NULL)
+  /*
+    Check that taking the lock is allowed
+    - if not allowed to enter lock queue, return if lock exists
+    - wait until allowed
+    - increase global lock count
+  */
+  Thd_proc_info_guard thd_proc_info_guard(thd);
+  pthread_mutex_lock(&ndbcluster_global_schema_lock_mutex);
+  /* increase global lock count */
+  ndbcluster_global_schema_lock_is_locked_or_queued++;
+  if (no_lock_queue)
+  {
+    if (ndbcluster_global_schema_lock_is_locked_or_queued != 1)
+    {
+      /* Other thread has lock and this thread may not enter lock queue */
+      pthread_mutex_unlock(&ndbcluster_global_schema_lock_mutex);
+      thd_ndb->global_schema_lock_error= -1;
+      DBUG_PRINT("exit", ("aborting as lock exists"));
+      DBUG_RETURN(-1);
+    }
+    /* Mark that no other thread may be take lock */
+    ndbcluster_global_schema_lock_no_locking_allowed= 1;
+  }
+  else
+  {
+    while (ndbcluster_global_schema_lock_no_locking_allowed)
+    {
+      thd_proc_info(thd, "Waiting for allowed to take ndbcluster global schema lock");
+      /* Wait until locking is allowed */
+      pthread_mutex_unlock(&ndbcluster_global_schema_lock_mutex);
+      do_retry_sleep(50);
+      if (thd->killed)
+      {
+        thd_ndb->global_schema_lock_error= -1;
+        DBUG_RETURN(-1);
+      }
+      pthread_mutex_lock(&ndbcluster_global_schema_lock_mutex);
+    }
+  }
+  pthread_mutex_unlock(&ndbcluster_global_schema_lock_mutex);
+
+  /*
+    Take the lock
+  */
+  thd_proc_info(thd, "Waiting for ndbcluster global schema lock");
+  thd_ndb->global_schema_lock_trans=
+    ndbcluster_global_schema_lock_ext(thd, ndb, ndb_error, -1);
+
+  DBUG_EXECUTE_IF("sleep_after_global_schema_lock", my_sleep(6000000););
+
+  if (no_lock_queue)
+  {
+    pthread_mutex_lock(&ndbcluster_global_schema_lock_mutex);
+    /* Mark that other thread may be take lock */
+    ndbcluster_global_schema_lock_no_locking_allowed= 0;
+    pthread_mutex_unlock(&ndbcluster_global_schema_lock_mutex);
+  }
+
+  if (thd_ndb->global_schema_lock_trans)
   {
     DBUG_RETURN(0);
   }
@@ -838,6 +908,14 @@ static int ndbcluster_global_schema_unlo
     DBUG_RETURN(0);
   }
   thd_ndb->global_schema_lock_error= 0;
+
+  /*
+    Decrease global lock count
+  */
+  pthread_mutex_lock(&ndbcluster_global_schema_lock_mutex);
+  ndbcluster_global_schema_lock_is_locked_or_queued--;
+  pthread_mutex_unlock(&ndbcluster_global_schema_lock_mutex);
+
   if (trans)
   {
     thd_ndb->global_schema_lock_trans= NULL;
@@ -879,7 +957,7 @@ static int ndbcluster_binlog_func(handle
     ndbcluster_binlog_index_purge_file(thd, (const char *)arg);
     break;
   case BFN_GLOBAL_SCHEMA_LOCK:
-    DBUG_RETURN(ndbcluster_global_schema_lock(thd, 1));
+    DBUG_RETURN(ndbcluster_global_schema_lock(thd, *(int*)arg, 1));
     break;
   case BFN_GLOBAL_SCHEMA_UNLOCK:
     ndbcluster_global_schema_unlock(thd);
@@ -906,7 +984,7 @@ int Ndbcluster_global_schema_lock_guard:
     of calls to lock and unlock need to match up.
   */
   m_lock= 1;
-  return ndbcluster_global_schema_lock(m_thd, 0);
+  return ndbcluster_global_schema_lock(m_thd, 0, 0);
 }
 
 void ndbcluster_binlog_init_handlerton()

=== modified file 'sql/ha_ndbcluster_binlog.h'
--- a/sql/ha_ndbcluster_binlog.h	2008-10-07 08:19:31 +0000
+++ b/sql/ha_ndbcluster_binlog.h	2008-10-29 13:09:15 +0000
@@ -135,7 +135,20 @@ private:
   int m_invalidate;
 };
 
+class Thd_proc_info_guard
+{
+public:
+  Thd_proc_info_guard(THD *thd)
+   : m_thd(thd), m_proc_info(thd->proc_info) {}
+  ~Thd_proc_info_guard() { m_thd->proc_info= m_proc_info; }
+private:
+  THD *m_thd;
+  const char *m_proc_info;
+};
+
 extern Ndb_cluster_connection* g_ndb_cluster_connection;
+void ndbcluster_global_schema_lock_init();
+void ndbcluster_global_schema_lock_deinit();
 
 #ifdef HAVE_NDB_BINLOG
 extern pthread_t ndb_binlog_thread;

=== modified file 'sql/handler.cc'
--- a/sql/handler.cc	2008-10-28 14:02:09 +0000
+++ b/sql/handler.cc	2008-10-29 13:09:15 +0000
@@ -3891,14 +3891,15 @@ static my_bool binlog_func_foreach(THD *
 {
   hton_list_st hton_list;
   uint i, sz;
+  int res= 0;
 
   hton_list.sz= 0;
   plugin_foreach(thd, binlog_func_list,
                  MYSQL_STORAGE_ENGINE_PLUGIN, &hton_list);
 
   for (i= 0, sz= hton_list.sz; i < sz ; i++)
-    hton_list.hton[i]->binlog_func(hton_list.hton[i], thd, bfn->fn, bfn->arg);
-  return FALSE;
+    res|= hton_list.hton[i]->binlog_func(hton_list.hton[i], thd, bfn->fn,
bfn->arg);
+  return res != 0;
 }
 
 int ha_reset_logs(THD *thd)
@@ -3941,11 +3942,11 @@ int ha_binlog_index_purge_file(THD *thd,
   return 0;
 }
 
-static int ha_global_schema_lock(THD *thd)
+static int ha_global_schema_lock(THD *thd, int no_queue)
 {
-  binlog_func_st bfn= {BFN_GLOBAL_SCHEMA_LOCK, 0};
-  binlog_func_foreach(thd, &bfn);
-  if (thd->main_da.is_error())
+  binlog_func_st bfn= {BFN_GLOBAL_SCHEMA_LOCK, (void *)&no_queue};
+  int res= binlog_func_foreach(thd, &bfn);
+  if (res || thd->main_da.is_error())
     return 1;
   return 0;
 }
@@ -3970,11 +3971,11 @@ Ha_global_schema_lock_guard::~Ha_global_
     ha_global_schema_unlock(m_thd);
 }
 
-int Ha_global_schema_lock_guard::lock()
+int Ha_global_schema_lock_guard::lock(int no_queue)
 {
   DBUG_ASSERT(m_lock == 0);
   m_lock= 1;
-  return ha_global_schema_lock(m_thd);
+  return ha_global_schema_lock(m_thd, no_queue);
 }
 
 struct binlog_log_query_st

=== modified file 'sql/handler.h'
--- a/sql/handler.h	2008-10-28 14:02:09 +0000
+++ b/sql/handler.h	2008-10-29 13:09:15 +0000
@@ -2198,7 +2198,7 @@ class Ha_global_schema_lock_guard
 public:
   Ha_global_schema_lock_guard(THD *thd);
   ~Ha_global_schema_lock_guard();
-  int lock();
+  int lock(int no_queue= 0);
 private:
   THD *m_thd;
   int m_lock;
@@ -2216,6 +2216,6 @@ class Ha_global_schema_lock_guard
 public:
   Ha_global_schema_lock_guard(THD *thd) {}
   ~Ha_global_schema_lock_guard() {}
-  int lock() { return 0; }
+  int lock(int no_queue= 0) { return 0; }
 };
 #endif

=== modified file 'sql/sql_table.cc'
--- a/sql/sql_table.cc	2008-10-28 14:02:09 +0000
+++ b/sql/sql_table.cc	2008-10-29 13:09:15 +0000
@@ -6639,16 +6639,24 @@ view_err:
   if (table_type == DB_TYPE_NDBCLUSTER ||
       (create_info->db_type && create_info->db_type->db_type ==
DB_TYPE_NDBCLUSTER))
   {
-    /*
-      To avoid deadlock in this situation
-    */
     if (thd->locked_tables)
     {
-      my_message(ER_LOCK_OR_ACTIVE_TRANSACTION,
-                 ER(ER_LOCK_OR_ACTIVE_TRANSACTION), MYF(0));
-      DBUG_RETURN(TRUE);
+      /*
+        To avoid deadlock in this situation:
+        - if other thread has lock do not enter lock queue
+        and report an error instead
+      */
+      if (global_schema_lock_guard.lock(1))
+      {
+        my_message(ER_LOCK_OR_ACTIVE_TRANSACTION,
+                   ER(ER_LOCK_OR_ACTIVE_TRANSACTION), MYF(0));
+        DBUG_RETURN(TRUE);
+      }
+    }
+    else
+    {
+      global_schema_lock_guard.lock();
     }
-    global_schema_lock_guard.lock();
   }
   if (!(table= open_n_lock_single_table(thd, table_list, TL_WRITE_ALLOW_READ)))
     DBUG_RETURN(TRUE);