-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
MDEV-19248 Implement MASTER_BIND for CHANGE MASTER #3883
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
# Is `''` when not specified | ||
CHANGE MASTER 'named' TO master_host='example.com'; | ||
CHANGE MASTER TO master_host='127.0.0.1', master_ssl_verify_server_cert=0; | ||
SELECT Connection_name, Master_Bind FROM information_schema.SLAVE_STATUS; | ||
Connection_name Master_Bind | ||
|
||
named | ||
# Replace when specified | ||
CHANGE MASTER 'named' TO master_bind='localhost'; | ||
CHANGE MASTER TO master_bind='127.0.0.1'; | ||
SELECT Connection_name, Master_Bind FROM information_schema.SLAVE_STATUS; | ||
Connection_name Master_Bind | ||
127.0.0.1 | ||
named localhost | ||
# SHOW SLAVE STATUS also show the configuations | ||
Connection_name = '' | ||
Connection_name = 'named' | ||
Master_Bind = '127.0.0.1' | ||
Master_Bind = 'localhost' | ||
Master_Bind = '127.0.0.1' | ||
Master_Bind = 'localhost' | ||
# Restore specified config on restart | ||
# restart: --skip-slave-start | ||
SELECT Connection_name, Master_Bind FROM information_schema.SLAVE_STATUS; | ||
Connection_name Master_Bind | ||
127.0.0.1 | ||
named localhost | ||
# Keep specified config on RESET REPLICA | ||
RESET REPLICA 'named'; | ||
RESET REPLICA; | ||
SELECT Connection_name, Master_Bind FROM information_schema.SLAVE_STATUS; | ||
Connection_name Master_Bind | ||
127.0.0.1 | ||
named localhost | ||
# Don't replace when not specified | ||
CHANGE MASTER TO master_user='root'; | ||
CHANGE MASTER 'named' TO master_user='root'; | ||
SELECT Connection_name, Master_Bind FROM information_schema.SLAVE_STATUS; | ||
Connection_name Master_Bind | ||
127.0.0.1 | ||
named localhost | ||
# Numeric | ||
CHANGE MASTER TO master_bind=10; | ||
ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '10' at line 1 | ||
CHANGE MASTER 'named' TO master_bind=11; | ||
ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '11' at line 1 | ||
# `''` also counts as specified | ||
CHANGE MASTER TO master_bind=''; | ||
CHANGE MASTER 'named' TO master_bind=''; | ||
SELECT Connection_name, Master_Bind FROM information_schema.SLAVE_STATUS; | ||
Connection_name Master_Bind | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can add a test with a non-existing network interface value. SHOW SLAVE STATUS should error out. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Be it interface or address, currently the field doesn’t check its string value and rely on the replicating client to check its options at START REPLICA. And I think that’s fine: technically one might upgrade their hardware upgrade inbetween CHANGE MASTER nand START REPLICA. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok. Shall cover it at QA stage. |
||
named | ||
# Cleanup | ||
RESET REPLICA 'named' ALL; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
# MDEV-19248: Test the `Master_Bind` field of | ||
# CHANGE MASTER [name] TO and SHOW SLAVE [name] STATUS & co. (no feature testing) | ||
# Two connections tests that the field is per-connection. | ||
|
||
--echo # Is `''` when not specified | ||
CHANGE MASTER 'named' TO master_host='example.com'; | ||
CHANGE MASTER TO master_host='127.0.0.1', master_ssl_verify_server_cert=0; | ||
SELECT Connection_name, Master_Bind FROM information_schema.SLAVE_STATUS; | ||
|
||
--echo # Replace when specified | ||
CHANGE MASTER 'named' TO master_bind='localhost'; | ||
# Default master does not replace named master | ||
CHANGE MASTER TO master_bind='127.0.0.1'; | ||
SELECT Connection_name, Master_Bind FROM information_schema.SLAVE_STATUS; | ||
|
||
--echo # SHOW SLAVE STATUS also show the configuations | ||
--let $all_slaves_status= 1 | ||
--let $status_items= Connection_name, Master_Bind | ||
--source include/show_slave_status.inc | ||
--let $all_slaves_status= 0 | ||
--let $status_items= Master_Bind | ||
--source include/show_slave_status.inc | ||
--let $slave_name= 'named' | ||
--source include/show_slave_status.inc | ||
|
||
--echo # Restore specified config on restart | ||
--let $restart_parameters= --skip-slave-start | ||
--source include/restart_mysqld.inc # not_embedded | ||
SELECT Connection_name, Master_Bind FROM information_schema.SLAVE_STATUS; | ||
|
||
--echo # Keep specified config on RESET REPLICA | ||
RESET REPLICA 'named'; | ||
RESET REPLICA; | ||
SELECT Connection_name, Master_Bind FROM information_schema.SLAVE_STATUS; | ||
|
||
--echo # Don't replace when not specified | ||
CHANGE MASTER TO master_user='root'; | ||
CHANGE MASTER 'named' TO master_user='root'; | ||
SELECT Connection_name, Master_Bind FROM information_schema.SLAVE_STATUS; | ||
|
||
--echo # Numeric | ||
--error ER_PARSE_ERROR | ||
CHANGE MASTER TO master_bind=10; | ||
--error ER_PARSE_ERROR | ||
CHANGE MASTER 'named' TO master_bind=11; | ||
|
||
--echo # `''` also counts as specified | ||
CHANGE MASTER TO master_bind=''; | ||
CHANGE MASTER 'named' TO master_bind=''; | ||
SELECT Connection_name, Master_Bind FROM information_schema.SLAVE_STATUS; | ||
|
||
--echo # Cleanup | ||
RESET REPLICA 'named' ALL; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO master_bind is for network interface name. But here in the test we've IP address and hostname instead. Shouldn't we test with network interface name?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good call. Why did MySQL document as and test with IP addresses 😕?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't find any MySQL test which does MASTER_BIND testing. But the doc https://dev.mysql.com/doc/refman/8.0/en/change-master-to.html says MASTER_BIND = 'interface_name'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one linked in PR description mysql/mysql-server@9174364, or its version on HEAD https://github.com/mysql/mysql-server/blob/6b6d3ed/mysql-test/common/rpl/change_replication_source.test#L285-L313
P.S. and https://dev.mysql.com/doc/refman/9.2/en/change-replication-source-to.html#crs-opt-source_bind
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the pointers. MASTER_BIND is network interface IP address only. The doc should have MASTER_BIND = 'interface_ip_address' instead. But do we need MASTER_HOST if MASTER_BIND is set?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MASTER_BIND
= address of this replica to connect with (optional)MASTER_HOST
= address of the primary to connect to (mandatory)