From: Sergey Vojtovich
Date: November 26 2010 12:01pm
Subject: Re: bzr commit into mysql-5.1-bugteam branch (Dmitry.Shulga:3511)
 Bug#54486
List-Archive: http://lists.mysql.com/commits/125127
Message-Id: <20101126120105.GA27093@june>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii

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?

>  	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)?
Why tabs?

> +      }
>      }
>    }
>    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) ?

>    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?

>  	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