MDEV-36082 Race condition between log_t::resize_start() and log_t::resize_abort()

log_t::writer_update(): Add the parameter bool resizing,
to indicate whether log resizing is in progress.
We must enable log_writer_resizing only if resize_lsn>1,
to ensure that log_t::resize_abort() will not choose the wrong
log_sys.log_writer.

log_t::resize_initiator: The thread that successfully invoked
resize_start().

log_t::resize_start(): Simplify some logic, and assign resize_initiator
if we successfully started log resizing.

log_t::resize_abort(): Abort log resizing if we are the
resize_initiator.

innodb_log_file_size_update(): Clean up some logic.

Reviewed by: Debarun Banerjee
This commit is contained in:
Marko Mäkelä 2025-02-17 15:55:58 +02:00
parent 43c5d1303f
commit 7e001b2a3c
4 changed files with 56 additions and 43 deletions

View File

@ -1912,7 +1912,8 @@ inline void log_t::write_checkpoint(lsn_t end_lsn) noexcept
resize_flush_buf= nullptr; resize_flush_buf= nullptr;
resize_target= 0; resize_target= 0;
resize_lsn.store(0, std::memory_order_relaxed); resize_lsn.store(0, std::memory_order_relaxed);
writer_update(); resize_initiator= nullptr;
writer_update(false);
} }
log_resize_release(); log_resize_release();

View File

@ -18605,7 +18605,7 @@ static void innodb_log_file_size_update(THD *thd, st_mysql_sys_var*,
" innodb_log_buffer_size=%u", MYF(0), log_sys.buf_size); " innodb_log_buffer_size=%u", MYF(0), log_sys.buf_size);
else else
{ {
switch (log_sys.resize_start(*static_cast<const ulonglong*>(save))) { switch (log_sys.resize_start(*static_cast<const ulonglong*>(save), thd)) {
case log_t::RESIZE_NO_CHANGE: case log_t::RESIZE_NO_CHANGE:
break; break;
case log_t::RESIZE_IN_PROGRESS: case log_t::RESIZE_IN_PROGRESS:
@ -18617,12 +18617,11 @@ static void innodb_log_file_size_update(THD *thd, st_mysql_sys_var*,
ib_senderrf(thd, IB_LOG_LEVEL_ERROR, ER_CANT_CREATE_HANDLER_FILE); ib_senderrf(thd, IB_LOG_LEVEL_ERROR, ER_CANT_CREATE_HANDLER_FILE);
break; break;
case log_t::RESIZE_STARTED: case log_t::RESIZE_STARTED:
const lsn_t start{log_sys.resize_in_progress()};
for (timespec abstime;;) for (timespec abstime;;)
{ {
if (thd_kill_level(thd)) if (thd_kill_level(thd))
{ {
log_sys.resize_abort(); log_sys.resize_abort(thd);
break; break;
} }
@ -18637,13 +18636,15 @@ static void innodb_log_file_size_update(THD *thd, st_mysql_sys_var*,
resizing= log_sys.resize_in_progress(); resizing= log_sys.resize_in_progress();
} }
mysql_mutex_unlock(&buf_pool.flush_list_mutex); mysql_mutex_unlock(&buf_pool.flush_list_mutex);
if (start > log_sys.get_lsn()) if (!resizing || !log_sys.resize_running(thd))
break;
if (resizing > log_sys.get_lsn())
{ {
ut_ad(!log_sys.is_mmap()); ut_ad(!log_sys.is_mmap());
/* The server is almost idle. Write dummy FILE_CHECKPOINT records /* The server is almost idle. Write dummy FILE_CHECKPOINT records
to ensure that the log resizing will complete. */ to ensure that the log resizing will complete. */
log_sys.latch.wr_lock(SRW_LOCK_CALL); log_sys.latch.wr_lock(SRW_LOCK_CALL);
while (start > log_sys.get_lsn()) while (resizing > log_sys.get_lsn())
{ {
mtr_t mtr; mtr_t mtr;
mtr.start(); mtr.start();
@ -18651,8 +18652,6 @@ static void innodb_log_file_size_update(THD *thd, st_mysql_sys_var*,
} }
log_sys.latch.wr_unlock(); log_sys.latch.wr_unlock();
} }
if (!resizing || resizing > start /* only wait for our resize */)
break;
} }
} }
} }

View File

@ -303,7 +303,6 @@ public:
bool log_maybe_unbuffered; bool log_maybe_unbuffered;
# endif # endif
#endif #endif
/** Fields involved in checkpoints @{ */ /** Fields involved in checkpoints @{ */
lsn_t log_capacity; /*!< capacity of the log; if lsn_t log_capacity; /*!< capacity of the log; if
the checkpoint age exceeds this, it is the checkpoint age exceeds this, it is
@ -326,6 +325,8 @@ public:
/* @} */ /* @} */
private: private:
/** the thread that initiated resize_lsn() */
Atomic_relaxed<void*> resize_initiator;
/** A lock when the spin-only lock_lsn() is not being used */ /** A lock when the spin-only lock_lsn() is not being used */
log_lsn_lock lsn_lock; log_lsn_lock lsn_lock;
public: public:
@ -367,11 +368,17 @@ public:
/** Start resizing the log and release the exclusive latch. /** Start resizing the log and release the exclusive latch.
@param size requested new file_size @param size requested new file_size
@param thd the current thread identifier
@return whether the resizing was started successfully */ @return whether the resizing was started successfully */
resize_start_status resize_start(os_offset_t size) noexcept; resize_start_status resize_start(os_offset_t size, void *thd) noexcept;
/** Abort any resize_start(). */ /** Abort a resize_start() that we started.
void resize_abort() noexcept; @param thd thread identifier that had been passed to resize_start() */
void resize_abort(void *thd) noexcept;
/** @return whether a particular resize_start() is in progress */
bool resize_running(void *thd) const noexcept
{ return thd == resize_initiator; }
/** Replicate a write to the log. /** Replicate a write to the log.
@param lsn start LSN @param lsn start LSN
@ -481,7 +488,7 @@ public:
private: private:
/** Update writer and mtr_t::finisher */ /** Update writer and mtr_t::finisher */
void writer_update() noexcept; void writer_update(bool resizing) noexcept;
/** Wait in append_prepare() for buffer to become available /** Wait in append_prepare() for buffer to become available
@tparam spin whether to use the spin-only lock_lsn() @tparam spin whether to use the spin-only lock_lsn()

View File

@ -353,7 +353,7 @@ bool log_t::attach(log_file_t file, os_offset_t size)
# endif # endif
buf= static_cast<byte*>(ptr); buf= static_cast<byte*>(ptr);
max_buf_free= 1; max_buf_free= 1;
writer_update(); writer_update(false);
# ifdef HAVE_PMEM # ifdef HAVE_PMEM
if (is_pmem) if (is_pmem)
return true; return true;
@ -395,7 +395,7 @@ bool log_t::attach(log_file_t file, os_offset_t size)
TRASH_ALLOC(buf, buf_size); TRASH_ALLOC(buf, buf_size);
TRASH_ALLOC(flush_buf, buf_size); TRASH_ALLOC(flush_buf, buf_size);
max_buf_free= buf_size / LOG_BUF_FLUSH_RATIO - LOG_BUF_FLUSH_MARGIN; max_buf_free= buf_size / LOG_BUF_FLUSH_RATIO - LOG_BUF_FLUSH_MARGIN;
writer_update(); writer_update(false);
memset_aligned<512>(checkpoint_buf, 0, write_size); memset_aligned<512>(checkpoint_buf, 0, write_size);
func_exit: func_exit:
@ -570,31 +570,35 @@ void log_t::set_buffered(bool buffered)
/** Start resizing the log and release the exclusive latch. /** Start resizing the log and release the exclusive latch.
@param size requested new file_size @param size requested new file_size
@param thd the current thread identifier
@return whether the resizing was started successfully */ @return whether the resizing was started successfully */
log_t::resize_start_status log_t::resize_start(os_offset_t size) noexcept log_t::resize_start_status log_t::resize_start(os_offset_t size, void *thd)
noexcept
{ {
ut_ad(size >= 4U << 20); ut_ad(size >= 4U << 20);
ut_ad(!(size & 4095)); ut_ad(!(size & 4095));
ut_ad(!srv_read_only_mode); ut_ad(!srv_read_only_mode);
ut_ad(thd);
log_resize_acquire(); log_resize_acquire();
resize_start_status status= RESIZE_NO_CHANGE; resize_start_status status;
lsn_t start_lsn{0};
#ifdef HAVE_PMEM
bool is_pmem{false};
#endif
if (resize_in_progress()) if (size == file_size)
status= RESIZE_NO_CHANGE;
else if (resize_in_progress())
status= RESIZE_IN_PROGRESS; status= RESIZE_IN_PROGRESS;
else if (size != file_size) else
{ {
lsn_t start_lsn;
ut_ad(!resize_in_progress()); ut_ad(!resize_in_progress());
ut_ad(!resize_log.is_opened()); ut_ad(!resize_log.is_opened());
ut_ad(!resize_buf); ut_ad(!resize_buf);
ut_ad(!resize_flush_buf); ut_ad(!resize_flush_buf);
ut_ad(!resize_initiator);
std::string path{get_log_file_path("ib_logfile101")}; std::string path{get_log_file_path("ib_logfile101")};
bool success; bool success;
resize_initiator= thd;
resize_lsn.store(1, std::memory_order_relaxed); resize_lsn.store(1, std::memory_order_relaxed);
resize_target= 0; resize_target= 0;
resize_log.m_file= resize_log.m_file=
@ -612,6 +616,7 @@ log_t::resize_start_status log_t::resize_start(os_offset_t size) noexcept
#ifdef HAVE_PMEM #ifdef HAVE_PMEM
else if (is_mmap()) else if (is_mmap())
{ {
bool is_pmem{false};
ptr= ::log_mmap(resize_log.m_file, is_pmem, size); ptr= ::log_mmap(resize_log.m_file, is_pmem, size);
if (ptr == MAP_FAILED) if (ptr == MAP_FAILED)
@ -661,34 +666,33 @@ log_t::resize_start_status log_t::resize_start(os_offset_t size) noexcept
else if (!is_opened()) else if (!is_opened())
resize_log.close(); resize_log.close();
writer_update(); resize_lsn.store(start_lsn, std::memory_order_relaxed);
writer_update(true);
log_resize_release();
mysql_mutex_lock(&buf_pool.flush_list_mutex);
lsn_t target_lsn= buf_pool.get_oldest_modification(0);
mysql_mutex_unlock(&buf_pool.flush_list_mutex);
buf_flush_ahead(start_lsn < target_lsn ? target_lsn + 1 : start_lsn,
false);
return RESIZE_STARTED;
} }
status= success ? RESIZE_STARTED : RESIZE_FAILED;
} }
resize_lsn.store(start_lsn, std::memory_order_relaxed); resize_initiator= nullptr;
resize_lsn.store(0, std::memory_order_relaxed);
status= RESIZE_FAILED;
} }
log_resize_release(); log_resize_release();
if (start_lsn)
{
mysql_mutex_lock(&buf_pool.flush_list_mutex);
lsn_t target_lsn= buf_pool.get_oldest_modification(0);
if (start_lsn < target_lsn)
start_lsn= target_lsn + 1;
mysql_mutex_unlock(&buf_pool.flush_list_mutex);
buf_flush_ahead(start_lsn, false);
}
return status; return status;
} }
/** Abort log resizing. */ /** Abort a resize_start() that we started. */
void log_t::resize_abort() noexcept void log_t::resize_abort(void *thd) noexcept
{ {
log_resize_acquire(); log_resize_acquire();
if (resize_in_progress() > 1) if (resize_running(thd))
{ {
#ifdef HAVE_PMEM #ifdef HAVE_PMEM
const bool is_mmap{this->is_mmap()}; const bool is_mmap{this->is_mmap()};
@ -715,11 +719,12 @@ void log_t::resize_abort() noexcept
resize_buf= nullptr; resize_buf= nullptr;
resize_target= 0; resize_target= 0;
resize_lsn.store(0, std::memory_order_relaxed); resize_lsn.store(0, std::memory_order_relaxed);
resize_initiator= nullptr;
std::string path{get_log_file_path("ib_logfile101")}; std::string path{get_log_file_path("ib_logfile101")};
IF_WIN(DeleteFile(path.c_str()), unlink(path.c_str())); IF_WIN(DeleteFile(path.c_str()), unlink(path.c_str()));
writer_update(false);
} }
writer_update();
log_resize_release(); log_resize_release();
} }
@ -1190,10 +1195,11 @@ ATTRIBUTE_COLD static lsn_t log_writer_resizing() noexcept
return log_sys.write_buf<log_t::RESIZING>(); return log_sys.write_buf<log_t::RESIZING>();
} }
void log_t::writer_update() noexcept void log_t::writer_update(bool resizing) noexcept
{ {
ut_ad(latch_have_wr()); ut_ad(latch_have_wr());
writer= resize_in_progress() ? log_writer_resizing : log_writer; ut_ad(resizing == (resize_in_progress() > 1));
writer= resizing ? log_writer_resizing : log_writer;
mtr_t::finisher_update(); mtr_t::finisher_update();
} }