From f68f3de7478c2b53081d707d9c5d317514b5daa0 Mon Sep 17 00:00:00 2001 From: dgzhongly <52107085+dgzhongly@users.noreply.github.com> Date: Fri, 26 Apr 2024 02:26:41 +0800 Subject: [PATCH] Fix bug #113812 alter table tablename engine=innodb cause data loss Fix bug #113812 alter table tablename engine=innodb cause data loss When user call "alter table tablename engine=innodb", MySQL will rebuild the table online. MySQL scan the original table, add record to a key_buffer. when key_buffer is full, page latch will be release and use PCursor to store cursor. PCursor::savepoint() call m_pcur->move_to_prev_on_page() before store position to aviod storing "supremum". PCursor::resume() call m_pcur->move_to_next_on_page() after restore position. PCursor::resume() use restore_position() to restore from persistent cursor. When record store in persistent cursor has been purge, restore_position() will set cursor to a previous record. On this situation, page_cur_move_to_next() will be called to move cursor to its right position. So, PCursor::resume() may move cursor to next record twice and it cause cursor skip a record. Here is a example: (.) a (X) b (.) c <--cursor (.) d (.) e 1. Cursor point to record 'c' and copy 'c' to builder and builder try to add it to key_buffer, but key_buffer is full. Start bulk insert. 2. To release page latch, PCursor::savepoint() will be called. Cursor move to record 'b' and store 'b' to persistent cursor. 3. During bulk insert, purge thread remove record 'b'. 4. Bulk insert is done, go on record scan. PCursor::resume() will be called. 5. First, PCursor::resume() calls restore_position(). restore_position() makes cursor restore to record 'a', then move to record 'c'. 6. Second, PCursor::resume() calls m_pcur->move_to_next_on_page(). Cursor move to recode 'd'. 7. Builder contiunes to add record 'c' to key_buffer. after that, cursor move to next record, which is record 'e'. 8. Record 'd' is skipped. PCursor::savepoint() use m_pcur->store_position() to store cursor. m_pcur->store_position() will store previous record when cursor pointing to "supremum". so, it is no need to call m_pcur->move_to_prev_on_page() in m_pcur->move_to_prev_on_page(). Thus, calling m_pcur->move_to_next_on_page() in PCursor::resume() is unnecessary and remove it could avoid a record skip. --- storage/innobase/row/row0pread.cc | 8 -------- 1 file changed, 8 deletions(-) diff --git a/storage/innobase/row/row0pread.cc b/storage/innobase/row/row0pread.cc index 15d075943fa0..bb6a690c2126 100644 --- a/storage/innobase/row/row0pread.cc +++ b/storage/innobase/row/row0pread.cc @@ -330,9 +330,6 @@ buf_block_t *Parallel_reader::Scan_ctx::block_get_s_latched( } void PCursor::savepoint() noexcept { - /* Store the cursor position on the previous user record on the page. */ - m_pcur->move_to_prev_on_page(); - m_pcur->store_position(m_mtr); m_mtr->commit(); @@ -347,11 +344,6 @@ void PCursor::resume() noexcept { was purged meanwhile. */ restore_position(); - - if (!m_pcur->is_after_last_on_page()) { - /* Move to the successor of the saved record. */ - m_pcur->move_to_next_on_page(); - } } dberr_t PCursor::move_to_user_rec() noexcept {