Skip to content

MDEV-40063 Corruption due to race in SET GLOBAL innodb_log_archive=ON #5253

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

Merged
dr-m merged 1 commit into from
Jun 18, 2026
+16 −11
Merged

MDEV-40063 Corruption due to race in SET GLOBAL innodb_log_archive=ON #5253

Show file tree
Hide file tree
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
25 changes: 15 additions & 10 deletions storage/innobase/buf/buf0flu.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1993,8 +1993,6 @@ inline lsn_t log_t::write_checkpoint(lsn_t checkpoint, lsn_t end_lsn) noexcept
(C[-1] && my_betoh64(C[-1]) < d));
ut_ad(!*C || offset + o == START_OFFSET - 8);
*C= my_htobe64(d);
if (resize_log.m_file == OS_FILE_CLOSED)
resize_log= log; /* Block concurrent set_archive() */
goto write_checkpoint;
}

Expand Down Expand Up @@ -2029,6 +2027,8 @@ inline lsn_t log_t::write_checkpoint(lsn_t checkpoint, lsn_t end_lsn) noexcept
{
write_checkpoint:
ut_ad(!is_mmap());
if (resize_log.m_file == OS_FILE_CLOSED)
resize_log= log; /* Block concurrent set_archive() */
latch.wr_unlock();
log_write_and_flush_prepare();
resizing= resize_lsn.load(std::memory_order_relaxed);
Expand Down Expand Up @@ -2059,7 +2059,18 @@ inline lsn_t log_t::write_checkpoint(lsn_t checkpoint, lsn_t end_lsn) noexcept
last_checkpoint_lsn= checkpoint;
this->end_lsn= end_lsn;
if (!archive)
{
archived_lsn= end_lsn;
checkpoint_completed:
if (resize_log.m_file == log.m_file)
{
/* We may have assigned resize_log= log to keep set_archived() out. */
#ifdef HAVE_PMEM
ut_ad(!is_mmap() || (!resize_log.is_opened() && is_mmap_writeable()));
#endif
resize_log.m_file= OS_FILE_CLOSED;
}
}
Comment on lines 2061 to +2073

@gemini-code-assist gemini-code-assist Bot Jun 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Jumping into an if block using goto is a highly discouraged control flow pattern in C++ that hurts readability and maintainability. We can completely avoid the goto statement and the checkpoint_completed label by restructuring the code. Since resize_log.m_file != log.m_file is guaranteed when archive_header_was_reset is true, we can safely place the resize_log.m_file cleanup block directly after the if-else chain. This simplifies the logic and makes it much cleaner.

  if (!archive)
  {
    archived_lsn= end_lsn;
  }

else if (archive_header_was_reset)
{
ut_ad(resize_log.m_file != log.m_file);
Expand All @@ -2080,14 +2091,8 @@ inline lsn_t log_t::write_checkpoint(lsn_t checkpoint, lsn_t end_lsn) noexcept
resize_log.close();
#endif
}
else if (resize_log.m_file == log.m_file)
{
/* We may have assigned resize_log= log to keep set_archived() out. */
#ifdef HAVE_PMEM
ut_ad(!is_mmap());
#endif
resize_log.m_file= OS_FILE_CLOSED;
}
else
goto checkpoint_completed;
Comment on lines +2094 to +2095

@gemini-code-assist gemini-code-assist Bot Jun 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

By placing the cleanup block here after the if-else chain, we can completely eliminate the goto checkpoint_completed; statement and the corresponding label inside the if (!archive) block. This makes the control flow linear and much easier to follow.

  if (resize_log.m_file == log.m_file)
  {
    /* We may have assigned resize_log= log to keep set_archived() out. */
#ifdef HAVE_PMEM
    ut_ad(!is_mmap());
#endif
    resize_log.m_file= OS_FILE_CLOSED;
  }


DBUG_PRINT("ib_log", ("checkpoint ended at " LSN_PF ", flushed to " LSN_PF,
checkpoint, get_flushed_lsn()));
Expand Down
2 changes: 1 addition & 1 deletion storage/innobase/log/log0log.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -993,7 +993,7 @@ log_t::resize_start_status log_t::resize_start(os_offset_t size, void *thd)
{
lsn_t start_lsn;
ut_ad(!resize_in_progress());
ut_ad(!resize_log.is_opened());
ut_ad(!resize_log.is_opened() || resize_log.m_file == log.m_file);
ut_ad(!resize_buf);
ut_ad(!resize_flush_buf);
ut_ad(!resize_initiator);
Expand Down