maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Michael!

On May 04, Michael Widenius wrote:
> revision-id: 9083627c50f (mariadb-10.5.2-562-g9083627c50f)
> parent(s): 105cda3b1cb
> author: Michael Widenius <michael.widenius@xxxxxxxxx>
> committer: Michael Widenius <michael.widenius@xxxxxxxxx>
> timestamp: 2021-03-25 12:06:34 +0200
> message:
> 
> Make rename atomic/repeatable in MyISAM and Aria
> 
> This is required to make Atomic RENAME TABLE work for these engines
> 
> The requirement is that if we have a server crash in the middle of a
> storage engine rename call, the upcoming ddl log recovery should be able
> to finalize it by re-execute the rename.
> 
> diff --git a/storage/maria/ma_rename.c b/storage/maria/ma_rename.c
> index a4388596f6b..64dac32a51c 100644
> --- a/storage/maria/ma_rename.c
> +++ b/storage/maria/ma_rename.c
> @@ -106,28 +106,37 @@ int maria_rename(const char *old_name, const char *new_name)
>    _ma_reset_state(info);
>    maria_close(info);
>  
> +  /*
> +    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 ?

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

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

Regards,
Sergei
VP of MariaDB Server Engineering
and security@xxxxxxxxxxx