From: Dmitry Lenev Date: September 30 2010 1:29pm Subject: bzr commit into mysql-5.5-runtime branch (Dmitry.Lenev:3150) Bug#56405 List-Archive: http://lists.mysql.com/commits/119541 X-Bug: 56405 Message-Id: <20100930132934.205FF1E54A9@mockturtle> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============0276672196==" --===============0276672196== 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-mrg/ based on revid:jon.hauglid@stripped 3150 Dmitry Lenev 2010-09-30 Reverted a temporary workaround for bug #56405 "Deadlock in the MDL deadlock detector". It is no longer needed as a better fix for this bug has been pushed. modified: mysql-test/suite/perfschema/r/dml_setup_instruments.result sql/mdl.cc sql/mdl.h sql/sql_base.cc sql/sql_base.h sql/table.cc === modified file 'mysql-test/suite/perfschema/r/dml_setup_instruments.result' --- a/mysql-test/suite/perfschema/r/dml_setup_instruments.result 2010-09-24 16:26:24 +0000 +++ b/mysql-test/suite/perfschema/r/dml_setup_instruments.result 2010-09-30 13:29:12 +0000 @@ -13,7 +13,7 @@ wait/synch/mutex/sql/LOCK_active_mi YES wait/synch/mutex/sql/LOCK_audit_mask YES YES wait/synch/mutex/sql/LOCK_connection_count YES YES wait/synch/mutex/sql/LOCK_crypt YES YES -wait/synch/mutex/sql/LOCK_dd_owns_lock_open YES YES +wait/synch/mutex/sql/LOCK_delayed_create YES YES select * from performance_schema.SETUP_INSTRUMENTS where name like 'Wait/Synch/Rwlock/sql/%' and name not in ('wait/synch/rwlock/sql/CRYPTO_dynlock_value::lock') === modified file 'sql/mdl.cc' --- a/sql/mdl.cc 2010-09-06 17:29:02 +0000 +++ b/sql/mdl.cc 2010-09-30 13:29:12 +0000 @@ -124,6 +124,7 @@ public: Deadlock_detection_visitor(MDL_context *start_node_arg) : m_start_node(start_node_arg), m_victim(NULL), + m_current_search_depth(0), m_found_deadlock(FALSE) {} virtual bool enter_node(MDL_context *node); @@ -132,8 +133,6 @@ public: virtual bool inspect_edge(MDL_context *dest); MDL_context *get_victim() const { return m_victim; } - - void abort_traversal(MDL_context *node); private: /** Change the deadlock victim to a new one if it has lower deadlock @@ -148,6 +147,13 @@ private: MDL_context *m_start_node; /** If a deadlock is found, the context that identifies the victim. */ MDL_context *m_victim; + /** Set to the 0 at start. Increased whenever + we descend into another MDL context (aka traverse to the next + wait-for graph node). When MAX_SEARCH_DEPTH is reached, we + assume that a deadlock is found, even if we have not found a + loop. + */ + uint m_current_search_depth; /** TRUE if we found a deadlock. */ bool m_found_deadlock; /** @@ -181,7 +187,7 @@ private: bool Deadlock_detection_visitor::enter_node(MDL_context *node) { - m_found_deadlock= m_current_search_depth >= MAX_SEARCH_DEPTH; + m_found_deadlock= ++m_current_search_depth >= MAX_SEARCH_DEPTH; if (m_found_deadlock) { DBUG_ASSERT(! m_victim); @@ -201,6 +207,7 @@ bool Deadlock_detection_visitor::enter_n void Deadlock_detection_visitor::leave_node(MDL_context *node) { + --m_current_search_depth; if (m_found_deadlock) opt_change_victim_to(node); } @@ -245,21 +252,6 @@ Deadlock_detection_visitor::opt_change_v /** - Abort traversal of a wait-for graph and report a deadlock. - - @param node Node which we were about to visit when abort - was initiated. -*/ - -void Deadlock_detection_visitor::abort_traversal(MDL_context *node) -{ - DBUG_ASSERT(! m_victim); - m_found_deadlock= TRUE; - opt_change_victim_to(node); -} - - -/** Get a bit corresponding to enum_mdl_type value in a granted/waiting bitmaps and compatibility matrices. */ @@ -2064,13 +2056,8 @@ bool MDL_lock::visit_subgraph(MDL_ticket are visiting it but this is OK: in the worst case we might do some extra work and one more context might be chosen as a victim. */ - ++gvisitor->m_current_search_depth; - if (gvisitor->enter_node(src_ctx)) - { - --gvisitor->m_current_search_depth; goto end; - } /* We do a breadth-first search first -- that is, inspect all @@ -2127,7 +2114,6 @@ bool MDL_lock::visit_subgraph(MDL_ticket end_leave_node: gvisitor->leave_node(src_ctx); - --gvisitor->m_current_search_depth; end: mysql_prlock_unlock(&m_rwlock); === modified file 'sql/mdl.h' --- a/sql/mdl.h 2010-09-06 17:29:02 +0000 +++ b/sql/mdl.h 2010-09-30 13:29:12 +0000 @@ -385,10 +385,7 @@ public: virtual bool inspect_edge(MDL_context *dest) = 0; virtual ~MDL_wait_for_graph_visitor(); - MDL_wait_for_graph_visitor() :m_lock_open_count(0), - m_current_search_depth(0) - { } - virtual void abort_traversal(MDL_context *node) = 0; + MDL_wait_for_graph_visitor() :m_lock_open_count(0) {} public: /** XXX, hack: During deadlock search, we may need to @@ -399,17 +396,6 @@ public: LOCK_open since it has significant performance impacts. */ uint m_lock_open_count; - /** - Set to the 0 at start. Increased whenever - we descend into another MDL context (aka traverse to the next - wait-for graph node). When MAX_SEARCH_DEPTH is reached, we - assume that a deadlock is found, even if we have not found a - loop. - - XXX: This member belongs to this class only temporarily until - bug #56405 is fixed. - */ - uint m_current_search_depth; }; /** === modified file 'sql/sql_base.cc' --- a/sql/sql_base.cc 2010-09-30 10:43:43 +0000 +++ b/sql/sql_base.cc 2010-09-30 13:29:12 +0000 @@ -100,14 +100,11 @@ bool No_such_table_error_handler::safely TABLE_SHAREs, refresh_version and the table id counter. */ mysql_mutex_t LOCK_open; -mysql_mutex_t LOCK_dd_owns_lock_open; -uint dd_owns_lock_open= 0; #ifdef HAVE_PSI_INTERFACE -static PSI_mutex_key key_LOCK_open, key_LOCK_dd_owns_lock_open; +static PSI_mutex_key key_LOCK_open; static PSI_mutex_info all_tdc_mutexes[]= { - { &key_LOCK_open, "LOCK_open", PSI_FLAG_GLOBAL }, - { &key_LOCK_dd_owns_lock_open, "LOCK_dd_owns_lock_open", PSI_FLAG_GLOBAL } + { &key_LOCK_open, "LOCK_open", PSI_FLAG_GLOBAL } }; /** @@ -302,8 +299,6 @@ bool table_def_init(void) init_tdc_psi_keys(); #endif mysql_mutex_init(key_LOCK_open, &LOCK_open, MY_MUTEX_INIT_FAST); - mysql_mutex_init(key_LOCK_dd_owns_lock_open, &LOCK_dd_owns_lock_open, - MY_MUTEX_INIT_FAST); oldest_unused_share= &end_of_unused_share; end_of_unused_share.prev= &oldest_unused_share; @@ -347,7 +342,6 @@ void table_def_free(void) table_def_inited= 0; /* Free table definitions. */ my_hash_free(&table_def_cache); - mysql_mutex_destroy(&LOCK_dd_owns_lock_open); mysql_mutex_destroy(&LOCK_open); } DBUG_VOID_RETURN; === modified file 'sql/sql_base.h' --- a/sql/sql_base.h 2010-09-30 10:43:43 +0000 +++ b/sql/sql_base.h 2010-09-30 13:29:12 +0000 @@ -70,8 +70,6 @@ enum enum_tdc_remove_table_type {TDC_RT_ bool check_dup(const char *db, const char *name, TABLE_LIST *tables); extern mysql_mutex_t LOCK_open; -extern mysql_mutex_t LOCK_dd_owns_lock_open; -extern uint dd_owns_lock_open; bool table_cache_init(void); void table_cache_free(void); bool table_def_init(void); === modified file 'sql/table.cc' --- a/sql/table.cc 2010-09-30 10:43:43 +0000 +++ b/sql/table.cc 2010-09-30 13:29:12 +0000 @@ -3085,30 +3085,7 @@ bool TABLE_SHARE::visit_subgraph(Wait_fo holding a write-lock on MDL_lock::m_rwlock. */ if (gvisitor->m_lock_open_count++ == 0) - { - /* - To circumvent bug #56405 "Deadlock in the MDL deadlock detector" - we don't try to lock LOCK_open mutex if some thread doing - deadlock detection already owns it and current search depth is - greater than 0. Instead we report a deadlock. - - TODO/FIXME: The proper fix for this bug is to use rwlocks for - protection of table shares/instead of LOCK_open. - Unfortunately it requires more effort/has significant - performance effect. - */ - mysql_mutex_lock(&LOCK_dd_owns_lock_open); - if (gvisitor->m_current_search_depth > 0 && dd_owns_lock_open > 0) - { - mysql_mutex_unlock(&LOCK_dd_owns_lock_open); - --gvisitor->m_lock_open_count; - gvisitor->abort_traversal(src_ctx); - return TRUE; - } - ++dd_owns_lock_open; - mysql_mutex_unlock(&LOCK_dd_owns_lock_open); mysql_mutex_lock(&LOCK_open); - } I_P_List_iterator tables_it(used_tables); @@ -3123,12 +3100,8 @@ bool TABLE_SHARE::visit_subgraph(Wait_fo goto end; } - ++gvisitor->m_current_search_depth; if (gvisitor->enter_node(src_ctx)) - { - --gvisitor->m_current_search_depth; goto end; - } while ((table= tables_it++)) { @@ -3151,16 +3124,10 @@ bool TABLE_SHARE::visit_subgraph(Wait_fo end_leave_node: gvisitor->leave_node(src_ctx); - --gvisitor->m_current_search_depth; end: if (gvisitor->m_lock_open_count-- == 1) - { mysql_mutex_unlock(&LOCK_open); - mysql_mutex_lock(&LOCK_dd_owns_lock_open); - --dd_owns_lock_open; - mysql_mutex_unlock(&LOCK_dd_owns_lock_open); - } return result; } --===============0276672196== 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-mrg/ # testament_sha1: 897e05b2fc511ec0f6f1df16a9fc3024437a6e0e # timestamp: 2010-09-30 17:29:33 +0400 # base_revision_id: jon.hauglid@stripped\ # awoh52s2kc9x9psk # # Begin bundle IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWaJVhMMABTBfgFAwWX///3/u 3OC////6YAmOFr5wEFLbQpMGNKABQSoCUQFPSj0yTU3hKPNCaTQB6gHtUNMmRjQmahEp5TanonpB tQDIHqNABkGmgAGmgAcZMmhiMTRgEYCYQBgJpo0yNAMJESaaNJNTaPSSeymJiJiANANA00GgAcZM mhiMTRgEYCYQBgJpo0yNAMJJBBoACABCYjRTaMSZDTQ9QNNGyjkL9dNZkttbnzbqmSUa4P55S+6+ 5rs64n7ex2NZKaFJt9xD7tdcI5+vDh+7Sq1Vh4cjKDSYZXdDr3so0icSU87Uxm2OOjrT88qVrY4J KQU8PX/2/HH0M7RfiLcJtpsbENoG02HD2/oBv137t87qm2+3KN7M2sWajwmAnCB4WL0xteU2scqz kVgvExElXZ2UIKKr1i9jUUarpMjJgKszbO/qy+OKq7yV5K/ZLquyM1REWDl2gRdK4wEU31Rg6xUm QqBo2xWSRCQFHWPOGSKAjqDEkaWAkJQWMHQlT2UTutx6OLTpsuak7m0eapVXQhxS9Amp+Nc2W3Z2 BU6DhCYfba8y+243d3rmPu3288iPPUbM1FeY4jj9UC5/qJicXEA8ydrncj5Ol1N5QeyrJHMNixNf 6v0cb3vxOZgwYC2RDGbhUQHk8XBKuGFRdi52ed7e3IS3shi5Gc7DVtpEtFOqFVqlPGT0hKOcGgUH DKgkt4YBIsJUTlABgnKuSSclJ0UBiPFoLHfhnTAMVszh3I2kFSSiuDXQZYUQKgMkCQudZJSmegyq 6TvOoobkg5YtptvXC7sROO3xYiA+nDNuQ9UkyKFHE5qbc61orzJfG2mO6NoBmrBkMeyDHK9Z5uyv DfXbQxAKoGA6LGoD0Y1OnWuU5Jq3Z6wsbQuJvZiGAUHtoNxdeeG9RW8LjUW0iaI1IFhE2RCH0KJF tt8tFGZ0TF8Z3ICBbcIB64FN7CHg2psnFEqnznKAaEHz5BW+IxYbACBaaRQeKxPEicSRNUOOJ6hT ZkC1GYrADsAd5lntM7q5NVtv4lAorEyzE2AdGROtzSVYLQiTAIC7RQAdql9QoNF3J5TBEiRiWDzi Rpexk1Vw7gSIIpBiV6eRNiP2M0bD6gED6GhnTi9ms5OdlMr2xxoAsQ8JCXqLIUhFSmvR3ToMLF76 BxNTTEsLuRIYywsNU7C1DkONJjSUxrZhscV4nsXNli92RlY/faO3lciFQcTyFVb9UZlN284CyPVQ +fcZ0fyvMH4ZzOecTlczoFTnN+pYkkoPGLnNo4zjB4SX6TCy3YdVJE/MeSJrRjxOw7QnLVcdrMbr zyjHeY4QhA3WAW9C2g3jFxUZsOLdo4rxKDsMj4cNFWV1RkVBUqiee4iSHHJEayBAzc4Y4sb6zbfS TDjHLXUC8kXj4joFg40NpwunvawsdRdLYGjng4lZPaTKTGlBvDeRlR7LEznbM3aldhavNsJJKFjM oOhTSsGciDh5IpeAabowvcJFQ4eWFOsJ5w1IhmZj7yTxxAwLSBIiVkiJZqLQT8opUTWyhAyLIeKW tpArADs8EskGOT7ez5Xf50vF9HvcUcgNmQtSRtoGN8PkFn2+NVE0B6ev2HtM/ADYxNrtd3DA3EEw 6unmJus3GA6/A2kThpaGAh291lG8Ql9VQqQU4SY4GETwHqK87TSf7D+jcfATRn0AvjyBxu5oXI4R U/afJiBDDJPVz70gnRQRlvxqCLNfuNaRETGfPYL9jgBqpEU2c95SmwfZkASYwjkFIl5VBA+6dHRm EnfZlGGOcKERLYvrwUCR75nvY5YuRIuOkTLUX/NJbTXgcD9CB00OA4mOKPzHFBiRJHtFLoSJET+n 33bbl6zQA98CKmEg64rJhYDr3DXlg88C8PzJrzpidRUTzm0qPuQTnRVFxaVmPi4NTyPUdwa8/rG4 ZAC8p7DlELjx6/OXjysOZTI23wiOlD3HTOaJxPvswOgxMqzUoNHFoFYFUTcQ6m07jwLSpIMp+R1L y84/MsK95uqZGjzkl6xbSAN+BvSYUODM3bKnu9XcP0Ccflp2e5WGMYCaYvN5nRjfdyNxURIeZ4eg qxc+DBgw60VqDvLDkWdDu0/GC0CBMWEKIznpZV5+rx3Nfo9EHTxZvm4stVfCHeF/v1SBptpokUJz gJA+Er4bn6TFcesiX5QxoWSEeLfOdBWPYS1UqvTcDd5sRE2B2OvqDjT6DiRImS7q0tiJz+ovcLRK SsRk/7G9zHuYc73nkZmAjdTEWKO4lKwpM0b8vEcwQPaJ45EOYlTCYTnCwmj9fbELjgabEKCKxubZ Gjm0nzQvMTo4d6VAgePq7IGoVsPjy0mzQb5l4iOJ3gDRcCLXCwWBibFWL0OpwRSrRxXs6isFajT6 YuQR+At6YZNwDcUoNHhAUk7wF5q4BOBUnZAegsKjhZMF1/y3EcPDqyBYHiO/T6S6t08zivj8mGa3 QTKbabxB8DVAceyHLCxEqUsrUX8i/dN9S+hMxUB1HOEzZir8P4aFXZA640EiKTychRZdiPAjFVHl khZTCbQNfYZ+nmxqfgpGw2o6ont7C+3NIVshMB8aINPcYcAfkAYrEbeAnIPsugnPV+waUaWLHrMy 8kFi3cmEwFJFKdHrRWcyF1LkUbRcAqkAWl6PQ5F6SrRuOhIpZmvcu95zAGeIDvkO2zg8uuHpagwH sy9kyWlfKsNEbUWwzruNoU74OJUAz2cIHB2BOYmvDU3VFoD5YTzYoFfMRAtgs0u1+M8KO9t5o6VM yOHybHrv2oNVTdMlvUWNfJhNFr1k2g5ARZ4igFbcWKonjKlxdgaFTwVmYR8Cav5CGjSXfmMqnHd4 EFFRwpSCOBjAHLT0jlejgAZrfzLhdC3U8Ly5ZI7AEVDINmZ1RjtGBFrapnMI65gDzu0Pd2mmM4Ng NqvMkJ+Zws59PxPA8RRANxNwHtvTAjYPORap+ZwXkcgDiTZmKUvM94BQar9v/i7kinChIUSrCYY= --===============0276672196==--