#At file:///opt/local/work/mysql-6.0-3726/
2653 Konstantin Osipov 2008-05-27
Implement code review fixes for WL#3726 "DDL locking for all metadata
objects": cleanup the code from share->mutex acquisitions, which
are now obsolete.
modified:
sql/ha_partition.cc
sql/ha_partition.h
sql/sql_base.cc
sql/sql_view.cc
sql/table.cc
sql/table.h
storage/myisam/ha_myisam.cc
per-file comments:
sql/ha_partition.cc
Rename share->mutex to share->LOCK_ha_data. The mutex never
protected the entire share. The right way to protect a share
is to acquire an MDL lock.
sql/ha_partition.h
Rename share->mutex to share->LOCK_ha_data.
sql/sql_base.cc
Remove LOCK_table_share. Do not acquire share->mutex when
deleting a table share or incrementing its ref_count.
All these operations are and must continue to be protected by LOCK_open
and respective MDL locks.
sql/sql_view.cc
Remove acquisition of share->mutex when incrementing share->ref_count.
sql/table.cc
Simplify release_table_shar() by removing dead code related
to share->mutex from it.
sql/table.h
Rename TABLE_SHARE::mutex to TABLE_SHARE::LOCK_ha_data
to better reflect its purpose.
storage/myisam/ha_myisam.cc
Rename share->mutex to share->LOCK_ha_data.
=== modified file 'sql/ha_partition.cc'
--- a/sql/ha_partition.cc 2008-05-14 13:49:41 +0000
+++ b/sql/ha_partition.cc 2008-05-27 12:15:44 +0000
@@ -2402,7 +2402,7 @@
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;
@@ -2416,7 +2416,7 @@
bzero(ha_data, sizeof(HA_DATA_PARTITION));
}
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
=== modified file 'sql/ha_partition.h'
--- a/sql/ha_partition.h 2008-05-08 13:01:30 +0000
+++ b/sql/ha_partition.h 2008-05-27 12:15:44 +0000
@@ -861,7 +861,7 @@
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()
@@ -874,7 +874,7 @@
*/
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;
}
}
=== modified file 'sql/sql_base.cc'
--- a/sql/sql_base.cc 2008-05-27 09:45:34 +0000
+++ b/sql/sql_base.cc 2008-05-27 12:15:44 +0000
@@ -103,7 +103,6 @@
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,
@@ -242,13 +241,12 @@
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;
@@ -258,7 +256,6 @@
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;
@@ -276,7 +273,6 @@
/* 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. */
hash_free(&table_def_cache);
}
@@ -463,12 +459,6 @@
}
/*
- 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
and using it for the sole purpose of serializing accesses to a
@@ -497,7 +487,6 @@
share->ref_count++; // Mark in use
DBUG_PRINT("exit", ("share: %p ref_count: %u",
share, share->ref_count));
- (void) pthread_mutex_unlock(&share->mutex);
DBUG_RETURN(share);
found:
@@ -505,20 +494,15 @@
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);
}
@@ -529,22 +513,16 @@
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);
hash_delete(&table_def_cache, (uchar*) oldest_unused_share);
- }
DBUG_PRINT("exit", ("share: %p ref_count: %u",
share, share->ref_count));
@@ -652,7 +630,6 @@
safe_mutex_assert_owner(&LOCK_open);
- pthread_mutex_lock(&share->mutex);
if (!--share->ref_count)
{
if (share->version != refresh_version)
@@ -663,12 +640,10 @@
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);
}
@@ -678,9 +653,7 @@
{
DBUG_PRINT("info", ("Deleting share"));
hash_delete(&table_def_cache, (uchar*) share);
- DBUG_VOID_RETURN;
}
- pthread_mutex_unlock(&share->mutex);
DBUG_VOID_RETURN;
}
@@ -723,9 +696,8 @@
{
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;
@@ -1021,10 +993,7 @@
free_cache_entry(unused_tables);
/* Free table shares */
while (oldest_unused_share->next)
- {
- pthread_mutex_lock(&oldest_unused_share->mutex);
(void) hash_delete(&table_def_cache, (uchar*) oldest_unused_share);
- }
if (!wait_for_refresh)
{
@@ -8525,10 +8494,7 @@
DBUG_PRINT("info", ("share version: %lu ref_count: %u",
share->version, share->ref_count));
if (share->ref_count == 0)
- {
- pthread_mutex_lock(&share->mutex);
hash_delete(&table_def_cache, (uchar*) share);
- }
}
if (result && (flags & RTFC_WAIT_OTHER_THREAD_FLAG))
@@ -8666,10 +8632,7 @@
{
DBUG_ASSERT(leave_thd || share->ref_count == 0);
if (share->ref_count == 0)
- {
- pthread_mutex_lock(&share->mutex);
hash_delete(&table_def_cache, (uchar*) share);
- }
}
}
=== modified file 'sql/sql_view.cc'
--- a/sql/sql_view.cc 2008-05-27 09:45:34 +0000
+++ b/sql/sql_view.cc 2008-05-27 12:15:44 +0000
@@ -1633,10 +1633,8 @@
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);
=== modified file 'sql/table.cc'
--- a/sql/table.cc 2008-05-27 09:45:34 +0000
+++ b/sql/table.cc 2008-05-27 12:15:44 +0000
@@ -346,7 +346,7 @@
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);
}
@@ -435,18 +435,11 @@
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);
hash_free(&share->name_hash);
-
+
plugin_unlock(NULL, share->db_plugin);
share->db_plugin= NULL;
=== modified file 'sql/table.h'
--- a/sql/table.h 2008-05-27 09:45:34 +0000
+++ b/sql/table.h 2008-05-27 12:15:44 +0000
@@ -258,7 +258,7 @@
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 */
struct st_table_share *next, /* Link to unused shares */
**prev;
=== modified file 'storage/myisam/ha_myisam.cc'
--- a/storage/myisam/ha_myisam.cc 2008-05-08 16:01:15 +0000
+++ b/storage/myisam/ha_myisam.cc 2008-05-27 12:15:44 +0000
@@ -1669,7 +1669,7 @@
/* 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);
@@ -1679,7 +1679,7 @@
(char*) misam_info.rec_per_key,
sizeof(table->key_info[0].rec_per_key)*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
| Thread |
|---|
| • commit into mysql-6.0 branch (konstantin:2653) WL#3726 | Konstantin Osipov | 27 May |
