Skip to content
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.

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

PS-9144 : Missing rows after ALGORITHM=INPLACE ALTER under same workl… #5354

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
33 changes: 33 additions & 0 deletions mysql-test/suite/innodb/r/percona_alter_debug.result
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#
# Test for PS-9144 : Missing rows after ALGORITHM=INPLACE ALTER under
# same workload as PS-9092.
#
CREATE TABLE t1 (pk CHAR(5) PRIMARY KEY);
INSERT INTO t1 VALUES ('aaaaa'), ('bbbbb'), ('bbbcc'), ('ccccc'), ('ddddd'), ('eeeee');
set global innodb_purge_stop_now=ON;
DELETE FROM t1 WHERE pk = 'bbbcc';
connect con1, localhost, root,,;
SET DEBUG='+d,ddl_buf_add_two';
SET DEBUG_SYNC='ddl_bulk_inserter_latches_released SIGNAL latches_released WAIT_FOR go';
# Send ALTER TABLE INPLACE which rebuilds table.
ALTER TABLE t1 ENGINE=InnoDB, ALGORITHM=INPLACE;
connection default;
SET DEBUG_SYNC='now WAIT_FOR latches_released';
SET GLOBAL innodb_purge_run_now=ON;
SET DEBUG_SYNC='now SIGNAL go';
connection con1;
# Reap ALTER TABLE
# Before the fix row 'ddddd' was missing from the table after ALTER.
SELECT * FROM t1;
pk
aaaaa
bbbbb
ccccc
ddddd
eeeee
SET DEBUG='-d,ddl_buf_add_two';
disconnect con1;
connection default;
# Cleanup.
SET DEBUG_SYNC= 'RESET';
DROP TABLE t1;
44 changes: 44 additions & 0 deletions mysql-test/suite/innodb/t/percona_alter_debug.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# Tests in this file use both debug-only facilities as well as debug sync point.
--source include/have_debug.inc
--source include/have_debug_sync.inc

--echo #
--echo # Test for PS-9144 : Missing rows after ALGORITHM=INPLACE ALTER under
--echo # same workload as PS-9092.
--echo #
--enable_connect_log
CREATE TABLE t1 (pk CHAR(5) PRIMARY KEY);
INSERT INTO t1 VALUES ('aaaaa'), ('bbbbb'), ('bbbcc'), ('ccccc'), ('ddddd'), ('eeeee');

set global innodb_purge_stop_now=ON;
DELETE FROM t1 WHERE pk = 'bbbcc';

--connect (con1, localhost, root,,)
SET DEBUG='+d,ddl_buf_add_two';
SET DEBUG_SYNC='ddl_bulk_inserter_latches_released SIGNAL latches_released WAIT_FOR go';
--echo # Send ALTER TABLE INPLACE which rebuilds table.
--send ALTER TABLE t1 ENGINE=InnoDB, ALGORITHM=INPLACE

--connection default
SET DEBUG_SYNC='now WAIT_FOR latches_released';
SET GLOBAL innodb_purge_run_now=ON;
--source include/wait_innodb_all_purged.inc
SET DEBUG_SYNC='now SIGNAL go';

--connection con1
--echo # Reap ALTER TABLE
--reap

--echo # Before the fix row 'ddddd' was missing from the table after ALTER.
SELECT * FROM t1;

SET DEBUG='-d,ddl_buf_add_two';

--disconnect con1
--source include/wait_until_disconnected.inc

--connection default
--echo # Cleanup.
SET DEBUG_SYNC= 'RESET';
DROP TABLE t1;
--disable_connect_log
1 change: 1 addition & 0 deletions storage/innobase/ddl/ddl0par-scan.cc
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ dberr_t Parallel_cursor::scan(Builders &builders) noexcept {
thread_ctx->get_state() != Parallel_reader::State::THREAD) {
thread_ctx->savepoint();
latches_released = true;
DEBUG_SYNC_C("ddl_bulk_inserter_latches_released");
}
return DB_SUCCESS;
});
Expand Down
84 changes: 23 additions & 61 deletions storage/innobase/row/row0pread.cc
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,27 @@ class PCursor {
PCursor(btr_pcur_t *pcur, mtr_t *mtr, size_t read_level)
: m_mtr(mtr), m_pcur(pcur), m_read_level(read_level) {}

/**
Restore position of cursor created from Scan_ctx::Range object.
Adjust it if starting record of range has been purged.
*/
void restore_position_for_range() noexcept {
/*
Restore position on the record or its predecessor if the record
was purged meanwhile. In the latter case we need to adjust position by
moving one step forward as predecessor belongs to another range.

We rely on return value from restore_position() to detect this scenario.
This can be done when saved position points to user record which is
always the case for saved start of Scan_ctx::Range.
*/
ut_ad(m_pcur->m_rel_pos == BTR_PCUR_ON);

if (!m_pcur->restore_position(BTR_SEARCH_LEAF, m_mtr, UT_LOCATION_HERE)) {
page_cur_move_to_next(m_pcur->get_page_cur());
}
}

/** Create a savepoint and commit the mini-transaction.*/
void savepoint() noexcept;

Expand All @@ -225,65 +246,6 @@ class PCursor {
@return DB_SUCCESS or error code. */
[[nodiscard]] dberr_t move_to_next_block(dict_index_t *index);

/** Restore the cursor position. */
void restore_position() noexcept {
constexpr auto MODE = BTR_SEARCH_LEAF;
const auto relative = m_pcur->m_rel_pos;
auto equal = m_pcur->restore_position(MODE, m_mtr, UT_LOCATION_HERE);

#ifdef UNIV_DEBUG
if (m_pcur->m_pos_state == BTR_PCUR_IS_POSITIONED_OPTIMISTIC) {
ut_ad(m_pcur->m_rel_pos == BTR_PCUR_BEFORE ||
m_pcur->m_rel_pos == BTR_PCUR_AFTER);
} else {
ut_ad(m_pcur->m_pos_state == BTR_PCUR_IS_POSITIONED);
ut_ad((m_pcur->m_rel_pos == BTR_PCUR_ON) == m_pcur->is_on_user_rec());
}
#endif /* UNIV_DEBUG */

switch (relative) {
case BTR_PCUR_ON:
if (!equal) {
page_cur_move_to_next(m_pcur->get_page_cur());
}
break;

case BTR_PCUR_UNSET:
case BTR_PCUR_BEFORE_FIRST_IN_TREE:
ut_error;
break;

case BTR_PCUR_AFTER:
case BTR_PCUR_AFTER_LAST_IN_TREE:
break;

case BTR_PCUR_BEFORE:
/* For non-optimistic restoration:
The position is now set to the record before pcur->old_rec.

For optimistic restoration:
The position also needs to take the previous search_mode into
consideration. */
switch (m_pcur->m_pos_state) {
case BTR_PCUR_IS_POSITIONED_OPTIMISTIC:
m_pcur->m_pos_state = BTR_PCUR_IS_POSITIONED;
/* The cursor always moves "up" ie. in ascending order. */
break;

case BTR_PCUR_IS_POSITIONED:
if (m_pcur->is_on_user_rec()) {
m_pcur->move_to_next(m_mtr);
}
break;

case BTR_PCUR_NOT_POSITIONED:
case BTR_PCUR_WAS_POSITIONED:
ut_error;
}
break;
}
}

/** @return the current page cursor. */
[[nodiscard]] page_cur_t *get_page_cursor() noexcept {
return m_pcur->get_page_cur();
Expand Down Expand Up @@ -347,7 +309,7 @@ void PCursor::resume() noexcept {
/* Restore position on the record, or its predecessor if the record
was purged meanwhile. */

restore_position();
m_pcur->restore_position(BTR_SEARCH_LEAF, m_mtr, UT_LOCATION_HERE);

if (!m_pcur->is_after_last_on_page()) {
/* Move to the successor of the saved record. */
Expand Down Expand Up @@ -625,7 +587,7 @@ dberr_t Parallel_reader::Ctx::traverse() {
auto &from = m_range.first;

PCursor pcursor(from->m_pcur, &mtr, m_scan_ctx->m_config.m_read_level);
pcursor.restore_position();
pcursor.restore_position_for_range();

dberr_t err{DB_SUCCESS};

Expand Down
Loading