Re: [Maria-developers] 9083627c50f: Make rename atomic/repeatable in MyISAM and Aria

Hi, Michael! On May 04, Michael Widenius wrote:

revision-id: 9083627c50f (mariadb-10.5.2-562-g9083627c50f) parent(s): 105cda3b1cb author: Michael Widenius committer: Michael Widenius 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@mariadb.org