Skip to content

MDEV-38849 slave_connections_needed_for_purge prevents independent machine from purging binary logs #5559

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
ParadoxV5 wants to merge 1 commit into
base: 11.4
Choose a base branch
from
Open

MDEV-38849 slave_connections_needed_for_purge prevents independent machine from purging binary logs #5559

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
1 change: 1 addition & 0 deletions mysql-test/include/mtr_warnings.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ INSERT INTO global_suppressions VALUES
("Slave: .*Duplicate entry"),

("Statement may not be safe to log in statement format"),
("slave_connections_needed_for_purge"),

/* innodb foreign key tests that fail in ALTER or RENAME produce this */
("InnoDB: Error: in ALTER TABLE `test`.`t[123]`"),
Expand Down
3 changes: 1 addition & 2 deletions mysql-test/main/mysqld--help.result
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1320,7 +1320,6 @@ The following specify which files/extra groups are read (specified before remain
Minimum number of connected slaves required for automatic
binary log purge with max_binlog_total_size,
binlog_expire_logs_seconds or binlog_expire_logs_days.
Default is 0 when Galera is enabled and 1 otherwise.
--slave-ddl-exec-mode=name
How replication events should be executed. Legal values
are STRICT and IDEMPOTENT (default). In IDEMPOTENT mode,
Expand Down Expand Up @@ -1954,7 +1953,7 @@ skip-networking FALSE
skip-show-database FALSE
skip-slave-start FALSE
slave-compressed-protocol FALSE
slave-connections-needed-for-purge 1
slave-connections-needed-for-purge 0
slave-ddl-exec-mode IDEMPOTENT
slave-domain-parallel-threads 0
slave-exec-mode STRICT
Expand Down
4 changes: 0 additions & 4 deletions mysql-test/suite/binlog/my.cnf
View file
Open in desktop

This file was deleted.

1 change: 0 additions & 1 deletion mysql-test/suite/binlog_encryption/binlog_index-master.opt
View file
Open in desktop

This file was deleted.

6 changes: 0 additions & 6 deletions mysql-test/suite/galera/r/basic.result
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,5 @@
connection node_2;
connection node_1;
select @@slave_connections_needed_for_purge;
@@slave_connections_needed_for_purge
0
select VARIABLE_NAME, GLOBAL_VALUE, GLOBAL_VALUE_ORIGIN from information_schema.system_variables where variable_name="slave_connections_needed_for_purge";
VARIABLE_NAME GLOBAL_VALUE GLOBAL_VALUE_ORIGIN
SLAVE_CONNECTIONS_NEEDED_FOR_PURGE 0 AUTO
USE test;
CREATE TABLE t1(c1 INT PRIMARY KEY) ENGINE=INNODB;
INSERT INTO t1 VALUES (1), (2), (3), (4), (5);
Expand Down
3 changes: 0 additions & 3 deletions mysql-test/suite/galera/t/basic.test
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,6 @@
--source include/galera_cluster.inc
--source include/have_innodb.inc

select @@slave_connections_needed_for_purge;
select VARIABLE_NAME, GLOBAL_VALUE, GLOBAL_VALUE_ORIGIN from information_schema.system_variables where variable_name="slave_connections_needed_for_purge";

USE test;
CREATE TABLE t1(c1 INT PRIMARY KEY) ENGINE=INNODB;
INSERT INTO t1 VALUES (1), (2), (3), (4), (5);
Expand Down
2 changes: 0 additions & 2 deletions mysql-test/suite/rpl/my.cnf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,3 @@
# E.g. after !include ../my.cnf, in your `test.cnf`, specify your configuration
# in option group e.g [mysqld.x], so that number `x` corresponds to the number
# in the rpl server topology.
[mariadbd]
slave_connections_needed_for_purge=0
25 changes: 25 additions & 0 deletions mysql-test/suite/rpl/r/warn_slaves_not_needed_for_purge.result
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
include/master-slave.inc
[connection master]
NOT FOUND /slave_connections_needed_for_purge/ in mysqld.1.err
connection slave;
START SLAVE IO_THREAD;
include/wait_for_slave_io_to_start.inc
FOUND 1 /slave_connections_needed_for_purge/ in mysqld.1.err
include/stop_slave_io.inc
START SLAVE IO_THREAD;
include/wait_for_slave_io_to_start.inc
include/stop_slave_io.inc
include/rpl_restart_server.inc [server_number=1 parameters: --slave-connections-needed-for-purge 0]
connection slave;
START SLAVE IO_THREAD;
include/wait_for_slave_io_to_start.inc
include/stop_slave_io.inc
include/rpl_restart_server.inc [server_number=1]
connection master;
SET @@GLOBAL.slave_connections_needed_for_purge= DEFAULT;
connection slave;
START SLAVE IO_THREAD;
include/wait_for_slave_io_to_start.inc
connection master;
FOUND 1 /slave_connections_needed_for_purge/ in mysqld.1.err
include/rpl_end.inc
52 changes: 52 additions & 0 deletions mysql-test/suite/rpl/t/warn_slaves_not_needed_for_purge.test
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
# MDEV-38849: Test the warning that the _default_ for
# `@@slave_connections_needed_for_purge` does not match the replication setup.
# (Do _not_ warn if `@@slave_connections_needed_for_purge` is manually set.)

--source include/have_binlog_format_mixed.inc # no actual binlog content
--let $rpl_skip_start_slave= 1
--source include/master-slave.inc


--let SEARCH_FILE= `SELECT @@log_error`
--let SEARCH_PATTERN= slave_connections_needed_for_purge
# Not connected: should not warn
--source include/search_pattern_in_file.inc


--connection slave
START SLAVE IO_THREAD;
--source include/wait_for_slave_io_to_start.inc
# Connected when left as default: should warn
--source include/search_pattern_in_file.inc


--source include/stop_slave_io.inc
START SLAVE IO_THREAD;
--source include/wait_for_slave_io_to_start.inc
# Reconnected: should not warn a second time

--source include/stop_slave_io.inc
--let $rpl_server_parameters= --slave-connections-needed-for-purge 0
--let $rpl_server_number= 1
--source include/rpl_restart_server.inc
--connection slave
START SLAVE IO_THREAD;
--source include/wait_for_slave_io_to_start.inc
# Explicitly set to 0 in server options: should not warn

--source include/stop_slave_io.inc
--let $rpl_server_parameters=
--source include/rpl_restart_server.inc
--connection master
SET @@GLOBAL.slave_connections_needed_for_purge= DEFAULT;
--connection slave
START SLAVE IO_THREAD;
--source include/wait_for_slave_io_to_start.inc
# Explicitly set to DEFAULT in system variables: should _not_ warn

--connection master
--source include/search_pattern_in_file.inc


--let $rpl_only_running_threads= 1
--source include/rpl_end.inc
2 changes: 1 addition & 1 deletion mysql-test/suite/sys_vars/r/sysvars_server_embedded.result
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3465,7 +3465,7 @@ COMMAND_LINE_ARGUMENT OPTIONAL
VARIABLE_NAME SLAVE_CONNECTIONS_NEEDED_FOR_PURGE
VARIABLE_SCOPE GLOBAL
VARIABLE_TYPE INT UNSIGNED
VARIABLE_COMMENT Minimum number of connected slaves required for automatic binary log purge with max_binlog_total_size, binlog_expire_logs_seconds or binlog_expire_logs_days. Default is 0 when Galera is enabled and 1 otherwise.
VARIABLE_COMMENT Minimum number of connected slaves required for automatic binary log purge with max_binlog_total_size, binlog_expire_logs_seconds or binlog_expire_logs_days.
NUMERIC_MIN_VALUE 0
NUMERIC_MAX_VALUE 4294967295
NUMERIC_BLOCK_SIZE 1
Expand Down
2 changes: 1 addition & 1 deletion mysql-test/suite/sys_vars/r/sysvars_server_notembedded.result
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4035,7 +4035,7 @@ COMMAND_LINE_ARGUMENT OPTIONAL
VARIABLE_NAME SLAVE_CONNECTIONS_NEEDED_FOR_PURGE
VARIABLE_SCOPE GLOBAL
VARIABLE_TYPE INT UNSIGNED
VARIABLE_COMMENT Minimum number of connected slaves required for automatic binary log purge with max_binlog_total_size, binlog_expire_logs_seconds or binlog_expire_logs_days. Default is 0 when Galera is enabled and 1 otherwise.
VARIABLE_COMMENT Minimum number of connected slaves required for automatic binary log purge with max_binlog_total_size, binlog_expire_logs_seconds or binlog_expire_logs_days.
NUMERIC_MIN_VALUE 0
NUMERIC_MAX_VALUE 4294967295
NUMERIC_BLOCK_SIZE 1
Expand Down
15 changes: 5 additions & 10 deletions sql/mysqld.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5956,16 +5956,6 @@ int mysqld_main(int argc, char **argv)
SYSVAR_AUTOSIZE(global_system_variables.binlog_format, BINLOG_FORMAT_ROW);
}
binlog_format_used= 1;
if (IS_SYSVAR_AUTOSIZE(&internal_slave_connections_needed_for_purge))
{
slave_connections_needed_for_purge=
internal_slave_connections_needed_for_purge= 0;
SYSVAR_AUTOSIZE(internal_slave_connections_needed_for_purge, 0);
sql_print_information(
"slave_connections_needed_for_purge changed to 0 because "
"of Galera. Change it to 1 or higher if this Galera node "
"is also Master in a normal replication setup");
Comment on lines -5964 to -5967

@ParadoxV5 ParadoxV5 Aug 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Should Galera slaves not count towards “when registering a slave”?
If not, what’s the definitive way to distinguish Galera mode?

}
}
#endif /* WITH_WSREP */

Expand Down Expand Up @@ -8353,6 +8343,11 @@ mysqld_get_one_option(const struct my_option *opt, const char *argument,
}

#ifdef HAVE_REPLICATION
case OPT_SLAVE_CONNECTIONS_NEEDED_FOR_PURGE:
warn_slaves_not_needed_for_purge.store(false,
std::memory_order_relaxed); // no other threads to sync with
break;

case (int)OPT_REPLICATE_IGNORE_DB:
{
cur_rpl_filter->add_ignore_db(argument);
Expand Down
1 change: 1 addition & 0 deletions sql/mysqld.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -828,6 +828,7 @@ enum options_mysqld
OPT_COSTS_ROWID_COPY_COST,
OPT_EXPIRE_LOGS_DAYS,
OPT_BINLOG_EXPIRE_LOGS_SECONDS,
OPT_SLAVE_CONNECTIONS_NEEDED_FOR_PURGE,
OPT_CONSOLE,
OPT_DEBUG_SYNC_TIMEOUT,
OPT_REMOVED_OPTION,
Expand Down
8 changes: 8 additions & 0 deletions sql/repl_failsafe.cc
View file
Open in desktop

@ParadoxV5 ParadoxV5 Aug 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Implementation is tricky because we don’t want to warn multiple times when the only slave disconnects and reconnects.

My preferred solution is using a std::atomic<bool>.
It starts with “do warn” and becomes “don’t warn” upon warning or when @@slave_connections_needed_for_purge is changed.

However, “when changed” turned out to be impractical to implement, for Server Options and System Variables don’t actually share code; specifically, Options do not call Variables’ ON_CHECK/ON_UPDATE callbacks.
Instead, as shown in mysqld.cc and sys_vars.cc, there must be additional copies of code to cover the Options.
Although this design does offer flexibility, such as how options don’t need to worry about synchronizing user threads, it is still inherently error-prone and has led me on a wild goose chase.

I also had the alternative of a call-once block, implemented with the initialization of a function-local static.

  • ➕ In the absence of infrastructure improvements, abandoning changing a state in Options/Variables leaves this site as the only accessor of this std::atomic<bool>, so a self-contained solution is suitable.
    • ➕ It does not even require this explicit std::atomic<bool> anymore, since the compiler will include one.
  • Its main downside on paper is the lacklustre expressiveness, but not like our ancient code is much more expressive either.
  • ➖ In practice, though, I don’t have another way to quickly distinguish whether the variable is left as default.
    The statistic would be sys_var::value_origin, but sys_var requires mutex synchronization, which is why IS_SYSVAR_AUTOSIZE() is banned after server startup.

Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ struct Slave_info
};


std::atomic<bool> warn_slaves_not_needed_for_purge= {true};
Atomic_counter<uint32_t> binlog_dump_thread_count;
ulong rpl_status=RPL_NULL;
mysql_mutex_t LOCK_rpl_status;
Expand Down Expand Up @@ -120,6 +121,13 @@ int THD::register_slave(uchar *packet, size_t packet_length)
Slave_info *si;
uchar *p= packet, *p_end= packet + packet_length;
const char *errmsg= "Wrong parameters to function register_slave";
if (variables.log_warnings >= 1 && // Verbosity Level "Binlog/Replication"
warn_slaves_not_needed_for_purge.exchange(false,
// no need to sync with `@@slave_connections_needed_for_purge` itself
std::memory_order_relaxed))

@ParadoxV5 ParadoxV5 Aug 22, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

std::memory_order_relaxed should be sufficient here, since the only ordering that relates to this flag is the log output.

  • && is not reörderable, rïght?
  • This procedure doesn’t need to care about whether @@slave_connections_needed_for_purge is actually still 0.
    As the variable currently stands, it’d need mutex synchronization to care, too.

sql_print_warning(
"`@@slave_connections_needed_for_purge` defaults to 0, which means "
"this master does not retain older logs even if slaves need them.");

if (check_access(this, PRIV_COM_REGISTER_SLAVE, any_db.str, NULL,NULL,0,0))
return 1;
Expand Down
9 changes: 9 additions & 0 deletions sql/sql_repl.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,15 @@

struct slave_connection_state;

/**
Whether to log a warning once about an unfit
@ref slave_connections_needed_for_purge upon @ref COM_REGISTER_SLAVE
* Starts as `true` (do warn), and becomes `false`
(don't warn) when warning for the first time
* Also becomes `false` when the user intetionally sets
`@@slave_connections_needed_for_purge`
*/
extern std::atomic<bool> warn_slaves_not_needed_for_purge;
extern my_bool opt_show_slave_auth_info;
extern char *master_host, *master_info_file;

Expand Down
22 changes: 16 additions & 6 deletions sql/sys_vars.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1326,13 +1326,23 @@ Sys_slave_connections_needed_for_purge(
"slave_connections_needed_for_purge",
"Minimum number of connected slaves required for automatic binary "
"log purge with max_binlog_total_size, binlog_expire_logs_seconds "
"or binlog_expire_logs_days. Default is 0 when Galera is enabled and 1 "
"otherwise.",
"or binlog_expire_logs_days.",
GLOBAL_VAR(internal_slave_connections_needed_for_purge),
CMD_LINE(REQUIRED_ARG),
VALID_RANGE(0, UINT_MAX), DEFAULT(1), BLOCK_SIZE(1),
NO_MUTEX_GUARD, NOT_IN_BINLOG, ON_CHECK(0),
ON_UPDATE(update_binlog_space_limit));
CMD_LINE(REQUIRED_ARG, OPT_SLAVE_CONNECTIONS_NEEDED_FOR_PURGE),
VALID_RANGE(0, UINT_MAX), DEFAULT(0), BLOCK_SIZE(1),
NO_MUTEX_GUARD, NOT_IN_BINLOG,
#ifdef HAVE_REPLICATION
ON_CHECK([](sys_var *, THD *, set_var *)
{
warn_slaves_not_needed_for_purge.store(false,
// no need to sync with the variable itself
std::memory_order_relaxed);
return false;
})
#else
ON_CHECK(0)
#endif
, ON_UPDATE(update_binlog_space_limit));


static Sys_var_mybool Sys_flush(
Expand Down