#At file:///home/dlenev/src/bzr/mysql-6.1-mil13/ based on revid:dlenev@stripped
2731 Dmitry Lenev 2009-07-04
WL#148 "Foreign keys".
Milestone 13 "DDL checks and changes: ALTER, CREATE INDEX, DROP INDEX words".
Work in progress. Making MDL lock upgrade for main and parent tables atomic,
cleaning up error handling in some rare cases.
modified:
sql/sql_base.cc
sql/sql_table.cc
=== modified file 'sql/sql_base.cc'
--- a/sql/sql_base.cc 2009-06-25 08:43:04 +0000
+++ b/sql/sql_base.cc 2009-07-04 12:52:55 +0000
@@ -4047,6 +4047,7 @@ int open_tables(THD *thd, TABLE_LIST **s
about kind of alter that we perform so we can pre-lock
only relevant parents.
QQ: Does this requires introduction of helper object ?
+ Or can we get by with yet another member in Query_tables_list ?
*/
if (tables == (*start) &&
(flags & MYSQL_OPEN_PRELOCK_PARENTS) &&
=== modified file 'sql/sql_table.cc'
--- a/sql/sql_table.cc 2009-07-02 13:38:41 +0000
+++ b/sql/sql_table.cc 2009-07-04 12:52:55 +0000
@@ -5328,8 +5328,12 @@ prepare_alter_table(TABLE_LIST *alter_ta
if (table)
{
- if (m_related_tables.push_back(table, m_thd->mem_root))
- return TRUE;
+ /*
+ TODO/FIXME: - Ensure that both m_related_tables and parent_tables
+ lists contain only unique elements.
+ */
+ if (m_related_tables.push_back(table, m_thd->mem_root))
+ return TRUE;
if (prepare_parent_table_description(m_thd, table))
return TRUE;
@@ -7823,6 +7827,153 @@ TABLE *create_altered_table(THD *thd,
}
+/**
+ Upgrade meta-data locks on table to be altered and parent tables for
+ which we have to update .FRMs.
+
+ @param thd Thread context.
+ @param altered_table Table to be altered and list of parent tables
+ for which foreign keys are added.
+ @param parent_tables List of parent tables for which foreign keys
+ are dropped.
+
+ @retval FALSE Success.
+ @retval TRUE Failure (E.g. statement was killed).
+*/
+
+static bool
+upgrade_mdl_for_altered_and_parent_tables(THD *thd,
+ TABLE_LIST *altered_table,
+ TABLE_LIST *parent_tables)
+{
+ TABLE_LIST *table, *err_table;
+
+ if (wait_while_table_is_used(thd, altered_table->table,
+ HA_EXTRA_PREPARE_FOR_RENAME))
+ return TRUE;
+
+ if (opt_fk_all_engines)
+ {
+ for (table= altered_table->next_local; table; table= table->next_local)
+ {
+ table->mdl_request.ticket= table->table->mdl_ticket;
+ if (wait_while_table_is_used(thd, table->table, HA_EXTRA_FORCE_REOPEN))
+ {
+ err_table= table;
+ goto err_fk_created_parents;
+ }
+ }
+
+ for (table= parent_tables; table; table= table->next_local)
+ {
+ table->mdl_request.ticket= table->table->mdl_ticket;
+ if (wait_while_table_is_used(thd, table->table, HA_EXTRA_FORCE_REOPEN))
+ {
+ err_table= table;
+ goto err_fk_dropped_parents;
+ }
+ }
+
+ return FALSE;
+
+err_fk_dropped_parents:
+ for (table= parent_tables; table != err_table; table= table->next_local)
+ {
+ mysql_lock_downgrade_write(thd, table->table,
+ table->table->reginfo.lock_type);
+ table->mdl_request.ticket->downgrade_exclusive_lock();
+ }
+ err_table= 0;
+
+err_fk_created_parents:
+ /* Note that below loop also handles downgrade for table being altered. */
+ for (table= altered_table; table != err_table; table= table->next_local)
+ {
+ mysql_lock_downgrade_write(thd, table->table,
+ table->table->reginfo.lock_type);
+ table->mdl_request.ticket->downgrade_exclusive_lock();
+ }
+ return TRUE;
+ }
+ else
+ {
+ return FALSE;
+ }
+}
+
+
+/**
+ Downgrade meta-data locks on table which were altered and parent tables
+ for which we have updated .FRMs.
+
+ @param thd Thread context.
+ @param altered_table Table which was altered and list of parent tables
+ for which foreign keys were added.
+ @param parent_tables List of parent tables for which foreign keys were
+ dropped.
+
+ @retval FALSE Success.
+ @retval TRUE Failure (E.g. statement was killed).
+*/
+
+static void
+downgrade_mdl_for_altered_and_parent_tables(TABLE_LIST *altered_table,
+ TABLE_LIST *parent_tables)
+{
+ TABLE_LIST *table;
+
+ if (opt_fk_all_engines)
+ {
+ for (table= parent_tables; table; table= table->next_local)
+ table->mdl_request.ticket->downgrade_exclusive_lock();
+
+ /*
+ Note that below loop also handles downgrade for table which was altered.
+ */
+ for (table= altered_table; table; table= table->next_local)
+ table->mdl_request.ticket->downgrade_exclusive_lock();
+ }
+ else
+ altered_table->mdl_request.ticket->downgrade_exclusive_lock();
+}
+
+
+/**
+ Release meta-data locks on table for which we failed to finalize ALTER TABLE
+ and parent tables for which we were supposed to update .FRMs.
+
+ @param thd Thread context.
+ @param altered_table Table which was supposed to be altered and list of
+ parent tables for which foreign keys were supposed
+ to be added.
+ @param parent_tables List of parent tables for which foreign keys were
+ supposed to be dropped.
+
+ @retval FALSE Success.
+ @retval TRUE Failure (E.g. statement was killed).
+*/
+
+static void
+release_mdl_for_altered_and_parent_tables(THD *thd, TABLE_LIST *altered_table,
+ TABLE_LIST *parent_tables)
+{
+ TABLE_LIST *table;
+
+ if (opt_fk_all_engines)
+ {
+ for (table= parent_tables; table; table= table->next_local)
+ thd->mdl_context.release_all_locks_for_name(table->mdl_request.ticket);
+
+ /* Note that below loop also handles table which was altered. */
+ for (table= altered_table; table; table= table->next_local)
+ thd->mdl_context.release_all_locks_for_name(table->mdl_request.ticket);
+ }
+ else
+ thd->mdl_context.release_all_locks_for_name(altered_table->
+ mdl_request.ticket);
+}
+
+
/*
Perform a fast or on-line alter table
@@ -7889,38 +8040,20 @@ mysql_fast_or_online_alter_table(THD *th
create_info, alter_info,
ha_alter_flags))
DBUG_RETURN(1);
-
- /*
- Make the metadata lock available to open_table() called to
- reopen the table down the road.
- */
- table_list->mdl_request.ticket= table->mdl_ticket;
}
/*
Upgrade the shared metadata lock to exclusive and close all
- instances of the table in the TDC except those used in this thread.
+ instances of the table in the TDC except those used in this
+ thread.
+
+ Also upgrade metadata locks on all parent tables for which
+ we are going to change .FRMs.
*/
- if (wait_while_table_is_used(thd, table, HA_EXTRA_PREPARE_FOR_RENAME))
+ if (upgrade_mdl_for_altered_and_parent_tables(thd, table_list,
+ parent_tables))
DBUG_RETURN(1);
- if (opt_fk_all_engines)
- {
- for (tab= table_list->next_local; tab; tab= tab->next_local)
- {
- tab->mdl_request.ticket= tab->table->mdl_ticket;
- if (wait_while_table_is_used(thd, tab->table, HA_EXTRA_FORCE_REOPEN))
- DBUG_RETURN(1);
- }
-
- for (tab= parent_tables; tab; tab= tab->next_local)
- {
- tab->mdl_request.ticket= tab->table->mdl_ticket;
- if (wait_while_table_is_used(thd, tab->table, HA_EXTRA_FORCE_REOPEN))
- DBUG_RETURN(1);
- }
- }
-
alter_table_manage_keys(table, table->file->indexes_are_disabled(),
keys_onoff);
@@ -8839,7 +8972,7 @@ err:
@retval TRUE Failure (due to OOM or KILL statement).
*/
-bool
+static bool
alter_table_lock_fk_names_under_lock_tables(THD *thd, TABLE_LIST *table_list,
Alter_info *alter_info,
List<Foreign_key_name> *fk_names_drop,
@@ -9008,7 +9141,7 @@ alter_table_lock_fk_names_under_lock_tab
downgraded.
*/
-void
+static void
alter_table_restore_fk_name_locks_under_lock_tables(THD *thd,
List<Foreign_key_name> &fk_names_drop,
List<Foreign_key_name> &fk_names_add,
@@ -9045,11 +9178,11 @@ alter_table_restore_fk_name_locks_under_
foreign keys, locks for which should be downgraded.
*/
-void
-alter_table_release_fk_name_locks_under_lock_tables(THD *thd,
- List<Foreign_key_name> &fk_names_drop,
- List<Foreign_key_name> &fk_names_add,
- List<Foreign_key_name> &fk_names_replace)
+static void
+alter_table_adjust_fk_name_locks_under_lock_tables(THD *thd,
+ List<Foreign_key_name> &fk_names_drop,
+ List<Foreign_key_name> &fk_names_add,
+ List<Foreign_key_name> &fk_names_replace)
{
List_iterator<Foreign_key_name> fk_drop_it(fk_names_drop);
List_iterator<Foreign_key_name> fk_add_it(fk_names_add);
@@ -9072,6 +9205,42 @@ alter_table_release_fk_name_locks_under_
/**
+ Release metadata locks on foreign key names after failure to finalize
+ ALTER TABLE under LOCK TABLES.
+
+ @param thd Thread context.
+ @param fk_names_drop List of names of foreign keys which were supposed
+ to be dropped locks for which should be released.
+ @param fk_names_add List of names of foreign keys which were supposed
+ to be added locks for which should be released.
+ @param fk_names_replace List of names of foreign keys which were supposed
+ to replace old foreign keys locks for which should
+ be downgraded.
+*/
+
+static void
+alter_table_release_fk_name_locks_under_lock_tables(THD *thd,
+ List<Foreign_key_name> &fk_names_drop,
+ List<Foreign_key_name> &fk_names_add,
+ List<Foreign_key_name> &fk_names_replace)
+{
+ List_iterator<Foreign_key_name> fk_drop_it(fk_names_drop);
+ List_iterator<Foreign_key_name> fk_add_it(fk_names_add);
+ List_iterator<Foreign_key_name> fk_replace_it(fk_names_replace);
+ Foreign_key_name *fk_name;
+
+ while ((fk_name= fk_drop_it++))
+ thd->mdl_context.release_lock(fk_name->get_mdl_request()->ticket);
+
+ while ((fk_name= fk_add_it++))
+ thd->mdl_context.release_lock(fk_name->get_mdl_request()->ticket);
+
+ while ((fk_name= fk_replace_it++))
+ thd->mdl_context.release_lock(fk_name->get_mdl_request()->ticket);
+}
+
+
+/**
Check if table being altered participates in some foreign key as parent
which will be preserved in its new version.
@@ -9145,7 +9314,6 @@ bool mysql_alter_table(THD *thd,char *ne
uint order_num, ORDER *order, bool ignore)
{
TABLE *table, *new_table= 0, *new_table_2= 0;
- MDL_ticket *mdl_ticket;
MDL_request target_mdl_request;
bool has_target_mdl_lock= FALSE;
int error= 0;
@@ -9160,7 +9328,7 @@ bool mysql_alter_table(THD *thd,char *ne
uint fast_alter_partition= 0;
bool partition_changed= FALSE;
#endif
- TABLE_LIST *tab, *parent_tables;
+ TABLE_LIST *parent_tables;
List<Foreign_key_name> fk_names_add, fk_names_drop, fk_names_replace;
DBUG_ENTER("mysql_alter_table");
@@ -9364,7 +9532,12 @@ view_err:
table= table_list->table;
table->use_all_columns();
- mdl_ticket= table->mdl_ticket;
+
+ /*
+ Make the metadata lock available to code handling downgrade of metadata
+ locks and to open_table() called by mysql_fast_or_online_alter_table().
+ */
+ table_list->mdl_request.ticket= table->mdl_ticket;
Foreign_key_ddl_rcontext fk_ctx(thd);
Parent_info alter_table_parent_info (create_info,
@@ -9644,17 +9817,24 @@ view_err:
*/
if (new_name != table_name || new_db != db)
{
- thd->mdl_context.release_all_locks_for_name(mdl_ticket);
+ thd->mdl_context.release_all_locks_for_name(table_list->
+ mdl_request.ticket);
}
else
- mdl_ticket->downgrade_exclusive_lock();
+ table_list->mdl_request.ticket->downgrade_exclusive_lock();
}
DBUG_RETURN(error);
}
/* We have to do full alter table. */
+ /*
+ QQ: Alternatively to checking for table->s->tmp_table == NO_TMP_TABLE
+ we can release these locks after end_temporaru label.
+ */
+
if (opt_fk_all_engines &&
+ table->s->tmp_table == NO_TMP_TABLE &&
(thd->locked_tables_mode == LTM_LOCK_TABLES ||
thd->locked_tables_mode == LTM_PRELOCKED_UNDER_LOCK_TABLES) &&
alter_table_lock_fk_names_under_lock_tables(thd, table_list, alter_info,
@@ -9894,6 +10074,10 @@ view_err:
A death-defying stunt - creation of second instance of TABLE object
for a temporary table.
+
+ QQ: What is the correct approach to this ? May be we should rely on the
+ new interface for cursors ? Should we call mysql_lock_tables() and
+ such?
*/
if (! (new_table_2= (TABLE*) my_malloc(sizeof(*new_table_2), MYF(MY_WME))) ||
open_table_from_share(thd, new_table->s, tmp_name,
@@ -10019,32 +10203,10 @@ view_err:
if (lower_case_table_names)
my_casedn_str(files_charset_info, old_name);
- if (wait_while_table_is_used(thd, table, HA_EXTRA_PREPARE_FOR_RENAME))
+ if (upgrade_mdl_for_altered_and_parent_tables(thd, table_list,
+ parent_tables))
goto err_new_table_cleanup;
- /*
- TODO/FIXME: - Reorganize code below in atomic function.
- - Also think about what happens in case when we fail to
- install new parent frms or new version of table being
- altered.
- */
- if (opt_fk_all_engines)
- {
- for (tab= table_list->next_local; tab; tab= tab->next_local)
- {
- tab->mdl_request.ticket= tab->table->mdl_ticket;
- if (wait_while_table_is_used(thd, tab->table, HA_EXTRA_FORCE_REOPEN))
- goto err_new_table_cleanup;
- }
-
- for (tab= parent_tables; tab; tab= tab->next_local)
- {
- tab->mdl_request.ticket= tab->table->mdl_ticket;
- if (wait_while_table_is_used(thd, tab->table, HA_EXTRA_FORCE_REOPEN))
- goto err_new_table_cleanup;
- }
- }
-
close_all_tables_for_name(thd, table->s,
new_name != table_name || new_db != db);
@@ -10054,7 +10216,10 @@ view_err:
if (fk_ctx.install_new_frms())
- goto err_new_table_cleanup;
+ {
+ (void) quick_rm_table(new_db_type, new_db, tmp_name, FN_IS_TMP);
+ goto err_with_mdl;
+ }
/*
This leads to the storage engine (SE) not being notified for renames in
@@ -10163,17 +10328,18 @@ end_online:
if (thd->locked_tables_mode == LTM_LOCK_TABLES ||
thd->locked_tables_mode == LTM_PRELOCKED_UNDER_LOCK_TABLES)
{
+ downgrade_mdl_for_altered_and_parent_tables(table_list, parent_tables);
+
if ((new_name != table_name || new_db != db))
{
- thd->mdl_context.release_all_locks_for_name(mdl_ticket);
+ thd->mdl_context.release_all_locks_for_name(table_list->
+ mdl_request.ticket);
}
- else
- mdl_ticket->downgrade_exclusive_lock();
if (opt_fk_all_engines)
- alter_table_release_fk_name_locks_under_lock_tables(thd, fk_names_drop,
- fk_names_add,
- fk_names_replace);
+ alter_table_adjust_fk_name_locks_under_lock_tables(thd, fk_names_drop,
+ fk_names_add,
+ fk_names_replace);
}
end_temporary:
@@ -10247,29 +10413,39 @@ err:
err_with_mdl:
/*
- An error happened while we were holding exclusive name metadata lock
+ An error happened while we were holding exclusive metadata lock
on table being altered. To be safe under LOCK TABLES we should
remove all references to the altered table from the list of locked
tables and release the exclusive metadata lock.
+
+ Also to make cleanup simple we do the same thing for all parent tables.
+ To do that we close all parent tables which are still open by calling
+ Foreign_key_ddl_rcontext::cleanup() first.
*/
+ fk_ctx.cleanup();
+
thd->locked_tables_list.unlink_all_closed_tables();
+
if (has_target_mdl_lock)
{
thd->mdl_context.release_lock(target_mdl_request.ticket);
thd->mdl_context.remove_request(&target_mdl_request);
}
- thd->mdl_context.release_all_locks_for_name(mdl_ticket);
/*
- QQ: May be it makes sense to do something special about FK names in
- this case instead of restoring original locks?
+ Under LOCK TABLES we can't rely on close_thread_tables() releasing all
+ metadata locks so we have to do it explicitly.
*/
- if (opt_fk_all_engines &&
- (thd->locked_tables_mode == LTM_LOCK_TABLES ||
- thd->locked_tables_mode == LTM_PRELOCKED_UNDER_LOCK_TABLES))
- alter_table_restore_fk_name_locks_under_lock_tables(thd, fk_names_drop,
- fk_names_add,
- fk_names_replace);
+ if (thd->locked_tables_mode == LTM_LOCK_TABLES ||
+ thd->locked_tables_mode == LTM_PRELOCKED_UNDER_LOCK_TABLES)
+ {
+ release_mdl_for_altered_and_parent_tables(thd, table_list, parent_tables);
+
+ if (opt_fk_all_engines)
+ alter_table_release_fk_name_locks_under_lock_tables(thd, fk_names_drop,
+ fk_names_add,
+ fk_names_replace);
+ }
DBUG_RETURN(TRUE);
}
/* mysql_alter_table */
Attachment: [text/bzr-bundle] bzr/dlenev@mysql.com-20090704125255-ccaze2efarjq4jz4.bundle
| Thread |
|---|
| • bzr commit into mysql-6.1-fk branch (dlenev:2731) WL#148 | Dmitry Lenev | 4 Jul |
