From f8f5ed22805f7c269d812b95db3b159b8f2639a3 Mon Sep 17 00:00:00 2001 From: Kristian Nielsen Date: Tue, 15 Aug 2023 12:19:34 +0200 Subject: [PATCH] Revert: MDEV-22351 InnoDB may recover wrong information after RESET MASTER This commit can cause the wrong (old) binlog position to be recovered by mariabackup --prepare. It implements that the value of the FIL_PAGE_LSN is compared to determine which binlog position is the last one and should be recoved. However, it is not guaranteed that the FIL_PAGE_LSN order matches the commit order, as is assumed by the code. This is because the page LSN could be modified by an unrelated update of the page after the commit. In one example, the recovery first encountered this in trx_rseg_mem_restore(): lsn=27282754 binlog position (./master-bin.000001, 472908) and then later: lsn=27282699 binlog position (./master-bin.000001, 477164) The last one 477164 is the correct position. However, because the LSN encountered for the first one is higher, that position is recovered instead. This results in too old binlog position, and a newly provisioned slave will start replicating too early and get duplicate key error or similar. Signed-off-by: Kristian Nielsen --- storage/innobase/include/trx0sys.h | 2 -- storage/innobase/trx/trx0rseg.cc | 45 +++++++++++++++++------------- 2 files changed, 26 insertions(+), 21 deletions(-) diff --git a/storage/innobase/include/trx0sys.h b/storage/innobase/include/trx0sys.h index 0f12cd90495..5f518d799bd 100644 --- a/storage/innobase/include/trx0sys.h +++ b/storage/innobase/include/trx0sys.h @@ -867,8 +867,6 @@ public: uint64_t recovered_binlog_offset; /** Latest recovered binlog file name */ char recovered_binlog_filename[TRX_SYS_MYSQL_LOG_NAME_LEN]; - /** FIL_PAGE_LSN of the page with the latest recovered binlog metadata */ - lsn_t recovered_binlog_lsn; /** diff --git a/storage/innobase/trx/trx0rseg.cc b/storage/innobase/trx/trx0rseg.cc index 49b93541620..10a863b0379 100644 --- a/storage/innobase/trx/trx0rseg.cc +++ b/storage/innobase/trx/trx0rseg.cc @@ -451,13 +451,8 @@ static dberr_t trx_undo_lists_init(trx_rseg_t *rseg, trx_id_t &max_trx_id, static dberr_t trx_rseg_mem_restore(trx_rseg_t *rseg, trx_id_t &max_trx_id, mtr_t *mtr) { - /* This is based on trx_rsegf_get_new(). - We need to access buf_block_t. */ - buf_block_t *block = buf_page_get( - page_id_t(rseg->space->id, rseg->page_no), 0, RW_S_LATCH, mtr); - buf_block_dbg_add_level(block, SYNC_RSEG_HEADER_NEW); - - const trx_rsegf_t* rseg_header = TRX_RSEG + block->frame; + trx_rsegf_t* rseg_header = trx_rsegf_get_new( + rseg->space->id, rseg->page_no, mtr); if (mach_read_from_4(rseg_header + TRX_RSEG_FORMAT) == 0) { trx_id_t id = mach_read_from_8(rseg_header @@ -468,20 +463,32 @@ static dberr_t trx_rseg_mem_restore(trx_rseg_t *rseg, trx_id_t &max_trx_id, } if (rseg_header[TRX_RSEG_BINLOG_NAME]) { - lsn_t lsn = std::max(block->page.newest_modification, - mach_read_from_8(FIL_PAGE_LSN - + block->frame)); + const char* binlog_name = reinterpret_cast + (rseg_header) + TRX_RSEG_BINLOG_NAME; compile_time_assert(TRX_RSEG_BINLOG_NAME_LEN == sizeof trx_sys.recovered_binlog_filename); - if (lsn > trx_sys.recovered_binlog_lsn) { - trx_sys.recovered_binlog_lsn = lsn; - trx_sys.recovered_binlog_offset - = mach_read_from_8( - rseg_header - + TRX_RSEG_BINLOG_OFFSET); - memcpy(trx_sys.recovered_binlog_filename, - rseg_header + TRX_RSEG_BINLOG_NAME, - TRX_RSEG_BINLOG_NAME_LEN); + + int cmp = *trx_sys.recovered_binlog_filename + ? strncmp(binlog_name, + trx_sys.recovered_binlog_filename, + TRX_RSEG_BINLOG_NAME_LEN) + : 1; + + if (cmp >= 0) { + uint64_t binlog_offset = mach_read_from_8( + rseg_header + TRX_RSEG_BINLOG_OFFSET); + if (cmp) { + memcpy(trx_sys. + recovered_binlog_filename, + binlog_name, + TRX_RSEG_BINLOG_NAME_LEN); + trx_sys.recovered_binlog_offset + = binlog_offset; + } else if (binlog_offset > + trx_sys.recovered_binlog_offset) { + trx_sys.recovered_binlog_offset + = binlog_offset; + } } #ifdef WITH_WSREP