Bug 1551634: xtrabackup --slave-info does not handle multi-master replication #362
Conversation
| @@ -265,6 +276,18 @@ free_mysql_variables(mysql_variable *vars) | |||
| } | |||
| } | |||
|
|
|||
| static | |||
| void | |||
| free_and_zero_mysql_variables(mysql_variable *vars) | |||
There was a problem hiding this comment.
Why add new function instead of making free_mysql_variables to zero values?
| {NULL, NULL} | ||
| }; | ||
|
|
||
| mysql_variable variables[] = { | ||
| {"gtid_slave_pos", >id_slave_pos}, | ||
| {NULL, NULL} | ||
| }; | ||
|
|
||
| read_mysql_variables(connection, "SHOW SLAVE STATUS", status, false); | ||
| // read_mysql_variables(connection, "SHOW SLAVE STATUS", status, false); |
| "SHOW SLAVE STATUS", true, true); | ||
| int masters_count = 0; | ||
| while (read_mysql_variables_from_result(slave_status_res, status, | ||
| false) && ++masters_count) { |
There was a problem hiding this comment.
Incrementing counter inside of the loop body looks more straightforward to me
| ut_a(asprintf(&mysql_slave_position, | ||
| "master host '%s', filename '%s', position '%s'", | ||
| master, filename, position) != -1); | ||
| if (mysql_slave_position) { |
There was a problem hiding this comment.
Probably C++ strings will be easier to use and I see no reason not to...
| "CHANGE MASTER TO MASTER_LOG_FILE='%s', " | ||
| "MASTER_LOG_POS=%s %s;\n", | ||
| filename, position, channel_info); | ||
| ut_a(asprintf(&tmp_mysql_slave_position, |
There was a problem hiding this comment.
Maybe include channel_name as well?
| "CHANGE MASTER TO MASTER_AUTO_POSITION=1 %s;\n", | ||
| channel_info); | ||
|
|
||
| ut_a(asprintf(&tmp_mysql_slave_position, |
There was a problem hiding this comment.
Maybe include channel_name as well?
| START SLAVE | ||
| MASTER_PORT=${SRV_MYSQLD_PORT[$master_id_arg]} | ||
| $extra | ||
| FOR CHANNEL 'master-$master_id_arg'; |
There was a problem hiding this comment.
This won't work on older servers, we need to make sure they are treated well
There was a problem hiding this comment.
Fixed, now one have to explicitly specify USE_CHANNELS as an argument to the setup_slave in order to get named channels in CHANGE MASTER statement.
There was a problem hiding this comment.
Added explicit argument USE_CHANNELS to the setup_slave.
| vlog "Backing up slave" | ||
| switch_server $slave_id | ||
|
|
||
| innobackupex --no-timestamp --slave-info --safe-slave-backup $topdir/backup |
There was a problem hiding this comment.
Lets replace with xtrabackup. innobackupex is deprecated and should be removed in future major releases.
| multi_row_insert test.m2 \({1..13}\) | ||
|
|
||
| start_server_with_id $slave_id \ | ||
| --master-info-repository=TABLE \ |
There was a problem hiding this comment.
Why do we need to set master-info-repository and relay-info-repository for this test?
There was a problem hiding this comment.
That is required to have multiple channels on slave. If we start slave server without both --master-info-repository=TABLE and --relay-log-info-repository=TABLE \, then setting up server as slave with CHANGE MASTER ... FOR CHANNEL '...' fails with:
ERROR 3077 (HY000) at line 1: To have multiple channels, repository cannot be of type FILE; Please check the repository configuration and convert them to TABLE.
VasilyNemkov
force-pushed
the
2.4-xb-bug1551634
branch
from
78b9290
to
decb34e
Compare
…lication Fixed xtrabackup_slave_info for multi-source replication: providing master info and channel name for each channel. Added a test case.
VasilyNemkov
force-pushed
the
2.4-xb-bug1551634
branch
from
decb34e
to
2551b14
Compare
|
Fixed, new param build: http://jenkins.percona.com/view/PXB%202.3/job/percona-xtrabackup-2.4-param/187/ |
|
Thanks. Looks good to me. |
Writing master info for every channel to xtrabackup_slave_info;
Added a test case that verifies multi-source replication for both binlog and gtid mode.
https://bugs.launchpad.net/percona-xtrabackup/+bug/1551634