Hi Dmitry,
ok to push, but there are a few minor suggestions inline.
On Fri, Dec 03, 2010 at 02:08:08PM +0000, Dmitry Shulga wrote:
> #At file:///Users/shulga/projects/mysql/5.1-bugteam-bug54486/ based on
> revid:vasil.dimov@stripped
>
> 3511 Dmitry Shulga 2010-12-03
> Fixed bug#54486 - assert in my_seek, concurrent
> DROP/CREATE SCHEMA, CREATE TABLE, REPAIR.
>
> The cause of assert was concurrent execution of
> DROP DATABASE and REPAIR TABLE where first statement
> deleted table's file .TMD at the same time when
> REPAIR TABLE statement tried replace
> old file by temporary file that has just been removed.
>
> Additionally was fixed trouble when DROP TABLE try delete
> all files belong to table being dropped at the same time
> when REPAIR TABLE statement has just deleted .TMD file.
>
> Regression test isn't added because for such one need insert
> synchonization point (DEBUG_SYNC) to function my_redel() defined in
> mysys/my_redel.c. DEBUG_SYNC is defined in sql/debug_sync.h.
> So, in order to add this one we need add additional dependency,
> i.e. mysys/my_redel.c will depend on sql.debug_sync.h, that is wrong.
You can easily make tests using DBUG_EXECUTE_IF(), but I have no strong
opinion if we really need it.
> @ sql/sql_db.cc
> mysql_rm_known_files() modified: ignore possible errors
> when trying delete all table's files. Such aggressigve
> algorithm permits skip already deleted (in another thread)
> files.
Hmm... it doesn't sound up to date.
> @ storage/myisam/mi_check.c
> mi_repair() was modified: set param->retry_repair= 0
> in order to don't call following failover procedure
> in ha_myisam::repair(); set an error in diagnostic area in
> case of error in access to .TMD file.
>
> modified:
> sql/sql_db.cc
> storage/myisam/mi_check.c
> === modified file 'sql/sql_db.cc'
> --- a/sql/sql_db.cc 2010-10-19 10:27:09 +0000
> +++ b/sql/sql_db.cc 2010-12-03 14:07:56 +0000
> @@ -948,9 +948,6 @@ bool mysql_rm_db(THD *thd,char *db,bool
> remove_db_from_cache(db);
> pthread_mutex_unlock(&LOCK_open);
>
> - Drop_table_error_handler err_handler(thd->get_internal_handler());
> - thd->push_internal_handler(&err_handler);
> -
> error= -1;
> /*
> We temporarily disable the binary log while dropping the objects
> @@ -983,8 +980,11 @@ bool mysql_rm_db(THD *thd,char *db,bool
> error = 0;
> reenable_binlog(thd);
> }
> - thd->pop_internal_handler();
> }
> + DBUG_ASSERT(deleted >= 0 ||
> + (deleted < 0 && thd->main_da.is_set()) ||
> + thd->killed);
> +
It makes me think that:
- diagnostics area state is irrelevant when deleted >= 0;
- diagnostics area state is irrelevant (though it must be non-empty)
when deleted < 0;
- diagnostics area state as well as deleted state is irrelevant
when thd->killed.
It confuses me, but I can leave with it. So I can only hope that all
points above are acceptable.
> if (!silent && deleted>=0)
> {
> const char *query;
> @@ -1213,16 +1213,36 @@ static long mysql_rm_known_files(THD *th
> else
> {
> strxmov(filePath, org_path, "/", file->name, NullS);
> - if (my_delete_with_symlink(filePath,MYF(MY_WME)))
> + /*
> + We ignore ENOENT error in order to skip files that was deleted
> + by concurrently running statement like REAPIR TABLE ...
> + */
> + if (my_delete_with_symlink(filePath, MYF(0)) &&
> + my_errno != ENOENT)
> {
> - goto err;
> + my_error(EE_DELETE, MYF(0), filePath, my_errno);
> + goto err;
> }
> }
> }
> - if (thd->killed ||
> - (tot_list && mysql_rm_table_part2(thd, tot_list, 1, 0, 1, 1)))
> +
> + if (thd->killed)
> goto err;
>
> + if (tot_list)
> + {
> + int res= 0;
> + {
Why extra level of braces and indentation?
> + Drop_table_error_handler err_handler(thd->get_internal_handler());
> +
> + thd->push_internal_handler(&err_handler);
> + res= mysql_rm_table_part2(thd, tot_list, 1, 0, 1, 1);
> + thd->pop_internal_handler();
> + }
> + if (res)
> + goto err;
> + }
> +
> /* Remove RAID directories */
> {
> List_iterator<String> it(raid_dirs);
>
> === modified file 'storage/myisam/mi_check.c'
> --- a/storage/myisam/mi_check.c 2010-03-25 12:08:21 +0000
> +++ b/storage/myisam/mi_check.c 2010-12-03 14:07:56 +0000
> @@ -1741,6 +1741,8 @@ err:
> MYF(MY_REDEL_MAKE_BACKUP): MYF(0))) ||
> mi_open_datafile(info,share,name,-1))
> got_error=1;
> +
> + param->retry_repair= 0;
> }
> }
> if (got_error)
>
>
> --
> MySQL Code Commits Mailing List
> For list archives: http://lists.mysql.com/commits
> To unsubscribe: http://lists.mysql.com/commits?unsub=1
Regards,
Sergey
--
Sergey Vojtovich <svoj@stripped>
MySQL AB, Software Engineer
Izhevsk, Russia, www.mysql.com
