Skip to content

Commit 4d37b1c

Browse files
committed
MDEV-36886 log_t::get_lsn_approx() isn't lower bound
If the execution of the two reads in log_t::get_lsn_approx() is interleaved with concurrent writes of those fields in log_t::write_buf() or log_t::persist(), the returned approximation will be an upper bound. If log_t::append_prepare_wait() is pending, the approximation could be a lower bound. We must adjust each caller of log_t::get_lsn_approx() for the possibility that the return value is larger than MAX(oldest_modification) in buf_pool.flush_list. af_needed_for_redo(): Add a comment that explains why the glitch is not a problem. page_cleaner_flush_pages_recommendation(): Revise the logic for the unlikely case cur_lsn < oldest_lsn. The original logic would have invoked af_get_pct_for_lsn() with a very large age value, which would likely cause an overflow of the local variable lsn_age_factor, and make pct_for_lsn a "random number". Based on that value, total_ratio would be normalized to something between 0.0 and 1.0. Nothing extremely bad should have happened in this case; the innodb_io_capacity_max should not be exceeded.
1 parent 7b4b759 commit 4d37b1c

File tree

2 files changed

+13
-4
lines changed

2 files changed

+13
-4
lines changed

storage/innobase/buf/buf0flu.cc

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2294,6 +2294,12 @@ redo log capacity filled threshold.
22942294
@return true if adaptive flushing is recommended. */
22952295
static bool af_needed_for_redo(lsn_t oldest_lsn) noexcept
22962296
{
2297+
/* We may have oldest_lsn > log_sys.get_lsn_approx() if
2298+
log_t::write_buf() or log_t::persist() are executing concurrently
2299+
with this. In that case, age > af_lwm should hold, and
2300+
buf_flush_page_cleaner() would execute one more timed wait. (Not a
2301+
big problem.) */
2302+
22972303
lsn_t age= log_sys.get_lsn_approx() - oldest_lsn;
22982304
lsn_t af_lwm= static_cast<lsn_t>(srv_adaptive_flushing_lwm *
22992305
static_cast<double>(log_sys.log_capacity) / 100);
@@ -2355,8 +2361,10 @@ static ulint page_cleaner_flush_pages_recommendation(ulint last_pages_in,
23552361
ulint n_pages = 0;
23562362

23572363
const lsn_t cur_lsn = log_sys.get_lsn_approx();
2358-
ut_ad(oldest_lsn <= cur_lsn);
2359-
ulint pct_for_lsn = af_get_pct_for_lsn(cur_lsn - oldest_lsn);
2364+
/* We may have cur_lsn < oldest_lsn if
2365+
log_t::write_buf() or log_t::persist() were executing concurrently. */
2366+
ulint pct_for_lsn = cur_lsn < oldest_lsn
2367+
? 0 : af_get_pct_for_lsn(cur_lsn - oldest_lsn);
23602368
time_t curr_time = time(nullptr);
23612369
const double max_pct = srv_max_buf_pool_modified_pct;
23622370

storage/innobase/include/log0log.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -406,9 +406,10 @@ struct log_t
406406
@param encrypted whether the log is encrypted */
407407
static void header_write(byte *buf, lsn_t lsn, bool encrypted) noexcept;
408408

409-
/** @return a lower bound estimate of get_lsn(),
409+
/** @return an estimate of get_lsn(),
410410
using acquire-release ordering with write_buf() or persist();
411-
this is exact unless append_prepare_wait() is pending */
411+
an upper bound if said functions have updated only one of the fields,
412+
a lower bound if append_prepare_wait() is pending, otherwise exact */
412413
lsn_t get_lsn_approx() const noexcept
413414
{
414415
/* acquire-release ordering with write_buf() and persist() */

0 commit comments

Comments
 (0)