← Back to team overview

maria-developers team mailing list archive

Re: 9083627c50f: Make rename atomic/repeatable in MyISAM and Aria

 

Hi!

> > +  /*
> > +    This code is written so that it should be possible to re-run a
> > +    failed rename (even if there is a server crash in between the
> > +    renames) and complete it.
> > +  */
> >    fn_format(from,old_name,"",MARIA_NAME_IEXT,MY_UNPACK_FILENAME|MY_APPEND_EXT);
> >    fn_format(to,new_name,"",MARIA_NAME_IEXT,MY_UNPACK_FILENAME|MY_APPEND_EXT);
> >    if (mysql_file_rename_with_symlink(key_file_kfile, from, to,
> >                                       MYF(MY_WME | sync_dir)))
> > -    DBUG_RETURN(my_errno);
> > +    index_file_rename_error= my_errno;
>
> Shouldn't you continue only if index_file_rename_error == ENOENT ?

I thought about that, but in the end thought that there in almost all
real world cases
not be any harm in trying to rename the other file too.  If there
would be some kind of privilege error, we would probably get it for
the other file too.

> also, I only very quickly looked at mysql_file_rename_with_symlink,
> but it didn't look atomic to me when symlinks are present.

Yes, very likely. I did however not had time to look at symlinks as I
with the big hard disks of today
I don't think many are using symlinks.  Should be checked at some point.

> >    fn_format(from,old_name,"",MARIA_NAME_DEXT,MY_UNPACK_FILENAME|MY_APPEND_EXT);
> >    fn_format(to,new_name,"",MARIA_NAME_DEXT,MY_UNPACK_FILENAME|MY_APPEND_EXT);
> ...
> > diff --git a/storage/myisam/mi_rename.c b/storage/myisam/mi_rename.c
> > index 19df2e54213..13cf8c60897 100644
> > --- a/storage/myisam/mi_rename.c
> > +++ b/storage/myisam/mi_rename.c
> > @@ -22,6 +22,7 @@
> >  int mi_rename(const char *old_name, const char *new_name)
> >  {
> >    char from[FN_REFLEN],to[FN_REFLEN];
> > +  int save_errno= 0;
> >    DBUG_ENTER("mi_rename");
> >
> >  #ifdef EXTRA_DEBUG
> > @@ -32,10 +33,13 @@ int mi_rename(const char *old_name, const char *new_name)
> >    fn_format(from,old_name,"",MI_NAME_IEXT,MY_UNPACK_FILENAME|MY_APPEND_EXT);
> >    fn_format(to,new_name,"",MI_NAME_IEXT,MY_UNPACK_FILENAME|MY_APPEND_EXT);
> >    if (mysql_file_rename_with_symlink(mi_key_file_kfile, from, to, MYF(MY_WME)))
> > -    DBUG_RETURN(my_errno);
> > +    save_errno= my_errno;
> >    fn_format(from,old_name,"",MI_NAME_DEXT,MY_UNPACK_FILENAME|MY_APPEND_EXT);
> >    fn_format(to,new_name,"",MI_NAME_DEXT,MY_UNPACK_FILENAME|MY_APPEND_EXT);
> > -  DBUG_RETURN(mysql_file_rename_with_symlink(mi_key_file_dfile,
> > -                                             from, to,
> > -                                             MYF(MY_WME)) ? my_errno : 0);
> > +  if (mysql_file_rename_with_symlink(mi_key_file_dfile,
> > +                                     from, to,
> > +                                     MYF(MY_WME)))
> > +    if (save_errno)
> > +      save_errno= my_errno;
> > +  DBUG_RETURN(save_errno);
>
> Why mi_rename isn't trying to undo a half-failed rename like ma_rename does?

Older code and also a very unlikely case.

When thinking about it, the code in Aria is probably wrong and should be fixed.
We have already written a log entry that we are going to rename the tables.
If we revert the table name change, the aria log entry will on
recovery force the table to be
renamed anyway, which is not good. Should be fixed at some point.

Regards,
Monty


References