Skip to content

Commit a7b5c46

Browse files
author
Konstantin Osipov
committed
A new implementation for the TABLE_SHARE cache in MDL
subsystem. Fix a number of caveates that the previous implementation suffered from, including unprotected access to shared data and lax resource accounting (share->ref_count) that could lead to deadlocks. The new implementation still suffers from a number of potential deadlocks in some edge cases, and this is still not enabled by default. Especially since performance testing has shown that it gives only marginable (not even exceeding measuring accuracy) improvements. @todo: - Remove calls to close_cached_tables() with REFRESH_FAST, and have_lock, because they break the MDL cache. - rework FLUSH TABLES <list> to not use close_cached_tables() - make sure that whenever we set TABLE_SHARE::version to 0 we free MDL cache references to it. --BZR-- revision-id: [email protected] property-branch-nick: trunk-runtime-mdl-cache property-file-info: ld7:file_id40:mdl.cc-20080523121737-j62pi0m62eaw1hq6-17:message134:We may cache references to TABLE_SHARE objects in property-file-info: MDL_lock objects for tables. Create a separate property-file-info: MDL_lock class to represent a table.4:path10:sql/mdl.cced7:file_id39:mdl.h-20080523121748-o4y2wcq3maotb9do-17:message42:Adjust the MDL caching API to avoid races.4:path9:sql/mdl.hed7:file_id64:sp1f-sql_base.cc-19700101030959-w7tul2gb2n4jzayjwlslj3ybmf3uhk6a7:message154:Move all caching functionality close together. property-file-info: Implement a solution for deadlocks caused by property-file-info: close_cached_tables() when MDL cache is enabled (incomplete).4:path15:sql/sql_base.cced7:file_id64:sp1f-sql_yacc.yy-19700101030959-wvn4qyy2drpmge7kaq3dysprbhlrv27j7:message158:Adjust FLUSH rule to do the necessary initialization of property-file-info: TABLE_LIST elements used in for FLUSH TABLES <list>, and thus property-file-info: work OK with flush_mdl_cache() function.4:path15:sql/sql_yacc.yyee testament3-sha1: 9cb593d806d0531fdc31d259a0dc8a20b26ba047
1 parent 172a513 commit a7b5c46

File tree

4 files changed

+228
-80
lines changed

4 files changed

+228
-80
lines changed

‎sql/mdl.cc‎

Lines changed: 53 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -284,8 +284,8 @@ class MDL_lock
284284
public:
285285
/** The key of the object (data) being protected. */
286286
MDL_key key;
287-
void *cached_object;
288-
mdl_cached_object_release_hook cached_object_release_hook;
287+
/** A cached reference to the TABLE_SHARE. Protected by LOCK_open. */
288+
void *m_cached_object;
289289
/**
290290
Read-write lock protecting this lock context.
291291
@@ -362,15 +362,16 @@ class MDL_lock
362362

363363
MDL_lock(const MDL_key *key_arg)
364364
: key(key_arg),
365-
cached_object(NULL),
366-
cached_object_release_hook(NULL),
365+
m_cached_object(NULL),
367366
m_ref_usage(0),
368367
m_ref_release(0),
369368
m_is_destroyed(FALSE)
370369
{
371370
mysql_prlock_init(key_MDL_lock_rwlock, &m_rwlock);
372371
}
373372

373+
/* Overridden for TABLE objects, to support TABLE_SHARE cache in MDL. */
374+
virtual void release_cached_object() {}
374375
virtual ~MDL_lock()
375376
{
376377
mysql_prlock_destroy(&m_rwlock);
@@ -458,6 +459,25 @@ class MDL_object_lock : public MDL_lock
458459
};
459460

460461

462+
/**
463+
A lock implementation for MDL_key::TABLE.
464+
*/
465+
466+
class MDL_table_lock: public MDL_object_lock
467+
{
468+
public:
469+
MDL_table_lock(const MDL_key *key_arg)
470+
: MDL_object_lock(key_arg)
471+
{ }
472+
#ifdef DISABLED_UNTIL_GRL_IS_MADE_PART_OF_MDL
473+
virtual void release_cached_object()
474+
{
475+
tdc_release_cached_share(&m_cached_object);
476+
}
477+
#endif
478+
};
479+
480+
461481
static MDL_map mdl_locks;
462482

463483
extern "C"
@@ -674,8 +694,7 @@ void MDL_map::remove(MDL_lock *lock)
674694
{
675695
uint ref_usage, ref_release;
676696

677-
if (lock->cached_object)
678-
(*lock->cached_object_release_hook)(lock->cached_object);
697+
lock->release_cached_object();
679698

680699
/*
681700
Destroy the MDL_lock object, but ensure that anyone that is
@@ -839,6 +858,8 @@ inline MDL_lock *MDL_lock::create(const MDL_key *mdl_key)
839858
{
840859
case MDL_key::GLOBAL:
841860
return new MDL_global_lock(mdl_key);
861+
case MDL_key::TABLE:
862+
return new MDL_table_lock(mdl_key);
842863
default:
843864
return new MDL_object_lock(mdl_key);
844865
}
@@ -1181,11 +1202,6 @@ void MDL_lock::reschedule_waiters()
11811202
*/
11821203
m_waiting.remove_ticket(ticket);
11831204
m_granted.add_ticket(ticket);
1184-
1185-
/* If we are granting an X lock, release the cached object. */
1186-
if (ticket->get_type() == MDL_EXCLUSIVE && cached_object)
1187-
(*cached_object_release_hook)(cached_object);
1188-
cached_object= NULL;
11891205
}
11901206
/*
11911207
If we could not update the wait slot of the waiter,
@@ -1655,14 +1671,13 @@ MDL_context::try_acquire_lock_impl(MDL_request *mdl_request,
16551671
{
16561672
lock->m_granted.add_ticket(ticket);
16571673

1658-
if (mdl_request->type == MDL_EXCLUSIVE && lock->cached_object)
1659-
(*lock->cached_object_release_hook)(lock->cached_object);
1660-
lock->cached_object= NULL;
1661-
16621674
mysql_prlock_unlock(&lock->m_rwlock);
16631675

16641676
m_tickets.push_front(ticket);
16651677

1678+
if (ticket->get_type() == MDL_EXCLUSIVE)
1679+
ticket->clear_cached_object();
1680+
16661681
mdl_request->ticket= ticket;
16671682
}
16681683
else
@@ -1864,6 +1879,9 @@ MDL_context::acquire_lock(MDL_request *mdl_request, ulong lock_wait_timeout)
18641879
*/
18651880
DBUG_ASSERT(wait_status == MDL_wait::GRANTED);
18661881

1882+
if (ticket->get_type() == MDL_EXCLUSIVE)
1883+
ticket->clear_cached_object();
1884+
18671885
m_tickets.push_front(ticket);
18681886

18691887
mdl_request->ticket= ticket;
@@ -2450,7 +2468,7 @@ bool MDL_ticket::has_pending_conflicting_lock() const
24502468
24512469
This function has the following usage pattern:
24522470
- try to acquire an MDL lock
2453-
- when done, call for mdl_get_cached_object(). If it returns NULL, our
2471+
- when done, call for get_cached_object(). If it returns NULL, our
24542472
thread has the only lock on this table.
24552473
- look up TABLE_SHARE in the table definition cache
24562474
- call mdl_set_cache_object() to assign the share to the opaque pointer.
@@ -2460,28 +2478,33 @@ bool MDL_ticket::has_pending_conflicting_lock() const
24602478
*/
24612479

24622480
void
2463-
MDL_ticket::set_cached_object(void *cached_object,
2464-
mdl_cached_object_release_hook release_hook)
2481+
MDL_ticket::set_cached_object(void *cached_object)
24652482
{
2466-
DBUG_ENTER("mdl_set_cached_object");
2483+
DBUG_ENTER("MDL_ticket::set_cached_object");
24672484
DBUG_PRINT("enter", ("db=%s name=%s cached_object=%p",
24682485
m_lock->key.db_name(), m_lock->key.name(),
24692486
cached_object));
2470-
/*
2471-
TODO: This assumption works now since we do get_cached_object()
2472-
and set_cached_object() in the same critical section. Once
2473-
this becomes false we will have to call release_hook here and
2474-
use additional mutex protecting 'cached_object' member.
2475-
*/
2476-
DBUG_ASSERT(!m_lock->cached_object);
2487+
mysql_mutex_assert_owner(&LOCK_open);
2488+
DBUG_ASSERT(m_lock->key.mdl_namespace() == MDL_key::TABLE);
2489+
DBUG_ASSERT(!m_lock->m_cached_object);
24772490

2478-
m_lock->cached_object= cached_object;
2479-
m_lock->cached_object_release_hook= release_hook;
2491+
m_lock->m_cached_object= cached_object;
24802492

24812493
DBUG_VOID_RETURN;
24822494
}
24832495

24842496

2497+
/**
2498+
A helper function to flush the table share cached in MDL.
2499+
@pre The ticket is acquired.
2500+
*/
2501+
2502+
void MDL_ticket::clear_cached_object()
2503+
{
2504+
m_lock->release_cached_object();
2505+
}
2506+
2507+
24852508
/**
24862509
Get a pointer to an opaque object that associated with the lock.
24872510
@@ -2492,7 +2515,8 @@ MDL_ticket::set_cached_object(void *cached_object,
24922515

24932516
void *MDL_ticket::get_cached_object()
24942517
{
2495-
return m_lock->cached_object;
2518+
mysql_mutex_assert_owner(&LOCK_open);
2519+
return m_lock->m_cached_object;
24962520
}
24972521

24982522

‎sql/mdl.h‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -397,8 +397,8 @@ class MDL_ticket
397397
bool has_pending_conflicting_lock() const;
398398

399399
void *get_cached_object();
400-
void set_cached_object(void *cached_object,
401-
mdl_cached_object_release_hook release_hook);
400+
void set_cached_object(void *cached_object);
401+
void clear_cached_object();
402402
MDL_context *get_ctx() const { return m_ctx; }
403403
bool is_upgradable_or_exclusive() const
404404
{
@@ -724,6 +724,7 @@ extern "C" const char *set_thd_proc_info(void *thd_arg, const char *info,
724724
const char *calling_function,
725725
const char *calling_file,
726726
const unsigned int calling_line);
727+
extern void tdc_release_cached_share(void *ptr);
727728
#ifndef DBUG_OFF
728729
extern mysql_mutex_t LOCK_open;
729730
#endif

0 commit comments

Comments
 (0)