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 Lenev | 26 Oct |
