Li-Bing.Song@stripped wrote:
> #At file:///home/anders/work/bzrwork/September/bug13963/mysql-next-bugfixing/ based
> on revid:alik@stripped
>
> 2818 Li-Bing.Song@stripped 2009-09-24
> Bug #13963 SHOW SLAVE HOSTS is unreliable
>
> Slaves only appear in the output of SHOW SLAVE HOSTS when report-host option
> is set. If an expected slave does not appear in the list, nobody knows
> whether the slave does not connect or has started without the "report-host"
> option. The output also contains a strange field "Rpl_recovery_rank" which has
>
> never been implemented and the manual of MySQL6.0 declares that the field has
> been removed from MySQL6.0.
>
Please add use past tense or use 'before the patch' to make it clear
that what you described above is the old behavior.
BTW: There is no MySQL 6.0 manual any more, please replace it with 5.4
> This patch is done with these,
> According to the manual of MySQL6.0, "Rpl_recovery_rank" is removed.
> Slaves will register themself to master no matter if report_host option is set
themselves
> or not. When slaves are registering themself, their Server_ids, report_host
> and other information are together sent to master. Sever_ids are never null
> and is unique in one replication group. Slaves always can be identified with
> different Server_ids no matter if report_host exists.
> @ mysql-test/suite/rpl/r/rpl_mixed_ddl_dml.result
> 'Rpl_recovery_rank' field has been removed.
> @ sql/repl_failsafe.cc
> rpl_recovery_rank has been removed.
> @ sql/slave.cc
>
>
> added:
> mysql-test/include/wait_show_condition_all.inc
> mysql-test/suite/rpl/t/rpl_show_slave_hosts.cnf
> mysql-test/suite/rpl/t/rpl_show_slave_hosts.test
rpl_show_slave_hosts.result file is missing :)
> modified:
> mysql-test/suite/rpl/r/rpl_mixed_ddl_dml.result
> sql/repl_failsafe.cc
> sql/slave.cc
> === added file 'mysql-test/include/wait_show_condition_all.inc'
> --- a/mysql-test/include/wait_show_condition_all.inc 1970-01-01 00:00:00 +0000
> +++ b/mysql-test/include/wait_show_condition_all.inc 2009-09-24 09:35:20 +0000
Please considering adding an variable to wait_show_condition.inc to
control whether we want to wait for one or all rows to satisfy the
condition.
> @@ -0,0 +1,77 @@
> +# include/wait_show_condition_all.inc
> +#
> +# SUMMARY
> +#
> +# Waits until the show statement ($show_statement) has all of the rows of
> +# the result set for the field ($field) a value which fulfils a condition
> +# ($condition), or the operation times out.
> +#
> +#
> +# USAGE
> +#
> +# let $show_statement= SHOW PROCESSLIST; let $field= State;
> +# let $condition= = 'Updating';
> +# --source include/wait_show_condition_not.inc
> +#
> +# OR
> +#
> +# let $wait_timeout= 60; # Override default of 30 seconds with 60.
> +# let $show_statement= SHOW PROCESSLIST;
> +# let $field= State;
> +# let $condition= = 'Updating';
> +# --source include/wait_show_condition.inc
> +#
> +# Please do not use this use routine if you can replace the SHOW statement
> +# with a select. In such a case include/wait_condition.inc is recommended.
> +#
> +# Created: 2009-02-18 mleich
> +#
> +
> +let $max_run_time= 30;
> +if ($wait_timeout)
> +{
> + let $max_run_time= $wait_timeout;
> +}
> +# Reset $wait_timeout so that its value won't be used on subsequent
> +# calls, and default will be used instead.
> +let $wait_timeout= 0;
> +
> +# The smallest timespan till UNIX_TIMESTAMP() gets incremented is ~0 seconds.
> +# We add one second to avoid the case that somebody measures timespans on a
> +# real clock with fractions of seconds, detects that n seconds are sufficient,
> +# assigns n to this routine and suffers because he sometimes gets n - 1
> +# seconds in reality.
> +inc $max_run_time;
> +
> +let $found_all= 0;
> +let $max_end_time= `SELECT UNIX_TIMESTAMP() + $max_run_time`;
> +while (`SELECT UNIX_TIMESTAMP() <= $max_end_time AND $found_all = 0`)
> +{
> + # Sleep a bit to avoid too heavy load.
> + real_sleep 0.2;
> + let $rowno= 1;
> + let $process_result= 1;
> + while (`SELECT $process_result = 1 AND $found_all = 0`)
> + {
> + let $field_value= query_get_value($show_statement, $field, $rowno);
> + if (`SELECT '$field_value' = 'No such row'`)
> + {
> + # We are behind the last row of the result set.
> + let $found_all= 1;
> + }
> + if (`SELECT $found_all = 0 AND NOT '$field_value' $condition`)
> + {
> + let process_result= 0;
> + }
> + inc $rowno;
> + }
> +}
> +if (!$found_all)
> +{
> + echo # Timeout in include/wait_show_condition.inc for $wait_condition;
> + echo # show_statement : $show_statement;
> + echo # field : $field;
> + echo # condition : $condition;
> + echo # max_run_time : $max_run_time;
> +}
> +
>
> === modified file 'mysql-test/suite/rpl/r/rpl_mixed_ddl_dml.result'
> --- a/mysql-test/suite/rpl/r/rpl_mixed_ddl_dml.result 2008-11-04 17:07:14 +0000
> +++ b/mysql-test/suite/rpl/r/rpl_mixed_ddl_dml.result 2009-09-24 09:35:20 +0000
> @@ -13,8 +13,8 @@ n
> 2001
> 2002
> show slave hosts;
> -Server_id Host Port Rpl_recovery_rank Master_id
> -2 127.0.0.1 9999 0 1
> +Server_id Host Port Master_id
> +2 127.0.0.1 9999 1
> drop table t1;
> stop slave;
> create table t2(id int auto_increment primary key, created datetime);
>
> === added file 'mysql-test/suite/rpl/t/rpl_show_slave_hosts.cnf'
> --- a/mysql-test/suite/rpl/t/rpl_show_slave_hosts.cnf 1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_show_slave_hosts.cnf 2009-09-24 09:35:20 +0000
> @@ -0,0 +1,23 @@
> +!include ../my.cnf
> +
> +[mysqld.1]
> +server_id=1
> +mysql-backup=1
> +
> +[mysqld.2]
> +server_id=2
> +mysql-backup=1
> +report-host=
> +report-user=
> +
> +[mysqld.3]
> +server_id=3
> +mysql-backup=1
> +report-host=slave2
> +slave-net-timeout=5
> +
> +[ENV]
> +SLAVE_MYPORT2= @mysqld.3.port
> +SLAVE_MYSOCK2= @mysqld.3.socket
> +
> +
>
> === added file 'mysql-test/suite/rpl/t/rpl_show_slave_hosts.test'
> --- a/mysql-test/suite/rpl/t/rpl_show_slave_hosts.test 1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_show_slave_hosts.test 2009-09-24 09:35:20 +0000
> @@ -0,0 +1,43 @@
> +###############################################################################
> +# Bug#13963 SHOW SLAVE HOSTS is unreliable
> +#
> +# Slaves only appear in the output of SHOW SLAVE HOSTS when report-host option
> +# is set. If an expected slave does not appear in the list, nobody knows
> +# whether the slave does not connect or has started without the "report-host"
> +# option.
> +#
> +# Remove the "Rpl_recovery_rank" column from SHOW SLAVE HOSTS, It is not
> +# implemented.
> +#######################################################################
> +source include/master-slave.inc;
> +connect (slave2,127.0.0.1,root,,test,$SLAVE_MYPORT2,);
> +
> +connection slave2;
> +RESET SLAVE;
> +--eval CHANGE MASTER TO
> master_host='127.0.0.1',master_port=$MASTER_MYPORT,master_user='root'
> +START SLAVE IO_THREAD;
> +source include/wait_for_slave_io_to_start.inc;
> +
> +connection master;
> +let $show_statement= SHOW SLAVE HOSTS;
> +let $field= Server_id;
> +# 3 is server_id of slave2.
> +let $connection= ='3';
> +source include/wait_show_condition.inc;
> +SHOW SLAVE HOSTS;
> +
> +connection slave2;
> +STOP SLAVE IO_THREAD;
> +source include/wait_for_slave_io_to_stop.inc;
> +
> +connection master;
> +let $show_statement= SHOW SLAVE HOSTS;
> +let $field= Server_id;
> +# 3 is server_id of slave2.
> +let $condition= <> '3';
> +# All rows of 'SHOW SLAVE HOSTS' are not equal to 3. It mean that master has
> +# knew the leave of slave2 and has unregistered it.
> +source include/wait_show_condition_all.inc;
> +SHOW SLAVE HOSTS;
> +
> +source include/master-slave-end.inc;
>
> === modified file 'sql/repl_failsafe.cc'
> --- a/sql/repl_failsafe.cc 2009-05-15 13:45:06 +0000
> +++ b/sql/repl_failsafe.cc 2009-09-24 09:35:20 +0000
> @@ -183,12 +183,11 @@ int register_slave(THD* thd, uchar* pack
> get_object(p,si->host, "Failed to register slave: too long 'report-host'");
> get_object(p,si->user, "Failed to register slave: too long 'report-user'");
> get_object(p,si->password, "Failed to register slave; too long
> 'report-password'");
> - if (p+10 > p_end)
> + /*6 is the total length of port and master_id*/
> + if (p+6 != p_end)
> goto err;
> si->port= uint2korr(p);
> p += 2;
> - si->rpl_recovery_rank= uint4korr(p);
> - p += 4;
> if (!(si->master_id= uint4korr(p)))
> si->master_id= server_id;
> si->thd= thd;
> @@ -670,8 +669,6 @@ bool show_slave_hosts(THD* thd)
> field_list.push_back(new Item_empty_string("Password",20));
> }
> field_list.push_back(new Item_return_int("Port", 7, MYSQL_TYPE_LONG));
> - field_list.push_back(new Item_return_int("Rpl_recovery_rank", 7,
> - MYSQL_TYPE_LONG));
> field_list.push_back(new Item_return_int("Master_id", 10,
> MYSQL_TYPE_LONG));
>
> @@ -693,7 +690,6 @@ bool show_slave_hosts(THD* thd)
> protocol->store(si->password, &my_charset_bin);
> }
> protocol->store((uint32) si->port);
> - protocol->store((uint32) si->rpl_recovery_rank);
> protocol->store((uint32) si->master_id);
> if (protocol->write())
> {
>
> === modified file 'sql/slave.cc'
> --- a/sql/slave.cc 2009-09-14 16:03:38 +0000
> +++ b/sql/slave.cc 2009-09-24 09:35:20 +0000
> @@ -1672,13 +1672,12 @@ int register_slave_on_master(MYSQL* mysq
> bool *suppress_warnings)
> {
> uchar buf[1024], *pos= buf;
> - uint report_host_len, report_user_len=0, report_password_len=0;
> + uint report_host_len=0, report_user_len=0, report_password_len=0;
> DBUG_ENTER("register_slave_on_master");
>
> *suppress_warnings= FALSE;
> - if (!report_host)
> - DBUG_RETURN(0);
> - report_host_len= strlen(report_host);
> + if (report_host)
> + report_host_len= strlen(report_host);
> if (report_user)
> report_user_len= strlen(report_user);
> if (report_password)
> @@ -1686,14 +1685,19 @@ int register_slave_on_master(MYSQL* mysq
> /* 30 is a good safety margin */
> if (report_host_len + report_user_len + report_password_len + 30 >
> sizeof(buf))
> + {
> + sql_print_warning("The total length of report_host, report_user and "
> + "report_password is %d. It is larger than the max length(%d),
> so this "
> + "slave cannot be registered to the master.",
> + report_host_len + report_user_len + report_password_len,
> + sizeof(buf) - 30);
I'd suggest to check the length of each part does not exceed the limit.
Please check the struct st_slave_info:
...
char host[HOSTNAME_LENGTH+1];
char user[USERNAME_LENGTH+1];
char password[MAX_PASSWORD_LENGTH+1];
...
> DBUG_RETURN(0); // safety
> -
> + }
> int4store(pos, server_id); pos+= 4;
> pos= net_store_data(pos, (uchar*) report_host, report_host_len);
> pos= net_store_data(pos, (uchar*) report_user, report_user_len);
> pos= net_store_data(pos, (uchar*) report_password, report_password_len);
> int2store(pos, (uint16) report_port); pos+= 2;
> - int4store(pos, rpl_recovery_rank); pos+= 4;
> /* The master will fill in master_id */
> int4store(pos, 0); pos+= 4;
>
>
