List:Commits« Previous MessageNext Message »
From:Dmitry Lenev Date:October 26 2010 4:26am
Subject:bzr push into mysql-5.5-runtime branch (Dmitry.Lenev:3180 to 3181)
View as plain text  
 3181 Dmitry Lenev	2010-10-26
      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. Get rid of special GRL handling in
      CREATE/DROP PROCEDURE/FUNCTION.

    modified:
      sql/lock.cc
      sql/sql_class.h
      sql/sql_parse.cc
 3180 Dmitry Lenev	2010-10-25
      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.
      
      Extend test coverage for special kinds of SET statement.

    modified:
      mysql-test/r/flush_read_lock.result
      mysql-test/t/flush_read_lock.test
=== modified file 'sql/lock.cc'
--- a/sql/lock.cc	2010-10-25 15:16:12 +0000
+++ b/sql/lock.cc	2010-10-26 04:25:15 +0000
@@ -1145,36 +1145,6 @@ 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-22 07:16:34 +0000
+++ b/sql/sql_class.h	2010-10-26 04:25:15 +0000
@@ -1350,7 +1350,6 @@ 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,29 +3506,10 @@ 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 << 10)
+#define CF_CAN_GENERATE_ROW_EVENTS (1U << 9)
 
 /* Bits in server_command_flags */
 

=== modified file 'sql/sql_parse.cc'
--- a/sql/sql_parse.cc	2010-10-25 18:11:22 +0000
+++ b/sql/sql_parse.cc	2010-10-26 04:25:15 +0000
@@ -362,14 +362,10 @@ void init_update_queries(void)
   sql_command_flags[SQLCOM_REVOKE]=            CF_CHANGES_DATA;
   sql_command_flags[SQLCOM_OPTIMIZE]=          CF_CHANGES_DATA;
   sql_command_flags[SQLCOM_CREATE_FUNCTION]=   CF_CHANGES_DATA;
-  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_CREATE_PROCEDURE]=  CF_CHANGES_DATA | CF_AUTO_COMMIT_TRANS;
+  sql_command_flags[SQLCOM_CREATE_SPFUNCTION]= CF_CHANGES_DATA | CF_AUTO_COMMIT_TRANS;
+  sql_command_flags[SQLCOM_DROP_PROCEDURE]=    CF_CHANGES_DATA | CF_AUTO_COMMIT_TRANS;
+  sql_command_flags[SQLCOM_DROP_FUNCTION]=     CF_CHANGES_DATA | CF_AUTO_COMMIT_TRANS;
   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;
@@ -1765,7 +1761,6 @@ mysql_execute_command(THD *thd)
   /* have table map for update for multi-update statement (BUG#37051) */
   bool have_table_map_for_update= FALSE;
 #endif
-  MDL_ticket *grl_protection= NULL;
   DBUG_ENTER("mysql_execute_command");
 #ifdef WITH_PARTITION_STORAGE_ENGINE
   thd->work_part_info= 0;
@@ -1957,18 +1952,6 @@ 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");
@@ -3733,13 +3716,22 @@ end_with_restore_list:
       Security_context *backup= NULL;
       LEX_USER *definer= thd->lex->definer;
       /*
-        We're going to issue an implicit GRANT statement.
-        It takes metadata locks and updates system tables.
-        Make sure that sp_create_routine() did not leave any
-        locks in the MDL context, so there is no risk to
-        deadlock.
+        We're going to issue an implicit GRANT statement so we close all
+        open tables. We have to keep metadata locks as this ensures that
+        this statement is atomic against concurent FLUSH TABLES WITH READ
+        LOCK. Deadlocks which can arise due to fact that this implicit
+        statement takes metadata locks should be detected by a deadlock
+        detector in MDL subsystem and reported as errors.
+
+        No need to commit/rollback statement transaction, it's not started.
+
+        TODO: Long-term we should either ensure that implicit GRANT statement
+              is written into binary log as a separate statement or make both
+              creation of routine and implicit GRANT parts of one fully atomic
+              statement.
       */
-      close_mysql_tables(thd);
+      DBUG_ASSERT(thd->transaction.stmt.is_empty());
+      close_thread_tables(thd);
       /*
         Check if the definer exists on slave, 
         then use definer privilege to insert routine privileges to mysql.procs_priv.
@@ -3999,13 +3991,22 @@ create_sp_error:
 
 #ifndef NO_EMBEDDED_ACCESS_CHECKS
       /*
-        We're going to issue an implicit REVOKE statement.
-        It takes metadata locks and updates system tables.
-        Make sure that sp_create_routine() did not leave any
-        locks in the MDL context, so there is no risk to
-        deadlock.
+        We're going to issue an implicit REVOKE statement so we close all
+        open tables. We have to keep metadata locks as this ensures that
+        this statement is atomic against concurent FLUSH TABLES WITH READ
+        LOCK. Deadlocks which can arise due to fact that this implicit
+        statement takes metadata locks should be detected by a deadlock
+        detector in MDL subsystem and reported as errors.
+
+        No need to commit/rollback statement transaction, it's not started.
+
+        TODO: Long-term we should either ensure that implicit REVOKE statement
+              is written into binary log as a separate statement or make both
+              dropping of routine and implicit REVOKE parts of one fully atomic
+              statement.
       */
-      close_mysql_tables(thd);
+      DBUG_ASSERT(thd->transaction.stmt.is_empty());
+      close_thread_tables(thd);
 
       if (sp_result != SP_KEY_NOT_FOUND &&
           sp_automatic_privileges && !opt_noacl &&
@@ -4334,13 +4335,6 @@ finish:
     DEBUG_SYNC(thd, "execute_command_after_close_tables");
 #endif
 
-  /*
-    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. */

No bundle (reason: useless for push emails).
Thread
bzr push into mysql-5.5-runtime branch (Dmitry.Lenev:3180 to 3181) Dmitry Lenev26 Oct