From: Date: October 29 2008 2:09pm Subject: bzr commit into mysql-5.1 branch (tomas.ulin:2710) Bug#40246 List-Archive: http://lists.mysql.com/commits/57308 X-Bug: 40246 Message-Id: <20081029130927.8C00F5D8391@linux.local> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit #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);