From b2c4e675049e23ce8c7346985f40840fcd4e583f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Mon, 24 Aug 2026 10:44:48 +0300 Subject: [PATCH] MDEV-40728 Recovery wrongly fails if FILE_CREATE is followed by FILE_RENAME deferred_spaces.deferred_dblwr(): Skip newly created tablespaces to avoid a bogus invocation of fil_space_free(). 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. log_parse_file(): Parse each FILE_ record only once. In multi-batch recovery, there may be redundant calls that would break the logic of fil_name_process(). fil_delete_apply(): Wrappers for fil_space_free(). When recovering a log in innodb_log_archive=ON format, we must apply FILE_DELETE records in order to avoid a future clash with FILE_CREATE or FILE_RENAME. --- storage/innobase/log/log0recv.cc | 121 +++++++++++++++++++++++-------- 1 file changed, 92 insertions(+), 29 deletions(-) diff --git a/storage/innobase/log/log0recv.cc b/storage/innobase/log/log0recv.cc index 92a47bceb8c..736c226cac4 100644 --- a/storage/innobase/log/log0recv.cc +++ b/storage/innobase/log/log0recv.cc @@ -929,6 +929,18 @@ processed: 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) @@ -1208,6 +1220,50 @@ inline size_t recv_sys_t::files_size() return files.size(); } +/** Apply a FILE_DELETE record. */ +ATTRIBUTE_COLD static void fil_delete_apply(fil_space_t *space) noexcept +{ + if (log_sys.archive) + { + /* + In recovery with innodb_log_archive=ON there may be a situation like + the following: + + FILE_RENAME(123, t1.ibd, #sql-ib123.ibd) + FILE_CREATE(124, #sql-alter-.ibd) + FILE_RENAME(124, #sql-alter-.ibd, t1.ibd) + FILE_DELETE(123, #sql-ib123.ibd) + + In such a scenario, we must delete the old file in order to + avoid a name clash in recv_rename_files(). + + In normal crash recovery, at most one file operation can be + pending, and it will be handled by rollback or purge. + */ + if (FSP_FLAGS_HAS_DATA_DIR(space->flags)) + RemoteDatafile::delete_link_file(space->name()); + os_file_delete(innodb_data_file_key, space->chain.start->name); + } + fil_space_free(space->id, false); +} + +/** Apply a FILE_DELETE record to a tablespace that has not been loaded yet. */ +ATTRIBUTE_COLD static void fil_delete_apply(uint32_t id, const char *name) + noexcept +{ + if (log_sys.archive) + { + fil_space_t *space{nullptr}; + if (fil_ibd_load(id, name, space) == FIL_LOAD_OK) + { + ut_ad(space); + fil_delete_apply(space); + return; + } + ut_ad(!space); + } +} + /** Process a file name from a FILE_* record. @param[in] name file name @param[in] len length of the file name @@ -1234,32 +1290,32 @@ 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; - } + /* deferred_spaces must be a proper subset of recv_spaces. + Both here and in recv_validate_tablespace(), deferred_spaces.add() + may only be invoked for entries that exist in recv_spaces. */ + ut_ad(!d || !p.second); if (deleted) { -got_deleted: /* Got FILE_DELETE */ - if (!p.second && f.status != file_name_t::DELETED) { + if (d) { + d->deleted = true; + } + if (p.second) { + fil_delete_apply(space_id, f.name.c_str()); + } else if (f.status != file_name_t::DELETED) { f.status = file_name_t::DELETED; if (f.space != NULL) { - fil_space_free(space_id, false); + fil_delete_apply(f.space); f.space = NULL; } } 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 @@ -1273,7 +1329,8 @@ reload: 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); @@ -1308,25 +1365,28 @@ same_space: 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) { + 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 @@ -1346,11 +1406,10 @@ rename: 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 @@ -1382,8 +1441,6 @@ rename: " 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; } @@ -3187,6 +3244,12 @@ static recv_sys_t::parse_mtr_result log_parse_file(const page_id_t id, bool if_exists, const byte b, const byte *l, uint32_t rlen) { + if (recv_sys.scanned_lsn > recv_sys.lsn) + /* + We must already have parsed this record, in the skip_the_rest: + loop in recv_scan_log() before starting multi-batch recovery. + */ + return recv_sys_t::OK; if (UNIV_LIKELY(rlen)); else if (!id.raw() && b == FILE_CHECKPOINT + 2) /* FILE_CHECKPOINT followed by any number of NUL bytes could be