Skip to content

MDEV-40728 Recovery wrongly fails if FILE_CREATE is followed by FILE_RENAME #5531

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dr-m wants to merge 1 commit into
base: 10.11
Choose a base branch
from
+34 −27
Open

MDEV-40728 Recovery wrongly fails if FILE_CREATE is followed by FILE_RENAME #5531

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 34 additions & 27 deletions storage/innobase/log/log0recv.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -937,6 +937,18 @@ static struct
d++;
continue;
}
else
{
recv_spaces_t::iterator it{recv_spaces.find(d->first)};
if (UNIV_UNLIKELY(it == recv_spaces.end()))
ut_ad("inconsistent data structures" == 0);
else if (it->second.create_lsn)
/*
Because a FILE_CREATE record exists, the entire file must
be recoverable via log records.
*/
goto next_item;
}
const page_id_t page_id{d->first, 0};
const byte *page= recv_sys.dblwr.find_page(page_id, max_lsn);
if (!page)
Expand Down Expand Up @@ -1322,19 +1334,14 @@ static void fil_name_process(const char *name, ulint len, uint32_t space_id,
ut_ad(p.first->first == space_id);

file_name_t& f = p.first->second;

ut_ad(!f.space || f.space->id == space_id);
auto d = deferred_spaces.find(space_id);
if (d) {
if (deleted) {
d->deleted = true;
goto got_deleted;
}
goto reload;
}

if (deleted) {
got_deleted:
/* Got FILE_DELETE */
if (d) {
d->deleted = true;
}
if (!p.second && f.status != file_name_t::DELETED) {
f.status = file_name_t::DELETED;
if (f.space != NULL) {
Expand All @@ -1344,10 +1351,9 @@ static void fil_name_process(const char *name, ulint len, uint32_t space_id,
}

ut_ad(f.space == NULL);
goto reset_create;
} else if (p.second // the first FILE_MODIFY or FILE_RENAME
f.create_lsn = 0;
} else if (d || p.second /* the first FILE_MODIFY or FILE_RENAME */
|| f.name != fname.name) {
reload:
if (f.name.size() == 0) {
/* Augment the recv_spaces.emplace_hint() for the
FILE_MODIFY record that had been added by
Expand All @@ -1361,7 +1367,8 @@ static void fil_name_process(const char *name, ulint len, uint32_t space_id,
the space_id. If not, ignore the file after displaying
a note. Abort if there are multiple files with the
same space_id. */
switch (fil_ibd_load(space_id, fname.name.c_str(), space)) {
switch (fil_load_status s
= fil_ibd_load(space_id, fname.name.c_str(), space)) {
case FIL_LOAD_OK:
ut_ad(space != NULL);

Expand Down Expand Up @@ -1396,25 +1403,28 @@ static void fil_name_process(const char *name, ulint len, uint32_t space_id,
break;

case FIL_LOAD_ID_CHANGED:
ut_ad(space == NULL);
break;

case FIL_LOAD_NOT_FOUND:
/* No matching tablespace was found; maybe it
was renamed, and we will find a subsequent
FILE_* record. */
ut_ad(space == NULL);

if (srv_operation == SRV_OPERATION_RESTORE && d
&& ftype == FILE_RENAME) {
if (f.create_lsn) {

@Thirunarayanan Thirunarayanan Aug 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merging case FIL_LOAD_ID_CHANGED into case FIL_LOAD_NOT_FOUND does more than
simplify:

The two statuses mean opposite things:
NOT_FOUND is "no file, the name is free to claim",
ID_CHANGED is "the file is there and its page 0 belongs to a different tablespace".

Let's say space id is 7 is deferred with create_lsn set, we parse FILE_RENAME 7-> t2.ibd
t2.ibd on disk has space id 9 in page 0. In this case, this patch sets deferred file name to t2.ibd (to space id 7). reinit_all() binds space 7 to t2.ibd and writes space 7 pages to space 9 silently.

Can we have separate case like we used to do before the patch ? By doing this, we don't need fil_load_status s variable at all.

In mtr_t::commit_file(), we also flush the redo log before doing file operation.

  /* Durably write the log for the file system operation. */
  log_write_and_flush();

  log_sys.latch.wr_unlock();
  m_latch_ex= false;

  char *old_name= space.chain.start->name;
  bool success= true;

  if (name)
  {
    char *new_name= mem_strdup(name);
    mysql_mutex_lock(&fil_system.mutex);
    success= os_file_rename(innodb_data_file_key, old_name, name);
    if (success)
      space.chain.start->name= new_name;
    else
      old_name= new_name;
    mysql_mutex_unlock(&fil_system.mutex);
    ut_free(old_name);
  }

@dr-m dr-m Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason why I merged the paths is that the FILE_RENAME handling needs to be executed also in case we got FIL_LOAD_ID_CHANGED. The scenario could be similar to the one in the #5530 function fil_delete_apply(). That is, the FILE_CREATE will be followed by a FILE_RENAME that would replace a file that unexpectedly exists in the data directory.

Apart from innodb_log_archive=ON recovery, this could be the case when a file has been copied to the data directory in order to prepare for ALTER TABLE…IMPORT TABLESPACE. Such copying could happen shortly before or after the server was killed. The existence of such a file would result in the following recovery failure message:

2026-08-11 13:21:35 3 [ERROR] InnoDB: Expected tablespace id 171 but found 147 in the file ./test/t1.ibd

Note: This message does not mention the newly created file at all.

If we retain this adjustment, the recovery would fail a little later with a more specific message:

2026-08-11 14:06:30 0 [ERROR] InnoDB: Cannot replay rename of tablespace 171 from './test/#sql-alter-1148a1-13-22.ibd' to './test/t1.ibd' because the target file exists

(In #5530 this failure may be prevented for innodb_log_archive=ON recovery by replaying the FILE_DELETE on the old t1.ibd file.)

@Thirunarayanan Thirunarayanan Aug 12, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

      else if (!os_file_status(new_name, &exists, &ftype) || exists)
      {
        sql_print_error("InnoDB: Cannot replay rename of tablespace " UINT32PF
                        " from '%s' to '%s'%s",
                        id, old, new_name, exists ?
                        " because the target file exists" : "");
        err= DB_TABLESPACE_EXISTS;

This error is being called in recv_rename_files(). If the tablespace exist in deferred space (d) that means fil_space_t::get(id) will throw nullptr.

    if (!space)
    {                   
      i++;              
      continue;
    }

Scenario:

Scenario:
1) FILE_MODIFY S, name N  (p.second) → fil_ibd_load (returns) NOT_FOUND; f.space stays NULL
2) FILE_CREATE S, name N  (f.create_lsn = lsn;)
3) FILE_RENAME S (file_name) M  FIL_LOAD_DEFER-> deferred_spaces.add(S, M)-> now d exists
4) FILE_RENAME S (file_name) P fil_ibd_load(S, P): ID_CHANGED and sets d->file_name = P;

@dr-m dr-m Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that I must create an ib_logfile0 corresponding to the data.tar.xz that is attached to MDEV-40728. Then we can test how recovery will actually perform on this data directory.

@dr-m dr-m Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please refer to this comment in MDEV-40728 on how to test this. If I revert the changes to this switch statement (retaining only the reset_create: removal), the file t1.ibd will be recovered incorrectly, with the old contents (tablespace ID 147, not 171). Recovery reports no error:

2026-08-12 17:49:40 0 [Note] InnoDB: Starting crash recovery from checkpoint LSN=1034042
2026-08-12 17:49:40 0 [Note] InnoDB: Ignoring data file './test/#sql-alter-1148a1-13-1f.ibd' with space ID 147. Another data file called ./test/t1.ibd exists with the same space ID.
2026-08-12 17:49:40 0 [Note] InnoDB: Ignoring data file './test/#sql-ib181.ibd' with space ID 147. Another data file called ./test/t1.ibd exists with the same space ID.
2026-08-12 17:49:40 0 [Note] InnoDB: End of log at LSN=1153119
2026-08-12 17:49:40 0 [ERROR] InnoDB: The change buffer is corrupted or has been removed on upgrade to MariaDB 11.0 or later
2026-08-12 17:49:40 0 [Note] InnoDB: To recover: 29 pages
2026-08-12 17:49:40 0 [Note] InnoDB: Resizing redo log from 4.000MiB to 8.000GiB; LSN=1153119
2026-08-12 17:49:40 0 [Warning] InnoDB: Cannot change innodb_undo_tablespaces=0 because previous shutdown was not with innodb_fast_shutdown=0
2026-08-12 17:49:40 0 [Note] InnoDB: 128 rollback segments in 3 undo tablespaces are active.
2026-08-12 17:49:40 0 [Note] InnoDB: Setting file './ibtmp1' size to 12.000MiB. Physically writing the file full; Please wait ...
2026-08-12 17:49:40 0 [Note] InnoDB: File './ibtmp1' size is now 12.000MiB.
2026-08-12 17:49:40 0 [Note] InnoDB: log sequence number 1153119 (memory-mapped); transaction id 639

If I try to access the table from SQL, an error will be reported:

2026-08-12 17:52:10 3 [ERROR] InnoDB: Expected tablespace id 171 but found 147 in the file ./test/t1.ibd
2026-08-12 17:52:10 3 [ERROR] mariadbd: Table `test`.`t1` is corrupted. Please drop the table and recreate.

This erroneous recovery would be a serious regression.

if (d && ftype == FILE_RENAME) {
rename:
d->file_name = fname.name;
f.name = fname.name;
d->file_name = fname.name;
f.name = fname.name;
}
break;
}

if (f.create_lsn) {
return;
if (ftype == FILE_CREATE) {
f.create_lsn = lsn;
break;
}

if (s == FIL_LOAD_ID_CHANGED) {
break;
}

if (srv_force_recovery
Expand All @@ -1434,11 +1444,10 @@ static void fil_name_process(const char *name, ulint len, uint32_t space_id,
int(fname.name.size()),
fname.name.data(), space_id);
}
return;
break;

case FIL_LOAD_DEFER:
if (d && ftype == FILE_RENAME
&& srv_operation == SRV_OPERATION_RESTORE) {
if (d && ftype == FILE_RENAME && f.create_lsn) {
goto rename;
}
/* Skip the deferred spaces
Expand Down Expand Up @@ -1470,8 +1479,6 @@ static void fil_name_process(const char *name, ulint len, uint32_t space_id,
" due to innodb_force_recovery",
int(len), name, space_id);
}
reset_create:
f.create_lsn = 0;
} else if (ftype == FILE_CREATE && !f.space) {
f.create_lsn = lsn;
}
Expand Down