Commit e4e85cd2 authored by Michael Widenius's avatar Michael Widenius

Fixed lp:925377 "Querying myisam table metadata while 'alter table..enable...

Fixed lp:925377 "Querying myisam table metadata while 'alter table..enable keys' is running may corrupt the table"
Fixed wrong mutex order bug in Aria when flush_log_for_bitmap() was called when table is not yet marked for change.

include/my_base.h:
  Added flag that table is opened only for status
mysql-test/r/myisam-big.result:
  Test case for lp:925377
mysql-test/t/myisam-big.test:
  Test case for lp:925377
sql/sql_base.cc:
  If thd->version == 0 (happens only when we are opening a table that is flushed under  MYSQL_LOCK_IGNORE_FLUSH), open the table in HA_OPEN_FOR_STATUS mode
storage/maria/ma_bitmap.c:
  Fixed wrong mutex order bug in Aria when flush_log_for_bitmap() was called when table is not yet marked for change.
storage/maria/ma_dbug.c:
  Ignore last_version <= 1 as these are either flushed or only opened for status
storage/maria/ma_open.c:
  Use last_version=1 as a marker that table was opened with HA_OPEN_FOR_STATUS.
  In this case we just open a new version of the table in read only mode.
storage/myisam/mi_create.c:
  Update prototype
storage/myisam/mi_dbug.c:
  Ignore last_version <= 1 as these are either flushed or only opened for status
storage/myisam/mi_open.c:
  Use last_version=1 as a marker that table was opened with HA_OPEN_FOR_STATUS.
  If HA_OPEN_FOR_STATUS is used, we will not assert if there is an old not-to-be-used version of the table existing.
  In this case we just open a new version of the table in read only mode.
storage/myisam/myisamdef.h:
  Updated prototype
parent d2064806
...@@ -54,6 +54,7 @@ ...@@ -54,6 +54,7 @@
/* Internal temp table, used for temporary results */ /* Internal temp table, used for temporary results */
#define HA_OPEN_INTERNAL_TABLE 512 #define HA_OPEN_INTERNAL_TABLE 512
#define HA_OPEN_MERGE_TABLE 1024 #define HA_OPEN_MERGE_TABLE 1024
#define HA_OPEN_FOR_STATUS 2048
/* The following is parameter to ha_rkey() how to use key */ /* The following is parameter to ha_rkey() how to use key */
......
drop table if exists t1,t2;
create table t1 (id int, sometext varchar(100)) engine=myisam;
insert into t1 values (1, "hello"),(2, "hello2"),(4, "hello3"),(4, "hello4");
create table t2 like t1;
insert into t1 select * from t1;
insert into t2 select * from t1;
insert into t1 select * from t1;
insert into t2 select * from t1;
insert into t1 select * from t1;
insert into t2 select * from t1;
insert into t1 select * from t1;
insert into t2 select * from t1;
insert into t1 select * from t1;
insert into t2 select * from t1;
insert into t1 select * from t1;
insert into t2 select * from t1;
insert into t1 select * from t1;
insert into t2 select * from t1;
insert into t1 select * from t1;
insert into t2 select * from t1;
insert into t1 select * from t1;
insert into t2 select * from t1;
insert into t1 select * from t1;
insert into t2 select * from t1;
insert into t1 select * from t1;
insert into t2 select * from t1;
insert into t1 select * from t1;
insert into t2 select * from t1;
insert into t1 select * from t1;
insert into t2 select * from t1;
insert into t1 select * from t1;
insert into t2 select * from t1;
insert into t1 select * from t1;
select count(*) from t1;
count(*)
131072
alter table t1 add index (id), add index(sometext), add index(sometext,id);
alter table t1 disable keys;
alter table t1 enable keys;
drop table t1,t2;
#
# Test bugs in the MyISAM code that require more space/time
--source include/big_test.inc
# Initialise
--disable_warnings
drop table if exists t1,t2;
--enable_warnings
#
# BUG#925377:
# Querying myisam table metadata while 'alter table..enable keys' is
# running may corrupt the table
#
create table t1 (id int, sometext varchar(100)) engine=myisam;
insert into t1 values (1, "hello"),(2, "hello2"),(4, "hello3"),(4, "hello4");
create table t2 like t1;
insert into t1 select * from t1;
insert into t2 select * from t1;
insert into t1 select * from t1;
insert into t2 select * from t1;
insert into t1 select * from t1;
insert into t2 select * from t1;
insert into t1 select * from t1;
insert into t2 select * from t1;
insert into t1 select * from t1;
insert into t2 select * from t1;
insert into t1 select * from t1;
insert into t2 select * from t1;
insert into t1 select * from t1;
insert into t2 select * from t1;
insert into t1 select * from t1;
insert into t2 select * from t1;
insert into t1 select * from t1;
insert into t2 select * from t1;
insert into t1 select * from t1;
insert into t2 select * from t1;
insert into t1 select * from t1;
insert into t2 select * from t1;
insert into t1 select * from t1;
insert into t2 select * from t1;
insert into t1 select * from t1;
insert into t2 select * from t1;
insert into t1 select * from t1;
insert into t2 select * from t1;
insert into t1 select * from t1;
select count(*) from t1;
connect (con2,localhost,root,,);
connection con2;
alter table t1 add index (id), add index(sometext), add index(sometext,id);
alter table t1 disable keys;
send alter table t1 enable keys;
connection default;
--sleep 1
--disable_query_log
--disable_result_log
show table status like 't1';
--enable_query_log
--enable_result_log
connection con2;
reap;
disconnect con2;
connection default;
drop table t1,t2;
...@@ -2985,7 +2985,9 @@ TABLE *open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root, ...@@ -2985,7 +2985,9 @@ TABLE *open_table(THD *thd, TABLE_LIST *table_list, MEM_ROOT *mem_root,
} }
error= open_unireg_entry(thd, table, table_list, alias, key, key_length, error= open_unireg_entry(thd, table, table_list, alias, key, key_length,
mem_root, (flags & OPEN_VIEW_NO_PARSE)); mem_root,
(flags & (OPEN_VIEW_NO_PARSE |
MYSQL_LOCK_IGNORE_FLUSH)));
if (error > 0) if (error > 0)
{ {
my_free((uchar*)table, MYF(0)); my_free((uchar*)table, MYF(0));
...@@ -4074,8 +4076,11 @@ static int open_unireg_entry(THD *thd, TABLE *entry, TABLE_LIST *table_list, ...@@ -4074,8 +4076,11 @@ static int open_unireg_entry(THD *thd, TABLE *entry, TABLE_LIST *table_list,
HA_GET_INDEX | HA_TRY_READ_ONLY), HA_GET_INDEX | HA_TRY_READ_ONLY),
READ_KEYINFO | COMPUTE_TYPES | EXTRA_RECORD | READ_KEYINFO | COMPUTE_TYPES | EXTRA_RECORD |
(flags & OPEN_VIEW_NO_PARSE), (flags & OPEN_VIEW_NO_PARSE),
thd->open_options, entry, table_list, thd->open_options |
mem_root); (thd->version == 0 &&
(flags & MYSQL_LOCK_IGNORE_FLUSH) ?
HA_OPEN_FOR_STATUS : 0),
entry, table_list, mem_root);
if (error) if (error)
goto err; goto err;
/* TODO: Don't free this */ /* TODO: Don't free this */
...@@ -4113,7 +4118,11 @@ static int open_unireg_entry(THD *thd, TABLE *entry, TABLE_LIST *table_list, ...@@ -4113,7 +4118,11 @@ static int open_unireg_entry(THD *thd, TABLE *entry, TABLE_LIST *table_list,
HA_TRY_READ_ONLY), HA_TRY_READ_ONLY),
(READ_KEYINFO | COMPUTE_TYPES | (READ_KEYINFO | COMPUTE_TYPES |
EXTRA_RECORD), EXTRA_RECORD),
thd->open_options, entry, FALSE))) thd->open_options |
(thd->version == 0 &&
(flags & MYSQL_LOCK_IGNORE_FLUSH) ?
HA_OPEN_FOR_STATUS : 0),
entry, FALSE)))
{ {
if (error == 7) // Table def changed if (error == 7) // Table def changed
{ {
......
...@@ -368,7 +368,7 @@ static inline void _ma_bitmap_mark_file_changed(MARIA_SHARE *share, ...@@ -368,7 +368,7 @@ static inline void _ma_bitmap_mark_file_changed(MARIA_SHARE *share,
if (flush_translog && share->now_transactional) if (flush_translog && share->now_transactional)
(void) translog_flush(share->state.logrec_file_id); (void) translog_flush(share->state.logrec_file_id);
_ma_mark_file_changed(share); _ma_mark_file_changed_now(share);
pthread_mutex_lock(&share->bitmap.bitmap_lock); pthread_mutex_lock(&share->bitmap.bitmap_lock);
/* purecov: end */ /* purecov: end */
} }
......
...@@ -186,7 +186,7 @@ my_bool _ma_check_table_is_closed(const char *name, const char *where) ...@@ -186,7 +186,7 @@ my_bool _ma_check_table_is_closed(const char *name, const char *where)
MARIA_SHARE *share= info->s; MARIA_SHARE *share= info->s;
if (!strcmp(share->unique_file_name.str, filename)) if (!strcmp(share->unique_file_name.str, filename))
{ {
if (share->last_version) if (share->last_version > 1)
{ {
fprintf(stderr,"Warning: Table: %s is open on %s\n", name,where); fprintf(stderr,"Warning: Table: %s is open on %s\n", name,where);
DBUG_PRINT("warning",("Table: %s is open on %s", name,where)); DBUG_PRINT("warning",("Table: %s is open on %s", name,where));
......
...@@ -62,7 +62,8 @@ MARIA_HA *_ma_test_if_reopen(const char *filename) ...@@ -62,7 +62,8 @@ MARIA_HA *_ma_test_if_reopen(const char *filename)
{ {
MARIA_HA *info=(MARIA_HA*) pos->data; MARIA_HA *info=(MARIA_HA*) pos->data;
MARIA_SHARE *share= info->s; MARIA_SHARE *share= info->s;
if (!strcmp(share->unique_file_name.str,filename) && share->last_version) if (!strcmp(share->unique_file_name.str,filename) &&
share->last_version > 1)
return info; return info;
} }
return 0; return 0;
...@@ -840,7 +841,12 @@ MARIA_HA *maria_open(const char *name, int mode, uint open_flags) ...@@ -840,7 +841,12 @@ MARIA_HA *maria_open(const char *name, int mode, uint open_flags)
share->base.key_parts=key_parts; share->base.key_parts=key_parts;
share->base.all_key_parts=key_parts+unique_key_parts; share->base.all_key_parts=key_parts+unique_key_parts;
if (!(share->last_version=share->state.version)) if (!(share->last_version=share->state.version))
share->last_version=1; /* Safety */ share->last_version= 2; /* Safety */
if (open_flags & HA_OPEN_FOR_STATUS)
{
share->last_version= 1; /* Not reusable version */
share->options|= HA_OPTION_READ_ONLY_DATA;
}
share->rec_reflength=share->base.rec_reflength; /* May be changed */ share->rec_reflength=share->base.rec_reflength; /* May be changed */
share->base.margin_key_file_length=(share->base.max_key_file_length - share->base.margin_key_file_length=(share->base.max_key_file_length -
(keys ? MARIA_INDEX_BLOCK_MARGIN * (keys ? MARIA_INDEX_BLOCK_MARGIN *
......
...@@ -643,7 +643,7 @@ int mi_create(const char *name,uint keys,MI_KEYDEF *keydefs, ...@@ -643,7 +643,7 @@ int mi_create(const char *name,uint keys,MI_KEYDEF *keydefs,
NOTE: The filename is compared against unique_file_name of every NOTE: The filename is compared against unique_file_name of every
open table. Hence we need a real path here. open table. Hence we need a real path here.
*/ */
if (test_if_reopen(filename)) if (test_if_reopen(filename, 1))
{ {
my_printf_error(0, "MyISAM table '%s' is in use " my_printf_error(0, "MyISAM table '%s' is in use "
"(most likely by a MERGE table). Try FLUSH TABLES.", "(most likely by a MERGE table). Try FLUSH TABLES.",
......
...@@ -183,7 +183,7 @@ my_bool check_table_is_closed(const char *name, const char *where) ...@@ -183,7 +183,7 @@ my_bool check_table_is_closed(const char *name, const char *where)
MYISAM_SHARE *share=info->s; MYISAM_SHARE *share=info->s;
if (!strcmp(share->unique_file_name,filename)) if (!strcmp(share->unique_file_name,filename))
{ {
if (share->last_version) if (share->last_version > 1)
{ {
fprintf(stderr,"Warning: Table: %s is open on %s\n", name,where); fprintf(stderr,"Warning: Table: %s is open on %s\n", name,where);
DBUG_PRINT("warning",("Table: %s is open on %s", name,where)); DBUG_PRINT("warning",("Table: %s is open on %s", name,where));
......
...@@ -52,7 +52,8 @@ if (pos > end_pos) \ ...@@ -52,7 +52,8 @@ if (pos > end_pos) \
** In MySQL the server will handle version issues. ** In MySQL the server will handle version issues.
******************************************************************************/ ******************************************************************************/
MI_INFO *test_if_reopen(char *filename) MI_INFO *test_if_reopen(char *filename,
my_bool ignore_last_version __attribute__((unused)))
{ {
LIST *pos; LIST *pos;
...@@ -61,8 +62,8 @@ MI_INFO *test_if_reopen(char *filename) ...@@ -61,8 +62,8 @@ MI_INFO *test_if_reopen(char *filename)
MI_INFO *info=(MI_INFO*) pos->data; MI_INFO *info=(MI_INFO*) pos->data;
MYISAM_SHARE *share=info->s; MYISAM_SHARE *share=info->s;
DBUG_ASSERT(strcmp(share->unique_file_name,filename) || DBUG_ASSERT(strcmp(share->unique_file_name,filename) ||
share->last_version); share->last_version > 1 || ignore_last_version);
if (!strcmp(share->unique_file_name,filename) && share->last_version) if (!strcmp(share->unique_file_name,filename) && share->last_version > 1)
return info; return info;
} }
return 0; return 0;
...@@ -109,7 +110,8 @@ MI_INFO *mi_open(const char *name, int mode, uint open_flags) ...@@ -109,7 +110,8 @@ MI_INFO *mi_open(const char *name, int mode, uint open_flags)
} }
pthread_mutex_lock(&THR_LOCK_myisam); pthread_mutex_lock(&THR_LOCK_myisam);
if (!(old_info=test_if_reopen(name_buff))) if (!(old_info=test_if_reopen(name_buff,
test(open_flags & HA_OPEN_FOR_STATUS))))
{ {
share= &share_buff; share= &share_buff;
bzero((uchar*) &share_buff,sizeof(share_buff)); bzero((uchar*) &share_buff,sizeof(share_buff));
...@@ -511,7 +513,12 @@ MI_INFO *mi_open(const char *name, int mode, uint open_flags) ...@@ -511,7 +513,12 @@ MI_INFO *mi_open(const char *name, int mode, uint open_flags)
share->base.key_parts=key_parts; share->base.key_parts=key_parts;
share->base.all_key_parts=key_parts+unique_key_parts; share->base.all_key_parts=key_parts+unique_key_parts;
if (!(share->last_version=share->state.version)) if (!(share->last_version=share->state.version))
share->last_version=1; /* Safety */ share->last_version= 2; /* Safety */
if (open_flags & HA_OPEN_FOR_STATUS)
{
share->last_version= 1; /* Not reusable version */
share->options|= HA_OPTION_READ_ONLY_DATA;
}
share->rec_reflength=share->base.rec_reflength; /* May be changed */ share->rec_reflength=share->base.rec_reflength; /* May be changed */
share->base.margin_key_file_length=(share->base.max_key_file_length - share->base.margin_key_file_length=(share->base.max_key_file_length -
(keys ? MI_INDEX_BLOCK_MARGIN * (keys ? MI_INDEX_BLOCK_MARGIN *
......
...@@ -726,7 +726,7 @@ my_bool mi_check_status(void *param); ...@@ -726,7 +726,7 @@ my_bool mi_check_status(void *param);
void mi_fix_status(MI_INFO *org_table, MI_INFO *new_table); void mi_fix_status(MI_INFO *org_table, MI_INFO *new_table);
void mi_disable_non_unique_index(MI_INFO *info, ha_rows rows); void mi_disable_non_unique_index(MI_INFO *info, ha_rows rows);
extern MI_INFO *test_if_reopen(char *filename); extern MI_INFO *test_if_reopen(char *filename, my_bool ignore_last_version);
my_bool check_table_is_closed(const char *name, const char *where); my_bool check_table_is_closed(const char *name, const char *where);
int mi_open_datafile(MI_INFO *info, MYISAM_SHARE *share, const char *orn_name, int mi_open_datafile(MI_INFO *info, MYISAM_SHARE *share, const char *orn_name,
File file_to_dup); File file_to_dup);
......
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