From: Dmitry Lenev Date: November 8 2010 2:19pm Subject: bzr push into mysql-5.5-runtime branch (Dmitry.Lenev:3189 to 3190) List-Archive: http://lists.mysql.com/commits/123089 Message-Id: <20101108141951.264BB2F0E23@mockturtle> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit 3190 Dmitry Lenev 2010-11-08 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. Minor after review fixes. modified: sql/sql_base.cc sql/sql_parse.cc 3189 Dmitry Lenev 2010-11-08 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. Minor after review fixes. modified: sql/lock.cc sql/sql_base.cc sql/sql_class.h sql/sql_insert.cc sql/sql_parse.cc sql/sql_parse.h === modified file 'sql/sql_base.cc' --- a/sql/sql_base.cc 2010-11-08 09:55:43 +0000 +++ b/sql/sql_base.cc 2010-11-08 14:10:47 +0000 @@ -2784,64 +2784,61 @@ bool open_table(THD *thd, TABLE_LIST *ta This is the normal use case. */ - /* - We are not under LOCK TABLES and going to acquire write-lock/ - modify the base table. We need to acquire protection against - global read lock until end of this statement in order to have - this statement blocked by active FLUSH TABLES WITH READ LOCK. - - We don't block acquire this protection under LOCK TABLES as - such protection already acquired at LOCK TABLES time and - not released until UNLOCK TABLES. - - We don't block statements which modify only temporary tables - as these tables are not preserved by backup by any form of - backup which uses FLUSH TABLES WITH READ LOCK. - - TODO: The fact that we sometimes acquire protection against - GRL only when we encounter table to be write-locked - slightly increases probability of deadlock. - This problem will be solved once Alik pushes his - temporary table refactoring patch and we can start - pre-acquiring metadata locks at the beggining of - open_tables() call. - - QQ: What about MYSQL_OPEN_HAS_MDL_LOCK flag? - */ - if (table_list->mdl_request.type >= MDL_SHARED_WRITE && - ! (flags & (MYSQL_OPEN_IGNORE_GLOBAL_READ_LOCK | - MYSQL_OPEN_FORCE_SHARED_MDL | - MYSQL_OPEN_FORCE_SHARED_HIGH_PRIO_MDL | - MYSQL_OPEN_SKIP_SCOPED_MDL_LOCK)) && - ! thd->locked_tables_mode && - ! ot_ctx->has_protection_against_grl()) + if (! (flags & MYSQL_OPEN_HAS_MDL_LOCK)) { - MDL_request protection_request; - MDL_deadlock_handler mdl_deadlock_handler(ot_ctx); + /* + We are not under LOCK TABLES and going to acquire write-lock/ + modify the base table. We need to acquire protection against + global read lock until end of this statement in order to have + this statement blocked by active FLUSH TABLES WITH READ LOCK. + + We don't block acquire this protection under LOCK TABLES as + such protection already acquired at LOCK TABLES time and + not released until UNLOCK TABLES. + + We don't block statements which modify only temporary tables + as these tables are not preserved by backup by any form of + backup which uses FLUSH TABLES WITH READ LOCK. + + TODO: The fact that we sometimes acquire protection against + GRL only when we encounter table to be write-locked + slightly increases probability of deadlock. + This problem will be solved once Alik pushes his + temporary table refactoring patch and we can start + pre-acquiring metadata locks at the beggining of + open_tables() call. + */ + if (table_list->mdl_request.type >= MDL_SHARED_WRITE && + ! (flags & (MYSQL_OPEN_IGNORE_GLOBAL_READ_LOCK | + MYSQL_OPEN_FORCE_SHARED_MDL | + MYSQL_OPEN_FORCE_SHARED_HIGH_PRIO_MDL | + MYSQL_OPEN_SKIP_SCOPED_MDL_LOCK)) && + ! ot_ctx->has_protection_against_grl()) + { + MDL_request protection_request; + MDL_deadlock_handler mdl_deadlock_handler(ot_ctx); - if (thd->global_read_lock.can_acquire_protection()) - DBUG_RETURN(TRUE); + if (thd->global_read_lock.can_acquire_protection()) + DBUG_RETURN(TRUE); - protection_request.init(MDL_key::GLOBAL, "", "", MDL_INTENTION_EXCLUSIVE, - MDL_STATEMENT); - /* - Install error handler which if possible will convert deadlock error - into request to back-off and restart process of opening tables. - */ - thd->push_internal_handler(&mdl_deadlock_handler); - bool result= thd->mdl_context.acquire_lock(&protection_request, - ot_ctx->get_timeout()); - thd->pop_internal_handler(); + protection_request.init(MDL_key::GLOBAL, "", "", MDL_INTENTION_EXCLUSIVE, + MDL_STATEMENT); - if (result) - DBUG_RETURN(TRUE); + /* + Install error handler which if possible will convert deadlock error + into request to back-off and restart process of opening tables. + */ + thd->push_internal_handler(&mdl_deadlock_handler); + bool result= thd->mdl_context.acquire_lock(&protection_request, + ot_ctx->get_timeout()); + thd->pop_internal_handler(); - ot_ctx->set_has_protection_against_grl(); - } + if (result) + DBUG_RETURN(TRUE); + ot_ctx->set_has_protection_against_grl(); + } - if (! (flags & MYSQL_OPEN_HAS_MDL_LOCK)) - { if (open_table_get_mdl_lock(thd, ot_ctx, &table_list->mdl_request, flags, &mdl_ticket) || mdl_ticket == NULL) === modified file 'sql/sql_parse.cc' --- a/sql/sql_parse.cc 2010-11-08 09:55:43 +0000 +++ b/sql/sql_parse.cc 2010-11-08 14:10:47 +0000 @@ -403,9 +403,7 @@ void init_update_queries(void) sql_command_flags[SQLCOM_FLUSH]= CF_AUTO_COMMIT_TRANS; sql_command_flags[SQLCOM_RESET]= CF_AUTO_COMMIT_TRANS; sql_command_flags[SQLCOM_CREATE_SERVER]= CF_AUTO_COMMIT_TRANS; - // TODO/FIXME Investigate effect of implicit close_cached_tables. sql_command_flags[SQLCOM_ALTER_SERVER]= CF_AUTO_COMMIT_TRANS; - // TODO/FIXME Investigate effect of implicit close_cached_tables. sql_command_flags[SQLCOM_DROP_SERVER]= CF_AUTO_COMMIT_TRANS; } No bundle (reason: useless for push emails).