From 511c68fbd461efecd5bd9f7c08b42916fa8416c9 Mon Sep 17 00:00:00 2001 From: Konstantin Osipov Date: Mon, 30 Nov 2009 22:38:25 +0300 Subject: [PATCH] Backport of: ------------------------------------------------------------------- revno: 2630.6.6 committer: Konstantin Osipov branch nick: mysql-6.0-3726 timestamp: Tue 2008-05-27 16:15:44 +0400 message: Implement code review fixes for WL#3726 "DDL locking for all metadata objects": cleanup the code from share->mutex acquisitions, which are now obsolete. --- sql/ha_partition.cc | 6 +++--- sql/ha_partition.h | 4 ++-- sql/sql_base.cc | 41 ++----------------------------------- sql/sql_view.cc | 2 -- sql/table.cc | 15 ++++---------- sql/table.h | 2 +- storage/myisam/ha_myisam.cc | 4 ++-- 7 files changed, 14 insertions(+), 60 deletions(-) diff --git a/sql/ha_partition.cc b/sql/ha_partition.cc index 2ec92173d14..1514651c1c5 100644 --- a/sql/ha_partition.cc +++ b/sql/ha_partition.cc @@ -2606,7 +2606,7 @@ int ha_partition::open(const char *name, int mode, uint test_if_locked) for the same table. */ if (is_not_tmp_table) - pthread_mutex_lock(&table_share->mutex); + pthread_mutex_lock(&table_share->LOCK_ha_data); if (!table_share->ha_data) { HA_DATA_PARTITION *ha_data; @@ -2617,7 +2617,7 @@ int ha_partition::open(const char *name, int mode, uint test_if_locked) if (!ha_data) { if (is_not_tmp_table) - pthread_mutex_unlock(&table_share->mutex); + pthread_mutex_unlock(&table_share->LOCK_ha_data); goto err_handler; } DBUG_PRINT("info", ("table_share->ha_data 0x%p", ha_data)); @@ -2626,7 +2626,7 @@ int ha_partition::open(const char *name, int mode, uint test_if_locked) pthread_mutex_init(&ha_data->mutex, MY_MUTEX_INIT_FAST); } if (is_not_tmp_table) - pthread_mutex_unlock(&table_share->mutex); + pthread_mutex_unlock(&table_share->LOCK_ha_data); /* Some handlers update statistics as part of the open call. This will in some cases corrupt the statistics of the partition handler and thus diff --git a/sql/ha_partition.h b/sql/ha_partition.h index d4579d013fd..fc0e98348db 100644 --- a/sql/ha_partition.h +++ b/sql/ha_partition.h @@ -933,7 +933,7 @@ private: if(table_share->tmp_table == NO_TMP_TABLE) { auto_increment_lock= TRUE; - pthread_mutex_lock(&table_share->mutex); + pthread_mutex_lock(&table_share->LOCK_ha_data); } } virtual void unlock_auto_increment() @@ -946,7 +946,7 @@ private: */ if(auto_increment_lock && !auto_increment_safe_stmt_log_lock) { - pthread_mutex_unlock(&table_share->mutex); + pthread_mutex_unlock(&table_share->LOCK_ha_data); auto_increment_lock= FALSE; } } diff --git a/sql/sql_base.cc b/sql/sql_base.cc index f963d74102b..1356a2cd16f 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -112,7 +112,6 @@ uint table_cache_count= 0; TABLE *unused_tables; HASH table_def_cache; static TABLE_SHARE *oldest_unused_share, end_of_unused_share; -static pthread_mutex_t LOCK_table_share; static bool table_def_inited= 0; static bool check_and_update_table_version(THD *thd, TABLE_LIST *tables, @@ -253,13 +252,12 @@ extern "C" uchar *table_def_key(const uchar *record, size_t *length, static void table_def_free_entry(TABLE_SHARE *share) { DBUG_ENTER("table_def_free_entry"); + safe_mutex_assert_owner(&LOCK_open); if (share->prev) { /* remove from old_unused_share list */ - pthread_mutex_lock(&LOCK_table_share); *share->prev= share->next; share->next->prev= share->prev; - pthread_mutex_unlock(&LOCK_table_share); } free_table_share(share); DBUG_VOID_RETURN; @@ -269,7 +267,6 @@ static void table_def_free_entry(TABLE_SHARE *share) bool table_def_init(void) { table_def_inited= 1; - pthread_mutex_init(&LOCK_table_share, MY_MUTEX_INIT_FAST); oldest_unused_share= &end_of_unused_share; end_of_unused_share.prev= &oldest_unused_share; @@ -287,7 +284,6 @@ void table_def_free(void) /* Free all open TABLEs first. */ close_cached_tables(NULL, NULL, FALSE, FALSE); table_def_inited= 0; - pthread_mutex_destroy(&LOCK_table_share); /* Free table definitions. */ my_hash_free(&table_def_cache); } @@ -473,12 +469,6 @@ TABLE_SHARE *get_table_share(THD *thd, TABLE_LIST *table_list, char *key, DBUG_RETURN(0); } - /* - Lock mutex to be able to read table definition from file without - conflicts - */ - (void) pthread_mutex_lock(&share->mutex); - /* We assign a new table id under the protection of the LOCK_open and the share's own mutex. We do this insted of creating a new mutex @@ -508,7 +498,6 @@ TABLE_SHARE *get_table_share(THD *thd, TABLE_LIST *table_list, char *key, share->ref_count++; // Mark in use DBUG_PRINT("exit", ("share: 0x%lx ref_count: %u", (ulong) share, share->ref_count)); - (void) pthread_mutex_unlock(&share->mutex); DBUG_RETURN(share); found: @@ -516,20 +505,15 @@ found: We found an existing table definition. Return it if we didn't get an error when reading the table definition from file. */ - - /* We must do a lock to ensure that the structure is initialized */ - (void) pthread_mutex_lock(&share->mutex); if (share->error) { /* Table definition contained an error */ open_table_error(share, share->error, share->open_errno, share->errarg); - (void) pthread_mutex_unlock(&share->mutex); DBUG_RETURN(0); } if (share->is_view && !(db_flags & OPEN_VIEW)) { open_table_error(share, 1, ENOENT, 0); - (void) pthread_mutex_unlock(&share->mutex); DBUG_RETURN(0); } @@ -540,22 +524,16 @@ found: Unlink share from this list */ DBUG_PRINT("info", ("Unlinking from not used list")); - pthread_mutex_lock(&LOCK_table_share); *share->prev= share->next; share->next->prev= share->prev; share->next= 0; share->prev= 0; - pthread_mutex_unlock(&LOCK_table_share); } - (void) pthread_mutex_unlock(&share->mutex); /* Free cache if too big */ while (table_def_cache.records > table_def_size && oldest_unused_share->next) - { - pthread_mutex_lock(&oldest_unused_share->mutex); my_hash_delete(&table_def_cache, (uchar*) oldest_unused_share); - } DBUG_PRINT("exit", ("share: 0x%lx ref_count: %u", (ulong) share, share->ref_count)); @@ -672,7 +650,6 @@ void release_table_share(TABLE_SHARE *share) safe_mutex_assert_owner(&LOCK_open); - pthread_mutex_lock(&share->mutex); DBUG_ASSERT(share->ref_count); if (!--share->ref_count) { @@ -684,12 +661,10 @@ void release_table_share(TABLE_SHARE *share) DBUG_PRINT("info",("moving share to unused list")); DBUG_ASSERT(share->next == 0); - pthread_mutex_lock(&LOCK_table_share); share->prev= end_of_unused_share.prev; *end_of_unused_share.prev= share; end_of_unused_share.prev= &share->next; share->next= &end_of_unused_share; - pthread_mutex_unlock(&LOCK_table_share); to_be_deleted= (table_def_cache.records > table_def_size); } @@ -699,9 +674,7 @@ void release_table_share(TABLE_SHARE *share) { DBUG_PRINT("info", ("Deleting share")); my_hash_delete(&table_def_cache, (uchar*) share); - DBUG_VOID_RETURN; } - pthread_mutex_unlock(&share->mutex); DBUG_VOID_RETURN; } @@ -745,9 +718,8 @@ static void reference_table_share(TABLE_SHARE *share) { DBUG_ENTER("reference_table_share"); DBUG_ASSERT(share->ref_count); - pthread_mutex_lock(&share->mutex); + safe_mutex_assert_owner(&LOCK_open); share->ref_count++; - pthread_mutex_unlock(&share->mutex); DBUG_PRINT("exit", ("share: 0x%lx ref_count: %u", (ulong) share, share->ref_count)); DBUG_VOID_RETURN; @@ -1043,10 +1015,7 @@ bool close_cached_tables(THD *thd, TABLE_LIST *tables, bool have_lock, free_cache_entry(unused_tables); /* Free table shares */ while (oldest_unused_share->next) - { - pthread_mutex_lock(&oldest_unused_share->mutex); (void) my_hash_delete(&table_def_cache, (uchar*) oldest_unused_share); - } if (!wait_for_refresh) { @@ -8581,10 +8550,7 @@ bool remove_table_from_cache(THD *thd, const char *db, const char *table_name, DBUG_PRINT("info", ("share version: %lu ref_count: %u", share->version, share->ref_count)); if (share->ref_count == 0) - { - pthread_mutex_lock(&share->mutex); my_hash_delete(&table_def_cache, (uchar*) share); - } } if (result && (flags & RTFC_WAIT_OTHER_THREAD_FLAG)) @@ -8722,10 +8688,7 @@ void expel_table_from_cache(THD *leave_thd, const char *db, const char *table_na { DBUG_ASSERT(leave_thd || share->ref_count == 0); if (share->ref_count == 0) - { - pthread_mutex_lock(&share->mutex); my_hash_delete(&table_def_cache, (uchar*) share); - } } } diff --git a/sql/sql_view.cc b/sql/sql_view.cc index 3dc5ca4e09c..55181a58e53 100644 --- a/sql/sql_view.cc +++ b/sql/sql_view.cc @@ -1643,10 +1643,8 @@ bool mysql_drop_view(THD *thd, TABLE_LIST *views, enum_drop_mode drop_mode) if ((share= get_cached_table_share(view->db, view->table_name))) { DBUG_ASSERT(share->ref_count == 0); - pthread_mutex_lock(&share->mutex); share->ref_count++; share->version= 0; - pthread_mutex_unlock(&share->mutex); release_table_share(share); } query_cache_invalidate3(thd, view, 0); diff --git a/sql/table.cc b/sql/table.cc index b232ba23a89..c8814cff685 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -320,7 +320,7 @@ TABLE_SHARE *alloc_table_share(TABLE_LIST *table_list, char *key, share->free_tables.empty(); memcpy((char*) &share->mem_root, (char*) &mem_root, sizeof(mem_root)); - pthread_mutex_init(&share->mutex, MY_MUTEX_INIT_FAST); + pthread_mutex_init(&share->LOCK_ha_data, MY_MUTEX_INIT_FAST); } DBUG_RETURN(share); } @@ -411,18 +411,11 @@ void free_table_share(TABLE_SHARE *share) DBUG_PRINT("enter", ("table: %s.%s", share->db.str, share->table_name.str)); DBUG_ASSERT(share->ref_count == 0); - /* - If someone is waiting for this to be deleted, inform it about this. - Don't do a delete until we know that no one is refering to this anymore. - */ + /* The mutex is initialized only for shares that are part of the TDC */ if (share->tmp_table == NO_TMP_TABLE) - { - /* No thread refers to this anymore */ - pthread_mutex_unlock(&share->mutex); - pthread_mutex_destroy(&share->mutex); - } + pthread_mutex_destroy(&share->LOCK_ha_data); my_hash_free(&share->name_hash); - + plugin_unlock(NULL, share->db_plugin); share->db_plugin= NULL; diff --git a/sql/table.h b/sql/table.h index 016a99e5452..60560029725 100644 --- a/sql/table.h +++ b/sql/table.h @@ -312,7 +312,7 @@ struct TABLE_SHARE TYPELIB keynames; /* Pointers to keynames */ TYPELIB fieldnames; /* Pointer to fieldnames */ TYPELIB *intervals; /* pointer to interval info */ - pthread_mutex_t mutex; /* For locking the share */ + pthread_mutex_t LOCK_ha_data; /* To protect access to ha_data */ TABLE_SHARE *next, **prev; /* Link to unused shares */ /* diff --git a/storage/myisam/ha_myisam.cc b/storage/myisam/ha_myisam.cc index cb8333767f8..9269b331754 100644 --- a/storage/myisam/ha_myisam.cc +++ b/storage/myisam/ha_myisam.cc @@ -1812,7 +1812,7 @@ int ha_myisam::info(uint flag) /* Update share */ if (share->tmp_table == NO_TMP_TABLE) - pthread_mutex_lock(&share->mutex); + pthread_mutex_lock(&share->LOCK_ha_data); share->keys_in_use.set_prefix(share->keys); share->keys_in_use.intersect_extended(misam_info.key_map); share->keys_for_keyread.intersect(share->keys_in_use); @@ -1822,7 +1822,7 @@ int ha_myisam::info(uint flag) (char*) misam_info.rec_per_key, sizeof(table->key_info[0].rec_per_key[0])*share->key_parts); if (share->tmp_table == NO_TMP_TABLE) - pthread_mutex_unlock(&share->mutex); + pthread_mutex_unlock(&share->LOCK_ha_data); /* Set data_file_name and index_file_name to point at the symlink value