MDEV-19248 Implement MASTER_BIND for CHANGE MASTER #3883
Conversation
Add CHANGE MASTER TO & SHOW REPLICA STATUS field `Master_Bind` that configures the `MYSQL_OPT_BIND` of the embedded replication client This is a port of MySQL 5.6.2’s WL#3127.
| SELECT Connection_name, Master_Bind FROM information_schema.SLAVE_STATUS; | ||
| Connection_name Master_Bind | ||
| 127.0.0.1 | ||
| named localhost |
There was a problem hiding this comment.
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.
Good call. Why did MySQL document as and test with IP addresses 😕?
There was a problem hiding this comment.
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.
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.
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.
MASTER_BIND= address of this replica to connect with (optional)MASTER_HOST= address of the primary to connect to (mandatory)
| 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.
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.
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.
Ok. Shall cover it at QA stage.
There was a problem hiding this comment.
Unfortunately the underlying code doesn't actually work. I set up a network namespace and a veth connection to it, started a mariadb instance (as a primary) in the namespace, and then tried to set up replication to the server using MASTER_BIND using my regular NIC and again using the veth NIC, and unfortunately it always picked the veth NIC, no matter the MASTER_BIND value. Then I tried using MySQL 8.0 and its MASTER_BIND, and the correct interface was always chosen.
Looking into our sources, it looks like MYSQL_OPT_BIND might be a dead option. We'll have to investigate further in the next feature development cycle.
There was a problem hiding this comment.
The code generally looks good. But as Brandon points out, there is a problem
in libmariadb that prevents this from working currently.
You can see the problem in the function pvio_socket_connect() in
libmariadb/plugins/pvio/pvio_socket.c . The code actually tries to do so:
/* if client has multiple interfaces, we will bind socket to given
* bind_address */
if (cinfo->mysql->options.bind_address)
{
wait_gai= 1;
while ((gai_rc= getaddrinfo(cinfo->mysql->options.bind_address, 0,
&hints, &bind_res)) == EAI_AGAIN)
{
unsigned int timeout= mysql->options.connect_timeout ?
mysql->options.connect_timeout : DNS_TIMEOUT;
if (time(NULL) - start_t > (time_t)timeout)
break;
#ifndef _WIN32
usleep(wait_gai);
#else
Sleep(wait_gai);
#endif
wait_gai*= 2;
}
if (gai_rc != 0 || !bind_res)
{
PVIO_SET_ERROR(cinfo->mysql, CR_BIND_ADDR_FAILED, SQLSTATE_UNKNOWN,
CER(CR_BIND_ADDR_FAILED), cinfo->mysql->options.bind_address, gai_rc);
goto error;
}
}It looks up the specified address. But it looks like the author of the code
forgot to actually call bind(2) with the looked up results!
I think you should report this as a bug for libmariadb (CONC in Jira).
You might also try fixing the bug in the libmariadb and submitting a pull
request for that to Georg? It looks like it might simply be calling bind()
in the above code, passing the looked up address and address lenght.
And once libmariadb is fixed, we clearly need some testing done. Maybe
Brandon can help you with how he set up multiple interfaces. Basically it
needs to be tested that bind can be used and verified that traffic goes on
the specified interface. Maybe it can maybe be done simply by specifying
either the local (127.0.0.1) or the wifi/ethernet card of the machine, and
checking with tcpdump that the packets have the right client ip address.
I'm not sure an mtr testcase in the main suite is feasible, too many
assumptions needed about the environment (mtr tests are run by some users
too). Maybe with some --source include/have_multi_interface.inc, but it
might be tricky to get it to work.
I think it's ok to test manually, and describe briefly in commit comment or
pull request how it was tested.
|
I thought that replication code doesn't use libmariadb. And server implementation of the client library seems to ignore MYSQL_OPT_BIND |
|
Hm, yes Serg, now that you mention it that's what I remember too. And looking closer at the libmariadb code, it does in fact seem to call bind() correctly with the bind_address. So seems I was just confused, and it's the server-side protocol code that needs fixing to support client-side bind |
Which is interesting as I could only find some functions I grepped in libmariadb. I thought Server includes libmariadb to reuse its Client code. |
Description & Release Notes
Add CHANGE MASTER TO & SHOW REPLICA STATUS field
Master_Bindthat configures theMYSQL_OPT_BINDof the embedded replication clientThe
mariadboptionMYSQL_OPT_BIND(--bind-addresswith MDEV-21465) chooses the network interface (e.g., eternet port) that connects to the server (the Master in this case).What problem is the patch trying to solve?
Knowledge Base pages that need changing
How can this PR be tested?
good question.
TODO: modify the automated test suite to verify that the PR causes MariaDB to behave as intended.
If the changes are not amenable to automated testing, please explain why not and carefully describe how to test manually.
PR quality check
mainbranch.This is a bug fix, and the PR is based against the earliest maintained branch in which the bug can be reproduced.