Skip to content

MDEV-39500 STRICT execution mode slave silently overwrites record eve… #5066

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.

Sign up for GitHub

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

Open
andrelkin wants to merge 1 commit into
base: main
Choose a base branch
from
Open

MDEV-39500 STRICT execution mode slave silently overwrites record eve… #5066

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
66 changes: 66 additions & 0 deletions mysql-test/suite/rpl/r/rpl_mismatch_detect.result
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
include/master-slave.inc
[connection master]
#
# MDEV-39500: Replicated UPDATE silently succeeds despite row divergence on slave
#
connection master;
CREATE TABLE t1 (a INT PRIMARY KEY, c VARCHAR(255)) ENGINE=InnoDB;
INSERT INTO t1 VALUES (1, 'initial_state');
connection slave;
connection slave;
set @save_slave_exec_mode = @@global.slave_exec_mode;
set @@global.slave_exec_mode = STRICT;
UPDATE t1 SET c = 'diverged_to_pass';
SELECT * FROM t1;
a c
1 diverged_to_pass
connection master;
UPDATE t1 SET c = 'master_update1';
#
# Expect it to succeed (slave gets in sync to have applied it).
# Exposing MDEV-39500: The slave applier uses the PK to find the row but
# fails to reject the update despite the before-image mismatch, applying it
# silently without throwing HA_ERR_RECORD_CHANGED (or similar).
#
connection slave;
connection slave;
# The slave has silently synced and overwritten the local changes
SELECT * FROM t1;
a c
1 master_update1
include/diff_tables.inc [master:t1, slave:t1]
connection slave;
set @@global.slave_exec_mode = STRINGENT;
call mtr.add_suppression("Slave: Replicated 'Update_rows_v1' record.s before-image on table");
UPDATE t1 SET c = 'diverged_to_fail';
SELECT * FROM t1;
a c
1 diverged_to_fail
connection master;
UPDATE t1 SET c = 'master_update2';
connection slave;
#
# Expect it to fail.
#
include/wait_for_slave_sql_error.inc [errno=4264]
connection slave;
# the stopped slave deverges
SELECT * FROM t1;
a c
1 diverged_to_fail
set @@global.slave_exec_mode = STRICT;
include/start_slave.inc
Not anymore in STRICT mode
SELECT * FROM t1;
a c
1 master_update2
connection master;
connection slave;
#
# Cleanup
#
connection master;
DROP TABLE t1;
connection slave;
set @@global.slave_exec_mode = @save_slave_exec_mode;
include/rpl_end.inc
87 changes: 87 additions & 0 deletions mysql-test/suite/rpl/t/rpl_mismatch_detect.test
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
--source include/have_binlog_format_row.inc
--source include/have_innodb.inc
--source include/master-slave.inc

--echo #
--echo # MDEV-39500: Replicated UPDATE silently succeeds despite row divergence on slave
--echo #

# Having two slave_exec_mode modes conduct the following.
# 0. initially populate a PK-defined table with a record
# 1. makes slave data inconsistent with master via UPDATEing locally a
# replicated record, and run
# 2. a replicated UPDATE from master and expect, it to succeed
# in the default STRICT mode, or
# fail in the new STRINGENT.


connection master;
CREATE TABLE t1 (a INT PRIMARY KEY, c VARCHAR(255)) ENGINE=InnoDB;
INSERT INTO t1 VALUES (1, 'initial_state');
sync_slave_with_master;

connection slave;
set @save_slave_exec_mode = @@global.slave_exec_mode;
set @@global.slave_exec_mode = STRICT;
# Modify the non-PK columns to simulate divergence
UPDATE t1 SET c = 'diverged_to_pass';
SELECT * FROM t1;

connection master;
UPDATE t1 SET c = 'master_update1';

--echo #
--echo # Expect it to succeed (slave gets in sync to have applied it).
--echo # Exposing MDEV-39500: The slave applier uses the PK to find the row but
--echo # fails to reject the update despite the before-image mismatch, applying it
--echo # silently without throwing HA_ERR_RECORD_CHANGED (or similar).
--echo #
sync_slave_with_master;

connection slave;
--echo # The slave has silently synced and overwritten the local changes
SELECT * FROM t1;

--let $diff_tables= master:t1, slave:t1
--source include/diff_tables.inc

# the new mode
connection slave;
set @@global.slave_exec_mode = STRINGENT;
call mtr.add_suppression("Slave: Replicated 'Update_rows_v1' record.s before-image on table");

# Modify the non-PK columns to simulate divergence
UPDATE t1 SET c = 'diverged_to_fail';
SELECT * FROM t1;

connection master;
UPDATE t1 SET c = 'master_update2';

connection slave;
--echo #
--echo # Expect it to fail.
--echo #
# ER_INCONSISTENT_SLAVE_RECORD
--let $slave_sql_errno = 4264
--source include/wait_for_slave_sql_error.inc

connection slave;
--echo # the stopped slave deverges
SELECT * FROM t1;

set @@global.slave_exec_mode = STRICT;
--source include/start_slave.inc
--echo Not anymore in STRICT mode
SELECT * FROM t1;

connection master;
sync_slave_with_master;

--echo #
--echo # Cleanup
--echo #
connection master;
DROP TABLE t1;
sync_slave_with_master;
set @@global.slave_exec_mode = @save_slave_exec_mode;
--source include/rpl_end.inc
18 changes: 17 additions & 1 deletion sql/log_event_server.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8206,7 +8206,7 @@ int Rows_log_event::find_row(rpl_group_info *rgi)
int error= 0;
bool is_table_scan= false, is_index_scan= false;
Check_level_instant_set clis(table->in_use, CHECK_FIELD_IGNORE);

bool do_compare_records= slave_exec_mode == SLAVE_EXEC_MODE_STRINGENT;
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

When STRINGENT mode is enabled, the code performs a record_compare at the end of find_row. However, record_compare expects the before-image to be in table->record[1] and the found row in table->record[0]. In the index lookup path (e.g., PK lookup), table->record[0] is overwritten by the storage engine during the read operation, and the original before-image is lost because it was never copied to table->record[1]. This results in record_compare comparing the found row against uninitialized or stale data in record[1]. You must copy the before-image to record[1] before the index lookup occurs.

  bool do_compare_records= slave_exec_mode == SLAVE_EXEC_MODE_STRINGENT;
  if (do_compare_records)
    memcpy(table->record[1], table->record[0], table->s->reclength);

/*
rpl_row_tabledefs.test specifies that
if the extra field on the slave does not have a default value
Expand Down Expand Up @@ -8272,6 +8272,10 @@ int Rows_log_event::find_row(rpl_group_info *rgi)
error= row_not_found_error(rgi);
table->file->print_error(error, MYF(0));
}
else
{
goto comp_rec;
}
DBUG_RETURN(error);
}

Expand Down Expand Up @@ -8465,6 +8469,18 @@ int Rows_log_event::find_row(rpl_group_info *rgi)
if (is_table_scan || is_index_scan)
issue_long_find_row_warning(get_general_type_code(), m_table->alias.c_ptr(),
is_index_scan, rgi);
comp_rec:
if (error == 0 && do_compare_records && rgi->rli->mi /* !online alter */)
{
if (record_compare(table, m_vers_from_plain))
{
my_error(ER_INCONSISTENT_SLAVE_RECORD, MYF(0),
get_type_str(get_type_code()),
table->s->db.str, table->s->table_name.str);
error= 1; // not HA error, the caller will catch it as already reported
}
}

DBUG_RETURN(error);
}

Expand Down
2 changes: 2 additions & 0 deletions sql/share/errmsg-utf8.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12404,3 +12404,5 @@ ER_WARN_QB_NAME_PATH_VIEW_NOT_FOUND
eng "Hint %s is ignored. `%s` required at element #%u of the path is not found in the target query block."
ER_WARN_QB_NAME_PATH_NOT_SUPPORTED_INSIDE_VIEW
eng "Hint %s is ignored. QB_NAME hints with path are not supported inside view definitions."
ER_INCONSISTENT_SLAVE_RECORD
eng "Replicated '%s' record's before-image on table `%s.%s` does not match the local record"
3 changes: 2 additions & 1 deletion sql/sql_class.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,8 @@ enum enum_ha_read_modes { RFIRST, RNEXT, RPREV, RLAST, RKEY, RNEXT_SAME };
enum enum_duplicates { DUP_ERROR, DUP_REPLACE, DUP_UPDATE };
enum enum_delay_key_write { DELAY_KEY_WRITE_NONE, DELAY_KEY_WRITE_ON,
DELAY_KEY_WRITE_ALL };
enum enum_slave_exec_mode { SLAVE_EXEC_MODE_STRICT,
enum enum_slave_exec_mode { SLAVE_EXEC_MODE_STRINGENT,
SLAVE_EXEC_MODE_STRICT,
SLAVE_EXEC_MODE_IDEMPOTENT,
SLAVE_EXEC_MODE_LAST_BIT };
Comment on lines +122 to 125
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Adding SLAVE_EXEC_MODE_STRINGENT at the beginning of the enum_slave_exec_mode changes the numeric values of existing modes (STRICT becomes 1 instead of 0, IDEMPOTENT becomes 2 instead of 1). This is a breaking change for any external tools or scripts that rely on the numeric values of @@slave_exec_mode. Furthermore, if the default value is initialized to 0, the default behavior of the server will silently change from STRICT to STRINGENT. It is safer to add new execution modes at the end of the enum, before SLAVE_EXEC_MODE_LAST_BIT.

enum enum_slave_exec_mode { SLAVE_EXEC_MODE_STRICT,
                            SLAVE_EXEC_MODE_IDEMPOTENT,
                            SLAVE_EXEC_MODE_STRINGENT,
                            SLAVE_EXEC_MODE_LAST_BIT };

enum enum_slave_run_triggers_for_rbr { SLAVE_RUN_TRIGGERS_FOR_RBR_NO,
Expand Down
3 changes: 2 additions & 1 deletion sql/sys_vars.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3684,7 +3684,8 @@ Sys_slave_compressed_protocol(
DEFAULT(FALSE));

#ifdef HAVE_REPLICATION
static const char *slave_exec_mode_names[]= {"STRICT", "IDEMPOTENT", 0};
static const char *slave_exec_mode_names[]=
{"STRINGENT", "STRICT", "IDEMPOTENT", 0};
Comment on lines +3687 to +3688
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The order of names in slave_exec_mode_names must match the order of the constants in enum enum_slave_exec_mode. If you move STRINGENT to the end of the enum as suggested, you must also move it to the end of this array to maintain consistency and avoid breaking the default value mapping.

static const char *slave_exec_mode_names[]=
		    {"STRICT", "IDEMPOTENT", "STRINGENT", 0};

static Sys_var_on_access_global<Sys_var_enum,
PRIV_SET_SYSTEM_GLOBAL_VAR_SLAVE_EXEC_MODE>
Slave_exec_mode(
Expand Down
Loading