On 26.11.2010, at 18:01, Sergey Vojtovich wrote:
> Hi Dmitry,
>
> a few myisam specific questions.
>
> On Fri, Nov 19, 2010 at 05:43:31AM +0000, Dmitry Shulga wrote:
>> #At file:///Users/shulga/projects/mysql/5.1-bugteam-bug54486/ based on
> revid:vasil.dimov@stripped
>>
>> 3511 Dmitry Shulga 2010-11-19
>> 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.
>> @ 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.
>> @ 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() 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-11-19 05:43:28 +0000
>> @@ -985,6 +985,9 @@ bool mysql_rm_db(THD *thd,char *db,bool
>> }
>> thd->pop_internal_handler();
>> }
>> + DBUG_ASSERT(deleted >= 0 ||
>> + (deleted < 0 && thd->main_da.is_set()));
>> +
>> if (!silent && deleted>=0)
>> {
>> const char *query;
>> @@ -1213,10 +1216,7 @@ 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)))
>> - {
>> - goto err;
>> - }
>> + my_delete_with_symlink(filePath, MYF(MY_WME));
>> }
>> }
>> if (thd->killed ||
>>
>> === 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-11-19 05:43:28 +0000
>> @@ -51,6 +51,7 @@
>> #include <sys/mman.h>
>> #endif
>> #include "rt_index.h"
>> +#include "mysqld_error.h"
>>
>> #ifndef USE_RAID
>> #define my_raid_create(A,B,C,D,E,F,G) my_create(A,B,C,G)
>> @@ -1739,8 +1740,11 @@ err:
>> DATA_TMP_EXT, share->base.raid_chunks,
>> (param->testflag & T_BACKUP_DATA ?
>> MYF(MY_REDEL_MAKE_BACKUP): MYF(0))) ||
>> - mi_open_datafile(info,share,name,-1))
>> + mi_open_datafile(info,share,name,-1)) {
> Why open brace on the same line?
Fixed.
>
>> got_error=1;
>> + param->retry_repair= 0;
> Afair we agreed retry_repair must be 0 independently of reopen status.
> Because there are no more methods to failover.
>
>> + my_error(ER_CANT_DELETE_FILE, MYF(0), share->data_file_name, errno);
> Why my_error(ER_CANT_DELETE_FILE)?
I can't find more suitable constant for this case. I'll change it if you suggest me
another constant.
> Why tabs?
It's just my text editor's strangeness. Fixed.
>
>> + }
>> }
>> }
>> if (got_error)
>> @@ -1774,6 +1778,7 @@ err:
>> }
>> share->state.changed|= (STATE_NOT_OPTIMIZED_KEYS | STATE_NOT_SORTED_PAGES |
>> STATE_NOT_ANALYZED);
>> + DBUG_ASSERT(info->dfile >= 0 || (info->dfile < 0 &&
> !param->retry_repair));
> Why not DBUG_ASSERT(!param->retry_repair || info->dfile >= 0) ?
Fixed.
>
>> DBUG_RETURN(got_error);
>> }
>>
>> @@ -2587,6 +2592,7 @@ err:
>> share->state.header.options[0]&= (uchar) ~HA_OPTION_COMPRESS_RECORD;
>> share->pack.header_length=0;
>> }
>> + DBUG_ASSERT(info->dfile >= 0 || (info->dfile < 0 &&
> !param->retry_repair));
>> DBUG_RETURN(got_error);
>> }
>>
>> @@ -3126,6 +3132,7 @@ err:
>> share->state.header.options[0]&= (uchar) ~HA_OPTION_COMPRESS_RECORD;
>> share->pack.header_length=0;
>> }
>> + DBUG_ASSERT(info->dfile >= 0 || (info->dfile < 0 &&
> !param->retry_repair));
>> DBUG_RETURN(got_error);
>> #endif /* THREAD */
>> }
>> @@ -3268,8 +3275,8 @@ static int sort_get_next_record(MI_SORT_
>> {
>> if (sort_param->read_cache.error)
>> param->out_flag |= O_DATA_LOST;
>> - param->retry_repair=1;
>> - param->testflag|=T_RETRY_WITHOUT_QUICK;
>> + param->retry_repair=1;
>> + param->testflag|=T_RETRY_WITHOUT_QUICK;
> Ehm?
Just another editors's strangeness. :(
Fixed.
>> DBUG_RETURN(-1);
>> }
>> sort_param->start_recpos=sort_param->pos;
>>
>
> Regards,
> Sergey
> --
> Sergey Vojtovich <svoj@stripped>
> MySQL AB, Software Engineer
> Izhevsk, Russia, www.mysql.com
