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.
This commit is contained in:
parent
a53f61e6e8
commit
8322cad015
@ -13,7 +13,7 @@ wait/synch/mutex/sql/LOCK_active_mi YES YES
|
|||||||
wait/synch/mutex/sql/LOCK_audit_mask YES YES
|
wait/synch/mutex/sql/LOCK_audit_mask YES YES
|
||||||
wait/synch/mutex/sql/LOCK_connection_count YES YES
|
wait/synch/mutex/sql/LOCK_connection_count YES YES
|
||||||
wait/synch/mutex/sql/LOCK_crypt 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
|
select * from performance_schema.SETUP_INSTRUMENTS
|
||||||
where name like 'Wait/Synch/Rwlock/sql/%'
|
where name like 'Wait/Synch/Rwlock/sql/%'
|
||||||
and name not in ('wait/synch/rwlock/sql/CRYPTO_dynlock_value::lock')
|
and name not in ('wait/synch/rwlock/sql/CRYPTO_dynlock_value::lock')
|
||||||
|
34
sql/mdl.cc
34
sql/mdl.cc
@ -124,6 +124,7 @@ public:
|
|||||||
Deadlock_detection_visitor(MDL_context *start_node_arg)
|
Deadlock_detection_visitor(MDL_context *start_node_arg)
|
||||||
: m_start_node(start_node_arg),
|
: m_start_node(start_node_arg),
|
||||||
m_victim(NULL),
|
m_victim(NULL),
|
||||||
|
m_current_search_depth(0),
|
||||||
m_found_deadlock(FALSE)
|
m_found_deadlock(FALSE)
|
||||||
{}
|
{}
|
||||||
virtual bool enter_node(MDL_context *node);
|
virtual bool enter_node(MDL_context *node);
|
||||||
@ -132,8 +133,6 @@ public:
|
|||||||
virtual bool inspect_edge(MDL_context *dest);
|
virtual bool inspect_edge(MDL_context *dest);
|
||||||
|
|
||||||
MDL_context *get_victim() const { return m_victim; }
|
MDL_context *get_victim() const { return m_victim; }
|
||||||
|
|
||||||
void abort_traversal(MDL_context *node);
|
|
||||||
private:
|
private:
|
||||||
/**
|
/**
|
||||||
Change the deadlock victim to a new one if it has lower deadlock
|
Change the deadlock victim to a new one if it has lower deadlock
|
||||||
@ -148,6 +147,13 @@ private:
|
|||||||
MDL_context *m_start_node;
|
MDL_context *m_start_node;
|
||||||
/** If a deadlock is found, the context that identifies the victim. */
|
/** If a deadlock is found, the context that identifies the victim. */
|
||||||
MDL_context *m_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. */
|
/** TRUE if we found a deadlock. */
|
||||||
bool m_found_deadlock;
|
bool m_found_deadlock;
|
||||||
/**
|
/**
|
||||||
@ -181,7 +187,7 @@ private:
|
|||||||
|
|
||||||
bool Deadlock_detection_visitor::enter_node(MDL_context *node)
|
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)
|
if (m_found_deadlock)
|
||||||
{
|
{
|
||||||
DBUG_ASSERT(! m_victim);
|
DBUG_ASSERT(! m_victim);
|
||||||
@ -201,6 +207,7 @@ bool Deadlock_detection_visitor::enter_node(MDL_context *node)
|
|||||||
|
|
||||||
void Deadlock_detection_visitor::leave_node(MDL_context *node)
|
void Deadlock_detection_visitor::leave_node(MDL_context *node)
|
||||||
{
|
{
|
||||||
|
--m_current_search_depth;
|
||||||
if (m_found_deadlock)
|
if (m_found_deadlock)
|
||||||
opt_change_victim_to(node);
|
opt_change_victim_to(node);
|
||||||
}
|
}
|
||||||
@ -244,21 +251,6 @@ Deadlock_detection_visitor::opt_change_victim_to(MDL_context *new_victim)
|
|||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
/**
|
|
||||||
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
|
Get a bit corresponding to enum_mdl_type value in a granted/waiting bitmaps
|
||||||
and compatibility matrices.
|
and compatibility matrices.
|
||||||
@ -2064,13 +2056,8 @@ bool MDL_lock::visit_subgraph(MDL_ticket *waiting_ticket,
|
|||||||
are visiting it but this is OK: in the worst case we might do some
|
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.
|
extra work and one more context might be chosen as a victim.
|
||||||
*/
|
*/
|
||||||
++gvisitor->m_current_search_depth;
|
|
||||||
|
|
||||||
if (gvisitor->enter_node(src_ctx))
|
if (gvisitor->enter_node(src_ctx))
|
||||||
{
|
|
||||||
--gvisitor->m_current_search_depth;
|
|
||||||
goto end;
|
goto end;
|
||||||
}
|
|
||||||
|
|
||||||
/*
|
/*
|
||||||
We do a breadth-first search first -- that is, inspect all
|
We do a breadth-first search first -- that is, inspect all
|
||||||
@ -2127,7 +2114,6 @@ bool MDL_lock::visit_subgraph(MDL_ticket *waiting_ticket,
|
|||||||
|
|
||||||
end_leave_node:
|
end_leave_node:
|
||||||
gvisitor->leave_node(src_ctx);
|
gvisitor->leave_node(src_ctx);
|
||||||
--gvisitor->m_current_search_depth;
|
|
||||||
|
|
||||||
end:
|
end:
|
||||||
mysql_prlock_unlock(&m_rwlock);
|
mysql_prlock_unlock(&m_rwlock);
|
||||||
|
16
sql/mdl.h
16
sql/mdl.h
@ -385,10 +385,7 @@ public:
|
|||||||
|
|
||||||
virtual bool inspect_edge(MDL_context *dest) = 0;
|
virtual bool inspect_edge(MDL_context *dest) = 0;
|
||||||
virtual ~MDL_wait_for_graph_visitor();
|
virtual ~MDL_wait_for_graph_visitor();
|
||||||
MDL_wait_for_graph_visitor() :m_lock_open_count(0),
|
MDL_wait_for_graph_visitor() :m_lock_open_count(0) {}
|
||||||
m_current_search_depth(0)
|
|
||||||
{ }
|
|
||||||
virtual void abort_traversal(MDL_context *node) = 0;
|
|
||||||
public:
|
public:
|
||||||
/**
|
/**
|
||||||
XXX, hack: During deadlock search, we may need to
|
XXX, hack: During deadlock search, we may need to
|
||||||
@ -399,17 +396,6 @@ public:
|
|||||||
LOCK_open since it has significant performance impacts.
|
LOCK_open since it has significant performance impacts.
|
||||||
*/
|
*/
|
||||||
uint m_lock_open_count;
|
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;
|
|
||||||
};
|
};
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
@ -100,14 +100,11 @@ bool No_such_table_error_handler::safely_trapped_errors()
|
|||||||
TABLE_SHAREs, refresh_version and the table id counter.
|
TABLE_SHAREs, refresh_version and the table id counter.
|
||||||
*/
|
*/
|
||||||
mysql_mutex_t LOCK_open;
|
mysql_mutex_t LOCK_open;
|
||||||
mysql_mutex_t LOCK_dd_owns_lock_open;
|
|
||||||
uint dd_owns_lock_open= 0;
|
|
||||||
|
|
||||||
#ifdef HAVE_PSI_INTERFACE
|
#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[]= {
|
static PSI_mutex_info all_tdc_mutexes[]= {
|
||||||
{ &key_LOCK_open, "LOCK_open", PSI_FLAG_GLOBAL },
|
{ &key_LOCK_open, "LOCK_open", PSI_FLAG_GLOBAL }
|
||||||
{ &key_LOCK_dd_owns_lock_open, "LOCK_dd_owns_lock_open", PSI_FLAG_GLOBAL }
|
|
||||||
};
|
};
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@ -302,8 +299,6 @@ bool table_def_init(void)
|
|||||||
init_tdc_psi_keys();
|
init_tdc_psi_keys();
|
||||||
#endif
|
#endif
|
||||||
mysql_mutex_init(key_LOCK_open, &LOCK_open, MY_MUTEX_INIT_FAST);
|
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;
|
oldest_unused_share= &end_of_unused_share;
|
||||||
end_of_unused_share.prev= &oldest_unused_share;
|
end_of_unused_share.prev= &oldest_unused_share;
|
||||||
|
|
||||||
@ -347,7 +342,6 @@ void table_def_free(void)
|
|||||||
table_def_inited= 0;
|
table_def_inited= 0;
|
||||||
/* Free table definitions. */
|
/* Free table definitions. */
|
||||||
my_hash_free(&table_def_cache);
|
my_hash_free(&table_def_cache);
|
||||||
mysql_mutex_destroy(&LOCK_dd_owns_lock_open);
|
|
||||||
mysql_mutex_destroy(&LOCK_open);
|
mysql_mutex_destroy(&LOCK_open);
|
||||||
}
|
}
|
||||||
DBUG_VOID_RETURN;
|
DBUG_VOID_RETURN;
|
||||||
|
@ -70,8 +70,6 @@ enum enum_tdc_remove_table_type {TDC_RT_REMOVE_ALL, TDC_RT_REMOVE_NOT_OWN,
|
|||||||
|
|
||||||
bool check_dup(const char *db, const char *name, TABLE_LIST *tables);
|
bool check_dup(const char *db, const char *name, TABLE_LIST *tables);
|
||||||
extern mysql_mutex_t LOCK_open;
|
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);
|
bool table_cache_init(void);
|
||||||
void table_cache_free(void);
|
void table_cache_free(void);
|
||||||
bool table_def_init(void);
|
bool table_def_init(void);
|
||||||
|
33
sql/table.cc
33
sql/table.cc
@ -3085,30 +3085,7 @@ bool TABLE_SHARE::visit_subgraph(Wait_for_flush *wait_for_flush,
|
|||||||
holding a write-lock on MDL_lock::m_rwlock.
|
holding a write-lock on MDL_lock::m_rwlock.
|
||||||
*/
|
*/
|
||||||
if (gvisitor->m_lock_open_count++ == 0)
|
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);
|
mysql_mutex_lock(&LOCK_open);
|
||||||
}
|
|
||||||
|
|
||||||
I_P_List_iterator <TABLE, TABLE_share> tables_it(used_tables);
|
I_P_List_iterator <TABLE, TABLE_share> tables_it(used_tables);
|
||||||
|
|
||||||
@ -3123,12 +3100,8 @@ bool TABLE_SHARE::visit_subgraph(Wait_for_flush *wait_for_flush,
|
|||||||
goto end;
|
goto end;
|
||||||
}
|
}
|
||||||
|
|
||||||
++gvisitor->m_current_search_depth;
|
|
||||||
if (gvisitor->enter_node(src_ctx))
|
if (gvisitor->enter_node(src_ctx))
|
||||||
{
|
|
||||||
--gvisitor->m_current_search_depth;
|
|
||||||
goto end;
|
goto end;
|
||||||
}
|
|
||||||
|
|
||||||
while ((table= tables_it++))
|
while ((table= tables_it++))
|
||||||
{
|
{
|
||||||
@ -3151,16 +3124,10 @@ bool TABLE_SHARE::visit_subgraph(Wait_for_flush *wait_for_flush,
|
|||||||
|
|
||||||
end_leave_node:
|
end_leave_node:
|
||||||
gvisitor->leave_node(src_ctx);
|
gvisitor->leave_node(src_ctx);
|
||||||
--gvisitor->m_current_search_depth;
|
|
||||||
|
|
||||||
end:
|
end:
|
||||||
if (gvisitor->m_lock_open_count-- == 1)
|
if (gvisitor->m_lock_open_count-- == 1)
|
||||||
{
|
|
||||||
mysql_mutex_unlock(&LOCK_open);
|
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;
|
return result;
|
||||||
}
|
}
|
||||||
|
Loading…
x
Reference in New Issue
Block a user