From ce5c87a3d359d528111d84b5c75bdb8acfe3fb9c Mon Sep 17 00:00:00 2001 From: Konstantin Osipov Date: Tue, 8 Dec 2009 12:57:07 +0300 Subject: Backport of: ---------------------------------------------------------- revno: 2617.69.20 committer: Konstantin Osipov 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. --- sql/mdl.h | 85 +++++++++++++++++++++++++++++++++++++-------------------------- 1 file changed, 50 insertions(+), 35 deletions(-) (limited to 'sql/mdl.h') 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_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 > - Request_list; - - typedef Request_list::Iterator Request_iterator; - typedef I_P_List