MDEV-38497 Remove all slave related configurations on RESET SLAVE ALL#4641
MDEV-38497 Remove all slave related configurations on RESET SLAVE ALL#4641deo002 wants to merge 1 commit intoMariaDB:12.3from
Conversation
deo002
force-pushed
the
fix/reset_slave_all
branch
from
41c2e7f to
ce7d020
Compare
Fix RESET SLAVE ALL to fully clear replica configuration of the default unnamed slave, including SSL options, so subsequent CHANGE MASTER starts from a clean state. Signed-off-by: Dearsh Oberoi <oberoidearsh@gmail.com>
deo002
force-pushed
the
fix/reset_slave_all
branch
from
ce7d020 to
3bbf843
Compare
gkodinov
added
the
External Contribution
ParadoxV5
left a comment
ParadoxV5
left a comment
There was a problem hiding this comment.
Hello~ Thanks for taking this on. Here’s some early feedback before I go on break.
| mi->master_ssl_ca= nullptr; mi->master_ssl_capath= nullptr; mi->master_ssl_cert= nullptr; | ||
| mi->master_ssl_cipher= nullptr; mi->master_ssl_key= nullptr; mi->master_ssl_crl= nullptr; | ||
| mi->master_ssl_crlpath= nullptr; |
There was a problem hiding this comment.
I wonder if we should fix the earlier LTS versions as well.
In either case, this section is unique to a refactor on 12.3.
| init_ssl_config(this); | ||
| init_connection_config(this); | ||
| init_group_counters_config(this); |
There was a problem hiding this comment.
The project isn’t just about adding clears to RESET SLAVE, but also checking which one is done where and whether they could’ve been the same.
There was a problem hiding this comment.
I thought I saw RESET SLAVE already tested somewhere.
Maybe it didn’t cover RESET SLAVE ALL well.
| --source include/master-slave.inc | ||
|
|
||
| connection slave; | ||
|
|
||
| STOP REPLICA; | ||
| --source include/wait_for_slave_io_to_stop.inc | ||
| --source include/wait_for_slave_sql_to_stop.inc |
There was a problem hiding this comment.
There’s stop_slave.inc.
Though master-slave.inc has a parameter that skips starting the slave.
But really, since this test doesn’t require a master server or any actual replication work, it could be directly in the main suite.
| # Cleanup | ||
| RESET REPLICA ALL; | ||
|
|
||
| # --source include/rpl_end.inc |
There was a problem hiding this comment.
- Please use
SLAVEoverREPLICAto match the other tests for the time being. - Perhaps the second RESET is for clearing the
MASTER_HOST='localhost'part.
But MTR is okay if you leave certain configurations set to their defaults. - Interesting that the CI kept working with
rpl_end.inccommented out.
Well, at least until thisrpl.rpl_slave_statusyou reported on Zulip.
3 participants
Description
RESET SLAVE ALL on the default unnamed slave does not all clear of its configurations from the server. So, certain configurations get carried onto the subsequent CHANGE MASTER.
This PR fixes it with a proper clean up of all the config options.
Basing the PR against the correct MariaDB version
mainbranch.PR quality check