Skip to content
/ server Public

MDEV-7394 : Making slave_skip_errors writable at runtime when slave is stopped #4634

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
Mahmoud-kh1 wants to merge 1 commit into
base: main
Choose a base branch
from
+111 −6
Open

MDEV-7394 : Making slave_skip_errors writable at runtime when slave is stopped #4634

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
29 changes: 29 additions & 0 deletions mysql-test/suite/rpl/r/rpl_skip_error_dynamic.result
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# MDEV-7394 test dynamic slave_skip_error (writable when slaves stopped)
include/master-slave.inc
[connection master]
connection slave;
# should reduce error because slave is not stopped
SET GLOBAL slave_skip_errors = "1062";
ERROR HY000: This operation cannot be performed as you have a running slave ''; run STOP SLAVE '' first
SELECT @@global.slave_skip_errors;
@@global.slave_skip_errors
OFF
STOP SLAVE;
# should work corerctly because slaves stopped
SET GLOBAL slave_skip_errors = "1062"
SELECT @@global.slave_skip_errors;
@@global.slave_skip_errors
OFF
SET GLOBAL slave_skip_errors = "1050";
SELECT @@global.slave_skip_errors;
@@global.slave_skip_errors
1050
SET GLOBAL slave_skip_errors = "OFF";
SELECT @@global.slave_skip_errors;
@@global.slave_skip_errors
OFF
START SLAVE;
# should reduce error because slave is not stopped
SET GLOBAL slave_skip_errors = "1040";
ERROR HY000: This operation cannot be performed as you have a running slave ''; run STOP SLAVE '' first
include/rpl_end.inc
28 changes: 28 additions & 0 deletions mysql-test/suite/rpl/t/rpl_skip_error_dynamic.test
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
--echo # MDEV-7394 test dynamic slave_skip_error (writable when slaves stopped)

--source include/master-slave.inc

--connection slave

--echo # should reduce error because slave is not stopped
--error ER_SLAVE_MUST_STOP
SET GLOBAL slave_skip_errors = "1062";
SELECT @@global.slave_skip_errors;

STOP SLAVE;
echo # should work corerctly because slaves stopped
SET GLOBAL slave_skip_errors = "1062";
SELECT @@global.slave_skip_errors;

SET GLOBAL slave_skip_errors = "1050";
SELECT @@global.slave_skip_errors;

SET GLOBAL slave_skip_errors = "OFF";
SELECT @@global.slave_skip_errors;

START SLAVE;
--echo # should reduce error because slave is not stopped
--error ER_SLAVE_MUST_STOP
SET GLOBAL slave_skip_errors = "1040";
Copy link
Member

@gkodinov gkodinov Feb 10, 2026

Choose a reason for hiding this comment

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

please also add actual functionality tests for the newly set variable values.


--source include/rpl_end.inc
52 changes: 48 additions & 4 deletions sql/slave.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -711,8 +711,8 @@ static void make_slave_skip_errors_printable(void)
DBUG_ASSERT(sizeof(slave_skip_error_names) > MIN_ROOM);
DBUG_ASSERT(MAX_SLAVE_ERROR <= 999999); // 6 digits

/* Make @@slave_skip_errors show the nice human-readable value. */
opt_slave_skip_errors= slave_skip_error_names;
/* we should not touch opt_slave_skip_errors here. we just build the printable string only. */
(void) opt_slave_skip_errors;
Copy link
Member

@gkodinov gkodinov Feb 10, 2026

Choose a reason for hiding this comment

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

remove all these lines please.


if (!use_slave_mask || bitmap_is_clear_all(&slave_error_mask))
{
Expand Down Expand Up @@ -774,8 +774,15 @@ bool init_slave_skip_errors(const char* arg)
if (!arg || !*arg) // No errors defined
goto end;

if (my_bitmap_init(&slave_error_mask,0,MAX_SLAVE_ERROR))
DBUG_RETURN(1);
if (!slave_error_mask.bitmap)
{
if (my_bitmap_init(&slave_error_mask, 0, MAX_SLAVE_ERROR))
DBUG_RETURN(1);
}
else
{
bitmap_clear_all(&slave_error_mask);
}

use_slave_mask= 1;
for (;my_isspace(system_charset_info,*arg);++arg)
Expand All @@ -801,6 +808,43 @@ bool init_slave_skip_errors(const char* arg)
DBUG_RETURN(0);
}


bool check_slave_skip_errors(sys_var *self, THD *thd, set_var *var)
{
return give_error_if_slave_running(0);
}


bool update_slave_skip_errors(sys_var *self, THD *thd, enum_var_type type)
{
bool res= false;
mysql_mutex_unlock(&LOCK_global_system_variables);
Copy link
Member

@gkodinov gkodinov Feb 10, 2026

Choose a reason for hiding this comment

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

Why have you decided to unlock this lock?

mysql_mutex_lock(&LOCK_active_mi);
Copy link
Member

@gkodinov gkodinov Feb 10, 2026

Choose a reason for hiding this comment

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

I believe you have a locking issue to sort out. LOCK_active_mi is definitely not the right lock to take for this. I'm not going to pursue on this one and will leave it to the final review. But I believe you need to find a better lock and not just reuse a random existing one.


if (active_mi->slave_running)
{
my_error(ER_SLAVE_MUST_STOP, MYF(0));
res= true;
}
else
{
if (use_slave_mask)
my_bitmap_free(&slave_error_mask);

use_slave_mask= 0;

if (init_slave_skip_errors(opt_slave_skip_errors))
{
my_error(ER_WRONG_VALUE_FOR_VAR, MYF(0), "slave_skip_errors", opt_slave_skip_errors);
res= true;
}
}

Copy link
Member

@gkodinov gkodinov Feb 10, 2026

Choose a reason for hiding this comment

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

here you can do what you removed from slave.cc: assign the printable names to the variable so it's more readable.

mysql_mutex_unlock(&LOCK_active_mi);
mysql_mutex_lock(&LOCK_global_system_variables);
return res;
}

/**
Make printable version if slave_transaction_retry_errors
This is never empty as at least ER_LOCK_DEADLOCK and ER_LOCK_WAIT_TIMEOUT
Expand Down
2 changes: 2 additions & 0 deletions sql/slave.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,8 @@ extern const char *relay_log_basename;
int init_slave();
int init_recovery(Master_info* mi, const char** errmsg);
bool init_slave_skip_errors(const char* arg);
bool check_slave_skip_errors(sys_var *self, THD *thd, set_var *var);
bool update_slave_skip_errors(sys_var *self, THD *thd, enum_var_type type);
bool init_slave_transaction_retry_errors(const char* arg);
int register_slave_on_master(MYSQL* mysql);
int terminate_slave_threads(Master_info* mi, int thread_mask,
Expand Down
6 changes: 4 additions & 2 deletions sql/sys_vars.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6243,8 +6243,10 @@ static Sys_var_charptr Sys_slave_skip_errors(
"slave_skip_errors", "Tells the slave thread to continue "
"replication when a query event returns an error from the "
"provided list",
READ_ONLY GLOBAL_VAR(opt_slave_skip_errors), CMD_LINE(REQUIRED_ARG),
DEFAULT(0));
GLOBAL_VAR(opt_slave_skip_errors), CMD_LINE(REQUIRED_ARG),
DEFAULT("OFF"), NO_MUTEX_GUARD, NOT_IN_BINLOG,
ON_CHECK(check_slave_skip_errors),
ON_UPDATE(update_slave_skip_errors));

static Sys_var_on_access_global<Sys_var_ulonglong,
PRIV_SET_SYSTEM_GLOBAL_VAR_READ_BINLOG_SPEED_LIMIT>
Expand Down