From: Dmitry Lenev Date: October 26 2010 4:25am Subject: bzr commit into mysql-5.5-runtime branch (Dmitry.Lenev:3181) List-Archive: http://lists.mysql.com/commits/121874 Message-Id: <20101026042536.C46232F0E23@mockturtle> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0945751121==" --===============0945751121== MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline #At file:///home/dlenev/src/bzr/mysql-5.5-rt-grl/ based on revid:dmitry.lenev@stripped 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 === 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. */ --===============0945751121== MIME-Version: 1.0 Content-Type: text/bzr-bundle; charset="us-ascii"; name="bzr/dmitry.lenev@stripped" Content-Transfer-Encoding: 7bit Content-Disposition: inline # Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: dmitry.lenev@stripped # target_branch: file:///home/dlenev/src/bzr/mysql-5.5-rt-grl/ # testament_sha1: d3896f13fbd7edd5f97619716ee67a45a3dddca1 # timestamp: 2010-10-26 08:25:36 +0400 # base_revision_id: dmitry.lenev@stripped\ # 61iq513xbv7c2rld # # Begin bundle IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWbzplXQABAtfgHQQWOf//3/v /4q////0YAn8+t25bAAoG1hbNDQNAAAA4aij0h6gBkYmmgAyBphNAAAAAEpTTTU2mpqemTaSBtQN A0AyANGgAeoAOAYRhNMQwCAZADCNMmTCMBDQShAhNT1T1Hp6aNFB6EHqeiNMaTJtED2pqabUaA4B hGE0xDAIBkAMI0yZMIwENBJECABABGgmJqan6SeFGjaTEGCGmEgIhuCYq18tWSCqrk7fzk67i1yk j1HkDKXEjvDWKfUzTpm71Cfebdq7ioS+6yPBAT0qlqiLUUirJ7rZhqq6VFNv3SiJCPMkITMEJ8Kf K3uW1bEzMzIZkG7sQhRii5cKwINStZgZ0miYZRmJq5NFkVRVJSYeeAgdCCQpburz+A6KKWBNCZSV FTVh9whUhteUA5CgUgDC0NL5MFfco+DadKWFBIOIBQzCNFrMJCixp7inEmYiewmQePSBOw6hj6KQ iTUUszSdQOHk5HSOXIuHofmTM1EcvYsuu9heWVr7/rjzbVUTCFRkXF5Ej1g5bbMmQhqNayKK0VRU QHGLkzwMkwzi/4yseFGoNHDn6GjuYcpxJntRdEft6xIZBhEU43rUVGmRGYXy7wdJKYysxiFI8gNA Aug1M5o4uSQdkD4d45dO/VWph2luoiY92BvJWscri7fmwxMdJf6oqyYlI4L1DwopG1dKIWxg2sJ2 OvweSUkS2AGYqCRmNUwB8V7dxAXxETkUUedco2DTFJTLUMFsniv0wRGp1U2DXcJiRiwkfb0NDTPw zytuK73vzsttqj+eBi2TRq22q8YLFiiBlQ/hg8xDdt8N32CH1dNmCr4H8XoHtsnFrBNuIGi8rc1Z lpAXaIrX7CGtNKjg+mDtlkANRocN3KqKYRFEplhARkWwgtlVkaXmyyYqziFkcenX2SOikqy7NrXj hhDenKxShKlpXhW5SuE1EZmjqYtgeI72MGfIqY7CyaMjCRqGNq8bzsOC7S9Y5F+oZzSIZTRsI1bR ESlDOGWhbgRNRaOWhO1XYFh+qpdg4jk2dQ8Waa2JEGDSJgNmxHE71C7+76g42wLC01UtlB6xXI0I jRDPwW+gneVkkjrxDKpSkQeS0FSMmmWwYqeEaaUeu0nShwNuugwWzLVNVGcqTi+UuLiqbTCFBveo wJphg1kC1YjVjD44os2rjXt6HsPA24Gjtg6wfaQsIbndY7hoFZQ0JFY9e4psrs113ltctcYbICtI WE+V4+EXrFkzpMQMcjEwJkyRafv/sjDB1dCkL8UXTajGbIm6Go0QqFOpvo+cGX9tZ7w/QOnUr186 4wYTHd9y8+07klxPiB7w+SPkl+H6n2BIi0AvWpMKq6giQhmTChctyvB4KHzCdjUlAMHabUfIqiLa XnGKvKlVIFL0s+qbmqKBYTRoNSVPCeLytoCnBFDYLMFIYpA5PALAreFieLKi+ANpCk2n7B/tZFoV gPzfNdOtkhoD1lpeuIaHJZgfopBEK9uG8gQC+Qc0+NAsTJyJxWCsoVMHELCoJfqGhMOVFiRC8GDI MVqLQtCqQezgcFmtoWhiF4bGLzL8AlezbS1BUHA2Ax1b6NUt4fSleOEOL3JwIEEh2PEiQMA5BTpP jCIGkuIF47Og/BkOrkSIjgyn6Riss4Skg7tO9ERw8IHoTNQUBEkbCbi2h+B9Z11L/NzVm1R2ECxk qg3jwDdmkQGVBiogK9UKmBuCJ1lQlHJ6goHny3mszHWMU9uMvmvGQpQ11hYOOaws2I/uRSJoRAUy CdM95XFxK3GBLViarz+gvv6iRZsOevu1HVue6dRADeLNmGTsL4qCSmfYeQjNASCJvYqPy7TngWmA wMmcz4wgROIww49bpyXKsOtRKizxnQ1nK3jcipE7YSSYLALKuy0YwM035Ugvgbi88QumltD6zvCB FNomn0VjaDNFFhV/GAYPRhEYii0agskPQYNVYg5zPR+mHX5HH2r0Xmd7Gb3q4WBh5jGGJ9LQcbDZ agxuHMvXuPS1st+xF/cIn+JVzKr7G6fs5igJewhA0PqkJN0LA5mQwhmGTdE6PgWHMgclq6YSYsMN tsnKzrfIO+OJ6T+UEYI9XAAoLDFKfgLoL3crDauZ22EIntJniGoN6mI+9earUOwK1qivm9TOaEQP gayaCEuLdZknKvDvqD3KYjibl603gN3gWvARjaVjgjgZM4onHTQHoEHmUWJaIMFC1lWas3h6yHiB xYff1JNZ2hYb0EDchYz9R5CnMlZb3dZ1Fp+ffzrsWxkyOKobBSOhgdopGsBlvOIwBIaxaXHacS/e eusuEMwayV5coByFqONBdxiTHSMw7TQUoBERyCiiZaBkuxSSTCenPcJYA4Gw68lw6jyV6c4TN7Kf uzIhEZZhoFXAc7yhtNZMAdiooEVipJLhtwoD7ysKz2mIk/BYEkmV1oVoL1eZCNR1HGlJsFQOWMMo NQZLUZqJUU2Ex0oiZDJyQ1QBAHYYjribSZGi8DxMlELCpAmGTMjkOnNWaFf8jebVogLcROPiMeJ9 W4A7GuBksQTFZ6We0nZ8RjOCofuknF9LfykpngoqwH9DxOA2/zCo4m/gg6zOJ3OmR5p1MOVLF3LG SCsfsl3yXfc5kupd9R7BXgyYQ2yZcsS8wGD1OHYQSW0R4uLfxChYUl+JsIG6wZ2gUQOOeG3ziGHF 7aj5zEh7lZUQWKistX0Fz6uhgWFhgurs8iJEkw/sM8IRGdJN56UkcjsIMQN7kFyKyZEiTGIECqBk dbjLtEWhMr3hKevAmVpzWcnbYy3KIcygoK5FeBM9FxZmZnsXSDkAdzDWmtMx5iPMY6FazPaaCktZ 8xCwPE5q8wIjMOLCoiRD1xHLhj/G8mVRGODraQILtJSXkJ0cjQuQywQE5KckVFoxNSEuS3lYrcRx e8MCEA4GI/yVkrtp3EcRmMQ0K0iINFRE0VA+ge8ArTodaeJ3FNVytXb/Nxz5jnqNQc1iURoMUkep ZQK06S1lS4VERZBxY2DFgjyVZWUrGSoWEB1WMgOfVb/WZNehLBZ38QvqLA/i7kinChIXnTKugA== --===============0945751121==--