List:Commits« Previous MessageNext Message »
From:Dmitry Lenev Date:October 20 2010 7:52am
Subject:bzr commit into mysql-5.5-runtime branch (Dmitry.Lenev:3169)
View as plain text  
#At file:///home/dlenev/src/bzr/mysql-5.5-rt-grl/ based on revid:dmitry.lenev@stripped

 3169 Dmitry Lenev	2010-10-20
      More changes to draft patch refactoring global read
      lock implementation. Makes GRL yet another type of
      metadata lock and thus exposes it to deadlock detector
      in MDL subsystem.
      
      Solves bugs #54673 "It takes too long to get readlock for
      'FLUSH TABLES WITH READ LOCK'" and #57006 "Deadlock between
      HANDLER and FLUSH TABLES WITH READ LOCK".
      
      Work-in-progress. Solve problems with stored routine and
      events-related DDL statements.

    modified:
      mysql-test/r/flush_read_lock.result
      mysql-test/t/flush_read_lock.test
      sql/lock.cc
      sql/sql_class.h
      sql/sql_parse.cc
=== modified file 'mysql-test/r/flush_read_lock.result'
--- a/mysql-test/r/flush_read_lock.result	2010-10-18 12:33:49 +0000
+++ b/mysql-test/r/flush_read_lock.result	2010-10-20 07:52:42 +0000
@@ -10,6 +10,8 @@ drop view if exists v1;
 drop procedure if exists p2;
 drop function if exists f2_base;
 drop function if exists f2_temp;
+drop event if exists e1;
+drop event if exists e2;
 create table t1_base(i int) engine=myisam;
 create table t2_base(j int) engine=myisam;
 create table t3_trans(i int) engine=innodb;
@@ -31,6 +33,7 @@ begin
 insert into t1_temp values (1);
 return 0;
 end|
+create event e1 on schedule every 1 minute do begin end;
 #
 # Test compatibility of FLUSH TABLES WITH READ LOCK 
 # with various statements.
@@ -90,8 +93,9 @@ Success: FTWRL is blocked when 'alter vi
 #
 # 1.7) ALTER EVENT should be incompatible with FTWRL.
 #
-# TODO/FIXME: Unfortunately it releases protection against
-#             GRL a bit too early at this point.
+Success: Was not able to run 'alter event e1 comment 'test'' under FTWRL.
+Success: 'alter event e1 comment 'test'' is blocked by FTWRL active in another connection.
+Success: FTWRL is blocked when 'alter event e1 comment 'test'' is active in another connection.
 #
 # 1.x) The rest of ALTER statements are too special to
 #      to be tested here.
@@ -310,24 +314,21 @@ Success: FTWRL is blocked when 'create t
 #
 # 8.6) CREATE FUNCTION is incompatible with FTWRL.
 #
-# Skip last part of compatibility testing as this statement
-# is internally consists of two statements.
-# QQ: Is this a problem?
 Success: Was not able to run 'create function f2() returns int return 0' under FTWRL.
 Success: 'create function f2() returns int return 0' is blocked by FTWRL active in another connection.
+Success: FTWRL is blocked when 'create function f2() returns int return 0' is active in another connection.
 #
 # 8.7) CREATE PROCEDURE is incompatible with FTWRL.
 #
-# Skip last part of compatibility testing as this statement
-# is internally consists of two statements.
-# QQ: Is this a problem?
 Success: Was not able to run 'create procedure p3() begin end' under FTWRL.
 Success: 'create procedure p3() begin end' is blocked by FTWRL active in another connection.
+Success: FTWRL is blocked when 'create procedure p3() begin end' is active in another connection.
 #
 # 8.8) CREATE EVENT should be incompatible with FTWRL.
 #
-# TODO/FIXME: Unfortunately it releases protection against
-#             GRL a bit too early at this point.
+Success: Was not able to run 'create event e2 on schedule every 1 minute do begin end' under FTWRL.
+Success: 'create event e2 on schedule every 1 minute do begin end' is blocked by FTWRL active in another connection.
+Success: FTWRL is blocked when 'create event e2 on schedule every 1 minute do begin end' is active in another connection.
 #
 # 8.9) CREATE USER should be incompatible with FTWRL.
 #
@@ -504,19 +505,15 @@ Success: FTWRL is blocked when 'drop dat
 #
 # 13.4) DROP FUNCTION is incompatible with FTWRL.
 #
-# Skip last part of compatibility testing as this statement
-# is internally consists of two statements.
-# QQ: Is this a problem?
 Success: Was not able to run 'drop function f1' under FTWRL.
 Success: 'drop function f1' is blocked by FTWRL active in another connection.
+Success: FTWRL is blocked when 'drop function f1' is active in another connection.
 #
 # 13.5) DROP PROCEDURE is incompatible with FTWRL.
 #
-# Skip last part of compatibility testing as this statement
-# is internally consists of two statements.
-# QQ: Is this a problem?
 Success: Was not able to run 'drop procedure p1' under FTWRL.
 Success: 'drop procedure p1' is blocked by FTWRL active in another connection.
+Success: FTWRL is blocked when 'drop procedure p1' is active in another connection.
 #
 # 13.6) DROP USER should be incompatible with FTWRL.
 #
@@ -534,8 +531,9 @@ Success: FTWRL is blocked when 'drop vie
 #
 # 13.8) DROP EVENT should be incompatible with FTWRL.
 #
-# TODO/FIXME: Unfortunately it releases protection against
-#             GRL a bit too early at this point.
+Success: Was not able to run 'drop event e1' under FTWRL.
+Success: 'drop event e1' is blocked by FTWRL active in another connection.
+Success: FTWRL is blocked when 'drop event e1' is active in another connection.
 #
 # 13.9) DROP TRIGGER is incompatible with FTWRL.
 #
@@ -1283,6 +1281,7 @@ delete from t2_base;
 #
 # Clean-up.
 #
+drop event e1;
 drop function f2_temp;
 drop function f2_base;
 drop procedure p2;

=== modified file 'mysql-test/t/flush_read_lock.test'
--- a/mysql-test/t/flush_read_lock.test	2010-10-18 12:33:49 +0000
+++ b/mysql-test/t/flush_read_lock.test	2010-10-20 07:52:42 +0000
@@ -23,6 +23,8 @@ drop view if exists v1;
 drop procedure if exists p2;
 drop function if exists f2_base;
 drop function if exists f2_temp;
+drop event if exists e1;
+drop event if exists e2;
 --enable_warnings
 create table t1_base(i int) engine=myisam;
 create table t2_base(j int) engine=myisam;
@@ -47,6 +49,7 @@ begin
   return 0;
 end|
 delimiter ;|
+create event e1 on schedule every 1 minute do begin end;
 
 connect (con1,localhost,root,,);
 connect (con2,localhost,root,,);
@@ -127,8 +130,9 @@ let $cleanup_stmt1= alter view v1 as sel
 --echo #
 --echo # 1.7) ALTER EVENT should be incompatible with FTWRL.
 --echo #
---echo # TODO/FIXME: Unfortunately it releases protection against
---echo #             GRL a bit too early at this point.
+let $statement= alter event e1 comment 'test';
+let $cleanup_stmt1= alter event e1 comment '';
+--source include/check_ftwrl_incompatible.inc
 
 --echo #
 --echo # 1.x) The rest of ALTER statements are too special to
@@ -415,30 +419,21 @@ let $cleanup_stmt1= drop trigger t1_bi;
 --echo #
 let $statement= create function f2() returns int return 0;
 let $cleanup_stmt1= drop function f2;
---echo # Skip last part of compatibility testing as this statement
---echo # is internally consists of two statements.
---echo # QQ: Is this a problem?
-let $skip_3rd_check= 1;
 --source include/check_ftwrl_incompatible.inc
-let $skip_3rd_check= ;
 
 --echo #
 --echo # 8.7) CREATE PROCEDURE is incompatible with FTWRL.
 --echo #
 let $statement= create procedure p3() begin end;
 let $cleanup_stmt1= drop procedure p3;
---echo # Skip last part of compatibility testing as this statement
---echo # is internally consists of two statements.
---echo # QQ: Is this a problem?
-let $skip_3rd_check= 1;
 --source include/check_ftwrl_incompatible.inc
-let $skip_3rd_check= ;
 
 --echo #
 --echo # 8.8) CREATE EVENT should be incompatible with FTWRL.
 --echo #
---echo # TODO/FIXME: Unfortunately it releases protection against
---echo #             GRL a bit too early at this point.
+let $statement= create event e2 on schedule every 1 minute do begin end;
+let $cleanup_stmt1= drop event e2;
+--source include/check_ftwrl_incompatible.inc
 
 --echo #
 --echo # 8.9) CREATE USER should be incompatible with FTWRL.
@@ -668,24 +663,14 @@ let $cleanup_stmt1= create database mysq
 --echo #
 let $statement= drop function f1;
 let $cleanup_stmt1= create function f1() returns int return 0;
---echo # Skip last part of compatibility testing as this statement
---echo # is internally consists of two statements.
---echo # QQ: Is this a problem?
-let $skip_3rd_check= 1;
 --source include/check_ftwrl_incompatible.inc
-let $skip_3rd_check= ;
 
 --echo #
 --echo # 13.5) DROP PROCEDURE is incompatible with FTWRL.
 --echo #
 let $statement= drop procedure p1;
 let $cleanup_stmt1= create procedure p1() begin end;
---echo # Skip last part of compatibility testing as this statement
---echo # is internally consists of two statements.
---echo # QQ: Is this a problem?
-let $skip_3rd_check= 1;
 --source include/check_ftwrl_incompatible.inc
-let $skip_3rd_check= ;
 
 --echo #
 --echo # 13.6) DROP USER should be incompatible with FTWRL.
@@ -706,8 +691,9 @@ let $cleanup_stmt1= create view v1 as se
 --echo #
 --echo # 13.8) DROP EVENT should be incompatible with FTWRL.
 --echo #
---echo # TODO/FIXME: Unfortunately it releases protection against
---echo #             GRL a bit too early at this point.
+let $statement= drop event e1;
+let $cleanup_stmt1= create event e1 on schedule every 1 minute do begin end;
+--source include/check_ftwrl_incompatible.inc
 
 --echo #
 --echo # 13.9) DROP TRIGGER is incompatible with FTWRL.
@@ -1698,6 +1684,7 @@ delete from t2_base;
 --echo #
 --echo # Clean-up.
 --echo #
+drop event e1;
 drop function f2_temp;
 drop function f2_base;
 drop procedure p2;

=== modified file 'sql/lock.cc'
--- a/sql/lock.cc	2010-10-19 13:31:53 +0000
+++ b/sql/lock.cc	2010-10-20 07:52:42 +0000
@@ -1160,6 +1160,36 @@ void Global_read_lock::move_tickets_afte
     thd->mdl_context.move_ticket_after_trans_sentinel(m_mdl_blocks_commits_lock);
 }
 
+
+/**
+  Acquire metadata lock protecting statement against global read lock
+  and move it after transactional sentinel, so it won't be released
+  in the middle of statement execution.
+
+  @param thd  Reference to thread.
+
+  @retval non-NULL - Ticket for metadata lock providing protection.
+  @retval NULL     - Error.
+*/
+
+MDL_ticket* Global_read_lock::acquire_explicit_protection(THD *thd)
+{
+  MDL_request mdl_request;
+
+  if (can_acquire_protection())
+    return NULL;
+
+  init_protection_request(&mdl_request);
+
+  if (thd->mdl_context.acquire_lock(&mdl_request,
+                                    thd->variables.lock_wait_timeout))
+    return NULL;
+
+  thd->mdl_context.move_ticket_after_trans_sentinel(mdl_request.ticket);
+
+  return mdl_request.ticket;
+}
+
 /**
   @} (end of group Locking)
 */

=== modified file 'sql/sql_class.h'
--- a/sql/sql_class.h	2010-10-19 13:31:53 +0000
+++ b/sql/sql_class.h	2010-10-20 07:52:42 +0000
@@ -1351,6 +1351,7 @@ public:
   bool make_global_read_lock_block_commit(THD *thd);
   bool is_acquired() const { return m_state != GRL_NONE; }
   void move_tickets_after_trans_sentinel(THD *thd);
+  MDL_ticket *acquire_explicit_protection(THD *thd);
 private:
   enum_grl_state m_state;
   /**
@@ -3507,10 +3508,29 @@ public:
 #define CF_DIAGNOSTIC_STMT        (1U << 8)
 
 /**
+  SQL statements that require explicit protection against impending
+  global read lock.
+  For most of DML and DDL statements metadata lock providing protection
+  will be acquired automatically at the time when tables are opened/
+  metadata locks are acquired. This lock is released at the end of
+  statement execution.
+  Unfortunately, a few DDL statements release metadata locks too early
+  and so this flag has to be used. For statements marked by this flag
+  we acquire metadata lock providing protection against GRL early during
+  statement execution and protect it from premature release by moving
+  corresponding ticket behind transactional sentinel. We release this
+  lock at the end of statement execution.
+
+  Usage of this flag is discouraged as it this hack should be removed
+  long-term.
+*/
+#define CF_EXPLICIT_PROTECT_AGAINST_GRL (1U << 9)
+
+/**
   Identifies statements that may generate row events
   and that may end up in the binary log.
 */
-#define CF_CAN_GENERATE_ROW_EVENTS (1U << 9)
+#define CF_CAN_GENERATE_ROW_EVENTS (1U << 10)
 
 /* Bits in server_command_flags */
 

=== modified file 'sql/sql_parse.cc'
--- a/sql/sql_parse.cc	2010-10-18 12:33:49 +0000
+++ b/sql/sql_parse.cc	2010-10-20 07:52:42 +0000
@@ -278,12 +278,12 @@ void init_update_queries(void)
   sql_command_flags[SQLCOM_DROP_VIEW]=      CF_CHANGES_DATA | CF_AUTO_COMMIT_TRANS;
   sql_command_flags[SQLCOM_CREATE_TRIGGER]= CF_CHANGES_DATA | CF_AUTO_COMMIT_TRANS;
   sql_command_flags[SQLCOM_DROP_TRIGGER]=   CF_CHANGES_DATA | CF_AUTO_COMMIT_TRANS;
-  // TODO/FIXME Investigate early release of metadata locks and effect on GRL.
-  sql_command_flags[SQLCOM_CREATE_EVENT]=   CF_CHANGES_DATA | CF_AUTO_COMMIT_TRANS;
-  // TODO/FIXME Investigate early release of metadata locks and effect on GRL.
-  sql_command_flags[SQLCOM_ALTER_EVENT]=    CF_CHANGES_DATA | CF_AUTO_COMMIT_TRANS;
-  // TODO/FIXME Investigate early release of metadata locks and effect on GRL.
-  sql_command_flags[SQLCOM_DROP_EVENT]=     CF_CHANGES_DATA | CF_AUTO_COMMIT_TRANS;
+  sql_command_flags[SQLCOM_CREATE_EVENT]=   CF_CHANGES_DATA | CF_AUTO_COMMIT_TRANS |
+                                            CF_EXPLICIT_PROTECT_AGAINST_GRL;
+  sql_command_flags[SQLCOM_ALTER_EVENT]=    CF_CHANGES_DATA | CF_AUTO_COMMIT_TRANS |
+                                            CF_EXPLICIT_PROTECT_AGAINST_GRL;
+  sql_command_flags[SQLCOM_DROP_EVENT]=     CF_CHANGES_DATA | CF_AUTO_COMMIT_TRANS |
+                                            CF_EXPLICIT_PROTECT_AGAINST_GRL;
 
   sql_command_flags[SQLCOM_UPDATE]=	    CF_CHANGES_DATA | CF_REEXECUTION_FRAGILE |
                                             CF_CAN_GENERATE_ROW_EVENTS;
@@ -366,14 +366,14 @@ void init_update_queries(void)
   // QQ Looks somewhat strange but is not changed...
   sql_command_flags[SQLCOM_OPTIMIZE]=          CF_CHANGES_DATA;
   sql_command_flags[SQLCOM_CREATE_FUNCTION]=   CF_CHANGES_DATA;
-  // TODO/FIXME Investigate and fix consequences of two statements aproach for GRL.
-  sql_command_flags[SQLCOM_CREATE_PROCEDURE]=  CF_CHANGES_DATA | CF_AUTO_COMMIT_TRANS;
-  // TODO/FIXME Investigate and fix consequences of two statements aproach for GRL.
-  sql_command_flags[SQLCOM_CREATE_SPFUNCTION]= CF_CHANGES_DATA | CF_AUTO_COMMIT_TRANS;
-  // TODO/FIXME Investigate and fix consequences of two statements aproach for GRL.
-  sql_command_flags[SQLCOM_DROP_PROCEDURE]=    CF_CHANGES_DATA | CF_AUTO_COMMIT_TRANS;
-  // TODO/FIXME Investigate and fix consequences of two statements aproach for GRL.
-  sql_command_flags[SQLCOM_DROP_FUNCTION]=     CF_CHANGES_DATA | CF_AUTO_COMMIT_TRANS;
+  sql_command_flags[SQLCOM_CREATE_PROCEDURE]=  CF_CHANGES_DATA | CF_AUTO_COMMIT_TRANS |
+                                               CF_EXPLICIT_PROTECT_AGAINST_GRL;
+  sql_command_flags[SQLCOM_CREATE_SPFUNCTION]= CF_CHANGES_DATA | CF_AUTO_COMMIT_TRANS |
+                                               CF_EXPLICIT_PROTECT_AGAINST_GRL;
+  sql_command_flags[SQLCOM_DROP_PROCEDURE]=    CF_CHANGES_DATA | CF_AUTO_COMMIT_TRANS |
+                                               CF_EXPLICIT_PROTECT_AGAINST_GRL;
+  sql_command_flags[SQLCOM_DROP_FUNCTION]=     CF_CHANGES_DATA | CF_AUTO_COMMIT_TRANS |
+                                               CF_EXPLICIT_PROTECT_AGAINST_GRL;
   sql_command_flags[SQLCOM_ALTER_PROCEDURE]=   CF_CHANGES_DATA | CF_AUTO_COMMIT_TRANS;
   sql_command_flags[SQLCOM_ALTER_FUNCTION]=    CF_CHANGES_DATA | CF_AUTO_COMMIT_TRANS;
   sql_command_flags[SQLCOM_INSTALL_PLUGIN]=    CF_CHANGES_DATA;
@@ -1769,7 +1769,7 @@ mysql_execute_command(THD *thd)
   /* have table map for update for multi-update statement (BUG#37051) */
   bool have_table_map_for_update= FALSE;
 #endif
-  /* Saved variable value */
+  MDL_ticket *grl_protection= NULL;
   DBUG_ENTER("mysql_execute_command");
 #ifdef WITH_PARTITION_STORAGE_ENGINE
   thd->work_part_info= 0;
@@ -1961,6 +1961,18 @@ mysql_execute_command(THD *thd)
     thd->mdl_context.release_transactional_locks();
   }
 
+  /*
+    Check if this command needs explicit protection against the global
+    read lock. See comment for CF_EXPLICIT_PROTECT_AGAINST_GRL flag for
+    details. We release metadata lock providing protection at the end
+    of mysql_execute_command().
+  */
+  if (((sql_command_flags[lex->sql_command] &
+        CF_EXPLICIT_PROTECT_AGAINST_GRL) != 0) &&
+      !thd->locked_tables_mode &&
+      !(grl_protection= thd->global_read_lock.acquire_explicit_protection(thd)))
+    goto error;
+
 #ifndef DBUG_OFF
   if (lex->sql_command != SQLCOM_SET_OPTION)
     DEBUG_SYNC(thd,"before_execute_sql_command");
@@ -4328,6 +4340,13 @@ finish:
 
   DBUG_EVALUATE_IF("execute_command_crash", (abort(), 0), 0);
 
+  /*
+    It is time to release metadata lock protecting against global read
+    lock if it is one of statements which require explicit protection.
+  */
+  if (grl_protection)
+    thd->mdl_context.release_lock(grl_protection);
+
   if (stmt_causes_implicit_commit(thd, CF_IMPLICIT_COMMIT_END))
   {
     /* No transaction control allowed in sub-statements. */


Attachment: [text/bzr-bundle] bzr/dmitry.lenev@oracle.com-20101020075242-rokk6l7etbf06m9q.bundle
Thread
bzr commit into mysql-5.5-runtime branch (Dmitry.Lenev:3169) Dmitry Lenev20 Oct