Skip to content

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

Open
dr-m wants to merge 1 commit into
10.11from
circular-MDEV-40728
Open

MDEV-40728 Recovery wrongly fails if FILE_CREATE is followed by FILE_RENAME#5531
dr-m wants to merge 1 commit into
10.11from
circular-MDEV-40728

Conversation

@dr-m

@dr-m dr-m commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

fil_name_process(): Simplify the logic. If no matching tablespace is found but file_name_t::create_lsn had been set in response to parsing a FILE_CREATE record, try to apply FILE_RENAME to deferred_spaces.

deferred_spaces.deferred_dblwr(): Skip newly created tablespaces to avoid a bogus invocation of fil_space_free().

This is a variant of #5530 that does not depend on #4405.

@dr-m
dr-m requested a review from Thirunarayanan August 11, 2026 12:09
@dr-m dr-m self-assigned this Aug 11, 2026
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@dr-m
dr-m force-pushed the circular-MDEV-40728 branch from 7e45fd4 to 6623cd0 Compare August 12, 2026 09:00

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

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);
  }

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.)

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;

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.

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.

…RENAME

fil_name_process(): Simplify the logic. If no matching tablespace is
found but file_name_t::create_lsn had been set in response to parsing
a FILE_CREATE record, try to apply FILE_RENAME to deferred_spaces.

deferred_spaces.deferred_dblwr(): Skip newly created tablespaces
to avoid a bogus invocation of fil_space_free().
@dr-m
dr-m force-pushed the circular-MDEV-40728 branch from 6623cd0 to e5e840f Compare August 13, 2026 05:29
@dr-m
dr-m requested a review from Thirunarayanan August 13, 2026 09:31

@Thirunarayanan Thirunarayanan left a comment

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.

Proof of test case and I also written some basic test case to test my theory.
Please run RQG testing in 10.11 branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants