Commit df9ab0ff authored by Konstantin Osipov's avatar Konstantin Osipov

A pre-requisite patch for WL#5419 "LOCK_open scalability:

make tdc_refresh_version an atomic counter".

To avoid orphaned TABLE_SHARE objects left in the
cache, make sure that wherever we set table->s->version
we take care of removing all unused table share objects
from the table cache. 

Always set table->s->version under LOCK_open, to make sure
that no other connection sees an old value of the
version and adds the table to unused_tables list.

Add an assert to table_def_unuse_table() that we never
'unuse' a talbe of a share that has an old version.

With this patch, only three places are left in the code
that manipulate with table->s->version:
- tdc_remove_table(). In most cases we have an X mdl lock
in tdc_remove_table(), the two remaining cases when we
don't are 'FLUSH TABLE' and mysql_admin_table().
- sql_view.cc - a crude hack that needs a separate fix
- initial assignment from refresh_version in table.cc.
parent fdd7c28b
...@@ -439,6 +439,9 @@ static void table_def_unuse_table(TABLE *table) ...@@ -439,6 +439,9 @@ static void table_def_unuse_table(TABLE *table)
{ {
DBUG_ASSERT(table->in_use); DBUG_ASSERT(table->in_use);
/* We shouldn't put the table to 'unused' list if the share is old. */
DBUG_ASSERT(table->s->version == refresh_version);
table->in_use= 0; table->in_use= 0;
/* Remove table from the list of tables used in this share. */ /* Remove table from the list of tables used in this share. */
table->s->used_tables.remove(table); table->s->used_tables.remove(table);
...@@ -2233,11 +2236,14 @@ void drop_open_table(THD *thd, TABLE *table, const char *db_name, ...@@ -2233,11 +2236,14 @@ void drop_open_table(THD *thd, TABLE *table, const char *db_name,
DBUG_ASSERT(table == thd->open_tables); DBUG_ASSERT(table == thd->open_tables);
handlerton *table_type= table->s->db_type(); handlerton *table_type= table->s->db_type();
/* Ensure the table is removed from the cache. */
table->s->version= 0;
table->file->extra(HA_EXTRA_PREPARE_FOR_DROP); table->file->extra(HA_EXTRA_PREPARE_FOR_DROP);
close_thread_table(thd, &thd->open_tables); close_thread_table(thd, &thd->open_tables);
/* Remove the table share from the table cache. */
mysql_mutex_lock(&LOCK_open);
tdc_remove_table(thd, TDC_RT_REMOVE_ALL, db_name, table_name);
mysql_mutex_unlock(&LOCK_open);
/* Remove the table from the storage engine and rm the .frm. */
quick_rm_table(table_type, db_name, table_name, 0); quick_rm_table(table_type, db_name, table_name, 0);
} }
DBUG_VOID_RETURN; DBUG_VOID_RETURN;
...@@ -3028,17 +3034,11 @@ bool open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root, ...@@ -3028,17 +3034,11 @@ bool open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root,
my_free(table, MYF(0)); my_free(table, MYF(0));
if (error == 7) if (error == 7)
{
share->version= 0;
(void) ot_ctx->request_backoff_action(Open_table_context::OT_DISCOVER, (void) ot_ctx->request_backoff_action(Open_table_context::OT_DISCOVER,
table_list); table_list);
}
else if (share->crashed) else if (share->crashed)
{
share->version= 0;
(void) ot_ctx->request_backoff_action(Open_table_context::OT_REPAIR, (void) ot_ctx->request_backoff_action(Open_table_context::OT_REPAIR,
table_list); table_list);
}
goto err_unlock; goto err_unlock;
} }
...@@ -3824,7 +3824,7 @@ static bool auto_repair_table(THD *thd, TABLE_LIST *table_list) ...@@ -3824,7 +3824,7 @@ static bool auto_repair_table(THD *thd, TABLE_LIST *table_list)
TABLE_SHARE *share; TABLE_SHARE *share;
TABLE *entry; TABLE *entry;
int not_used; int not_used;
bool result= FALSE; bool result= TRUE;
my_hash_value_type hash_value; my_hash_value_type hash_value;
cache_key_length= create_table_def_key(thd, cache_key, table_list, 0); cache_key_length= create_table_def_key(thd, cache_key, table_list, 0);
...@@ -3839,20 +3839,19 @@ static bool auto_repair_table(THD *thd, TABLE_LIST *table_list) ...@@ -3839,20 +3839,19 @@ static bool auto_repair_table(THD *thd, TABLE_LIST *table_list)
cache_key_length, cache_key_length,
OPEN_VIEW, &not_used, OPEN_VIEW, &not_used,
hash_value))) hash_value)))
{ goto end_unlock;
mysql_mutex_unlock(&LOCK_open);
return TRUE;
}
if (share->is_view) if (share->is_view)
goto end_with_lock_open; {
release_table_share(share);
goto end_unlock;
}
if (!(entry= (TABLE*)my_malloc(sizeof(TABLE), MYF(MY_WME)))) if (!(entry= (TABLE*)my_malloc(sizeof(TABLE), MYF(MY_WME))))
{ {
result= TRUE; release_table_share(share);
goto end_with_lock_open; goto end_unlock;
} }
share->version= 0;
mysql_mutex_unlock(&LOCK_open); mysql_mutex_unlock(&LOCK_open);
if (open_table_from_share(thd, share, table_list->alias, if (open_table_from_share(thd, share, table_list->alias,
...@@ -3871,19 +3870,21 @@ static bool auto_repair_table(THD *thd, TABLE_LIST *table_list) ...@@ -3871,19 +3870,21 @@ static bool auto_repair_table(THD *thd, TABLE_LIST *table_list)
share->table_name.str); share->table_name.str);
if (entry->file) if (entry->file)
closefrm(entry, 0); closefrm(entry, 0);
result= TRUE;
} }
else else
{ {
thd->clear_error(); // Clear error message thd->clear_error(); // Clear error message
closefrm(entry, 0); closefrm(entry, 0);
result= FALSE;
} }
my_free(entry, MYF(0)); my_free(entry, MYF(0));
mysql_mutex_lock(&LOCK_open); mysql_mutex_lock(&LOCK_open);
end_with_lock_open:
release_table_share(share); release_table_share(share);
/* Remove the repaired share from the table cache. */
tdc_remove_table(thd, TDC_RT_REMOVE_ALL,
table_list->db, table_list->table_name);
end_unlock:
mysql_mutex_unlock(&LOCK_open); mysql_mutex_unlock(&LOCK_open);
return result; return result;
} }
......
...@@ -85,19 +85,6 @@ mysql_prepare_alter_table(THD *thd, TABLE *table, ...@@ -85,19 +85,6 @@ mysql_prepare_alter_table(THD *thd, TABLE *table,
HA_CREATE_INFO *create_info, HA_CREATE_INFO *create_info,
Alter_info *alter_info); Alter_info *alter_info);
#ifndef DBUG_OFF
/* Wait until we get a 'mysql_kill' signal */
static void wait_for_kill_signal(THD *thd)
{
while (thd->killed == 0)
sleep(1);
// Reset signal and continue as if nothing happend
thd->killed= THD::NOT_KILLED;
}
#endif
/** /**
@brief Helper function for explain_filename @brief Helper function for explain_filename
...@@ -4877,18 +4864,30 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables, ...@@ -4877,18 +4864,30 @@ static bool mysql_admin_table(THD* thd, TABLE_LIST* tables,
/* purecov: end */ /* purecov: end */
} }
/* Close all instances of the table to allow repair to rename files */ /*
if (lock_type == TL_WRITE && table->table->s->version) Close all instances of the table to allow MyISAM "repair"
to rename files.
@todo: This code does not close all instances of the table.
It only closes instances in other connections, but if this
connection has LOCK TABLE t1 a READ, t1 b WRITE,
both t1 instances will be kept open.
There is no need to execute this branch for InnoDB, which does
repair by recreate. There is no need to do it for OPTIMIZE,
which doesn't move files around.
Hence, this code should be moved to prepare_for_repair(),
and executed only for MyISAM engine.
*/
if (lock_type == TL_WRITE && !table->table->s->tmp_table)
{ {
if (wait_while_table_is_used(thd, table->table, if (wait_while_table_is_used(thd, table->table,
HA_EXTRA_PREPARE_FOR_RENAME)) HA_EXTRA_PREPARE_FOR_RENAME))
goto err; goto err;
DBUG_EXECUTE_IF("wait_in_mysql_admin_table",
wait_for_kill_signal(thd);
if (thd->killed)
goto err;);
/* Flush entries in the query cache involving this table. */ /* Flush entries in the query cache involving this table. */
query_cache_invalidate3(thd, table->table, 0); query_cache_invalidate3(thd, table->table, 0);
/*
XXX: hack: switch off open_for_modify to skip the
flush that is made later in the execution flow.
*/
open_for_modify= 0; open_for_modify= 0;
} }
...@@ -5148,20 +5147,21 @@ send_result_message: ...@@ -5148,20 +5147,21 @@ send_result_message:
} }
if (table->table) if (table->table)
{ {
if (fatal_error) if (table->table->s->tmp_table)
table->table->s->version=0; // Force close of table
else if (open_for_modify)
{ {
if (table->table->s->tmp_table) if (open_for_modify)
table->table->file->info(HA_STATUS_CONST); table->table->file->info(HA_STATUS_CONST);
else }
{ else if (open_for_modify || fatal_error)
TABLE_LIST *save_next_global= table->next_global; {
table->next_global= 0; mysql_mutex_lock(&LOCK_open);
close_cached_tables(thd, table, FALSE, FALSE); tdc_remove_table(thd, TDC_RT_REMOVE_UNUSED,
table->next_global= save_next_global; table->db, table->table_name);
} mysql_mutex_unlock(&LOCK_open);
/* May be something modified consequently we have to invalidate cache */ /*
May be something modified. Consequently, we have to
invalidate the query cache.
*/
query_cache_invalidate3(thd, table->table, 0); query_cache_invalidate3(thd, table->table, 0);
} }
} }
......
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