#At file:///home/dlenev/src/bzr/mysql-6.0-3726-w/
2652 Dmitry Lenev 2008-05-28
WL#3726 "DDL locking for all metadata objects".
After review fixes in progress. Removed unused code and
adjusted names of functions/methods to better reflect
their current function.
modified:
sql/mysql_priv.h
sql/sql_base.cc
sql/sql_handler.cc
sql/sql_insert.cc
sql/sql_partition.cc
sql/sql_table.cc
sql/sql_trigger.cc
sql/table.h
per-file comments:
sql/mysql_priv.h
Changed names of close_data_files_and_morph_locks() and
close_handle_and_leave_table_as_lock() to better reflect
their current function (locking is now responsibility
of metadata locking subsystem).
sql/sql_base.cc
Changed names of close_data_files_and_morph_locks() and
close_handle_and_leave_table_as_lock() to better reflect
their current function (locking is now responsibility
of metadata locking subsystem). Also adjusted comments
describing these functions.
Got rid of TABLE::open_placeholder since it is no longer
used (its value is never read anywhere).
TABLE::needs_reopen_or_name_lock() was renamed to needs_reopen()
since we no longer use name-locks.
sql/sql_handler.cc
TABLE::needs_reopen_or_name_lock() was renamed to needs_reopen()
since we no longer use name-locks.
sql/sql_insert.cc
TABLE::needs_reopen_or_name_lock() was renamed to needs_reopen()
since we no longer use name-locks.
sql/sql_partition.cc
Changed name of close_data_files_and_morph_locks() to
better reflect its current function (locking is now
responsibility of metadata locking subsystem).
sql/sql_table.cc
Changed names of close_data_files_and_morph_locks() and
close_handle_and_leave_table_as_lock() to better reflect
their current function (locking is now responsibility
of metadata locking subsystem).
Got rid of TABLE::open_placeholder since it is no longer
used.
sql/sql_trigger.cc
Changed name of close_data_files_and_morph_locks() to
better reflect its current function (locking is now
responsibility of metadata locking subsystem).
sql/table.h
Got rid of TABLE::open_placeholder which is no longer used
altough its value was set in several places no code reads it).
Removed unused TABLE::is_name_opened() method.
Finally TABLE::needs_reopen_or_name_lock() was renamed to
needs_reopen() since we no longer use name-locks.
=== modified file 'sql/mysql_priv.h'
--- a/sql/mysql_priv.h 2008-05-28 08:07:30 +0000
+++ b/sql/mysql_priv.h 2008-05-28 08:16:03 +0000
@@ -1335,9 +1335,9 @@
TABLE_LIST *new_child_list, TABLE_LIST **new_last);
bool reopen_table(TABLE *table);
bool reopen_tables(THD *thd, bool get_locks);
-void close_data_files_and_morph_locks(THD *thd, const char *db,
- const char *table_name);
-void close_handle_and_leave_table_as_lock(TABLE *table);
+void close_data_files_and_leave_as_placeholders(THD *thd, const char *db,
+ const char *table_name);
+void close_handle_and_leave_table_as_placeholder(TABLE *table);
bool open_new_frm(THD *thd, TABLE_SHARE *share, const char *alias,
uint db_stat, uint prgflag,
uint ha_open_flags, TABLE *outparam,
=== modified file 'sql/sql_base.cc'
--- a/sql/sql_base.cc 2008-05-28 08:07:30 +0000
+++ b/sql/sql_base.cc 2008-05-28 08:16:03 +0000
@@ -702,25 +702,23 @@
}
-/*
- Close file handle, but leave the table in the table cache
-
- SYNOPSIS
- close_handle_and_leave_table_as_lock()
- table Table handler
-
- NOTES
- By leaving the table in the table cache, it disallows any other thread
- to open the table
-
- thd->killed will be set if we run out of memory
-
- If closing a MERGE child, the calling function has to take care for
- closing the parent too, if necessary.
+/**
+ Close file handle, but leave the table in THD::open_tables list
+ to allow its future reopening.
+
+ @param table Table handler
+
+ @note THD::killed will be set if we run out of memory
+
+ @note If closing a MERGE child, the calling function has to
+ take care for closing the parent too, if necessary.
+
+ @todo Get rid of this function once we refactor LOCK TABLES
+ to keep around TABLE_LIST elements used for opening
+ of tables until UNLOCK TABLES.
*/
-
-void close_handle_and_leave_table_as_lock(TABLE *table)
+void close_handle_and_leave_table_as_placeholder(TABLE *table)
{
TABLE_SHARE *share, *old_share= table->s;
char *key_buff;
@@ -1024,8 +1022,8 @@
char dbname[NAME_LEN+1];
char tname[NAME_LEN+1];
/*
- Since close_data_files_and_morph_locks() frees share's memroot
- we need to make copies of database and table names.
+ Since close_data_files_and_leave_as_placeholders() frees share's
+ memroot we need to make copies of database and table names.
*/
strmov(dbname, tab->s->db.str);
strmov(tname, tab->s->table_name.str);
@@ -1035,7 +1033,7 @@
goto err_with_reopen;
}
pthread_mutex_lock(&LOCK_open);
- close_data_files_and_morph_locks(thd, dbname, tname);
+ close_data_files_and_leave_as_placeholders(thd, dbname, tname);
pthread_mutex_unlock(&LOCK_open);
}
}
@@ -1058,7 +1056,8 @@
goto err_with_reopen;
}
pthread_mutex_lock(&LOCK_open);
- close_data_files_and_morph_locks(thd, table->db, table->table_name);
+ close_data_files_and_leave_as_placeholders(thd, table->db,
+ table->table_name);
pthread_mutex_unlock(&LOCK_open);
}
}
@@ -1482,7 +1481,7 @@
detach_merge_children(table, TRUE);
table->mdl_lock= 0;
- if (table->needs_reopen_or_name_lock() ||
+ if (table->needs_reopen() ||
thd->version != refresh_version || !table->db_stat)
{
free_cache_entry(table);
@@ -1490,12 +1489,6 @@
}
else
{
- /*
- Open placeholders have TABLE::db_stat set to 0, so they should be
- handled by the first alternative.
- */
- DBUG_ASSERT(!table->open_placeholder);
-
/* Assert that MERGE children are not attached in unused_tables. */
DBUG_ASSERT(!table->is_children_attached());
@@ -3195,7 +3188,7 @@
/**
Close all instances of a table open by this thread and replace
- them with exclusive name-locks.
+ them with placeholder in THD::open_tables list for future reopening.
@param thd Thread context
@param db Database name for the table to be closed
@@ -3210,11 +3203,11 @@
the strings are used in a loop even after the share may be freed.
*/
-void close_data_files_and_morph_locks(THD *thd, const char *db,
- const char *table_name)
+void close_data_files_and_leave_as_placeholders(THD *thd, const char *db,
+ const char *table_name)
{
TABLE *table;
- DBUG_ENTER("close_data_files_and_morph_locks");
+ DBUG_ENTER("close_data_files_and_leave_as_placeholders");
safe_mutex_assert_owner(&LOCK_open);
@@ -3228,11 +3221,6 @@
thd->lock= 0;
}
- /*
- Note that open table list may contain a name-lock placeholder
- for target table name if we process ALTER TABLE ... RENAME.
- So loop below makes sense even if we are not under LOCK TABLES.
- */
for (table=thd->open_tables; table ; table=table->next)
{
if (!strcmp(table->s->table_name.str, table_name) &&
@@ -3249,15 +3237,13 @@
won't have multiple children with the same db.table_name.
*/
mysql_lock_remove(thd, thd->locked_tables, table->parent, TRUE);
- table->parent->open_placeholder= 1;
- close_handle_and_leave_table_as_lock(table->parent);
+ close_handle_and_leave_table_as_placeholder(table->parent);
}
else
mysql_lock_remove(thd, thd->locked_tables, table, TRUE);
}
- table->open_placeholder= 1;
table->s->version= 0;
- close_handle_and_leave_table_as_lock(table);
+ close_handle_and_leave_table_as_placeholder(table);
}
}
DBUG_VOID_RETURN;
=== modified file 'sql/sql_handler.cc'
--- a/sql/sql_handler.cc 2008-05-27 17:31:53 +0000
+++ b/sql/sql_handler.cc 2008-05-28 08:16:03 +0000
@@ -781,7 +781,7 @@
if (hash_tables->table &&
(hash_tables->table->mdl_lock &&
mdl_has_pending_conflicting_lock(hash_tables->table->mdl_lock) ||
- hash_tables->table->needs_reopen_or_name_lock()))
+ hash_tables->table->needs_reopen()))
{
mysql_ha_close_table(thd, hash_tables);
/* Mark table as closed, ready for re-open. */
=== modified file 'sql/sql_insert.cc'
--- a/sql/sql_insert.cc 2008-05-27 17:31:53 +0000
+++ b/sql/sql_insert.cc 2008-05-28 08:16:03 +0000
@@ -2581,7 +2581,7 @@
thd_proc_info(&thd, "insert");
max_rows= delayed_insert_limit;
- if (thd.killed || table->needs_reopen_or_name_lock())
+ if (thd.killed || table->needs_reopen())
{
thd.killed= THD::KILL_CONNECTION;
max_rows= ULONG_MAX; // Do as much as possible
=== modified file 'sql/sql_partition.cc'
--- a/sql/sql_partition.cc 2008-05-23 13:54:03 +0000
+++ b/sql/sql_partition.cc 2008-05-28 08:16:03 +0000
@@ -5828,7 +5828,7 @@
and we set db_stat to zero to ensure we don't close twice.
*/
pthread_mutex_lock(&LOCK_open);
- close_data_files_and_morph_locks(thd, db, table_name);
+ close_data_files_and_leave_as_placeholders(thd, db, table_name);
pthread_mutex_unlock(&LOCK_open);
DBUG_RETURN(0);
}
=== modified file 'sql/sql_table.cc'
--- a/sql/sql_table.cc 2008-05-28 08:07:30 +0000
+++ b/sql/sql_table.cc 2008-05-28 08:16:03 +0000
@@ -5778,9 +5778,8 @@
pthread_mutex_lock(&LOCK_open);
alter_table_manage_keys(table, table->file->indexes_are_disabled(),
keys_onoff);
- close_data_files_and_morph_locks(thd,
- table->pos_in_table_list->db,
- table->pos_in_table_list->table_name);
+ close_data_files_and_leave_as_placeholders(thd, table->pos_in_table_list->db,
+ table->pos_in_table_list->table_name);
if (mysql_rename_table(NULL,
altered_table->s->db.str,
altered_table->s->table_name.str,
@@ -5832,9 +5831,8 @@
object to make it suitable for reopening.
*/
DBUG_ASSERT(t_table == table);
- table->open_placeholder= 1;
pthread_mutex_lock(&LOCK_open);
- close_handle_and_leave_table_as_lock(table);
+ close_handle_and_leave_table_as_placeholder(table);
pthread_mutex_unlock(&LOCK_open);
}
@@ -7005,7 +7003,7 @@
pthread_mutex_lock(&LOCK_open);
- close_data_files_and_morph_locks(thd, db, table_name);
+ close_data_files_and_leave_as_placeholders(thd, db, table_name);
error=0;
save_old_db_type= old_db_type;
=== modified file 'sql/sql_trigger.cc'
--- a/sql/sql_trigger.cc 2008-05-23 13:54:03 +0000
+++ b/sql/sql_trigger.cc 2008-05-28 08:16:03 +0000
@@ -487,7 +487,8 @@
if (!result && thd->locked_tables)
{
/* Make table suitable for reopening */
- close_data_files_and_morph_locks(thd, tables->db, tables->table_name);
+ close_data_files_and_leave_as_placeholders(thd, tables->db,
+ tables->table_name);
thd->in_lock_tables= 1;
if (reopen_tables(thd, 1))
{
=== modified file 'sql/table.h'
--- a/sql/table.h 2008-05-27 12:15:44 +0000
+++ b/sql/table.h 2008-05-28 08:16:03 +0000
@@ -722,24 +722,6 @@
my_bool force_index;
my_bool distinct,const_table,no_rows;
my_bool key_read, no_keyread;
- /*
- Placeholder for an open table which prevents other connections
- from taking name-locks on this table. Typically used with
- TABLE_SHARE::version member to take an exclusive name-lock on
- this table name -- a name lock that not only prevents other
- threads from opening the table, but also blocks other name
- locks. This is achieved by:
- - setting open_placeholder to 1 - this will block other name
- locks, as wait_for_locked_table_name will be forced to wait,
- see table_is_used for details.
- - setting version to 0 - this will force other threads to close
- the instance of this table and wait (this is the same approach
- as used for usual name locks).
- An exclusively name-locked table currently can have no handler
- object associated with it (db_stat is always 0), but please do
- not rely on that.
- */
- my_bool open_placeholder;
my_bool locked_by_logger;
my_bool no_replicate;
my_bool locked_by_name;
@@ -803,12 +785,10 @@
read_set= &def_read_set;
write_set= &def_write_set;
}
- /* Is table open or should be treated as such by name-locking? */
- inline bool is_name_opened() { return db_stat || open_placeholder; }
/*
- Is this instance of the table should be reopen or represents a name-lock?
+ Is this instance of the table should be reopen?
*/
- inline bool needs_reopen_or_name_lock()
+ inline bool needs_reopen()
{ return s->version != refresh_version; }
bool is_children_attached(void);
};
| Thread |
|---|
| • commit into mysql-6.0 branch (dlenev:2652) WL#3726 | Dmitry Lenev | 28 May |