From: Dmitry Shulga
Date: November 29 2010 8:51am
Subject: Re: bzr commit into mysql-5.1-bugteam branch (Dmitry.Shulga:3511) Bug#54486
List-Archive: http://lists.mysql.com/commits/125306
Message-Id: <DB68E03D-6299-4F43-81DF-2B3ADC934899@oracle.com>
MIME-Version: 1.0 (Apple Message framework v1082)
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: quoted-printable


On 26.11.2010, at 18:01, Sergey Vojtovich wrote:

> Hi Dmitry,
>=20
> a few myisam specific questions.
>=20
> 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
>>=20
>> 3511 Dmitry Shulga	2010-11-19
>>      Fixed bug#54486 - assert in my_seek, concurrent
>>      DROP/CREATE SCHEMA, CREATE TABLE, REPAIR.
>>=20
>>      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.
>>=20
>>      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=20
>>        algorithm permits skip already deleted (in another thread)
>>        files.
>>     @ storage/myisam/mi_check.c
>>        mi_repair() was modified: set param->retry_repair=3D 0
>>        in order to don't call following failover procedure
>>        in ha_myisam::repair() in case of error in access to
>>        .TMD file.
>>=20
>>    modified:
>>      sql/sql_db.cc
>>      storage/myisam/mi_check.c
>> =3D=3D=3D 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=20
>>     }
>>     thd->pop_internal_handler();
>>   }
>> +  DBUG_ASSERT(deleted >=3D 0 ||
>> +              (deleted < 0 && thd->main_da.is_set()));
>> +
>>   if (!silent && deleted>=3D0)
>>   {
>>     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 ||
>>=20
>> =3D=3D=3D 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"
>>=20
>> #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.
>=20
>> 	got_error=3D1;
>> +	param->retry_repair=3D 0;
> Afair we agreed retry_repair must be 0 independently of reopen status.
> Because there are no more methods to failover.

>=20
>> +	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.

>=20
>> +      }
>>     }
>>   }
>>   if (got_error)
>> @@ -1774,6 +1778,7 @@ err:
>>   }
>>   share->state.changed|=3D (STATE_NOT_OPTIMIZED_KEYS | =
STATE_NOT_SORTED_PAGES |
>> 			  STATE_NOT_ANALYZED);
>> +  DBUG_ASSERT(info->dfile >=3D 0 || (info->dfile < 0 && =
!param->retry_repair));
> Why not DBUG_ASSERT(!param->retry_repair || info->dfile >=3D 0) ?
Fixed.
>=20
>>   DBUG_RETURN(got_error);
>> }
>>=20
>> @@ -2587,6 +2592,7 @@ err:
>>     share->state.header.options[0]&=3D (uchar) =
~HA_OPTION_COMPRESS_RECORD;
>>     share->pack.header_length=3D0;
>>   }
>> +  DBUG_ASSERT(info->dfile >=3D 0 || (info->dfile < 0 && =
!param->retry_repair));
>>   DBUG_RETURN(got_error);
>> }
>>=20
>> @@ -3126,6 +3132,7 @@ err:
>>     share->state.header.options[0]&=3D (uchar) =
~HA_OPTION_COMPRESS_RECORD;
>>     share->pack.header_length=3D0;
>>   }
>> +  DBUG_ASSERT(info->dfile >=3D 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 |=3D O_DATA_LOST;
>> -        param->retry_repair=3D1;
>> -        param->testflag|=3DT_RETRY_WITHOUT_QUICK;
>> +          param->retry_repair=3D1;
>> +          param->testflag|=3DT_RETRY_WITHOUT_QUICK;
> Ehm?
Just another editors's strangeness. :(
Fixed.

>> 	DBUG_RETURN(-1);
>>       }
>>       sort_param->start_recpos=3Dsort_param->pos;
>>=20
>=20
> Regards,
> Sergey
> --=20
> Sergey Vojtovich <svoj@stripped>
> MySQL AB, Software Engineer
> Izhevsk, Russia, www.mysql.com