diff options
| author | Konstantin Osipov <kostja@sun.com> | 2009-12-08 12:57:07 +0300 |
|---|---|---|
| committer | Konstantin Osipov <kostja@sun.com> | 2009-12-08 12:57:07 +0300 |
| commit | ce5c87a3d359d528111d84b5c75bdb8acfe3fb9c (patch) | |
| tree | b2b94a32418f0728974d2bad4d3fa70acc2d75ef | |
| parent | 478e09609c0922c7838c2ae1e25f4a7b2aaa5970 (diff) | |
| download | mariadb-git-ce5c87a3d359d528111d84b5c75bdb8acfe3fb9c.tar.gz | |
Backport of:
----------------------------------------------------------
revno: 2617.69.20
committer: Konstantin Osipov <kostja@sun.com>
branch nick: 5.4-4284-1-assert
timestamp: Thu 2009-08-13 18:29:55 +0400
message:
WL#4284 "Transactional DDL locking"
A review fix.
Since WL#4284 implementation separated MDL_request and MDL_ticket,
MDL_request becamse a utility object necessary only to get a ticket.
Store it by-value in TABLE_LIST with the intent to merge
MDL_request::key with table_list->table_name and table_list->db
in future.
Change the MDL subsystem to not require MDL_requests to
stay around till close_thread_tables().
Remove the list of requests from the MDL context.
Requests for shared metadata locks acquired in open_tables()
are only used as a list in recover_from_failed_open_table_attempt(),
which calls mdl_context.wait_for_locks() for this list.
To keep such list for recover_from_failed_open_table_attempt(),
introduce a context class (Open_table_context), that collects
all requests.
A lot of minor cleanups and simplications that became possible
with this change.
sql/event_db_repository.cc:
Remove alloc_mdl_requests(). Now MDL_request instance is a member
of TABLE_LIST, and init_one_table() initializes it.
sql/ha_ndbcluster_binlog.cc:
Remove now unnecessary declaration and initialization
of binlog_mdl_request.
sql/lock.cc:
No need to allocate MDL requests in lock_table_names() now.
sql/log.cc:
Use init_one_table() method, remove alloc_mdl_requests(),
which is now unnecessary.
sql/log_event.cc:
No need to allocate mdl_request separately now.
Use init_one_table() method.
sql/log_event_old.cc:
Update to the new signature of close_tables_for_reopen().
sql/mdl.cc:
Update try_acquire_exclusive_lock() to be more easy to use.
Function lock_table_name_if_not_cached() has been removed.
Make acquire_shared_lock() signature consistent with
try_acquire_exclusive_lock() signature.
Remove methods that are no longer used.
Update comments.
sql/mdl.h:
Implement an assignment operator that doesn't
copy MDL_key (MDL_key::operator= is private and
should remain private).
This is a hack to work-around assignment of TABLE_LIST
by value in several places. Such assignments violate
encapsulation, since only perform a shallow copy.
In most cases these assignments are a hack on their own.
sql/mysql_priv.h:
Update signatures of close_thread_tables() and close_tables_for_reopen().
sql/sp.cc:
Allocate TABLE_LIST in thd->mem_root.
Use init_one_table().
sql/sp_head.cc:
Use init_one_table().
Remove thd->locked_tables_root, it's no longer needed.
sql/sql_acl.cc:
Use init_mdl_requests() and init_one_table().
sql/sql_base.cc:
Update to new signatures of try_acquire_shared_lock() and
try_acquire_exclusive_lock().
Remove lock_table_name_if_not_cached().
Fix a bug in open_ltable() that would not return ER_LOCK_DEADLOCK
in case of a failed lock_tables() and a multi-statement
transaction.
Fix a bug in open_and_lock_tables_derived() that would
not return ER_LOCK_DEADLOCK in case of a multi-statement
transaction and a failure of lock_tables().
Move assignment of enum_open_table_action to a method of Open_table_context, a new class that maintains information
for backoff actions.
Minor rearrangements of the code.
Remove alloc_mdl_requests() in functions that work with system
tables: instead the patch ensures that callers always initialize
TABLE_LIST argument.
sql/sql_class.cc:
THD::locked_tables_root is no more.
sql/sql_class.h:
THD::locked_tables_root is no more.
Add a declaration for Open_table_context class.
sql/sql_delete.cc:
Update to use the simplified MDL API.
sql/sql_handler.cc:
TABLE_LIST::mdl_request is stored by-value now.
Ensure that mdl_request.ticket is NULL for every request
that is passed into MDL, to satisfy MDL asserts.
@ sql/sql_help.cc
Function open_system_tables_for_read() no longer initializes
mdl_requests.
Move TABLE_LIST::mdl_request initialization closer to
TABLE_LIST initialization.
sql/sql_help.cc:
Function open_system_tables_for_read() no longer initializes
mdl_requests.
Move TABLE_LIST::mdl_request initialization closer to
TABLE_LIST initialization.
sql/sql_insert.cc:
Remove assignment by-value of TABLE_LIST in
TABLEOP_HOOKS. We can't carry over a granted
MDL ticket from one table list to another.
sql/sql_parse.cc:
Change alloc_mdl_requests() -> init_mdl_requests().
@todo We can remove init_mdl_requests() altogether
in some places: all places that call add_table_to_list()
already have mdl requests initialized.
sql/sql_plugin.cc:
Use init_one_table().
THD::locked_tables_root is no more.
sql/sql_servers.cc:
Use init_one_table().
sql/sql_show.cc:
Update acquire_high_priority_shared_lock() to use
TABLE_LIST::mdl_request rather than allocate an own.
Fix get_trigger_table_impl() to use init_one_table(),
check for out of memory, follow the coding style.
sql/sql_table.cc:
Update to work with TABLE_LIST::mdl_request by-value.
Remove lock_table_name_if_not_cached().
The code that used to delegate to it is quite simple and
concise without it now.
sql/sql_udf.cc:
Use init_one_table().
sql/sql_update.cc:
Update to use the new signature of close_tables_for_reopen().
sql/table.cc:
Move re-setting of mdl_requests for prepared statements
and stored procedures from close_thread_tables() to
reinit_stmt_before_use().
Change alloc_mdl_requests() to init_mdl_requests().
init_mdl_requests() is a hack that can't be deleted
until we don't have a list-aware TABLE_LIST constructor.
Hopefully its use will be minimal
sql/table.h:
Change alloc_mdl_requests() to init_mdl_requests()
TABLE_LIST::mdl_request is stored by value.
sql/tztime.cc:
We no longer initialize mdl requests in open_system_tables_for*()
functions. Move this initialization closer to initialization
of the rest of TABLE_LIST members.
storage/myisammrg/ha_myisammrg.cc:
Simplify mdl_request initialization.
| -rw-r--r-- | sql/event_db_repository.cc | 4 | ||||
| -rw-r--r-- | sql/ha_ndbcluster_binlog.cc | 10 | ||||
| -rw-r--r-- | sql/lock.cc | 21 | ||||
| -rw-r--r-- | sql/log.cc | 42 | ||||
| -rw-r--r-- | sql/log_event.cc | 18 | ||||
| -rw-r--r-- | sql/log_event_old.cc | 2 | ||||
| -rw-r--r-- | sql/mdl.cc | 242 | ||||
| -rw-r--r-- | sql/mdl.h | 85 | ||||
| -rw-r--r-- | sql/mysql_priv.h | 8 | ||||
| -rw-r--r-- | sql/sp.cc | 15 | ||||
| -rw-r--r-- | sql/sp_head.cc | 10 | ||||
| -rw-r--r-- | sql/sql_acl.cc | 25 | ||||
| -rw-r--r-- | sql/sql_base.cc | 283 | ||||
| -rw-r--r-- | sql/sql_class.cc | 3 | ||||
| -rw-r--r-- | sql/sql_class.h | 53 | ||||
| -rw-r--r-- | sql/sql_delete.cc | 41 | ||||
| -rw-r--r-- | sql/sql_handler.cc | 8 | ||||
| -rw-r--r-- | sql/sql_help.cc | 1 | ||||
| -rw-r--r-- | sql/sql_insert.cc | 33 | ||||
| -rw-r--r-- | sql/sql_parse.cc | 10 | ||||
| -rw-r--r-- | sql/sql_plugin.cc | 19 | ||||
| -rw-r--r-- | sql/sql_servers.cc | 22 | ||||
| -rw-r--r-- | sql/sql_show.cc | 82 | ||||
| -rw-r--r-- | sql/sql_table.cc | 204 | ||||
| -rw-r--r-- | sql/sql_udf.cc | 26 | ||||
| -rw-r--r-- | sql/sql_update.cc | 4 | ||||
| -rw-r--r-- | sql/table.cc | 14 | ||||
| -rw-r--r-- | sql/table.h | 8 | ||||
| -rw-r--r-- | sql/tztime.cc | 2 | ||||
| -rw-r--r-- | storage/myisammrg/ha_myisammrg.cc | 8 |
30 files changed, 568 insertions, 735 deletions
diff --git a/sql/event_db_repository.cc b/sql/event_db_repository.cc index 134c8059e13..9f53a73a594 100644 --- a/sql/event_db_repository.cc +++ b/sql/event_db_repository.cc @@ -555,7 +555,6 @@ Event_db_repository::open_event_table(THD *thd, enum thr_lock_type lock_type, DBUG_ENTER("Event_db_repository::open_event_table"); tables.init_one_table("mysql", 5, "event", 5, "event", lock_type); - alloc_mdl_requests(&tables, thd->mem_root); if (simple_open_n_lock_tables(thd, &tables)) { @@ -1110,7 +1109,6 @@ Event_db_repository::check_system_tables(THD *thd) /* Check mysql.db */ tables.init_one_table("mysql", 5, "db", 2, "db", TL_READ); - alloc_mdl_requests(&tables, thd->mem_root); if (simple_open_n_lock_tables(thd, &tables)) { @@ -1128,7 +1126,6 @@ Event_db_repository::check_system_tables(THD *thd) } /* Check mysql.user */ tables.init_one_table("mysql", 5, "user", 4, "user", TL_READ); - alloc_mdl_requests(&tables, thd->mem_root); if (simple_open_n_lock_tables(thd, &tables)) { @@ -1149,7 +1146,6 @@ Event_db_repository::check_system_tables(THD *thd) } /* Check mysql.event */ tables.init_one_table("mysql", 5, "event", 5, "event", TL_READ); - alloc_mdl_requests(&tables, thd->mem_root); if (simple_open_n_lock_tables(thd, &tables)) { diff --git a/sql/ha_ndbcluster_binlog.cc b/sql/ha_ndbcluster_binlog.cc index 7ce318394d4..8b3764367c2 100644 --- a/sql/ha_ndbcluster_binlog.cc +++ b/sql/ha_ndbcluster_binlog.cc @@ -140,7 +140,6 @@ static Uint64 *p_latest_trans_gci= 0; */ static TABLE *ndb_binlog_index= 0; static TABLE_LIST binlog_tables; -static MDL_request binlog_mdl_request; /* Helper functions @@ -2337,13 +2336,10 @@ static int open_ndb_binlog_index(THD *thd, TABLE **ndb_binlog_index) const char *save_proc_info= thd->proc_info; TABLE_LIST *tables= &binlog_tables; - bzero((char*) tables, sizeof(*tables)); - tables->db= repdb; - tables->alias= tables->table_name= reptable; - tables->lock_type= TL_WRITE; + tables->init_one_table(repdb, strlen(repdb), reptable, strlen(reptable), + reptable, TL_WRITE); thd->proc_info= "Opening " NDB_REP_DB "." NDB_REP_TABLE; - binlog_mdl_request.init(0, tables->db, tables->table_name); - tables->mdl_request= &binlog_mdl_request; + tables->required_type= FRMTYPE_TABLE; uint counter; thd->clear_error(); diff --git a/sql/lock.cc b/sql/lock.cc index 170007d8f66..aea1bfbd0e6 100644 --- a/sql/lock.cc +++ b/sql/lock.cc @@ -952,26 +952,18 @@ static MYSQL_LOCK *get_lock_data(THD *thd, TABLE **table_ptr, uint count, bool lock_table_names(THD *thd, TABLE_LIST *table_list) { + MDL_request_list mdl_requests; TABLE_LIST *lock_table; - MDL_request *mdl_request; for (lock_table= table_list; lock_table; lock_table= lock_table->next_local) { - mdl_request= MDL_request::create(0, lock_table->db, lock_table->table_name, - thd->mem_root); - if (!mdl_request) - goto end; - mdl_request->set_type(MDL_EXCLUSIVE); - thd->mdl_context.add_request(mdl_request); - lock_table->mdl_request= mdl_request; + lock_table->mdl_request.init(0, lock_table->db, lock_table->table_name, + MDL_EXCLUSIVE); + mdl_requests.push_front(&lock_table->mdl_request); } - if (thd->mdl_context.acquire_exclusive_locks()) - goto end; + if (thd->mdl_context.acquire_exclusive_locks(&mdl_requests)) + return 1; return 0; - -end: - thd->mdl_context.remove_all_requests(); - return 1; } @@ -987,7 +979,6 @@ void unlock_table_names(THD *thd) { DBUG_ENTER("unlock_table_names"); thd->mdl_context.release_all_locks(); - thd->mdl_context.remove_all_requests(); DBUG_VOID_RETURN; } diff --git a/sql/log.cc b/sql/log.cc index 48e509e9275..a74fc94d09d 100644 --- a/sql/log.cc +++ b/sql/log.cc @@ -423,14 +423,10 @@ bool Log_to_csv_event_handler:: save_thd_options= thd->options; thd->options&= ~OPTION_BIN_LOG; - bzero(& table_list, sizeof(TABLE_LIST)); - table_list.alias= table_list.table_name= GENERAL_LOG_NAME.str; - table_list.table_name_length= GENERAL_LOG_NAME.length; - - table_list.lock_type= TL_WRITE_CONCURRENT_INSERT; - - table_list.db= MYSQL_SCHEMA_NAME.str; - table_list.db_length= MYSQL_SCHEMA_NAME.length; + table_list.init_one_table(MYSQL_SCHEMA_NAME.str, MYSQL_SCHEMA_NAME.length, + GENERAL_LOG_NAME.str, GENERAL_LOG_NAME.length, + GENERAL_LOG_NAME.str, + TL_WRITE_CONCURRENT_INSERT); /* 1) open_performance_schema_table generates an error of the @@ -588,14 +584,10 @@ bool Log_to_csv_event_handler:: */ save_time_zone_used= thd->time_zone_used; - bzero(& table_list, sizeof(TABLE_LIST)); - table_list.alias= table_list.table_name= SLOW_LOG_NAME.str; - table_list.table_name_length= SLOW_LOG_NAME.length; - - table_list.lock_type= TL_WRITE_CONCURRENT_INSERT; - - table_list.db= MYSQL_SCHEMA_NAME.str; - table_list.db_length= MYSQL_SCHEMA_NAME.length; + table_list.init_one_table(MYSQL_SCHEMA_NAME.str, MYSQL_SCHEMA_NAME.length, + SLOW_LOG_NAME.str, SLOW_LOG_NAME.length, + SLOW_LOG_NAME.str, + TL_WRITE_CONCURRENT_INSERT); if (!(table= open_performance_schema_table(thd, & table_list, & open_tables_backup))) @@ -733,29 +725,25 @@ int Log_to_csv_event_handler:: { TABLE_LIST table_list; TABLE *table; + LEX_STRING *UNINIT_VAR(log_name); int result; Open_tables_state open_tables_backup; DBUG_ENTER("Log_to_csv_event_handler::activate_log"); - bzero(& table_list, sizeof(TABLE_LIST)); - if (log_table_type == QUERY_LOG_GENERAL) { - table_list.alias= table_list.table_name= GENERAL_LOG_NAME.str; - table_list.table_name_length= GENERAL_LOG_NAME.length; + log_name= &GENERAL_LOG_NAME; } else { DBUG_ASSERT(log_table_type == QUERY_LOG_SLOW); - table_list.alias= table_list.table_name= SLOW_LOG_NAME.str; - table_list.table_name_length= SLOW_LOG_NAME.length; - } - table_list.lock_type= TL_WRITE_CONCURRENT_INSERT; - - table_list.db= MYSQL_SCHEMA_NAME.str; - table_list.db_length= MYSQL_SCHEMA_NAME.length; + log_name= &SLOW_LOG_NAME; + } + table_list.init_one_table(MYSQL_SCHEMA_NAME.str, MYSQL_SCHEMA_NAME.length, + log_name->str, log_name->length, log_name->str, + TL_WRITE_CONCURRENT_INSERT); table= open_performance_schema_table(thd, & table_list, & open_tables_backup); diff --git a/sql/log_event.cc b/sql/log_event.cc index aa8dd6e9bff..46d016b2c15 100644 --- a/sql/log_event.cc +++ b/sql/log_event.cc @@ -8067,7 +8067,6 @@ int Table_map_log_event::do_apply_event(Relay_log_info const *rli) { RPL_TABLE_LIST *table_list; char *db_mem, *tname_mem; - MDL_request *mdl_request; size_t dummy_len; void *memory; DBUG_ENTER("Table_map_log_event::do_apply_event(Relay_log_info*)"); @@ -8082,21 +8081,18 @@ int Table_map_log_event::do_apply_event(Relay_log_info const *rli) &table_list, (uint) sizeof(RPL_TABLE_LIST), &db_mem, (uint) NAME_LEN + 1, &tname_mem, (uint) NAME_LEN + 1, - &mdl_request, sizeof(MDL_request), NullS))) DBUG_RETURN(HA_ERR_OUT_OF_MEM); - bzero(table_list, sizeof(*table_list)); - table_list->db = db_mem; - table_list->alias= table_list->table_name = tname_mem; - table_list->lock_type= TL_WRITE; - table_list->next_global= table_list->next_local= 0; + strmov(db_mem, rpl_filter->get_rewrite_db(m_dbnam, &dummy_len)); + strmov(tname_mem, m_tblnam); + + table_list->init_one_table(db_mem, strlen(db_mem), + tname_mem, strlen(tname_mem), + tname_mem, TL_WRITE); + table_list->table_id= m_table_id; table_list->updating= 1; - strmov(table_list->db, rpl_filter->get_rewrite_db(m_dbnam, &dummy_len)); - strmov(table_list->table_name, m_tblnam); - mdl_request->init(0, table_list->db, table_list->table_name); - table_list->mdl_request= mdl_request; int error= 0; diff --git a/sql/log_event_old.cc b/sql/log_event_old.cc index 90ea8b6cefe..942396fc3da 100644 --- a/sql/log_event_old.cc +++ b/sql/log_event_old.cc @@ -1506,7 +1506,7 @@ int Old_rows_log_event::do_apply_event(Relay_log_info const *rli) */ thd->binlog_flush_pending_rows_event(false); TABLE_LIST *tables= rli->tables_to_lock; - close_tables_for_reopen(thd, &tables, FALSE); + close_tables_for_reopen(thd, &tables); uint tables_count= rli->tables_to_lock_count; if ((error= open_tables(thd, &tables, &tables_count, 0))) diff --git a/sql/mdl.cc b/sql/mdl.cc index eb8fcdb323e..566a7c96b3b 100644 --- a/sql/mdl.cc +++ b/sql/mdl.cc @@ -194,7 +194,6 @@ void MDL_context::init(THD *thd_arg) rely here on the default constructors of I_P_List to empty the list. */ - m_requests.empty(); m_tickets.empty(); } @@ -213,7 +212,6 @@ void MDL_context::init(THD *thd_arg) void MDL_context::destroy() { - DBUG_ASSERT(m_requests.is_empty()); DBUG_ASSERT(m_tickets.is_empty()); DBUG_ASSERT(! m_has_global_shared_lock); } @@ -231,10 +229,8 @@ void MDL_context::destroy() void MDL_context::backup_and_reset(MDL_context *backup) { - DBUG_ASSERT(backup->m_requests.is_empty()); DBUG_ASSERT(backup->m_tickets.is_empty()); - m_requests.swap(backup->m_requests); m_tickets.swap(backup->m_tickets); backup->m_has_global_shared_lock= m_has_global_shared_lock; @@ -254,11 +250,9 @@ void MDL_context::backup_and_reset(MDL_context *backup) void MDL_context::restore_from_backup(MDL_context *backup) { - DBUG_ASSERT(m_requests.is_empty()); DBUG_ASSERT(m_tickets.is_empty()); DBUG_ASSERT(m_has_global_shared_lock == FALSE); - m_requests.swap(backup->m_requests); m_tickets.swap(backup->m_tickets); m_has_global_shared_lock= backup->m_has_global_shared_lock; } @@ -271,18 +265,9 @@ void MDL_context::restore_from_backup(MDL_context *backup) void MDL_context::merge(MDL_context *src) { MDL_ticket *ticket; - MDL_request *mdl_request; DBUG_ASSERT(m_thd == src->m_thd); - if (!src->m_requests.is_empty()) - { - Request_iterator it(src->m_requests); - while ((mdl_request= it++)) - m_requests.push_front(mdl_request); - src->m_requests.empty(); - } - if (!src->m_tickets.is_empty()) { Ticket_iterator it(src->m_tickets); @@ -315,15 +300,11 @@ void MDL_context::merge(MDL_context *src) for example in the grant subsystem, to lock privilege tables. The MDL subsystem does not own or manage memory of lock requests. - Instead it assumes that the life time of every lock request (including - encompassed members db/name) encloses calls to MDL_context::add_request() - and MDL_context::remove_request() or MDL_context::remove_all_requests(). @param type Id of type of object to be locked @param db Name of database to which the object belongs @param name Name of of the object - - The initialized lock request will have MDL_SHARED type. + @param mdl_type The MDL lock type for the request. Suggested lock types: TABLE - 0 PROCEDURE - 1 FUNCTION - 2 Note that tables and views must have the same lock type, since @@ -332,10 +313,11 @@ void MDL_context::merge(MDL_context *src) void MDL_request::init(unsigned char type_arg, const char *db_arg, - const char *name_arg) + const char *name_arg, + enum enum_mdl_type mdl_type_arg) { key.mdl_key_init(type_arg, db_arg, name_arg); - type= MDL_SHARED; + type= mdl_type_arg; ticket= NULL; } @@ -360,92 +342,21 @@ void MDL_request::init(unsigned char type_arg, MDL_request * MDL_request::create(unsigned char type, const char *db, - const char *name, MEM_ROOT *root) + const char *name, enum_mdl_type mdl_type, + MEM_ROOT *root) { MDL_request *mdl_request; if (!(mdl_request= (MDL_request*) alloc_root(root, sizeof(MDL_request)))) return NULL; - mdl_request->init(type, db, name); + mdl_request->init(type, db, name, mdl_type); return mdl_request; } /** - Add a lock request to the list of lock requests of the context. - - The procedure to acquire metadata locks is: - - allocate and initialize lock requests - (MDL_request::create()) - - associate them with a context (MDL_context::add_request()) - - call MDL_context::acquire_shared_lock() and - MDL_context::release_lock() (maybe repeatedly). - - Associates a lock request with the given context. - There should be no more than one context per connection, to - avoid deadlocks. - - @param mdl_request The lock request to be added. -*/ - -void MDL_context::add_request(MDL_request *mdl_request) -{ - DBUG_ENTER("MDL_context::add_request"); - DBUG_ASSERT(mdl_request->ticket == NULL); - m_requests.push_front(mdl_request); - DBUG_VOID_RETURN; -} - - -/** - Remove a lock request from the list of lock requests. - - Disassociates a lock request from the given context. - - @param mdl_request The lock request to be removed. - - @pre The lock request being removed should correspond to a ticket that - was released or was not acquired. - - @note Resets lock request back to its initial state - (i.e. sets type to MDL_SHARED). -*/ - -void MDL_context::remove_request(MDL_request *mdl_request) -{ - DBUG_ENTER("MDL_context::remove_request"); - /* Reset lock request back to its initial state. */ - mdl_request->type= MDL_SHARED; - mdl_request->ticket= NULL; - m_requests.remove(mdl_request); - DBUG_VOID_RETURN; -} - - -/** - Clear all lock requests in the context. - Disassociates lock requests from the context. - - Also resets lock requests back to their initial state (i.e. MDL_SHARED). -*/ - -void MDL_context::remove_all_requests() -{ - MDL_request *mdl_request; - Request_iterator it(m_requests); - while ((mdl_request= it++)) - { - /* Reset lock request back to its initial state. */ - mdl_request->type= MDL_SHARED; - mdl_request->ticket= NULL; - } - m_requests.empty(); -} - - -/** Auxiliary functions needed for creation/destruction of MDL_lock objects. @todo This naive implementation should be replaced with one that saves @@ -636,8 +547,10 @@ MDL_global_lock::is_lock_type_compatible(enum_mdl_type type, /** Check if request for the lock can be satisfied given current state of lock. - @param lock Lock. - @param mdl_request Request for lock. + @param requestor_ctx The context that identifies the owner of the request. + @param type_arg The requested lock type. + @param is_upgrade Must be set to TRUE when we are upgrading + a shared upgradable lock to exclusive. @retval TRUE Lock request can be satisfied @retval FALSE There is some conflicting lock. @@ -731,7 +644,7 @@ MDL_lock::can_grant_lock(const MDL_context *requestor_ctx, enum_mdl_type type_ar /** Check whether the context already holds a compatible lock ticket - on a object. Only shared locks can be recursive. + on an object. @param mdl_request Lock request object for lock to be acquired @@ -744,8 +657,6 @@ MDL_context::find_ticket(MDL_request *mdl_request) MDL_ticket *ticket; Ticket_iterator it(m_tickets); - DBUG_ASSERT(mdl_request->is_shared()); - while ((ticket= it++)) { if (mdl_request->type == ticket->m_type && @@ -762,35 +673,33 @@ MDL_context::find_ticket(MDL_request *mdl_request) Unlike exclusive locks, shared locks are acquired one by one. This is interface is chosen to simplify introduction of - the new locking API to the system. MDL_context::acquire_shared_lock() + the new locking API to the system. MDL_context::try_acquire_shared_lock() is currently used from open_table(), and there we have only one table to work with. In future we may consider allocating multiple shared locks at once. - This function must be called after the lock is added to a context. - - @param mdl_request [in] Lock request object for lock to be acquired - @param retry [out] Indicates that conflicting lock exists and another - attempt should be made after releasing all current - locks and waiting for conflicting lock go away - (using MDL_context::wait_for_locks()). + @param mdl_request [in/out] Lock request object for lock to be acquired - @retval FALSE Success. - @retval TRUE Failure. Either error occurred or conflicting lock exists. - In the latter case "retry" parameter is set to TRUE. + @retval FALSE Success. The lock may have not been acquired. + Check the ticket, if it's NULL, a conflicting lock + exists and another attempt should be made after releasing + all current locks and waiting for conflicting lock go + away (using MDL_context::wait_for_locks()). + @retval TRUE Out of resources, an error has been reported. */ bool -MDL_context::acquire_shared_lock(MDL_request *mdl_request, bool *retry) +MDL_context::try_acquire_shared_lock(MDL_request *mdl_request) { MDL_lock *lock; MDL_key *key= &mdl_request->key; MDL_ticket *ticket; - *retry= FALSE; DBUG_ASSERT(mdl_request->is_shared() && mdl_request->ticket == NULL); + /* Don't take chances in production. */ + mdl_request->ticket= NULL; safe_mutex_assert_not_owner(&LOCK_open); if (m_has_global_shared_lock && @@ -807,6 +716,8 @@ MDL_context::acquire_shared_lock(MDL_request *mdl_request, bool *retry) if ((ticket= find_ticket(mdl_request))) { DBUG_ASSERT(ticket->m_state == MDL_ACQUIRED); + /* Only shared locks can be recursive. */ + DBUG_ASSERT(ticket->is_shared()); mdl_request->ticket= ticket; return FALSE; } @@ -816,8 +727,7 @@ MDL_context::acquire_shared_lock(MDL_request *mdl_request, bool *retry) if (!global_lock.is_lock_type_compatible(mdl_request->type, FALSE)) { pthread_mutex_unlock(&LOCK_mdl); - *retry= TRUE; - return TRUE; + return FALSE; } if (!(ticket= MDL_ticket::create(this, mdl_request->type))) @@ -854,13 +764,11 @@ MDL_context::acquire_shared_lock(MDL_request *mdl_request, bool *retry) { /* We can't get here if we allocated a new lock. */ DBUG_ASSERT(! lock->is_empty()); - *retry= TRUE; MDL_ticket::destroy(ticket); } - pthread_mutex_unlock(&LOCK_mdl); - return *retry; + return FALSE; } @@ -889,6 +797,19 @@ static bool notify_shared_lock(THD *thd, MDL_ticket *conflicting_ticket) /** + Acquire a single exclusive lock. A convenience + wrapper around the method acquiring a list of locks. +*/ + +bool MDL_context::acquire_exclusive_lock(MDL_request *mdl_request) +{ + MDL_request_list mdl_requests; + mdl_requests.push_front(mdl_request); + return acquire_exclusive_locks(&mdl_requests); +} + + +/** Acquire exclusive locks. The context must contain the list of locks to be acquired. There must be no granted locks in the context. @@ -903,7 +824,7 @@ static bool notify_shared_lock(THD *thd, MDL_ticket *conflicting_ticket) @retval TRUE Failure */ -bool MDL_context::acquire_exclusive_locks() +bool MDL_context::acquire_exclusive_locks(MDL_request_list *mdl_requests) { MDL_lock *lock; bool signalled= FALSE; @@ -911,9 +832,11 @@ bool MDL_context::acquire_exclusive_locks() MDL_request *mdl_request; MDL_ticket *ticket; st_my_thread_var *mysys_var= my_thread_var; - Request_iterator it(m_requests); + MDL_request_list::Iterator it(*mdl_requests); safe_mutex_assert_not_owner(&LOCK_open); + /* Exclusive locks must always be acquired first, all at once. */ + DBUG_ASSERT(! has_locks()); if (m_has_global_shared_lock) { @@ -931,6 +854,9 @@ bool MDL_context::acquire_exclusive_locks() DBUG_ASSERT(mdl_request->type == MDL_EXCLUSIVE && mdl_request->ticket == NULL); + /* Don't take chances in production. */ + mdl_request->ticket= NULL; + /* Early allocation: ticket is used as a shortcut to the lock. */ if (!(ticket= MDL_ticket::create(this, mdl_request->type))) goto err; @@ -1024,10 +950,7 @@ bool MDL_context::acquire_exclusive_locks() return FALSE; err: - /* - Remove our pending lock requests from the locks. - Ignore those lock requests which were not made MDL_PENDING. - */ + /* Remove our pending tickets from the locks. */ it.rewind(); while ((mdl_request= it++) && mdl_request->ticket) { @@ -1163,17 +1086,16 @@ MDL_ticket::upgrade_shared_lock_to_exclusive() @param mdl_request [in] The lock request @param conflict [out] Indicates that conflicting lock exists - @retval TRUE Failure either conflicting lock exists or some error - occurred (probably OOM). - @retval FALSE Success, lock was acquired. + @retval TRUE Failure: some error occurred (probably OOM). + @retval FALSE Success: the lock might have not been acquired, + check request.ticket to find out. FIXME: Compared to lock_table_name_if_not_cached() it gives slightly more false negatives. */ bool -MDL_context::try_acquire_exclusive_lock(MDL_request *mdl_request, - bool *conflict) +MDL_context::try_acquire_exclusive_lock(MDL_request *mdl_request) { MDL_lock *lock; MDL_ticket *ticket; @@ -1184,7 +1106,7 @@ MDL_context::try_acquire_exclusive_lock(MDL_request *mdl_request, safe_mutex_assert_not_owner(&LOCK_open); - *conflict= FALSE; + mdl_request->ticket= NULL; pthread_mutex_lock(&LOCK_mdl); @@ -1197,7 +1119,8 @@ MDL_context::try_acquire_exclusive_lock(MDL_request *mdl_request, { MDL_ticket::destroy(ticket); MDL_lock::destroy(lock); - goto err; + pthread_mutex_unlock(&LOCK_mdl); + return TRUE; } mdl_request->ticket= ticket; lock->type= MDL_lock::MDL_LOCK_EXCLUSIVE; @@ -1206,16 +1129,9 @@ MDL_context::try_acquire_exclusive_lock(MDL_request *mdl_request, ticket->m_state= MDL_ACQUIRED; ticket->m_lock= lock; global_lock.active_intention_exclusive++; - pthread_mutex_unlock(&LOCK_mdl); - return FALSE; } - - /* There is some lock for the object. */ - *conflict= TRUE; - -err: pthread_mutex_unlock(&LOCK_mdl); - return TRUE; + return FALSE; } @@ -1262,7 +1178,7 @@ bool MDL_context::acquire_global_shared_lock() /** Wait until there will be no locks that conflict with lock requests - in the context. + in the given list. This is a part of the locking protocol and must be used by the acquirer of shared locks after a back-off. @@ -1274,11 +1190,11 @@ bool MDL_context::acquire_global_shared_lock() */ bool -MDL_context::wait_for_locks() +MDL_context::wait_for_locks(MDL_request_list *mdl_requests) { MDL_lock *lock; MDL_request *mdl_request; - Request_iterator it(m_requests); + MDL_request_list::Iterator it(*mdl_requests); const char *old_msg; st_my_thread_var *mysys_var= my_thread_var; @@ -1380,8 +1296,7 @@ void MDL_context::release_ticket(MDL_ticket *ticket) /** - Release all locks associated with the context, but leave them - in the context as lock requests. + Release all locks associated with the context. This function is used to back off in case of a lock conflict. It is also used to release shared locks in the end of an SQL @@ -1396,15 +1311,6 @@ void MDL_context::release_all_locks() safe_mutex_assert_not_owner(&LOCK_open); - /* Detach lock tickets from the requests for back off. */ - { - MDL_request *mdl_request; - Request_iterator it(m_requests); - - while ((mdl_request= it++)) - mdl_request->ticket= NULL; - } - if (m_tickets.is_empty()) DBUG_VOID_RETURN; @@ -1444,7 +1350,7 @@ void MDL_context::release_lock(MDL_ticket *ticket) /** Release all locks in the context which correspond to the same name/ - object as this lock request, remove lock requests from the context. + object as this lock request. @param ticket One of the locks for the name/object for which all locks should be released. @@ -1455,19 +1361,6 @@ void MDL_context::release_all_locks_for_name(MDL_ticket *name) /* Use MDL_ticket::lock to identify other locks for the same object. */ MDL_lock *lock= name->m_lock; - /* Remove matching lock requests from the context. */ - MDL_request *mdl_request; - Request_iterator it_mdl_request(m_requests); - - while ((mdl_request= it_mdl_request++)) - { - DBUG_ASSERT(mdl_request->ticket && - mdl_request->ticket->m_state == MDL_ACQUIRED); - - if (mdl_request->ticket->m_lock == lock) - remove_request(mdl_request); - } - /* Remove matching lock tickets from the context. */ MDL_ticket *ticket; Ticket_iterator it_ticket(m_tickets); @@ -1537,16 +1430,11 @@ bool MDL_context::is_exclusive_lock_owner(unsigned char type, const char *db, const char *name) { - MDL_key key(type, db, name); - MDL_ticket *ticket; - MDL_context::Ticket_iterator it(m_tickets); + MDL_request mdl_request; + mdl_request.init(type, db, name, MDL_EXCLUSIVE); + MDL_ticket *ticket= find_ticket(&mdl_request); - while ((ticket= it++)) - { - if (ticket->m_lock->type == MDL_lock::MDL_LOCK_EXCLUSIVE && - ticket->m_lock->key.is_equal(&key)) - break; - } + DBUG_ASSERT(ticket == NULL || ticket->m_state == MDL_ACQUIRED); return ticket; } diff --git a/sql/mdl.h b/sql/mdl.h index e3e41652bf4..dc6f0a34443 100644 --- a/sql/mdl.h +++ b/sql/mdl.h @@ -109,7 +109,6 @@ public: mdl_key_init(type_arg, db_arg, name_arg); } MDL_key() {} /* To use when part of MDL_request. */ - private: char m_ptr[MAX_MDLKEY_LENGTH]; uint m_length; @@ -157,12 +156,20 @@ public: /** Pointers for participating in the list of lock requests for this context. */ - MDL_request *next_in_context; - MDL_request **prev_in_context; + MDL_request *next_in_list; + MDL_request **prev_in_list; + /** + Pointer to the lock ticket object for this lock request. + Valid only if this lock request is satisfied. + */ + MDL_ticket *ticket; + /** A lock is requested based on a fully qualified name and type. */ MDL_key key; - void init(unsigned char type_arg, const char *db_arg, const char *name_arg); +public: + void init(unsigned char type_arg, const char *db_arg, const char *name_arg, + enum_mdl_type mdl_type_arg); /** Set type of lock request. Can be only applied to pending locks. */ inline void set_type(enum_mdl_type type_arg) { @@ -171,15 +178,37 @@ public: } bool is_shared() const { return type < MDL_EXCLUSIVE; } - /** - Pointer to the lock ticket object for this lock request. - Valid only if this lock request is satisfied. - */ - MDL_ticket *ticket; - static MDL_request *create(unsigned char type, const char *db, - const char *name, MEM_ROOT *root); + const char *name, enum_mdl_type mdl_type, + MEM_ROOT *root); + + /* + This is to work around the ugliness of TABLE_LIST + compiler-generated assignment operator. It is currently used + in several places to quickly copy "most" of the members of the + table list. These places currently never assume that the mdl + request is carried over to the new TABLE_LIST, or shared + between lists. + + This method does not initialize the instance being assigned! + Use of init() for initialization after this assignment operator + is mandatory. Can only be used before the request has been + granted. + */ + MDL_request& operator=(const MDL_request &rhs) + { + ticket= NULL; + /* Do nothing, in particular, don't try to copy the key. */ + return *this; + } + /* Another piece of ugliness for TABLE_LIST constructor */ + MDL_request() {} + MDL_request(const MDL_request *rhs) + :type(rhs->type), + ticket(NULL), + key(&rhs->key) + {} }; @@ -248,6 +277,11 @@ private: }; +typedef I_P_List<MDL_request, I_P_List_adapter<MDL_request, + &MDL_request::next_in_list, + &MDL_request::prev_in_list> > + MDL_request_list; + /** Context of the owner of metadata locks. I.e. each server connection has such a context. @@ -256,14 +290,6 @@ private: class MDL_context { public: - typedef I_P_List<MDL_request, - I_P_List_adapter<MDL_request, - &MDL_request::next_in_context, - &MDL_request::prev_in_context> > - Request_list; - - typedef Request_list::Iterator Request_iterator; - typedef I_P_List<MDL_ticket, I_P_List_adapter<MDL_ticket, &MDL_ticket::next_in_context, @@ -279,16 +305,13 @@ public: void restore_from_backup(MDL_context *backup); void merge(MDL_context *source); - void add_request(MDL_request *mdl_request); - void remove_request(MDL_request *mdl_request); - void remove_all_requests(); - - bool acquire_shared_lock(MDL_request *mdl_request, bool *retry); - bool acquire_exclusive_locks(); - bool try_acquire_exclusive_lock(MDL_request *mdl_request, bool *conflict); + bool try_acquire_shared_lock(MDL_request *mdl_request); + bool acquire_exclusive_lock(MDL_request *mdl_request); + bool acquire_exclusive_locks(MDL_request_list *requests); + bool try_acquire_exclusive_lock(MDL_request *mdl_request); bool acquire_global_shared_lock(); - bool wait_for_locks(); + bool wait_for_locks(MDL_request_list *requests); void release_all_locks(); void release_all_locks_for_name(MDL_ticket *ticket); @@ -312,16 +335,8 @@ public: void rollback_to_savepoint(MDL_ticket *mdl_savepoint); - /** - Get iterator for walking through all lock requests in the context. - */ - inline Request_iterator get_requests() - { - return Request_iterator(m_requests); - } inline THD *get_thd() const { return m_thd; } private: - Request_list m_requests; Ticket_list m_tickets; bool m_has_global_shared_lock; THD *m_thd; diff --git a/sql/mysql_priv.h b/sql/mysql_priv.h index 74c5af92229..d65850e0d18 100644 --- a/sql/mysql_priv.h +++ b/sql/mysql_priv.h @@ -827,7 +827,7 @@ extern my_decimal decimal_zero; void free_items(Item *item); void cleanup_items(Item *item); class THD; -void close_thread_tables(THD *thd, bool is_back_off= 0); +void close_thread_tables(THD *thd); #ifndef NO_EMBEDDED_ACCESS_CHECKS bool check_one_table_access(THD *thd, ulong privilege, TABLE_LIST *tables); @@ -1232,10 +1232,8 @@ void release_table_share(TABLE_SHARE *share); TABLE_SHARE *get_cached_table_share(const char *db, const char *table_name); TABLE *open_ltable(THD *thd, TABLE_LIST *table_list, thr_lock_type update, uint lock_flags); -enum enum_open_table_action {OT_NO_ACTION= 0, OT_BACK_OFF_AND_RETRY, - OT_DISCOVER, OT_REPAIR}; bool open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT* mem, - enum_open_table_action *action, uint flags); + Open_table_context *backoff, uint flags); bool tdc_open_view(THD *thd, TABLE_LIST *table_list, const char *alias, char *cache_key, uint cache_key_length, MEM_ROOT *mem_root, uint flags); @@ -1504,7 +1502,7 @@ void free_io_cache(TABLE *entry); void intern_close_table(TABLE *entry); bool close_thread_table(THD *thd, TABLE **table_ptr); void close_temporary_tables(THD *thd); -void close_tables_for_reopen(THD *thd, TABLE_LIST **tables, bool skip_mdl); +void close_tables_for_reopen(THD *thd, TABLE_LIST **tables); TABLE_LIST *find_table_in_list(TABLE_LIST *table, TABLE_LIST *TABLE_LIST::*link, const char *db_name, diff --git a/sql/sp.cc b/sql/sp.cc index e6999b480f2..68c8ad395a1 100644 --- a/sql/sp.cc +++ b/sql/sp.cc @@ -263,13 +263,11 @@ Stored_routine_creation_ctx::load_from_db(THD *thd, TABLE *open_proc_table_for_read(THD *thd, Open_tables_state *backup) { + TABLE_LIST table; + DBUG_ENTER("open_proc_table_for_read"); - TABLE_LIST table; - bzero((char*) &table, sizeof(table)); - table.db= (char*) "mysql"; - table.table_name= table.alias= (char*)"proc"; - table.lock_type= TL_READ; + table.init_one_table("mysql", 5, "proc", 4, "proc", TL_READ); if (!open_system_tables_for_read(thd, &table, backup)) DBUG_RETURN(table.table); @@ -294,13 +292,10 @@ TABLE *open_proc_table_for_read(THD *thd, Open_tables_state *backup) static TABLE *open_proc_table_for_update(THD *thd) { + TABLE_LIST table; DBUG_ENTER("open_proc_table_for_update"); - TABLE_LIST table; - bzero((char*) &table, sizeof(table)); - table.db= (char*) "mysql"; - table.table_name= table.alias= (char*)"proc"; - table.lock_type= TL_WRITE; + table.init_one_table("mysql", 5, "proc", 4, "proc", TL_WRITE); DBUG_RETURN(open_system_table_for_update(thd, &table)); } diff --git a/sql/sp_head.cc b/sql/sp_head.cc index 92019299f6c..ae798d063ea 100644 --- a/sql/sp_head.cc +++ b/sql/sp_head.cc @@ -3981,10 +3981,7 @@ sp_head::add_used_tables_to_table_list(THD *thd, table->prelocking_placeholder= 1; table->belong_to_view= belong_to_view; table->trg_event_map= stab->trg_event_map; - table->mdl_request= MDL_request::create(0, table->db, table->table_name, - thd->locked_tables_root ? - thd->locked_tables_root : - thd->mem_root); + table->mdl_request.init(0, table->db, table->table_name, MDL_SHARED); /* Everyting else should be zeroed */ @@ -4026,10 +4023,7 @@ sp_add_to_query_tables(THD *thd, LEX *lex, table->lock_type= locktype; table->select_lex= lex->current_select; table->cacheable_table= 1; - table->mdl_request= MDL_request::create(0, table->db, table->table_name, - thd->locked_tables_root ? - thd->locked_tables_root : - thd->mem_root); + table->mdl_request.init(0, table->db, table->table_name, MDL_SHARED); lex->add_to_query_tables(table); return table; diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index f4a182b321f..fdcd68cc2ea 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -691,7 +691,7 @@ my_bool acl_reload(THD *thd) tables[0].lock_type=tables[1].lock_type=tables[2].lock_type=TL_READ; tables[0].skip_temporary= tables[1].skip_temporary= tables[2].skip_temporary= TRUE; - alloc_mdl_requests(tables, thd->mem_root); + init_mdl_requests(tables); if (simple_open_n_lock_tables(thd, tables)) { @@ -1596,10 +1596,7 @@ bool change_password(THD *thd, const char *host, const char *user, if (check_change_password(thd, host, user, new_password, new_password_len)) DBUG_RETURN(1); - bzero((char*) &tables, sizeof(tables)); - tables.alias= tables.table_name= (char*) "user"; - tables.db= (char*) "mysql"; - alloc_mdl_requests(&tables, thd->mem_root); + tables.init_one_table("mysql", 5, "user", 4, "user", TL_WRITE); #ifdef HAVE_REPLICATION /* @@ -3111,7 +3108,7 @@ int mysql_table_grant(THD *thd, TABLE_LIST *table_list, ? tables+2 : 0); tables[0].lock_type=tables[1].lock_type=tables[2].lock_type=TL_WRITE; tables[0].db=tables[1].db=tables[2].db=(char*) "mysql"; - alloc_mdl_requests(tables, thd->mem_root); + init_mdl_requests(tables); /* This statement will be replicated as a statement, even when using @@ -3329,7 +3326,7 @@ bool mysql_routine_grant(THD *thd, TABLE_LIST *table_list, bool is_proc, tables[0].next_local= tables[0].next_global= tables+1; tables[0].lock_type=tables[1].lock_type=TL_WRITE; tables[0].db=tables[1].db=(char*) "mysql"; - alloc_mdl_requests(tables, thd->mem_root); + init_mdl_requests(tables); /* This statement will be replicated as a statement, even when using @@ -3468,7 +3465,7 @@ bool mysql_grant(THD *thd, const char *db, List <LEX_USER> &list, tables[0].next_local= tables[0].next_global= tables+1; tables[0].lock_type=tables[1].lock_type=TL_WRITE; tables[0].db=tables[1].db=(char*) "mysql"; - alloc_mdl_requests(tables, thd->mem_root); + init_mdl_requests(tables); /* This statement will be replicated as a statement, even when using @@ -3797,12 +3794,10 @@ static my_bool grant_reload_procs_priv(THD *thd) my_bool return_val= FALSE; DBUG_ENTER("grant_reload_procs_priv"); - bzero((char*) &table, sizeof(table)); - table.alias= table.table_name= (char*) "procs_priv"; - table.db= (char *) "mysql"; - table.lock_type= TL_READ; + table.init_one_table("mysql", 5, "procs_priv", + strlen("procs_priv"), "procs_priv", + TL_READ); table.skip_temporary= 1; - alloc_mdl_requests(&table, thd->mem_root); if (simple_open_n_lock_tables(thd, &table)) { @@ -3869,7 +3864,7 @@ my_bool grant_reload(THD *thd) tables[0].next_local= tables[0].next_global= tables+1; tables[0].lock_type= tables[1].lock_type= TL_READ; tables[0].skip_temporary= tables[1].skip_temporary= TRUE; - alloc_mdl_requests(tables, thd->mem_root); + init_mdl_requests(tables); /* To avoid deadlocks we should obtain table locks before @@ -5219,7 +5214,7 @@ int open_grant_tables(THD *thd, TABLE_LIST *tables) (tables+4)->lock_type= TL_WRITE; tables->db= (tables+1)->db= (tables+2)->db= (tables+3)->db= (tables+4)->db= (char*) "mysql"; - alloc_mdl_requests(tables, thd->mem_root); + init_mdl_requests(tables); #ifdef HAVE_REPLICATION /* diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 02871c118ca..7650d854efb 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -125,7 +125,8 @@ static bool open_new_frm(THD *thd, TABLE_SHARE *share, const char *alias, uint db_stat, uint prgflag, uint ha_open_flags, TABLE *outparam, TABLE_LIST *table_desc, MEM_ROOT *mem_root); -static bool tdc_wait_for_old_versions(THD *thd, MDL_context *context); +static bool tdc_wait_for_old_versions(THD *thd, + MDL_request_list *mdl_requests); static bool has_write_table_with_auto_increment(TABLE_LIST *tables); @@ -1360,8 +1361,7 @@ close_all_tables_for_name(THD *thd, TABLE_SHARE *share, leave prelocked mode if needed. */ -void close_thread_tables(THD *thd, - bool is_back_off) +void close_thread_tables(THD *thd) { TABLE *table; DBUG_ENTER("close_thread_tables"); @@ -1471,10 +1471,6 @@ void close_thread_tables(THD *thd, thd->locked_tables_mode= LTM_NONE; - /* - Note that we are leaving prelocked mode so we don't need - to care about THD::locked_tables_root. - */ /* Fallthrough */ } @@ -1503,11 +1499,6 @@ void close_thread_tables(THD *thd, if (thd->open_tables) close_open_tables(thd); - if (!is_back_off) - { - thd->mdl_context.remove_all_requests(); - } - /* Defer the release of metadata locks until the current transaction is either committed or rolled back. This prevents other statements @@ -2316,10 +2307,10 @@ void table_share_release_hook(void *share) static bool open_table_get_mdl_lock(THD *thd, TABLE_LIST *table_list, MDL_request *mdl_request, - uint flags, - enum_open_table_action *action) + Open_table_context *ot_ctx, + uint flags) { - thd->mdl_context.add_request(mdl_request); + ot_ctx->add_request(mdl_request); if (table_list->lock_strategy) { @@ -2333,16 +2324,13 @@ open_table_get_mdl_lock(THD *thd, TABLE_LIST *table_list, enforced by asserts in metadata locking subsystem. */ mdl_request->set_type(MDL_EXCLUSIVE); - if (thd->mdl_context.acquire_exclusive_locks()) - { - thd->mdl_context.remove_request(mdl_request); + DBUG_ASSERT(! thd->mdl_context.has_locks()); + + if (thd->mdl_context.acquire_exclusive_lock(mdl_request)) return 1; - } } else { - bool retry; - /* There is no MDL_SHARED_UPGRADABLE_HIGH_PRIO type of metadata lock so we want to be sure that caller doesn't pass us both flags simultaneously. @@ -2356,12 +2344,11 @@ open_table_get_mdl_lock(THD *thd, TABLE_LIST *table_list, if (flags & MYSQL_LOCK_IGNORE_FLUSH) mdl_request->set_type(MDL_SHARED_HIGH_PRIO); - if (thd->mdl_context.acquire_shared_lock(mdl_request, &retry)) + if (thd->mdl_context.try_acquire_shared_lock(mdl_request)) + return 1; + if (mdl_request->ticket == NULL) { - if (retry) - *action= OT_BACK_OFF_AND_RETRY; - else - thd->mdl_context.remove_request(mdl_request); + (void) ot_ctx->request_backoff_action(Open_table_context::OT_WAIT); return 1; } } @@ -2416,7 +2403,7 @@ open_table_get_mdl_lock(THD *thd, TABLE_LIST *table_list, bool open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root, - enum_open_table_action *action, uint flags) + Open_table_context *ot_ctx, uint flags) { reg1 TABLE *table; char key[MAX_DBKEY_LENGTH]; @@ -2428,11 +2415,6 @@ bool open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root, TABLE_SHARE *share; DBUG_ENTER("open_table"); - /* Parsing of partitioning information from .frm needs thd->lex set up. */ - DBUG_ASSERT(thd->lex->is_lex_started); - - *action= OT_NO_ACTION; - /* an open table operation needs a lot of the stack space */ if (check_stack_overrun(thd, STACK_MIN_SIZE_FOR_OPEN, (uchar *)&alias)) DBUG_RETURN(TRUE); @@ -2602,12 +2584,15 @@ bool open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root, This is the normal use case. */ - mdl_request= table_list->mdl_request; + mdl_request= &table_list->mdl_request; if (! (flags & MYSQL_OPEN_HAS_MDL_LOCK)) { - if (open_table_get_mdl_lock(thd, table_list, mdl_request, flags, - action)) + if (open_table_get_mdl_lock(thd, table_list, mdl_request, ot_ctx, flags)) + { + DEBUG_SYNC(thd, "before_open_table_wait_refresh"); DBUG_RETURN(TRUE); + } + DEBUG_SYNC(thd, "after_open_table_mdl_shared"); } /* @@ -2633,8 +2618,8 @@ bool open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root, ! (flags & MYSQL_LOCK_IGNORE_FLUSH)) { /* Someone did a refresh while thread was opening tables */ - *action= OT_BACK_OFF_AND_RETRY; pthread_mutex_unlock(&LOCK_open); + (void) ot_ctx->request_backoff_action(Open_table_context::OT_WAIT); DBUG_RETURN(TRUE); } @@ -2766,9 +2751,9 @@ bool open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root, separately in the caller for old table versions to go away (see tdc_wait_for_old_versions()). */ - *action= OT_BACK_OFF_AND_RETRY; release_table_share(share); pthread_mutex_unlock(&LOCK_open); + (void) ot_ctx->request_backoff_action(Open_table_context::OT_WAIT); DBUG_RETURN(TRUE); } /* Force close at once after usage */ @@ -2808,12 +2793,12 @@ bool open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root, if (error == 7) { share->version= 0; - *action= OT_DISCOVER; + (void) ot_ctx->request_backoff_action(Open_table_context::OT_DISCOVER); } else if (share->crashed) { share->version= 0; - *action= OT_REPAIR; + (void) ot_ctx->request_backoff_action(Open_table_context::OT_REPAIR); } goto err_unlock; @@ -2891,10 +2876,8 @@ err_unlock: err_unlock2: pthread_mutex_unlock(&LOCK_open); if (! (flags & MYSQL_OPEN_HAS_MDL_LOCK)) - { thd->mdl_context.release_lock(mdl_ticket); - thd->mdl_context.remove_request(mdl_request); - } + DBUG_RETURN(TRUE); } @@ -3004,6 +2987,9 @@ Locked_tables_list::init_locked_tables(THD *thd) return TRUE; } + memcpy(db, src_table_list->db, db_len + 1); + memcpy(table_name, src_table_list->table_name, table_name_len + 1); + memcpy(alias, src_table_list->alias, alias_len + 1); /** Sic: remember the *actual* table level lock type taken, to acquire the exact same type in reopen_tables(). @@ -3014,11 +3000,9 @@ Locked_tables_list::init_locked_tables(THD *thd) dst_table_list->init_one_table(db, db_len, table_name, table_name_len, alias, src_table_list->table->reginfo.lock_type); - dst_table_list->mdl_request= src_table_list->mdl_request; dst_table_list->table= table; - memcpy(db, src_table_list->db, db_len + 1); - memcpy(table_name, src_table_list->table_name, table_name_len + 1); - memcpy(alias, src_table_list->alias, alias_len + 1); + dst_table_list->mdl_request.ticket= src_table_list->mdl_request.ticket; + /* Link last into the list of tables */ *(dst_table_list->prev_global= m_locked_tables_last)= dst_table_list; m_locked_tables_last= &dst_table_list->next_global; @@ -3227,7 +3211,7 @@ unlink_all_closed_tables(THD *thd, MYSQL_LOCK *lock, size_t reopen_count) bool Locked_tables_list::reopen_tables(THD *thd) { - enum enum_open_table_action ot_action_unused; + Open_table_context ot_ctx_unused(thd); bool lt_refresh_unused; size_t reopen_count= 0; MYSQL_LOCK *lock; @@ -3240,7 +3224,7 @@ Locked_tables_list::reopen_tables(THD *thd) continue; /* Links into thd->open_tables upon success */ - if (open_table(thd, table_list, thd->mem_root, &ot_action_unused, + if (open_table(thd, table_list, thd->mem_root, &ot_ctx_unused, MYSQL_OPEN_REOPEN)) { unlink_all_closed_tables(thd, 0, reopen_count); @@ -3591,6 +3575,43 @@ end_with_lock_open: } +/** Open_table_context */ + +Open_table_context::Open_table_context(THD *thd) + :m_action(OT_NO_ACTION), + m_can_deadlock(thd->in_multi_stmt_transaction() && + thd->mdl_context.has_locks()) +{} + + +/** + Check if we can back-off and set back off action if we can. + Otherwise report and return error. + + @retval TRUE if back-off is impossible. + @retval FALSE if we can back off. Back off action has been set. +*/ + +bool +Open_table_context:: +request_backoff_action(enum_open_table_action action_arg) +{ + /* + We have met a exclusive metadata lock or a old version of + table and we are inside a transaction that already hold locks. + We can't follow the locking protocol in this scenario as it + might lead to deadlocks. + */ + if (m_can_deadlock) + { + my_error(ER_LOCK_DEADLOCK, MYF(0)); + return TRUE; + } + m_action= action_arg; + return FALSE; +} + + /** Recover from failed attempt ot open table by performing requested action. @@ -3604,57 +3625,58 @@ end_with_lock_open: @retval TRUE - Error */ -static bool -recover_from_failed_open_table_attempt(THD *thd, TABLE_LIST *table, - enum_open_table_action action) +bool +Open_table_context:: +recover_from_failed_open_table_attempt(THD *thd, TABLE_LIST *table) { bool result= FALSE; - MDL_request *mdl_request= table->mdl_request; - - switch (action) + /* Execute the action. */ + switch (m_action) { - case OT_BACK_OFF_AND_RETRY: - result= (thd->mdl_context.wait_for_locks() || - tdc_wait_for_old_versions(thd, &thd->mdl_context)); - thd->mdl_context.remove_all_requests(); + case OT_WAIT: + result= (thd->mdl_context.wait_for_locks(&m_mdl_requests) || + tdc_wait_for_old_versions(thd, &m_mdl_requests)); break; case OT_DISCOVER: - mdl_request->set_type(MDL_EXCLUSIVE); - thd->mdl_context.add_request(mdl_request); - if (thd->mdl_context.acquire_exclusive_locks()) { - thd->mdl_context.remove_request(mdl_request); - return TRUE; + MDL_request mdl_xlock_request(&table->mdl_request); + mdl_xlock_request.set_type(MDL_EXCLUSIVE); + if ((result= + thd->mdl_context.acquire_exclusive_lock(&mdl_xlock_request))) + break; + pthread_mutex_lock(&LOCK_open); + tdc_remove_table(thd, TDC_RT_REMOVE_ALL, table->db, table->table_name); + ha_create_table_from_engine(thd, table->db, table->table_name); + pthread_mutex_unlock(&LOCK_open); + + thd->warning_info->clear_warning_info(thd->query_id); + thd->clear_error(); // Clear error message + thd->mdl_context.release_lock(mdl_xlock_request.ticket); + break; } - pthread_mutex_lock(&LOCK_open); - tdc_remove_table(thd, TDC_RT_REMOVE_ALL, table->db, table->table_name); - ha_create_table_from_engine(thd, table->db, table->table_name); - pthread_mutex_unlock(&LOCK_open); - - thd->warning_info->clear_warning_info(thd->query_id); - thd->clear_error(); // Clear error message - thd->mdl_context.release_lock(mdl_request->ticket); - thd->mdl_context.remove_request(mdl_request); - break; case OT_REPAIR: - mdl_request->set_type(MDL_EXCLUSIVE); - thd->mdl_context.add_request(mdl_request); - if (thd->mdl_context.acquire_exclusive_locks()) { - thd->mdl_context.remove_request(mdl_request); - return TRUE; - } - pthread_mutex_lock(&LOCK_open); - tdc_remove_table(thd, TDC_RT_REMOVE_ALL, table->db, table->table_name); - pthread_mutex_unlock(&LOCK_open); + MDL_request mdl_xlock_request(&table->mdl_request); + mdl_xlock_request.set_type(MDL_EXCLUSIVE); + if ((result= + thd->mdl_context.acquire_exclusive_lock(&mdl_xlock_request))) + break; - result= auto_repair_table(thd, table); - thd->mdl_context.release_lock(mdl_request->ticket); - thd->mdl_context.remove_request(mdl_request); - break; + pthread_mutex_lock(&LOCK_open); + tdc_remove_table(thd, TDC_RT_REMOVE_ALL, table->db, table->table_name); + pthread_mutex_unlock(&LOCK_open); + + result= auto_repair_table(thd, table); + thd->mdl_context.release_lock(mdl_xlock_request.ticket); + break; + } default: DBUG_ASSERT(0); } + /* Remove all old requests, they will be re-added. */ + m_mdl_requests.empty(); + /* Prepare for possible another back-off. */ + m_action= OT_NO_ACTION; return result; } @@ -3722,14 +3744,13 @@ thr_lock_type read_lock_type_for_table(THD *thd, TABLE *table) int open_tables(THD *thd, TABLE_LIST **start, uint *counter, uint flags) { TABLE_LIST *tables= NULL; - enum_open_table_action action; + Open_table_context ot_ctx(thd); int result=0; bool error; MEM_ROOT new_frm_mem; /* Also used for indicating that prelocking is need */ TABLE_LIST **query_tables_last_own; bool safe_to_ignore_table; - bool has_locks= thd->mdl_context.has_locks(); DBUG_ENTER("open_tables"); /* temporary mem_root for new .frm parsing. @@ -3856,32 +3877,20 @@ int open_tables(THD *thd, TABLE_LIST **start, uint *counter, uint flags) */ Prelock_error_handler prelock_handler; thd->push_internal_handler(& prelock_handler); - error= open_table(thd, tables, &new_frm_mem, &action, flags); + error= open_table(thd, tables, &new_frm_mem, &ot_ctx, flags); thd->pop_internal_handler(); safe_to_ignore_table= prelock_handler.safely_trapped_errors(); } else - error= open_table(thd, tables, &new_frm_mem, &action, flags); + error= open_table(thd, tables, &new_frm_mem, &ot_ctx, flags); free_root(&new_frm_mem, MYF(MY_KEEP_PREALLOC)); if (error) { - if (action) + if (ot_ctx.can_recover_from_failed_open_table()) { /* - We have met a exclusive metadata lock or a old version of table and - we are inside a transaction that already hold locks. We can't follow - the locking protocol in this scenario as it might lead to deadlocks. - */ - if (thd->in_multi_stmt_transaction() && has_locks) - { - my_error(ER_LOCK_DEADLOCK, MYF(0)); - result= -1; - goto err; - } - - /* We have met exclusive metadata lock or old version of table. Now we have to close all tables which are not up to date/release metadata locks. We also have to throw away set of prelocked tables (and thus @@ -3897,13 +3906,13 @@ int open_tables(THD *thd, TABLE_LIST **start, uint *counter, uint flags) */ if (query_tables_last_own) thd->lex->mark_as_requiring_prelocking(query_tables_last_own); - close_tables_for_reopen(thd, start, (action == OT_BACK_OFF_AND_RETRY)); + close_tables_for_reopen(thd, start); /* Here we rely on the fact that 'tables' still points to the valid TABLE_LIST element. Altough currently this assumption is valid it may change in future. */ - if (recover_from_failed_open_table_attempt(thd, tables, action)) + if (ot_ctx.recover_from_failed_open_table_attempt(thd, tables)) { result= -1; goto err; @@ -4210,7 +4219,7 @@ TABLE *open_ltable(THD *thd, TABLE_LIST *table_list, thr_lock_type lock_type, uint lock_flags) { TABLE *table; - enum_open_table_action action; + Open_table_context ot_ctx(thd); bool refresh; bool error; DBUG_ENTER("open_ltable"); @@ -4224,16 +4233,18 @@ TABLE *open_ltable(THD *thd, TABLE_LIST *table_list, thr_lock_type lock_type, table_list->required_type= FRMTYPE_TABLE; retry: - while ((error= open_table(thd, table_list, thd->mem_root, &action, 0)) && - action) + while ((error= open_table(thd, table_list, thd->mem_root, &ot_ctx, 0)) && + ot_ctx.can_recover_from_failed_open_table()) { /* - Even altough we have failed to open table we still need to - call close_thread_tables() to release metadata locks which + Even though we have failed to open table we still need to + call release_all_locks() to release metadata locks which might have been acquired successfully. */ - close_thread_tables(thd, (action == OT_BACK_OFF_AND_RETRY)); - if (recover_from_failed_open_table_attempt(thd, table_list, action)) + if (! thd->locked_tables_mode) + thd->mdl_context.release_all_locks(); + table_list->mdl_request.ticket= 0; + if (ot_ctx.recover_from_failed_open_table_attempt(thd, table_list)) break; } @@ -4272,8 +4283,20 @@ retry: { if (refresh) { - close_thread_tables(thd); - goto retry; + if (ot_ctx.can_deadlock()) + { + my_error(ER_LOCK_DEADLOCK, MYF(0)); + table= 0; + } + else + { + close_thread_tables(thd); + table_list->table= NULL; + table_list->mdl_request.ticket= NULL; + if (! thd->locked_tables_mode) + thd->mdl_context.release_all_locks(); + goto retry; + } } else table= 0; @@ -4283,7 +4306,7 @@ retry: else table= 0; - end: +end: thd_proc_info(thd, 0); DBUG_RETURN(table); } @@ -4320,6 +4343,7 @@ int open_and_lock_tables_derived(THD *thd, TABLE_LIST *tables, bool derived, { uint counter; bool need_reopen; + bool has_locks= thd->mdl_context.has_locks(); DBUG_ENTER("open_and_lock_tables_derived"); DBUG_PRINT("enter", ("derived handling: %d", derived)); @@ -4339,7 +4363,12 @@ int open_and_lock_tables_derived(THD *thd, TABLE_LIST *tables, bool derived, break; if (!need_reopen) DBUG_RETURN(-1); - close_tables_for_reopen(thd, &tables, FALSE); + if (thd->in_multi_stmt_transaction() && has_locks) + { + my_error(ER_LOCK_DEADLOCK, MYF(0)); + DBUG_RETURN(-1); + } + close_tables_for_reopen(thd, &tables); } if (derived && (mysql_handle_derived(thd->lex, &mysql_derived_prepare) || @@ -4677,10 +4706,6 @@ int lock_tables(THD *thd, TABLE_LIST *tables, uint count, table->table->query_id= thd->query_id; if (check_lock_and_start_stmt(thd, table->table, table->lock_type)) { - /* - This was an attempt to enter prelocked mode so there is no - need to care about THD::locked_tables_root here. - */ mysql_unlock_tables(thd, thd->lock); thd->lock= 0; DBUG_RETURN(-1); @@ -4767,7 +4792,7 @@ int lock_tables(THD *thd, TABLE_LIST *tables, uint count, */ -void close_tables_for_reopen(THD *thd, TABLE_LIST **tables, bool is_back_off) +void close_tables_for_reopen(THD *thd, TABLE_LIST **tables) { /* If table list consists only from tables from prelocking set, table list @@ -4778,8 +4803,11 @@ void close_tables_for_reopen(THD *thd, TABLE_LIST **tables, bool is_back_off) thd->lex->chop_off_not_own_tables(); sp_remove_not_own_routines(thd->lex); for (TABLE_LIST *tmp= *tables; tmp; tmp= tmp->next_global) + { tmp->table= 0; - close_thread_tables(thd, is_back_off); + tmp->mdl_request.ticket= NULL; + } + close_thread_tables(thd); if (!thd->locked_tables_mode) thd->mdl_context.release_all_locks(); } @@ -7855,7 +7883,8 @@ void tdc_remove_table(THD *thd, enum_tdc_remove_table_type remove_type, @param context Metadata locking context with locks. */ -static bool tdc_wait_for_old_versions(THD *thd, MDL_context *mdl_context) +static bool +tdc_wait_for_old_versions(THD *thd, MDL_request_list *mdl_requests) { TABLE_SHARE *share; const char *old_msg; @@ -7872,7 +7901,7 @@ static bool tdc_wait_for_old_versions(THD *thd, MDL_context *mdl_context) mysql_ha_flush(thd); pthread_mutex_lock(&LOCK_open); - MDL_context::Request_iterator it= mdl_context->get_requests(); + MDL_request_list::Iterator it(*mdl_requests); while ((mdl_request= it++)) { if ((share= get_cached_table_share(mdl_request->key.db_name(), @@ -8095,8 +8124,6 @@ open_system_tables_for_read(THD *thd, TABLE_LIST *table_list, DBUG_ENTER("open_system_tables_for_read"); - alloc_mdl_requests(table_list, thd->mem_root); - /* Besides using new Open_tables_state for opening system tables, we also have to backup and reset/and then restore part of LEX @@ -8170,8 +8197,6 @@ open_system_table_for_update(THD *thd, TABLE_LIST *one_table) { DBUG_ENTER("open_system_table_for_update"); - alloc_mdl_requests(one_table, thd->mem_root); - TABLE *table= open_ltable(thd, one_table, one_table->lock_type, 0); if (table) { @@ -8208,7 +8233,6 @@ open_performance_schema_table(THD *thd, TABLE_LIST *one_table, thd->reset_n_backup_open_tables_state(backup); - alloc_mdl_requests(one_table, thd->mem_root); if ((table= open_ltable(thd, one_table, one_table->lock_type, flags))) { DBUG_ASSERT(table->s->table_category == TABLE_CATEGORY_PERFORMANCE); @@ -8286,7 +8310,6 @@ void close_performance_schema_table(THD *thd, Open_tables_state *backup) pthread_mutex_unlock(&LOCK_open); thd->mdl_context.release_all_locks(); - thd->mdl_context.remove_all_requests(); thd->restore_backup_open_tables_state(backup); } diff --git a/sql/sql_class.cc b/sql/sql_class.cc index 7252f078f81..d83b60810ab 100644 --- a/sql/sql_class.cc +++ b/sql/sql_class.cc @@ -466,8 +466,7 @@ THD::THD() #if defined(ENABLED_DEBUG_SYNC) debug_sync_control(0), #endif /* defined(ENABLED_DEBUG_SYNC) */ - main_warning_info(0), - locked_tables_root(NULL) + main_warning_info(0) { ulong tmp; diff --git a/sql/sql_class.h b/sql/sql_class.h index 4b6564fb9da..9edb5f97713 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -1180,6 +1180,49 @@ private: Internal_error_handler *m_err_handler; }; + +/** + A context of open_tables() function, used to recover + from a failed open_table() attempt. + + Implemented in sql_base.cc. +*/ + +class Open_table_context +{ +public: + enum enum_open_table_action + { + OT_NO_ACTION= 0, + OT_WAIT, + OT_DISCOVER, + OT_REPAIR + }; + Open_table_context(THD *thd); + + bool recover_from_failed_open_table_attempt(THD *thd, TABLE_LIST *tables); + bool request_backoff_action(enum_open_table_action action_arg); + + void add_request(MDL_request *request) + { m_mdl_requests.push_front(request); } + + bool can_recover_from_failed_open_table() const + { return m_action != OT_NO_ACTION; } + bool can_deadlock() const { return m_can_deadlock; } +private: + /** List of requests for all locks taken so far. Used for waiting on locks. */ + MDL_request_list m_mdl_requests; + /** Back off action. */ + enum enum_open_table_action m_action; + /** + Whether we had any locks when this context was created. + If we did, they are from the previous statement of a transaction, + and we can't safely do back-off (and release them). + */ + bool m_can_deadlock; +}; + + /** Tables that were locked with LOCK TABLES statement. @@ -1236,7 +1279,6 @@ public: } bool init_locked_tables(THD *thd); TABLE_LIST *locked_tables() { return m_locked_tables; } - MEM_ROOT *locked_tables_root() { return &m_locked_tables_root; } void unlink_from_list(THD *thd, TABLE_LIST *table_list, bool remove_from_locked_tables); void unlink_all_closed_tables(THD *thd, @@ -1895,15 +1937,6 @@ public: /* Debug Sync facility. See debug_sync.cc. */ struct st_debug_sync_control *debug_sync_control; #endif /* defined(ENABLED_DEBUG_SYNC) */ - - /** - Points to the memory root of Locked_tables_list if - we're locking the tables for LOCK TABLES. Otherwise is NULL. - This is necessary to ensure that metadata locks allocated for - tables used in triggers will persist after statement end. - */ - MEM_ROOT *locked_tables_root; - THD(); ~THD(); diff --git a/sql/sql_delete.cc b/sql/sql_delete.cc index fc92dbee0a1..c3e205848de 100644 --- a/sql/sql_delete.cc +++ b/sql/sql_delete.cc @@ -1100,7 +1100,8 @@ bool mysql_truncate(THD *thd, TABLE_LIST *table_list, bool dont_send_ok) TABLE *table; bool error; uint path_length; - MDL_request *mdl_request= NULL; + MDL_request mdl_request; + bool has_mdl_lock= FALSE; DBUG_ENTER("mysql_truncate"); bzero((char*) &create_info,sizeof(create_info)); @@ -1145,6 +1146,12 @@ bool mysql_truncate(THD *thd, TABLE_LIST *table_list, bool dont_send_ok) if (!dont_send_ok) { enum legacy_db_type table_type; + /* + FIXME: Code of TRUNCATE breaks the meta-data + locking protocol since it tries to find out the table storage + engine and therefore accesses table in some way without holding + any kind of meta-data lock. + */ mysql_frm_type(thd, path, &table_type); if (table_type == DB_TYPE_UNKNOWN) { @@ -1170,20 +1177,12 @@ bool mysql_truncate(THD *thd, TABLE_LIST *table_list, bool dont_send_ok) thd->lex->alter_info.flags & ALTER_ADMIN_PARTITION) goto trunc_by_del; - /* - FIXME: Actually code of TRUNCATE breaks meta-data locking protocol since - tries to get table enging and therefore accesses table in some way - without holding any kind of meta-data lock. - */ - mdl_request= MDL_request::create(0, table_list->db, - table_list->table_name, thd->mem_root); - mdl_request->set_type(MDL_EXCLUSIVE); - thd->mdl_context.add_request(mdl_request); - if (thd->mdl_context.acquire_exclusive_locks()) - { - thd->mdl_context.remove_request(mdl_request); + mdl_request.init(0, table_list->db, table_list->table_name, MDL_EXCLUSIVE); + if (thd->mdl_context.acquire_exclusive_lock(&mdl_request)) DBUG_RETURN(TRUE); - } + + has_mdl_lock= TRUE; + pthread_mutex_lock(&LOCK_open); tdc_remove_table(thd, TDC_RT_REMOVE_ALL, table_list->db, table_list->table_name); @@ -1212,19 +1211,13 @@ end: write_bin_log(thd, TRUE, thd->query(), thd->query_length()); my_ok(thd); // This should return record count } - if (mdl_request) - { - thd->mdl_context.release_lock(mdl_request->ticket); - thd->mdl_context.remove_request(mdl_request); - } + if (has_mdl_lock) + thd->mdl_context.release_lock(mdl_request.ticket); } else if (error) { - if (mdl_request) - { - thd->mdl_context.release_lock(mdl_request->ticket); - thd->mdl_context.remove_request(mdl_request); - } + if (has_mdl_lock) + thd->mdl_context.release_lock(mdl_request.ticket); } DBUG_RETURN(error); diff --git a/sql/sql_handler.cc b/sql/sql_handler.cc index 4b07a00c779..1b7e45cec5d 100644 --- a/sql/sql_handler.cc +++ b/sql/sql_handler.cc @@ -150,7 +150,6 @@ static void mysql_ha_close_table(THD *thd, TABLE_LIST *tables) } pthread_mutex_unlock(&LOCK_open); thd->handler_mdl_context.release_lock(mdl_ticket); - thd->handler_mdl_context.remove_request(tables->mdl_request); } else if (tables->table) { @@ -163,6 +162,8 @@ static void mysql_ha_close_table(THD *thd, TABLE_LIST *tables) /* Mark table as closed, ready for re-open if necessary. */ tables->table= NULL; + /* Safety, cleanup the pointer to satisfy MDL assertions. */ + tables->mdl_request.ticket= NULL; } /* @@ -195,7 +196,6 @@ bool mysql_ha_open(THD *thd, TABLE_LIST *tables, bool reopen) int error; TABLE *backup_open_tables; MDL_context backup_mdl_context; - MDL_request *mdl_request; DBUG_ENTER("mysql_ha_open"); DBUG_PRINT("enter",("'%s'.'%s' as '%s' reopen: %d", tables->db, tables->table_name, tables->alias, @@ -246,7 +246,6 @@ bool mysql_ha_open(THD *thd, TABLE_LIST *tables, bool reopen) &db, (uint) dblen, &name, (uint) namelen, &alias, (uint) aliaslen, - &mdl_request, sizeof(MDL_request), NullS))) { DBUG_PRINT("exit",("ERROR")); @@ -260,8 +259,7 @@ bool mysql_ha_open(THD *thd, TABLE_LIST *tables, bool reopen) memcpy(hash_tables->db, tables->db, dblen); memcpy(hash_tables->table_name, tables->table_name, namelen); memcpy(hash_tables->alias, tables->alias, aliaslen); - mdl_request->init(0, db, name); - hash_tables->mdl_request= mdl_request; + hash_tables->mdl_request.init(0, db, name, MDL_SHARED); /* add to hash */ if (my_hash_insert(&thd->handler_tables_hash, (uchar*) hash_tables)) diff --git a/sql/sql_help.cc b/sql/sql_help.cc index 003741a7ddc..af67db45b36 100644 --- a/sql/sql_help.cc +++ b/sql/sql_help.cc @@ -653,6 +653,7 @@ bool mysqld_help(THD *thd, const char *mask) tables[3].alias= tables[3].table_name= (char*) "help_keyword"; tables[3].lock_type= TL_READ; tables[0].db= tables[1].db= tables[2].db= tables[3].db= (char*) "mysql"; + init_mdl_requests(tables); Open_tables_state open_tables_state_backup; if (open_system_tables_for_read(thd, tables, &open_tables_state_backup)) diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc index 8553db8cf2b..e0537c75e07 100644 --- a/sql/sql_insert.cc +++ b/sql/sql_insert.cc @@ -2399,7 +2399,7 @@ pthread_handler_t handle_delayed_insert(void *arg) thd->lex->set_stmt_unsafe(); thd->set_current_stmt_binlog_row_based_if_mixed(); - alloc_mdl_requests(&di->table_list, thd->mem_root); + init_mdl_requests(&di->table_list); if (di->open_and_lock_table()) goto err; @@ -3464,7 +3464,6 @@ static TABLE *create_table_from_items(THD *thd, HA_CREATE_INFO *create_info, Item *item; Field *tmp_field; bool not_used; - enum_open_table_action not_used2; DBUG_ENTER("create_table_from_items"); tmp_table.alias= 0; @@ -3543,13 +3542,13 @@ static TABLE *create_table_from_items(THD *thd, HA_CREATE_INFO *create_info, if (!(create_info->options & HA_LEX_CREATE_TMP_TABLE)) { - enum enum_open_table_action ot_action_unused; + Open_table_context ot_ctx_unused(thd); /* Here we open the destination table, on which we already have an exclusive metadata lock. */ if (open_table(thd, create_table, thd->mem_root, - &ot_action_unused, MYSQL_OPEN_REOPEN)) + &ot_ctx_unused, MYSQL_OPEN_REOPEN)) { pthread_mutex_lock(&LOCK_open); quick_rm_table(create_info->db_type, create_table->db, @@ -3562,7 +3561,8 @@ static TABLE *create_table_from_items(THD *thd, HA_CREATE_INFO *create_info, } else { - if (open_table(thd, create_table, thd->mem_root, ¬_used2, + Open_table_context ot_ctx_unused(thd); + if (open_table(thd, create_table, thd->mem_root, &ot_ctx_unused, MYSQL_OPEN_TEMPORARY_ONLY) && !create_info->table_existed) { @@ -3637,18 +3637,28 @@ select_create::prepare(List<Item> &values, SELECT_LEX_UNIT *u) */ class MY_HOOKS : public TABLEOP_HOOKS { public: - MY_HOOKS(select_create *x, TABLE_LIST *create_table, - TABLE_LIST *select_tables) - : ptr(x), all_tables(*create_table) + MY_HOOKS(select_create *x, TABLE_LIST *create_table_arg, + TABLE_LIST *select_tables_arg) + : ptr(x), + create_table(create_table_arg), + select_tables(select_tables_arg) { - all_tables.next_global= select_tables; } private: virtual int do_postlock(TABLE **tables, uint count) { + int error; THD *thd= const_cast<THD*>(ptr->get_thd()); - if (int error= decide_logging_format(thd, &all_tables)) + TABLE_LIST *save_next_global= create_table->next_global; + + create_table->next_global= select_tables; + + error= decide_logging_format(thd, create_table); + + create_table->next_global= save_next_global; + + if (error) return error; TABLE const *const table = *tables; @@ -3662,7 +3672,8 @@ select_create::prepare(List<Item> &values, SELECT_LEX_UNIT *u) } select_create *ptr; - TABLE_LIST all_tables; + TABLE_LIST *create_table; + TABLE_LIST *select_tables; }; MY_HOOKS hooks(this, create_table, select_tables); diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc index ddc163072a7..35c0973d103 100644 --- a/sql/sql_parse.cc +++ b/sql/sql_parse.cc @@ -1107,7 +1107,7 @@ bool dispatch_command(enum enum_server_command command, THD *thd, select_lex.table_list.link_in_list((uchar*) &table_list, (uchar**) &table_list.next_local); thd->lex->add_to_query_tables(&table_list); - alloc_mdl_requests(&table_list, thd->mem_root); + init_mdl_requests(&table_list); /* switch on VIEW optimisation: do not fill temporary tables */ thd->lex->sql_command= SQLCOM_SHOW_FIELDS; @@ -3337,18 +3337,16 @@ end_with_restore_list: !(need_start_waiting= !wait_if_global_read_lock(thd, 0, 1))) goto error; - alloc_mdl_requests(all_tables, thd->locked_tables_list.locked_tables_root()); + init_mdl_requests(all_tables); thd->options|= OPTION_TABLE_LOCK; thd->in_lock_tables=1; - thd->locked_tables_root= thd->locked_tables_list.locked_tables_root(); res= (open_and_lock_tables_derived(thd, all_tables, FALSE, MYSQL_OPEN_TAKE_UPGRADABLE_MDL) || thd->locked_tables_list.init_locked_tables(thd)); thd->in_lock_tables= 0; - thd->locked_tables_root= NULL; if (res) { @@ -6021,9 +6019,7 @@ TABLE_LIST *st_select_lex::add_table_to_list(THD *thd, ptr->next_name_resolution_table= NULL; /* Link table in global list (all used tables) */ lex->add_to_query_tables(ptr); - ptr->mdl_request= - MDL_request::create(0, ptr->db, ptr->table_name, thd->locked_tables_root ? - thd->locked_tables_root : thd->mem_root); + ptr->mdl_request.init(0, ptr->db, ptr->table_name, MDL_SHARED); DBUG_RETURN(ptr); } diff --git a/sql/sql_plugin.cc b/sql/sql_plugin.cc index b7efe13e26e..7dd7dd0d4b9 100644 --- a/sql/sql_plugin.cc +++ b/sql/sql_plugin.cc @@ -1349,7 +1349,6 @@ static void plugin_load(MEM_ROOT *tmp_root, int *argc, char **argv) #ifdef EMBEDDED_LIBRARY bool table_exists; #endif /* EMBEDDED_LIBRARY */ - MDL_request mdl_request; DBUG_ENTER("plugin_load"); if (!(new_thd= new THD)) @@ -1363,12 +1362,7 @@ static void plugin_load(MEM_ROOT *tmp_root, int *argc, char **argv) lex_start(new_thd); new_thd->db= my_strdup("mysql", MYF(0)); new_thd->db_length= 5; - bzero((uchar*)&tables, sizeof(tables)); - tables.alias= tables.table_name= (char*)"plugin"; - tables.lock_type= TL_READ; - tables.db= new_thd->db; - tables.mdl_request= &mdl_request; - mdl_request.init(0, tables.db, tables.table_name); + tables.init_one_table("mysql", 5, "plugin", 6, "plugin", TL_READ); #ifdef EMBEDDED_LIBRARY /* @@ -1656,14 +1650,10 @@ bool mysql_install_plugin(THD *thd, const LEX_STRING *name, const LEX_STRING *dl struct st_plugin_int *tmp; DBUG_ENTER("mysql_install_plugin"); - bzero(&tables, sizeof(tables)); - tables.db= (char *)"mysql"; - tables.table_name= tables.alias= (char *)"plugin"; + tables.init_one_table("mysql", 5, "plugin", 6, "plugin", TL_WRITE); if (check_table_access(thd, INSERT_ACL, &tables, FALSE, 1, FALSE)) DBUG_RETURN(TRUE); - alloc_mdl_requests(&tables, thd->mem_root); - /* need to open before acquiring LOCK_plugin or it will deadlock */ if (! (table = open_ltable(thd, &tables, TL_WRITE, 0))) DBUG_RETURN(TRUE); @@ -1734,10 +1724,7 @@ bool mysql_uninstall_plugin(THD *thd, const LEX_STRING *name) struct st_plugin_int *plugin; DBUG_ENTER("mysql_uninstall_plugin"); - bzero(&tables, sizeof(tables)); - tables.db= (char *)"mysql"; - tables.table_name= tables.alias= (char *)"plugin"; - alloc_mdl_requests(&tables, thd->mem_root); + tables.init_one_table("mysql", 5, "plugin", 6, "plugin", TL_WRITE); /* need to open before acquiring LOCK_plugin or it will deadlock */ if (! (table= open_ltable(thd, &tables, TL_WRITE, 0))) diff --git a/sql/sql_servers.cc b/sql/sql_servers.cc index c46a01cd534..8fafbbd8e70 100644 --- a/sql/sql_servers.cc +++ b/sql/sql_servers.cc @@ -228,11 +228,7 @@ bool servers_reload(THD *thd) DBUG_PRINT("info", ("locking servers_cache")); rw_wrlock(&THR_LOCK_servers); - bzero((char*) tables, sizeof(tables)); - tables[0].alias= tables[0].table_name= (char*) "servers"; - tables[0].db= (char*) "mysql"; - tables[0].lock_type= TL_READ; - alloc_mdl_requests(tables, thd->mem_root); + tables[0].init_one_table("mysql", 5, "servers", 7, "servers", TL_READ); if (simple_open_n_lock_tables(thd, tables)) { @@ -363,10 +359,7 @@ insert_server(THD *thd, FOREIGN_SERVER *server) DBUG_ENTER("insert_server"); - bzero((char*) &tables, sizeof(tables)); - tables.db= (char*) "mysql"; - tables.alias= tables.table_name= (char*) "servers"; - alloc_mdl_requests(&tables, thd->mem_root); + tables.init_one_table("mysql", 5, "servers", 7, "servers", TL_WRITE); /* need to open before acquiring THR_LOCK_plugin or it will deadlock */ if (! (table= open_ltable(thd, &tables, TL_WRITE, 0))) @@ -582,10 +575,7 @@ int drop_server(THD *thd, LEX_SERVER_OPTIONS *server_options) DBUG_PRINT("info", ("server name server->server_name %s", server_options->server_name)); - bzero((char*) &tables, sizeof(tables)); - tables.db= (char*) "mysql"; - tables.alias= tables.table_name= (char*) "servers"; - alloc_mdl_requests(&tables, thd->mem_root); + tables.init_one_table("mysql", 5, "servers", 7, "servers", TL_WRITE); rw_wrlock(&THR_LOCK_servers); @@ -707,10 +697,8 @@ int update_server(THD *thd, FOREIGN_SERVER *existing, FOREIGN_SERVER *altered) TABLE_LIST tables; DBUG_ENTER("update_server"); - bzero((char*) &tables, sizeof(tables)); - tables.db= (char*)"mysql"; - tables.alias= tables.table_name= (char*)"servers"; - alloc_mdl_requests(&tables, thd->mem_root); + tables.init_one_table("mysql", 5, "servers", 7, "servers", + TL_WRITE); if (!(table= open_ltable(thd, &tables, TL_WRITE, 0))) { diff --git a/sql/sql_show.cc b/sql/sql_show.cc index 9727498cb2e..6a5895f9446 100644 --- a/sql/sql_show.cc +++ b/sql/sql_show.cc @@ -2934,7 +2934,7 @@ fill_schema_show_cols_or_idxs(THD *thd, TABLE_LIST *tables, table, res, db_name, table_name)); thd->temporary_tables= 0; - close_tables_for_reopen(thd, &show_table_list, FALSE); + close_tables_for_reopen(thd, &show_table_list); DBUG_RETURN(error); } @@ -3065,30 +3065,21 @@ uint get_table_open_method(TABLE_LIST *tables, */ static bool -acquire_high_prio_shared_mdl_lock(THD *thd, MDL_request *mdl_request, - TABLE_LIST *table) -{ - bool retry; - - mdl_request->init(0, table->db, table->table_name); - table->mdl_request= mdl_request; - thd->mdl_context.add_request(mdl_request); - mdl_request->set_type(MDL_SHARED_HIGH_PRIO); - - while (1) - { - if (thd->mdl_context.acquire_shared_lock(mdl_request, &retry)) - { - if (!retry || thd->mdl_context.wait_for_locks()) - { - thd->mdl_context.remove_all_requests(); - return TRUE; - } - continue; - } - break; +acquire_high_prio_shared_mdl_lock(THD *thd, TABLE_LIST *table) +{ + bool error; + table->mdl_request.init(0, table->db, table->table_name, + MDL_SHARED_HIGH_PRIO); + while (!(error= + thd->mdl_context.try_acquire_shared_lock(&table->mdl_request)) && + !table->mdl_request.ticket) + { + MDL_request_list mdl_requests; + mdl_requests.push_front(&table->mdl_request); + if ((error= thd->mdl_context.wait_for_locks(&mdl_requests))) + break; } - return FALSE; + return error; } @@ -3123,7 +3114,6 @@ static int fill_schema_table_from_frm(THD *thd,TABLE *table, char key[MAX_DBKEY_LENGTH]; uint key_length; char db_name_buff[NAME_LEN + 1], table_name_buff[NAME_LEN + 1]; - MDL_request mdl_request; bzero((char*) &table_list, sizeof(TABLE_LIST)); bzero((char*) &tbl, sizeof(TABLE)); @@ -3153,7 +3143,7 @@ static int fill_schema_table_from_frm(THD *thd,TABLE *table, simply obtaining internal lock of data-dictionary (ATM it is LOCK_open) instead of obtaning full-blown metadata lock. */ - if (acquire_high_prio_shared_mdl_lock(thd, &mdl_request, &table_list)) + if (acquire_high_prio_shared_mdl_lock(thd, &table_list)) { /* Some error occured (most probably we have been killed while @@ -3213,8 +3203,7 @@ err_share: err_unlock: pthread_mutex_unlock(&LOCK_open); - thd->mdl_context.release_lock(mdl_request.ticket); - thd->mdl_context.remove_request(&mdl_request); + thd->mdl_context.release_lock(table_list.mdl_request.ticket); thd->clear_error(); return res; } @@ -3462,7 +3451,7 @@ int get_all_tables(THD *thd, TABLE_LIST *tables, COND *cond) res= schema_table->process_table(thd, show_table_list, table, res, &orig_db_name, &tmp_lex_string); - close_tables_for_reopen(thd, &show_table_list, FALSE); + close_tables_for_reopen(thd, &show_table_list); } DBUG_ASSERT(!lex->query_tables_own_last); if (res) @@ -7199,14 +7188,14 @@ static bool show_create_trigger_impl(THD *thd, - do not update Lex::query_tables in add_table_to_list(). */ -static TABLE_LIST *get_trigger_table_impl( - THD *thd, - const sp_name *trg_name) +static +TABLE_LIST *get_trigger_table_impl(THD *thd, const sp_name *trg_name) { char trn_path_buff[FN_REFLEN]; - LEX_STRING trn_path= { trn_path_buff, 0 }; + LEX_STRING db; LEX_STRING tbl_name; + TABLE_LIST *table; build_trn_path(thd, trg_name, &trn_path); @@ -7220,25 +7209,19 @@ static TABLE_LIST *get_trigger_table_impl( return NULL; /* We need to reset statement table list to be PS/SP friendly. */ - - TABLE_LIST *table; - - if (!(table= (TABLE_LIST *)thd->calloc(sizeof(TABLE_LIST)))) - { - my_error(ER_OUTOFMEMORY, MYF(0), sizeof(TABLE_LIST)); + if (!(table= (TABLE_LIST*) thd->alloc(sizeof(TABLE_LIST)))) return NULL; - } - table->db_length= trg_name->m_db.length; - table->db= thd->strmake(trg_name->m_db.str, trg_name->m_db.length); + db= trg_name->m_db; - table->table_name_length= tbl_name.length; - table->table_name= thd->strmake(tbl_name.str, tbl_name.length); + db.str= thd->strmake(db.str, db.length); + tbl_name.str= thd->strmake(tbl_name.str, tbl_name.length); - table->alias= thd->strmake(tbl_name.str, tbl_name.length); + if (db.str == NULL || tbl_name.str == NULL) + return NULL; - table->lock_type= TL_IGNORE; - table->cacheable_table= 0; + table->init_one_table(db.str, db.length, tbl_name.str, tbl_name.length, + tbl_name.str, TL_IGNORE); return table; } @@ -7254,7 +7237,8 @@ static TABLE_LIST *get_trigger_table_impl( @return TABLE_LIST object corresponding to the base table. */ -static TABLE_LIST *get_trigger_table(THD *thd, const sp_name *trg_name) +static +TABLE_LIST *get_trigger_table(THD *thd, const sp_name *trg_name) { /* Acquire LOCK_open (stop the server). */ @@ -7310,8 +7294,6 @@ bool show_create_trigger(THD *thd, const sp_name *trg_name) uint num_tables; /* NOTE: unused, only to pass to open_tables(). */ - alloc_mdl_requests(lst, thd->mem_root); - if (open_tables(thd, &lst, &num_tables, 0)) { my_error(ER_TRG_CANT_OPEN_TABLE, MYF(0), diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 527095f2c88..c389ef2aef3 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -16,6 +16,7 @@ /* drop and alter of tables */ #include "mysql_priv.h" +#include "debug_sync.h" #include <hash.h> #include <myisam.h> #include <my_dir.h> @@ -1907,14 +1908,25 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists, if (find_temporary_table(thd, table->db, table->table_name)) { /* - Since we don't acquire metadata lock if we have found temporary - table, we should do something to avoid releasing it at the end. + A temporary table. + + Don't try to find a corresponding MDL lock or assign it + to table->mdl_request.ticket. There can't be metadata + locks for temporary tables: they are local to the session. + + Later in this function we release the MDL lock only if + table->mdl_requeset.ticket is not NULL. Thus here we + ensure that we won't release the metadata lock on the base + table locked with LOCK TABLES as a side effect of temporary + table drop. */ - table->mdl_request= NULL; + DBUG_ASSERT(table->mdl_request.ticket == NULL); } else { /* + Not a temporary table. + Since 'tables' list can't contain duplicates (this is ensured by parser) it is safe to cache pointer to the TABLE instances in its elements. @@ -1923,7 +1935,7 @@ int mysql_rm_table_part2(THD *thd, TABLE_LIST *tables, bool if_exists, table->table_name); if (!table->table) DBUG_RETURN(1); - table->mdl_request->ticket= table->table->mdl_ticket; + table->mdl_request.ticket= table->table->mdl_ticket; } } } @@ -2188,7 +2200,7 @@ err: Under LOCK TABLES we should release meta-data locks on the tables which were dropped. Otherwise we can rely on close_thread_tables() doing this. Unfortunately in this case we are likely to get more - false positives in lock_table_name_if_not_cached() function. So + false positives in try_acquire_exclusive_lock() function. So it makes sense to remove exclusive meta-data locks in all cases. Leave LOCK TABLES mode if we managed to drop all tables which were @@ -2203,14 +2215,14 @@ err: } for (table= tables; table; table= table->next_local) { - if (table->mdl_request) + if (table->mdl_request.ticket) { /* Under LOCK TABLES we may have several instances of table open and locked and therefore have to remove several metadata lock requests associated with them. */ - thd->mdl_context.release_all_locks_for_name(table->mdl_request->ticket); + thd->mdl_context.release_all_locks_for_name(table->mdl_request.ticket); } } } @@ -4094,47 +4106,6 @@ warn: } -/** - Auxiliary function which obtains exclusive meta-data lock on the - table if there are no shared or exclusive on it already. - - See mdl_try_acquire_exclusive_lock() function for more info. - - TODO: This function is here mostly to simplify current patch - and probably should be removed. - TODO: Investigate if it is kosher to leave lock request in the - context in the case when we fail to obtain the lock. -*/ - -static bool lock_table_name_if_not_cached(THD *thd, const char *db, - const char *table_name, - MDL_request **mdl_request) -{ - bool conflict; - - if (!(*mdl_request= MDL_request::create(0, db, table_name, thd->mem_root))) - return TRUE; - (*mdl_request)->set_type(MDL_EXCLUSIVE); - thd->mdl_context.add_request(*mdl_request); - if (thd->mdl_context.try_acquire_exclusive_lock(*mdl_request, &conflict)) - { - /* - To simplify our life under LOCK TABLES we remove unsatisfied - lock request from the context. - */ - thd->mdl_context.remove_request(*mdl_request); - if (!conflict) - { - /* Probably OOM. */ - return TRUE; - } - else - *mdl_request= NULL; - } - return FALSE; -} - - /* Database and name-locking aware wrapper for mysql_create_table_no_lock(), */ @@ -4145,7 +4116,8 @@ bool mysql_create_table(THD *thd, const char *db, const char *table_name, bool internal_tmp_table, uint select_field_count) { - MDL_request *target_mdl_request= NULL; + MDL_request target_mdl_request; + bool has_target_mdl_lock= FALSE; bool result; DBUG_ENTER("mysql_create_table"); @@ -4168,13 +4140,15 @@ bool mysql_create_table(THD *thd, const char *db, const char *table_name, if (!(create_info->options & HA_LEX_CREATE_TMP_TABLE)) { - if (lock_table_name_if_not_cached(thd, db, table_name, &target_mdl_request)) + target_mdl_request.init(0, db, table_name, MDL_EXCLUSIVE); + if (thd->mdl_context.try_acquire_exclusive_lock(&target_mdl_request)) { result= TRUE; goto unlock; } - if (!target_mdl_request) + if (target_mdl_request.ticket == NULL) { + /* Table exists and is locked by some other thread. */ if (create_info->options & HA_LEX_CREATE_IF_NOT_EXISTS) { push_warning_printf(thd, MYSQL_ERROR::WARN_LEVEL_NOTE, @@ -4191,6 +4165,9 @@ bool mysql_create_table(THD *thd, const char *db, const char *table_name, } goto unlock; } + /* Got lock. */ + DEBUG_SYNC(thd, "locked_table_name"); + has_target_mdl_lock= TRUE; } result= mysql_create_table_no_lock(thd, db, table_name, create_info, @@ -4199,11 +4176,9 @@ bool mysql_create_table(THD *thd, const char *db, const char *table_name, select_field_count); unlock: - if (target_mdl_request) - { - thd->mdl_context.release_lock(target_mdl_request->ticket); - thd->mdl_context.remove_request(target_mdl_request); - } + if (has_target_mdl_lock) + thd->mdl_context.release_lock(target_mdl_request.ticket); + pthread_mutex_lock(&LOCK_lock_db); if (!--creating_table && creating_database) pthread_cond_signal(&COND_refresh); @@ -4364,11 +4339,11 @@ static int prepare_for_repair(THD *thd, TABLE_LIST *table_list, int error= 0; TABLE tmp_table, *table; TABLE_SHARE *share; + bool has_mdl_lock= FALSE; char from[FN_REFLEN],tmp[FN_REFLEN+32]; const char **ext; MY_STAT stat_info; - MDL_request *mdl_request= NULL; - enum enum_open_table_action ot_action_unused; + Open_table_context ot_ctx_unused(thd); DBUG_ENTER("prepare_for_repair"); uint reopen_for_repair_flags= (MYSQL_LOCK_IGNORE_FLUSH | MYSQL_OPEN_HAS_MDL_LOCK); @@ -4386,15 +4361,11 @@ static int prepare_for_repair(THD *thd, TABLE_LIST *table_list, uint key_length; key_length= create_table_def_key(thd, key, table_list, 0); - mdl_request= MDL_request::create(0, table_list->db, - table_list->table_name, thd->mem_root); - mdl_request->set_type(MDL_EXCLUSIVE); - thd->mdl_context.add_request(mdl_request); - if (thd->mdl_context.acquire_exclusive_locks()) - { - thd->mdl_context.remove_request(mdl_request); + table_list->mdl_request.init(0, table_list->db, table_list->table_name, + MDL_EXCLUSIVE); + if (thd->mdl_context.acquire_exclusive_lock(&table_list->mdl_request)) DBUG_RETURN(0); - } + has_mdl_lock= TRUE; pthread_mutex_lock(&LOCK_open); if (!(share= (get_table_share(thd, table_list, key, key_length, 0, @@ -4412,7 +4383,6 @@ static int prepare_for_repair(THD *thd, TABLE_LIST *table_list, } pthread_mutex_unlock(&LOCK_open); table= &tmp_table; - table_list->mdl_request= mdl_request; } /* A MERGE table must not come here. */ @@ -4507,7 +4477,7 @@ static int prepare_for_repair(THD *thd, TABLE_LIST *table_list, to finish the repair in the handler later on. */ if (open_table(thd, table_list, thd->mem_root, - &ot_action_unused, reopen_for_repair_flags)) + &ot_ctx_unused, reopen_for_repair_flags)) { error= send_check_errmsg(thd, table_list, "repair", "Failed to open partially repaired table"); @@ -4523,11 +4493,9 @@ end: pthread_mutex_unlock(&LOCK_open); } /* In case of a temporary table there will be no metadata lock. */ - if (error && mdl_request) - { - thd->mdl_context.release_lock(mdl_request->ticket); - thd->mdl_context.remove_request(mdl_request); - } + if (error && has_mdl_lock) + thd->mdl_context.release_lock(table_list->mdl_request.ticket); + DBUG_RETURN(error); } @@ -4938,6 +4906,8 @@ send_result_message: thd->mdl_context.release_all_locks(); if (!result_code) // recreation went ok { + /* Clear the ticket released in close_thread_tables(). */ + table->mdl_request.ticket= NULL; if ((table->table= open_ltable(thd, table, lock_type, 0)) && ((result_code= table->table->file->ha_analyze(thd, check_opt)) > 0)) result_code= 0; // analyze went ok @@ -5241,9 +5211,9 @@ bool mysql_create_like_schema_frm(THD* thd, TABLE_LIST* schema_table, bool mysql_create_like_table(THD* thd, TABLE_LIST* table, TABLE_LIST* src_table, HA_CREATE_INFO *create_info) { - MDL_request *target_mdl_request= NULL; - char src_path[FN_REFLEN], dst_path[FN_REFLEN + 1]; + char src_path[FN_REFLEN + 1], dst_path[FN_REFLEN + 1]; uint dst_path_length; + bool has_mdl_lock= FALSE; char *db= table->db; char *table_name= table->table_name; int err; @@ -5298,19 +5268,20 @@ bool mysql_create_like_table(THD* thd, TABLE_LIST* table, TABLE_LIST* src_table, } else { - if (lock_table_name_if_not_cached(thd, db, table_name, &target_mdl_request)) - goto err; - if (!target_mdl_request) + table->mdl_request.init(0, db, table_name, MDL_EXCLUSIVE); + if (thd->mdl_context.try_acquire_exclusive_lock(&table->mdl_request)) + DBUG_RETURN(TRUE); + + if (table->mdl_request.ticket == NULL) goto table_exists; + + DEBUG_SYNC(thd, "locked_table_name"); + has_mdl_lock= TRUE; + dst_path_length= build_table_filename(dst_path, sizeof(dst_path) - 1, db, table_name, reg_ext, 0); if (!access(dst_path, F_OK)) goto table_exists; - /* - Make the metadata lock available to open_table() called to - reopen the table down the road. - */ - table->mdl_request= target_mdl_request; } DBUG_EXECUTE_IF("sleep_create_like_before_copy", my_sleep(6000000);); @@ -5440,14 +5411,14 @@ binlog: char buf[2048]; String query(buf, sizeof(buf), system_charset_info); query.length(0); // Have to zero it since constructor doesn't - enum enum_open_table_action ot_action_unused; + Open_table_context ot_ctx_unused(thd); /* Here we open the destination table, on which we already have exclusive metadata lock. This is needed for store_create_info() to work. The table will be closed by close_thread_table() at the end of this branch. */ - if (open_table(thd, table, thd->mem_root, &ot_action_unused, + if (open_table(thd, table, thd->mem_root, &ot_ctx_unused, MYSQL_OPEN_REOPEN)) goto err; @@ -5481,11 +5452,9 @@ binlog: res= FALSE; err: - if (target_mdl_request) - { - thd->mdl_context.release_lock(target_mdl_request->ticket); - thd->mdl_context.remove_request(target_mdl_request); - } + if (has_mdl_lock) + thd->mdl_context.release_lock(table->mdl_request.ticket); + DBUG_RETURN(res); } @@ -5969,7 +5938,6 @@ bool alter_table_manage_keys(TABLE *table, int indexes_were_disabled, DBUG_RETURN(error); } - /** Prepare column and key definitions for CREATE TABLE in ALTER TABLE. @@ -6424,7 +6392,8 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name, { TABLE *table, *new_table= 0; MDL_ticket *mdl_ticket; - MDL_request *target_mdl_request= NULL; + MDL_request target_mdl_request; + bool has_target_mdl_lock= FALSE; int error= 0; char tmp_name[80],old_name[32],new_name_buff[FN_REFLEN + 1]; char new_alias_buff[FN_REFLEN], *table_name, *db, *new_alias, *alias; @@ -6647,15 +6616,21 @@ view_err: } else { - if (lock_table_name_if_not_cached(thd, new_db, new_name, - &target_mdl_request)) + target_mdl_request.init(0, new_db, new_name, MDL_EXCLUSIVE); + if (thd->mdl_context.try_acquire_exclusive_lock(&target_mdl_request)) DBUG_RETURN(TRUE); - if (!target_mdl_request) + if (target_mdl_request.ticket == NULL) { + /* Table exists and is locked by some thread. */ my_error(ER_TABLE_EXISTS_ERROR, MYF(0), new_alias); DBUG_RETURN(TRUE); } - + DEBUG_SYNC(thd, "locked_table_name"); + has_target_mdl_lock= TRUE; + /* + Table maybe does not exist, but we got an exclusive lock + on the name, now we can safely try to find out for sure. + */ build_table_filename(new_name_buff, sizeof(new_name_buff) - 1, new_db, new_name_buff, reg_ext, 0); if (!access(new_name_buff, F_OK)) @@ -6843,8 +6818,7 @@ view_err: */ if (new_name != table_name || new_db != db) { - thd->mdl_context.release_lock(target_mdl_request->ticket); - thd->mdl_context.remove_request(target_mdl_request); + thd->mdl_context.release_lock(target_mdl_request.ticket); thd->mdl_context.release_all_locks_for_name(mdl_ticket); } else @@ -7081,7 +7055,6 @@ view_err: #ifdef WITH_PARTITION_STORAGE_ENGINE if (fast_alter_partition) { - DBUG_ASSERT(!target_mdl_request); DBUG_RETURN(fast_alter_partition_table(thd, table, alter_info, create_info, table_list, db, table_name, @@ -7161,13 +7134,13 @@ view_err: { if (table->s->tmp_table) { - enum_open_table_action not_used; + Open_table_context ot_ctx_unused(thd); TABLE_LIST tbl; bzero((void*) &tbl, sizeof(tbl)); tbl.db= new_db; tbl.table_name= tbl.alias= tmp_name; /* Table is in thd->temporary_tables */ - (void) open_table(thd, &tbl, thd->mem_root, ¬_used, + (void) open_table(thd, &tbl, thd->mem_root, &ot_ctx_unused, MYSQL_LOCK_IGNORE_FLUSH); new_table= tbl.table; } @@ -7439,7 +7412,7 @@ view_err: To do this we need to obtain a handler object for it. NO need to tamper with MERGE tables. The real open is done later. */ - enum enum_open_table_action ot_action_unused; + Open_table_context ot_ctx_unused(thd); TABLE *t_table; if (new_name != table_name || new_db != db) { @@ -7448,7 +7421,7 @@ view_err: table_list->table_name_length= strlen(new_name); table_list->db= new_db; table_list->db_length= strlen(new_db); - table_list->mdl_request= target_mdl_request; + table_list->mdl_request.ticket= target_mdl_request.ticket; } else { @@ -7457,10 +7430,10 @@ view_err: points to a different instance than the one set initially to request the lock. */ - table_list->mdl_request->ticket= mdl_ticket; + table_list->mdl_request.ticket= mdl_ticket; } if (open_table(thd, table_list, thd->mem_root, - &ot_action_unused, MYSQL_OPEN_REOPEN)) + &ot_ctx_unused, MYSQL_OPEN_REOPEN)) { goto err_with_mdl; } @@ -7523,8 +7496,7 @@ view_err: { if ((new_name != table_name || new_db != db)) { - thd->mdl_context.release_lock(target_mdl_request->ticket); - thd->mdl_context.remove_request(target_mdl_request); + thd->mdl_context.release_lock(target_mdl_request.ticket); thd->mdl_context.release_all_locks_for_name(mdl_ticket); } else @@ -7583,11 +7555,9 @@ err: alter_info->datetime_field->field_name); thd->abort_on_warning= save_abort_on_warning; } - if (target_mdl_request) - { - thd->mdl_context.release_lock(target_mdl_request->ticket); - thd->mdl_context.remove_request(target_mdl_request); - } + if (has_target_mdl_lock) + thd->mdl_context.release_lock(target_mdl_request.ticket); + DBUG_RETURN(TRUE); err_with_mdl: @@ -7598,11 +7568,9 @@ err_with_mdl: tables and release the exclusive metadata lock. */ thd->locked_tables_list.unlink_all_closed_tables(thd, NULL, 0); - if (target_mdl_request) - { - thd->mdl_context.release_lock(target_mdl_request->ticket); - thd->mdl_context.remove_request(target_mdl_request); - } + if (has_target_mdl_lock) + thd->mdl_context.release_lock(target_mdl_request.ticket); + thd->mdl_context.release_all_locks_for_name(mdl_ticket); DBUG_RETURN(TRUE); } @@ -7851,6 +7819,8 @@ bool mysql_recreate_table(THD *thd, TABLE_LIST *table_list) uninitialized data. open_tables() could fail. */ table_list->table= NULL; + /* Same applies to MDL ticket. */ + table_list->mdl_request.ticket= NULL; bzero((char*) &create_info, sizeof(create_info)); create_info.row_type=ROW_TYPE_NOT_USED; diff --git a/sql/sql_udf.cc b/sql/sql_udf.cc index 7e557c3ce68..d27473c1959 100644 --- a/sql/sql_udf.cc +++ b/sql/sql_udf.cc @@ -138,11 +138,7 @@ void udf_init() lex_start(new_thd); new_thd->set_db(db, sizeof(db)-1); - bzero((uchar*) &tables,sizeof(tables)); - tables.alias= tables.table_name= (char*) "func"; - tables.lock_type = TL_READ; - tables.db= db; - alloc_mdl_requests(&tables, new_thd->mem_root); + tables.init_one_table(db, sizeof(db)-1, "func", 4, "func", TL_READ); if (simple_open_n_lock_tables(new_thd, &tables)) { @@ -483,10 +479,7 @@ int mysql_create_function(THD *thd,udf_func *udf) /* create entry in mysql.func table */ - bzero((char*) &tables,sizeof(tables)); - tables.db= (char*) "mysql"; - tables.table_name= tables.alias= (char*) "func"; - alloc_mdl_requests(&tables, thd->mem_root); + tables.init_one_table("mysql", 5, "func", 4, "func", TL_WRITE); /* Allow creation of functions even if we can't open func table */ if (!(table = open_ltable(thd, &tables, TL_WRITE, 0))) goto err; @@ -562,10 +555,8 @@ int mysql_drop_function(THD *thd,const LEX_STRING *udf_name) if (udf->dlhandle && !find_udf_dl(udf->dl)) dlclose(udf->dlhandle); - bzero((char*) &tables,sizeof(tables)); - tables.db=(char*) "mysql"; - tables.table_name= tables.alias= (char*) "func"; - alloc_mdl_requests(&tables, thd->mem_root); + tables.init_one_table("mysql", 5, "func", 4, "func", TL_WRITE); + if (!(table = open_ltable(thd, &tables, TL_WRITE, 0))) goto err; table->use_all_columns(); @@ -579,15 +570,16 @@ int mysql_drop_function(THD *thd,const LEX_STRING *udf_name) if ((error = table->file->ha_delete_row(table->record[0]))) table->file->print_error(error, MYF(0)); } - close_thread_tables(thd); - rw_unlock(&THR_LOCK_udf); - /* Binlog the drop function. */ + /* + Binlog the drop function. Keep the table open and locked + while binlogging, to avoid binlog inconsistency. + */ write_bin_log(thd, TRUE, thd->query(), thd->query_length()); DBUG_RETURN(0); - err: +err: rw_unlock(&THR_LOCK_udf); DBUG_RETURN(1); } diff --git a/sql/sql_update.cc b/sql/sql_update.cc index 7e033bc963a..b81fb30ec27 100644 --- a/sql/sql_update.cc +++ b/sql/sql_update.cc @@ -226,7 +226,7 @@ int mysql_update(THD *thd, break; if (!need_reopen) DBUG_RETURN(1); - close_tables_for_reopen(thd, &table_list, FALSE); + close_tables_for_reopen(thd, &table_list); } if (mysql_handle_derived(thd->lex, &mysql_derived_prepare) || @@ -1149,7 +1149,7 @@ reopen_tables: */ cleanup_items(thd->free_list); - close_tables_for_reopen(thd, &table_list, FALSE); + close_tables_for_reopen(thd, &table_list); goto reopen_tables; } diff --git a/sql/table.cc b/sql/table.cc index 8d28514e912..7fb9bbbd955 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -4583,6 +4583,14 @@ void TABLE_LIST::reinit_before_use(THD *thd) } while (parent_embedding && parent_embedding->nested_join->join_list.head() == embedded); + + mdl_request.ticket= NULL; + /* + Not strictly necessary, but we manipulate with the type in open_table(), + so it's "safe" to reset the lock request type to the parser default, to + restore things back to first-execution state. + */ + mdl_request.set_type(MDL_SHARED); } /* @@ -4811,11 +4819,11 @@ size_t max_row_length(TABLE *table, const uchar *data) objects for all elements of table list. */ -void alloc_mdl_requests(TABLE_LIST *table_list, MEM_ROOT *root) +void init_mdl_requests(TABLE_LIST *table_list) { for ( ; table_list ; table_list= table_list->next_global) - table_list->mdl_request= - MDL_request::create(0, table_list->db, table_list->table_name, root); + table_list->mdl_request.init(0, table_list->db, table_list->table_name, + MDL_SHARED); } diff --git a/sql/table.h b/sql/table.h index 1762ae8785d..82498428a11 100644 --- a/sql/table.h +++ b/sql/table.h @@ -18,6 +18,7 @@ #include "sql_plist.h" +#include "mdl.h" /* Structs that defines the TABLE */ @@ -30,8 +31,6 @@ class st_select_lex; class partition_info; class COND_EQUAL; class Security_context; -class MDL_request; -class MDL_ticket; /*************************************************************************/ @@ -1126,6 +1125,7 @@ struct TABLE_LIST table_name_length= table_name_length_arg; alias= (char*) alias_arg; lock_type= lock_type_arg; + mdl_request.init(0, db, table_name, MDL_SHARED); } /* @@ -1429,7 +1429,7 @@ struct TABLE_LIST uint table_open_method; enum enum_schema_table_state schema_table_state; - MDL_request *mdl_request; + MDL_request mdl_request; void calc_md5(char *buffer); void set_underlying_merge(); @@ -1798,6 +1798,6 @@ static inline void dbug_tmp_restore_column_maps(MY_BITMAP *read_set, size_t max_row_length(TABLE *table, const uchar *data); -void alloc_mdl_requests(TABLE_LIST *table_list, MEM_ROOT *root); +void init_mdl_requests(TABLE_LIST *table_list); #endif /* TABLE_INCLUDED */ diff --git a/sql/tztime.cc b/sql/tztime.cc index 9c49c286662..c17b37e27fb 100644 --- a/sql/tztime.cc +++ b/sql/tztime.cc @@ -1637,6 +1637,7 @@ my_tz_init(THD *org_thd, const char *default_tzname, my_bool bootstrap) tz_init_table_list(tz_tables+1); tz_tables[0].next_global= tz_tables[0].next_local= &tz_tables[1]; tz_tables[1].prev_global= &tz_tables[0].next_global; + init_mdl_requests(tz_tables); /* We need to open only mysql.time_zone_leap_second, but we try to @@ -2296,6 +2297,7 @@ my_tz_find(THD *thd, const String *name) Open_tables_state open_tables_state_backup; tz_init_table_list(tz_tables); + init_mdl_requests(tz_tables); if (!open_system_tables_for_read(thd, tz_tables, &open_tables_state_backup)) { diff --git a/storage/myisammrg/ha_myisammrg.cc b/storage/myisammrg/ha_myisammrg.cc index 21a7e2c43d3..e68971975bc 100644 --- a/storage/myisammrg/ha_myisammrg.cc +++ b/storage/myisammrg/ha_myisammrg.cc @@ -388,7 +388,6 @@ int ha_myisammrg::add_children_list(void) { TABLE_LIST *parent_l= this->table->pos_in_table_list; TABLE_LIST *child_l; - THD *thd= current_thd; DBUG_ENTER("ha_myisammrg::add_children_list"); DBUG_PRINT("myrg", ("table: '%s'.'%s' 0x%lx", this->table->s->db.str, this->table->s->table_name.str, (long) this->table)); @@ -434,15 +433,12 @@ int ha_myisammrg::add_children_list(void) /* Copy select_lex. Used in unique_table() at least. */ child_l->select_lex= parent_l->select_lex; - child_l->mdl_request= NULL; /* Safety, if alloc_mdl_requests fails. */ - /* Break when this was the last child. */ if (&child_l->next_global == this->children_last_l) break; } - alloc_mdl_requests(children_l, thd->locked_tables_root ? - thd->locked_tables_root : thd->mem_root); + init_mdl_requests(children_l); /* Insert children into the table list. */ if (parent_l->next_global) @@ -819,6 +815,8 @@ int ha_myisammrg::detach_children(void) Clear the table reference. */ child_l->table= NULL; + /* Similarly, clear the ticket reference. */ + child_l->mdl_request.ticket= NULL; /* Break when this was the last child. */ if (&child_l->next_global == this->children_last_l) |