Commit b1485a47 authored by Michael Widenius's avatar Michael Widenius

More fixes for LOCK TABLE and REPAIR/FLUSH

Changed HA_EXTRA_NORMAL to HA_EXTRA_NOT_USED (more clean)

mysql-test/suite/maria/lock.result:
  More extensive tests of LOCK TABLE with FLUSH and REPAIR
mysql-test/suite/maria/lock.test:
  More extensive tests of LOCK TABLE with FLUSH and REPAIR
sql/sql_admin.cc:
  Fix that REPAIR TABLE ... USE_FRM works with LOCK TABLES
sql/sql_base.cc:
  Ensure that transactions are closed in ARIA when doing flush
  HA_EXTRA_NORMAL -> HA_EXTRA_NOT_USED
  Don't call extra many times for a table in close_all_tables_for_name()
  Added test if table_list->table as this can happen in error situations
sql/sql_partition.cc:
  HA_EXTRA_NORMAL -> HA_EXTRA_NOT_USED
sql/sql_reload.cc:
  Fixed comment
sql/sql_table.cc:
  HA_EXTRA_NORMAL -> HA_EXTRA_NOT_USED
sql/sql_trigger.cc:
  HA_EXTRA_NORMAL -> HA_EXTRA_NOT_USED
sql/sql_truncate.cc:
  HA_EXTRA_FORCE_REOPEN -> HA_EXTRA_PREPARE_FOR_DROP for truncate, as this speeds up truncate by not having to flush the cache to disk.
parent 26cc22f3
...@@ -8,3 +8,22 @@ LOCK TABLE t1 WRITE, t2 WRITE; ...@@ -8,3 +8,22 @@ LOCK TABLE t1 WRITE, t2 WRITE;
DROP TABLE t1; DROP TABLE t1;
UNLOCK TABLES; UNLOCK TABLES;
DROP TABLE t2; DROP TABLE t2;
CREATE TABLE t1 (i INT) ENGINE=Aria;
CREATE TABLE t2 (i INT) ENGINE=Aria;
LOCK TABLE t1 WRITE, t2 WRITE;
FLUSH TABLE t1;
select * from t1;
i
unlock tables;
drop table t1,t2;
CREATE TABLE t1 (i INT) ENGINE=Aria;
CREATE TABLE t2 (i INT) ENGINE=Aria;
LOCK TABLE t1 WRITE, t2 WRITE;
repair table t1 use_frm;
Table Op Msg_type Msg_text
test.t1 repair status OK
select * from t1;
i
drop table t2;
unlock tables;
drop table t1;
...@@ -12,6 +12,8 @@ drop table if exists t1,t2; ...@@ -12,6 +12,8 @@ drop table if exists t1,t2;
# under LOCK # under LOCK
# #
# Test DROP TABLE
CREATE TABLE t1 (i INT) ENGINE=Aria; CREATE TABLE t1 (i INT) ENGINE=Aria;
CREATE TABLE t2 (i INT) ENGINE=Aria; CREATE TABLE t2 (i INT) ENGINE=Aria;
LOCK TABLE t1 WRITE, t2 WRITE; LOCK TABLE t1 WRITE, t2 WRITE;
...@@ -19,3 +21,24 @@ LOCK TABLE t1 WRITE, t2 WRITE; ...@@ -19,3 +21,24 @@ LOCK TABLE t1 WRITE, t2 WRITE;
DROP TABLE t1; DROP TABLE t1;
UNLOCK TABLES; UNLOCK TABLES;
DROP TABLE t2; DROP TABLE t2;
#Test FLUSH TABLE
CREATE TABLE t1 (i INT) ENGINE=Aria;
CREATE TABLE t2 (i INT) ENGINE=Aria;
LOCK TABLE t1 WRITE, t2 WRITE;
FLUSH TABLE t1;
select * from t1;
unlock tables;
drop table t1,t2;
# Test REPAIR ... USE_FRM and unlock tables last
CREATE TABLE t1 (i INT) ENGINE=Aria;
CREATE TABLE t2 (i INT) ENGINE=Aria;
LOCK TABLE t1 WRITE, t2 WRITE;
repair table t1 use_frm;
select * from t1;
drop table t2;
unlock tables;
drop table t1;
...@@ -80,6 +80,7 @@ static int prepare_for_repair(THD *thd, TABLE_LIST *table_list, ...@@ -80,6 +80,7 @@ static int prepare_for_repair(THD *thd, TABLE_LIST *table_list,
{ {
int error= 0; int error= 0;
TABLE tmp_table, *table; TABLE tmp_table, *table;
TABLE_LIST *pos_in_locked_tables= 0;
TABLE_SHARE *share; TABLE_SHARE *share;
bool has_mdl_lock= FALSE; bool has_mdl_lock= FALSE;
char from[FN_REFLEN],tmp[FN_REFLEN+32]; char from[FN_REFLEN],tmp[FN_REFLEN+32];
...@@ -194,9 +195,13 @@ static int prepare_for_repair(THD *thd, TABLE_LIST *table_list, ...@@ -194,9 +195,13 @@ static int prepare_for_repair(THD *thd, TABLE_LIST *table_list,
Table was successfully open in mysql_admin_table(). Now we need Table was successfully open in mysql_admin_table(). Now we need
to close it, but leave it protected by exclusive metadata lock. to close it, but leave it protected by exclusive metadata lock.
*/ */
if (wait_while_table_is_used(thd, table, HA_EXTRA_FORCE_REOPEN)) pos_in_locked_tables= table->pos_in_locked_tables;
if (wait_while_table_is_used(thd, table,
HA_EXTRA_PREPARE_FOR_FORCED_CLOSE))
goto end; goto end;
close_all_tables_for_name(thd, table_list->table->s, HA_EXTRA_NORMAL); /* Close table but don't remove from locked list */
close_all_tables_for_name(thd, table_list->table->s,
HA_EXTRA_NOT_USED);
table_list->table= 0; table_list->table= 0;
} }
/* /*
...@@ -230,18 +235,25 @@ static int prepare_for_repair(THD *thd, TABLE_LIST *table_list, ...@@ -230,18 +235,25 @@ static int prepare_for_repair(THD *thd, TABLE_LIST *table_list,
goto end; goto end;
} }
if (thd->locked_tables_list.reopen_tables(thd)) if (thd->locked_tables_list.locked_tables())
goto end;
/*
Now we should be able to open the partially repaired table
to finish the repair in the handler later on.
*/
if (open_table(thd, table_list, thd->mem_root, &ot_ctx))
{ {
error= send_check_errmsg(thd, table_list, "repair", if (thd->locked_tables_list.reopen_tables(thd))
"Failed to open partially repaired table"); goto end;
goto end; /* Restore the table in the table list with the new opened table */
table_list->table= pos_in_locked_tables->table;
}
else
{
/*
Now we should be able to open the partially repaired table
to finish the repair in the handler later on.
*/
if (open_table(thd, table_list, thd->mem_root, &ot_ctx))
{
error= send_check_errmsg(thd, table_list, "repair",
"Failed to open partially repaired table");
goto end;
}
} }
end: end:
......
...@@ -1097,7 +1097,7 @@ bool close_cached_tables(THD *thd, TABLE_LIST *tables, ...@@ -1097,7 +1097,7 @@ bool close_cached_tables(THD *thd, TABLE_LIST *tables,
TABLE_LIST *tables_to_reopen= (tables ? tables : TABLE_LIST *tables_to_reopen= (tables ? tables :
thd->locked_tables_list.locked_tables()); thd->locked_tables_list.locked_tables());
/* Close open HANLER instances to avoid self-deadlock. */ /* Close open HANDLER instances to avoid self-deadlock. */
mysql_ha_flush_tables(thd, tables_to_reopen); mysql_ha_flush_tables(thd, tables_to_reopen);
for (TABLE_LIST *table_list= tables_to_reopen; table_list; for (TABLE_LIST *table_list= tables_to_reopen; table_list;
...@@ -1111,12 +1111,13 @@ bool close_cached_tables(THD *thd, TABLE_LIST *tables, ...@@ -1111,12 +1111,13 @@ bool close_cached_tables(THD *thd, TABLE_LIST *tables,
if (! table) if (! table)
continue; continue;
if (wait_while_table_is_used(thd, table, HA_EXTRA_FORCE_REOPEN)) if (wait_while_table_is_used(thd, table,
HA_EXTRA_PREPARE_FOR_FORCED_CLOSE))
{ {
result= TRUE; result= TRUE;
goto err_with_reopen; goto err_with_reopen;
} }
close_all_tables_for_name(thd, table->s, HA_EXTRA_NORMAL); close_all_tables_for_name(thd, table->s, HA_EXTRA_NOT_USED);
} }
} }
...@@ -1382,9 +1383,11 @@ static void close_open_tables(THD *thd) ...@@ -1382,9 +1383,11 @@ static void close_open_tables(THD *thd)
@param[in] share table share, but is just a handy way to @param[in] share table share, but is just a handy way to
access the table cache key access the table cache key
@param[in] remove_from_locked_tables @param[in] extra
TRUE if the table is being dropped or renamed. HA_EXTRA_PREPRE_FOR_DROP if the table is being dropped
In that case the documented behaviour is to HA_EXTRA_PREPARE_FOR_REANME if the table is being renamed
HA_EXTRA_NOT_USED no drop/rename
In case of drop/reanme the documented behaviour is to
implicitly remove the table from LOCK TABLES implicitly remove the table from LOCK TABLES
list. list.
...@@ -1412,10 +1415,13 @@ close_all_tables_for_name(THD *thd, TABLE_SHARE *share, ...@@ -1412,10 +1415,13 @@ close_all_tables_for_name(THD *thd, TABLE_SHARE *share,
{ {
thd->locked_tables_list.unlink_from_list(thd, thd->locked_tables_list.unlink_from_list(thd,
table->pos_in_locked_tables, table->pos_in_locked_tables,
extra != HA_EXTRA_NORMAL); extra != HA_EXTRA_NOT_USED);
/* Inform handler that there is a drop table or rename going on */ /* Inform handler that there is a drop table or a rename going on */
if (extra != HA_EXTRA_NORMAL && table->db_stat) if (extra != HA_EXTRA_NOT_USED && table->db_stat)
{
table->file->extra(extra); table->file->extra(extra);
extra= HA_EXTRA_NOT_USED; // Call extra once!
}
/* /*
Does nothing if the table is not locked. Does nothing if the table is not locked.
...@@ -2312,7 +2318,7 @@ bool rename_temporary_table(THD* thd, TABLE *table, const char *db, ...@@ -2312,7 +2318,7 @@ bool rename_temporary_table(THD* thd, TABLE *table, const char *db,
@param function HA_EXTRA_PREPARE_FOR_DROP if table is to be deleted @param function HA_EXTRA_PREPARE_FOR_DROP if table is to be deleted
HA_EXTRA_FORCE_REOPEN if table is not be used HA_EXTRA_FORCE_REOPEN if table is not be used
HA_EXTRA_PREPARE_FOR_RENAME if table is to be renamed HA_EXTRA_PREPARE_FOR_RENAME if table is to be renamed
HA_EXTRA_NORMAL Don't call extra() HA_EXTRA_NOT_USED Don't call extra()
@note When returning, the table will be unusable for other threads @note When returning, the table will be unusable for other threads
until metadata lock is downgraded. until metadata lock is downgraded.
...@@ -2337,7 +2343,7 @@ bool wait_while_table_is_used(THD *thd, TABLE *table, ...@@ -2337,7 +2343,7 @@ bool wait_while_table_is_used(THD *thd, TABLE *table,
table->s->db.str, table->s->table_name.str, table->s->db.str, table->s->table_name.str,
FALSE); FALSE);
/* extra() call must come only after all instances above are closed */ /* extra() call must come only after all instances above are closed */
if (function != HA_EXTRA_NORMAL) if (function != HA_EXTRA_NOT_USED)
(void) table->file->extra(function); (void) table->file->extra(function);
DBUG_RETURN(FALSE); DBUG_RETURN(FALSE);
} }
...@@ -3386,7 +3392,6 @@ Locked_tables_list::init_locked_tables(THD *thd) ...@@ -3386,7 +3392,6 @@ Locked_tables_list::init_locked_tables(THD *thd)
void void
Locked_tables_list::unlock_locked_tables(THD *thd) Locked_tables_list::unlock_locked_tables(THD *thd)
{ {
if (thd) if (thd)
{ {
...@@ -3408,7 +3413,8 @@ Locked_tables_list::unlock_locked_tables(THD *thd) ...@@ -3408,7 +3413,8 @@ Locked_tables_list::unlock_locked_tables(THD *thd)
Clear the position in the list, the TABLE object will be Clear the position in the list, the TABLE object will be
returned to the table cache. returned to the table cache.
*/ */
table_list->table->pos_in_locked_tables= NULL; if (table_list->table) // If not closed
table_list->table->pos_in_locked_tables= NULL;
} }
thd->leave_locked_tables_mode(); thd->leave_locked_tables_mode();
......
...@@ -6270,7 +6270,7 @@ static void alter_partition_lock_handling(ALTER_PARTITION_PARAM_TYPE *lpt) ...@@ -6270,7 +6270,7 @@ static void alter_partition_lock_handling(ALTER_PARTITION_PARAM_TYPE *lpt)
THD *thd= lpt->thd; THD *thd= lpt->thd;
if (lpt->old_table) if (lpt->old_table)
close_all_tables_for_name(thd, lpt->old_table->s, HA_EXTRA_NORMAL); close_all_tables_for_name(thd, lpt->old_table->s, HA_EXTRA_NOT_USED);
if (lpt->table) if (lpt->table)
{ {
/* /*
...@@ -6307,7 +6307,7 @@ static int alter_close_tables(ALTER_PARTITION_PARAM_TYPE *lpt, bool close_old) ...@@ -6307,7 +6307,7 @@ static int alter_close_tables(ALTER_PARTITION_PARAM_TYPE *lpt, bool close_old)
} }
if (close_old && lpt->old_table) if (close_old && lpt->old_table)
{ {
close_all_tables_for_name(lpt->thd, lpt->old_table->s, HA_EXTRA_NORMAL); close_all_tables_for_name(lpt->thd, lpt->old_table->s, HA_EXTRA_NOT_USED);
lpt->old_table= 0; lpt->old_table= 0;
} }
DBUG_RETURN(0); DBUG_RETURN(0);
...@@ -6640,7 +6640,7 @@ uint fast_alter_partition_table(THD *thd, TABLE *table, ...@@ -6640,7 +6640,7 @@ uint fast_alter_partition_table(THD *thd, TABLE *table,
mysql_write_frm(lpt, WFRM_WRITE_SHADOW) || mysql_write_frm(lpt, WFRM_WRITE_SHADOW) ||
ERROR_INJECT_CRASH("crash_drop_partition_2") || ERROR_INJECT_CRASH("crash_drop_partition_2") ||
ERROR_INJECT_ERROR("fail_drop_partition_2") || ERROR_INJECT_ERROR("fail_drop_partition_2") ||
wait_while_table_is_used(thd, table, HA_EXTRA_NORMAL) || wait_while_table_is_used(thd, table, HA_EXTRA_NOT_USED) ||
ERROR_INJECT_CRASH("crash_drop_partition_3") || ERROR_INJECT_CRASH("crash_drop_partition_3") ||
ERROR_INJECT_ERROR("fail_drop_partition_3") || ERROR_INJECT_ERROR("fail_drop_partition_3") ||
(close_table_on_failure= TRUE, FALSE) || (close_table_on_failure= TRUE, FALSE) ||
...@@ -6714,7 +6714,7 @@ uint fast_alter_partition_table(THD *thd, TABLE *table, ...@@ -6714,7 +6714,7 @@ uint fast_alter_partition_table(THD *thd, TABLE *table,
mysql_write_frm(lpt, WFRM_WRITE_SHADOW) || mysql_write_frm(lpt, WFRM_WRITE_SHADOW) ||
ERROR_INJECT_CRASH("crash_add_partition_2") || ERROR_INJECT_CRASH("crash_add_partition_2") ||
ERROR_INJECT_ERROR("fail_add_partition_2") || ERROR_INJECT_ERROR("fail_add_partition_2") ||
wait_while_table_is_used(thd, table, HA_EXTRA_NORMAL) || wait_while_table_is_used(thd, table, HA_EXTRA_NOT_USED) ||
ERROR_INJECT_CRASH("crash_add_partition_3") || ERROR_INJECT_CRASH("crash_add_partition_3") ||
ERROR_INJECT_ERROR("fail_add_partition_3") || ERROR_INJECT_ERROR("fail_add_partition_3") ||
(close_table_on_failure= TRUE, FALSE) || (close_table_on_failure= TRUE, FALSE) ||
...@@ -6820,7 +6820,7 @@ uint fast_alter_partition_table(THD *thd, TABLE *table, ...@@ -6820,7 +6820,7 @@ uint fast_alter_partition_table(THD *thd, TABLE *table,
mysql_change_partitions(lpt) || mysql_change_partitions(lpt) ||
ERROR_INJECT_CRASH("crash_change_partition_4") || ERROR_INJECT_CRASH("crash_change_partition_4") ||
ERROR_INJECT_ERROR("fail_change_partition_4") || ERROR_INJECT_ERROR("fail_change_partition_4") ||
wait_while_table_is_used(thd, table, HA_EXTRA_NORMAL) || wait_while_table_is_used(thd, table, HA_EXTRA_NOT_USED) ||
ERROR_INJECT_CRASH("crash_change_partition_5") || ERROR_INJECT_CRASH("crash_change_partition_5") ||
ERROR_INJECT_ERROR("fail_change_partition_5") || ERROR_INJECT_ERROR("fail_change_partition_5") ||
write_log_final_change_partition(lpt) || write_log_final_change_partition(lpt) ||
......
...@@ -243,9 +243,9 @@ bool reload_acl_and_cache(THD *thd, unsigned long options, ...@@ -243,9 +243,9 @@ bool reload_acl_and_cache(THD *thd, unsigned long options,
{ {
/* /*
It is not safe to upgrade the metadata lock without GLOBAL IX lock. It is not safe to upgrade the metadata lock without GLOBAL IX lock.
This can happen with FLUSH TABLES <list> WITH READ LOCK as we in these This can happen with FLUSH TABLES <list> WITH READ LOCK as we in
cases don't take a GLOBAL IX lock in order to be compatible with these cases don't take a GLOBAL IX lock in order to be compatible
global read lock. with global read lock.
*/ */
if (thd->open_tables && if (thd->open_tables &&
!thd->mdl_context.is_lock_owner(MDL_key::GLOBAL, "", "", !thd->mdl_context.is_lock_owner(MDL_key::GLOBAL, "", "",
......
...@@ -6894,7 +6894,7 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name, ...@@ -6894,7 +6894,7 @@ bool mysql_alter_table(THD *thd,char *new_db, char *new_name,
close_all_tables_for_name(thd, table->s, close_all_tables_for_name(thd, table->s,
new_name != table_name || new_db != db ? new_name != table_name || new_db != db ?
HA_EXTRA_PREPARE_FOR_RENAME : HA_EXTRA_PREPARE_FOR_RENAME :
HA_EXTRA_NORMAL); HA_EXTRA_NOT_USED);
error=0; error=0;
table_list->table= table= 0; /* Safety */ table_list->table= table= 0; /* Safety */
......
...@@ -561,7 +561,7 @@ bool mysql_create_or_drop_trigger(THD *thd, TABLE_LIST *tables, bool create) ...@@ -561,7 +561,7 @@ bool mysql_create_or_drop_trigger(THD *thd, TABLE_LIST *tables, bool create)
if (result) if (result)
goto end; goto end;
close_all_tables_for_name(thd, table->s, HA_EXTRA_NORMAL); close_all_tables_for_name(thd, table->s, HA_EXTRA_NOT_USED);
/* /*
Reopen the table if we were under LOCK TABLES. Reopen the table if we were under LOCK TABLES.
Ignore the return value for now. It's better to Ignore the return value for now. It's better to
......
...@@ -358,12 +358,12 @@ bool Truncate_statement::lock_table(THD *thd, TABLE_LIST *table_ref, ...@@ -358,12 +358,12 @@ bool Truncate_statement::lock_table(THD *thd, TABLE_LIST *table_ref,
{ {
DEBUG_SYNC(thd, "upgrade_lock_for_truncate"); DEBUG_SYNC(thd, "upgrade_lock_for_truncate");
/* To remove the table from the cache we need an exclusive lock. */ /* To remove the table from the cache we need an exclusive lock. */
if (wait_while_table_is_used(thd, table, HA_EXTRA_FORCE_REOPEN)) if (wait_while_table_is_used(thd, table, HA_EXTRA_PREPARE_FOR_DROP))
DBUG_RETURN(TRUE); DBUG_RETURN(TRUE);
m_ticket_downgrade= table->mdl_ticket; m_ticket_downgrade= table->mdl_ticket;
/* Close if table is going to be recreated. */ /* Close if table is going to be recreated. */
if (*hton_can_recreate) if (*hton_can_recreate)
close_all_tables_for_name(thd, table->s, HA_EXTRA_PREPARE_FOR_DROP); close_all_tables_for_name(thd, table->s, HA_EXTRA_NOT_USED);
} }
else else
{ {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment