Hi Leonard,
See a in-line comment.
Leonard zhou wrote:
> Hi all,
>
> On the discuss today, we have another solution for this bug, that is
> "add a new command SQLCOM_SHOW_RELAYLOGS, and new function
> mysql_show_relay_logs.".
>
> But the function mysql_show_binlog_events() is more than 150 lines and
> if we add new function, most of content of them are the same. Only 3 or
> 4 lines are diffrent.
>
> To avoid adding more duplicate codes i think it is better that we add
> new command SQLCOM_SHOW_RELAYLOGS but use the same function(maybe we can
> change the name) and add a flag in the LEX_MASTER_INFO, in order to
> distinguish between relaylog and binlog in mysql_show_binlog_events().
>
> What do you think of this?
>
You can also keep two functions and refactory the
mysql_show_binlog_events()
to remove what is common and put into a third private/static function.
It is up to you. I have just on comment:
1 - Try to make the old function mysql_show_binlog_events() more generic
in the sense that you should avoid testing if the binlog is open
if you know that you will process the relay log and vice-versa.
Cheers.
> BR.
> Leonard
>
>
>
>
> He Zhenxing 写道:
>
>> Alfranio Correia wrote:
>>
>>> Hi Leonard,
>>>
>>> Sorry for not doing this review before.
>>> I don't know if you discussed this patch with Jasonh and other people
>>> and you agreed on
>>> the feature implemented here. So I am assuming that the
>>> architecture/implementation is
>>> still open for discussion. Said that, please find below some comments:
>>>
>>>
>> Yes, it's open.
>>
>>
>>> The patch does not show the relay log when the MySQL instance is also a
>>> master and
>>> the bin log is enabled. This should be fixed as follows:
>>>
>>>
>> No, it can show relay logs if binlog is enabled also, because usually
>> relay logs and binlogs have different name.
>>
>> But if user use the same name for binlog and relay logs (in different
>> directory), then it will not able to show relay logs that has the same
>> name with binlogs because currently it first searches in binlogs, and
>> only searches for relay logs if not found matching binlogs.
>>
>>
>>> 1 - Change the command's syntax a little bit:
>>>
>>> show binlog|relaylog ...;
>>>
>>>
>> OK, I think this is more clear.
>>
>>
>>> 2 - Add a flag in the LEX_MASTER_INFO, in order to distinguish between them.
>>>
>>>
>> OK
>>
>>
>>> 3 - The same function could be called but the flag in the
>>> LEX_MASTER_INFO will allow you
>>> to know which type of file do you want to process.
>>>
>>>
>> OK
>>
>>
>>> 4 - Introduce a variable mysql_relay_log which will be similar to
>>> mysql_bin_log.
>>>
>> I don't think we need this, use active_mi->rli.relay_log instead.
>>
>>
>>> We can discuss more on that as I don't think a global variable is a good
>>> idea. However,
>>> for now, we can keep the same design in here.
>>>
>>>
>> OK, we can discuss
>>
>>
>>> 5 - In mysql_show_binlog_events, we can decide which variable to use
>>> based on the flag in
>>> the LEX_MASTER_INFO.
>>>
>>>
>> OK
>>
>>
>>> I have also several comments regarding the test case but let's skip them
>>> for now.
>>>
>>>
>>> Cheers.
>>>
>>> Leonard Zhou wrote:
>>>
>>>> #At file:///home/zhl/mysql/rep/5.1/bug28777/
>>>>
>>>> 2793 Leonard Zhou 2009-02-11
>>>> Bug#28777 show binlog events can work on relay log
>>>>
>>>> Show binlog event can't work on relay log.
>>>>
>>>> Check relay log when do mysql 'show binlog event', so this command
> can work on both bin log and relay log.
>>>> added:
>>>> mysql-test/include/master-slave-2.inc
>>>> mysql-test/suite/rpl/r/rpl_bug28777-2.result
>>>> mysql-test/suite/rpl/r/rpl_bug28777.result
>>>> mysql-test/suite/rpl/t/rpl_bug28777-2-slave.opt
>>>> mysql-test/suite/rpl/t/rpl_bug28777-2.test
>>>> mysql-test/suite/rpl/t/rpl_bug28777-slave.opt
>>>> mysql-test/suite/rpl/t/rpl_bug28777.test
>>>> modified:
>>>> sql/sql_parse.cc
>>>> sql/sql_repl.cc
>>>> sql/sql_repl.h
>>>>
>>>> per-file messages:
>>>> mysql-test/include/master-slave-2.inc
>>>> Master-slave file but doesn't do 'restet master' in slave
>>>> mysql-test/suite/rpl/r/rpl_bug28777-2.result
>>>> Test result of bug28777-2
>>>> mysql-test/suite/rpl/r/rpl_bug28777.result
>>>> Test result of bug28777
>>>> mysql-test/suite/rpl/t/rpl_bug28777-2-slave.opt
>>>> slave option file for closing bin log in slave
>>>> mysql-test/suite/rpl/t/rpl_bug28777-2.test
>>>> Test case for bug28777 without bin logging in slave
>>>> mysql-test/suite/rpl/t/rpl_bug28777-slave.opt
>>>> slave option file to set relay log in different directory
>>>> mysql-test/suite/rpl/t/rpl_bug28777.test
>>>> Test case for bug28777
>>>> sql/sql_parse.cc
>>>> Pass master_info to mysql_show_binlog_events
>>>> sql/sql_repl.cc
>>>> Check relay log when do mysql 'show binlog event'
>>>> sql/sql_repl.h
>>>> Add master_info variable
>>>> === added file 'mysql-test/include/master-slave-2.inc'
>>>> --- a/mysql-test/include/master-slave-2.inc 1970-01-01 00:00:00 +0000
>>>> +++ b/mysql-test/include/master-slave-2.inc 2009-02-11 08:57:59 +0000
>>>> @@ -0,0 +1,48 @@
>>>> +# The different between master-slave-2.inc and mster-slave.inc is not do
> 'reset master' in slave
>>>> +# because we stop binlog in slave.
>>>> +# Replication tests need binlog
>>>> +source include/have_log_bin.inc;
>>>> +
>>>> +connect (master,127.0.0.1,root,,test,$MASTER_MYPORT,);
>>>> +connect (master1,127.0.0.1,root,,test,$MASTER_MYPORT,);
>>>> +connect (slave,127.0.0.1,root,,test,$SLAVE_MYPORT,);
>>>> +connect (slave1,127.0.0.1,root,,test,$SLAVE_MYPORT,);
>>>> +
>>>> +# Reset the master and the slave to start fresh.
>>>> +#
>>>> +# It is necessary to execute RESET MASTER and RESET SLAVE on both
>>>> +# master and slave since the replication setup might be circular.
>>>> +#
>>>> +# Since we expect STOP SLAVE to produce a warning as the slave is
>>>> +# stopped (the server was started with skip-slave-start), we disable
>>>> +# warnings when doing STOP SLAVE.
>>>> +
>>>> +connection slave;
>>>> +--disable_warnings
>>>> +stop slave;
>>>> +source include/wait_for_slave_to_stop.inc;
>>>> +--enable_warnings
>>>> +connection master;
>>>> +--disable_warnings
>>>> +--disable_query_log
>>>> +use test;
>>>> +--enable_query_log
>>>> +drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
>>>> +--enable_warnings
>>>> +reset master;
>>>> +--disable_query_log
>>>> +reset slave;
>>>> +--enable_query_log
>>>> +connection slave;
>>>> +reset slave;
>>>> +# Clean up old test tables
>>>> +--disable_warnings
>>>> +drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
>>>> +--enable_warnings
>>>> +--disable_query_log
>>>> +--enable_query_log
>>>> +start slave;
>>>> +source include/wait_for_slave_to_start.inc;
>>>> +
>>>> +# Set the default connection to 'master'
>>>> +connection master;
>>>>
>>>> === added file 'mysql-test/suite/rpl/r/rpl_bug28777-2.result'
>>>> --- a/mysql-test/suite/rpl/r/rpl_bug28777-2.result 1970-01-01 00:00:00
> +0000
>>>> +++ b/mysql-test/suite/rpl/r/rpl_bug28777-2.result 2009-02-11 08:57:59
> +0000
>>>> @@ -0,0 +1,37 @@
>>>> +stop slave;
>>>> +drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
>>>> +reset master;
>>>> +reset slave;
>>>> +drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
>>>> +start slave;
>>>> +==== Initialize ====
>>>> +[on master]
>>>> +CREATE TABLE t1 (a INT);
>>>> +SHOW CREATE TABLE t1;
>>>> +Table Create Table
>>>> +t1 CREATE TABLE `t1` (
>>>> + `a` int(11) DEFAULT NULL
>>>> +) ENGINE=MyISAM DEFAULT CHARSET=latin1
>>>> +INSERT INTO t1 VALUES (1);
>>>> +INSERT INTO t1 VALUES (2);
>>>> +INSERT INTO t1 VALUES (3);
>>>> +INSERT INTO t1 VALUES (4);
>>>> +INSERT INTO t1 VALUES (5);
>>>> +INSERT INTO t1 VALUES (6);
>>>> +SELECT * FROM t1 ORDER BY a;
>>>> +a
>>>> +1
>>>> +2
>>>> +3
>>>> +4
>>>> +5
>>>> +6
>>>> +[on slave]
>>>> +==== Verify results on slave ====
>>>> +show binlog events in 'slave-relay-bin.000002';
>>>> +Log_name Pos Event_type Server_id End_log_pos Info
>>>> +slave-relay-bin.000002 # Format_desc 2 # Server ver: 5.1.32-debug-log,
> Binlog ver: 4
>>>> +slave-relay-bin.000002 # Rotate 2 # slave-relay-bin.000003;pos=4
>>>> +==== Clean up ====
>>>> +[on master]
>>>> +DROP TABLE t1;
>>>>
>>>> === added file 'mysql-test/suite/rpl/r/rpl_bug28777.result'
>>>> --- a/mysql-test/suite/rpl/r/rpl_bug28777.result 1970-01-01 00:00:00
> +0000
>>>> +++ b/mysql-test/suite/rpl/r/rpl_bug28777.result 2009-02-11 08:57:59
> +0000
>>>> @@ -0,0 +1,47 @@
>>>> +stop slave;
>>>> +drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
>>>> +reset master;
>>>> +reset slave;
>>>> +drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9;
>>>> +start slave;
>>>> +==== Initialize ====
>>>> +[on master]
>>>> +CREATE TABLE t1 (a INT);
>>>> +SHOW CREATE TABLE t1;
>>>> +Table Create Table
>>>> +t1 CREATE TABLE `t1` (
>>>> + `a` int(11) DEFAULT NULL
>>>> +) ENGINE=MyISAM DEFAULT CHARSET=latin1
>>>> +INSERT INTO t1 VALUES (1);
>>>> +INSERT INTO t1 VALUES (2);
>>>> +INSERT INTO t1 VALUES (3);
>>>> +INSERT INTO t1 VALUES (4);
>>>> +INSERT INTO t1 VALUES (5);
>>>> +INSERT INTO t1 VALUES (6);
>>>> +SELECT * FROM t1 ORDER BY a;
>>>> +a
>>>> +1
>>>> +2
>>>> +3
>>>> +4
>>>> +5
>>>> +6
>>>> +[on slave]
>>>> +==== Verify results on slave ====
>>>> +show binlog events in 'slave-bin.000001';
>>>> +Log_name Pos Event_type Server_id End_log_pos Info
>>>> +slave-bin.000001 # Format_desc 2 # Server ver: 5.1.32-debug-log, Binlog
> ver: 4
>>>> +slave-bin.000001 # Query 1 # use `test`; CREATE TABLE t1 (a INT)
>>>> +slave-bin.000001 # Query 1 # use `test`; INSERT INTO t1 VALUES (1)
>>>> +slave-bin.000001 # Query 1 # use `test`; INSERT INTO t1 VALUES (2)
>>>> +slave-bin.000001 # Query 1 # use `test`; INSERT INTO t1 VALUES (3)
>>>> +slave-bin.000001 # Query 1 # use `test`; INSERT INTO t1 VALUES (4)
>>>> +slave-bin.000001 # Query 1 # use `test`; INSERT INTO t1 VALUES (5)
>>>> +slave-bin.000001 # Query 1 # use `test`; INSERT INTO t1 VALUES (6)
>>>> +show binlog events in 'slave-relay-bin.000002';
>>>> +Log_name Pos Event_type Server_id End_log_pos Info
>>>> +slave-relay-bin.000002 # Format_desc 2 # Server ver: 5.1.32-debug-log,
> Binlog ver: 4
>>>> +slave-relay-bin.000002 # Rotate 2 # slave-relay-bin.000003;pos=4
>>>> +==== Clean up ====
>>>> +[on master]
>>>> +DROP TABLE t1;
>>>>
>>>> === added file 'mysql-test/suite/rpl/t/rpl_bug28777-2-slave.opt'
>>>> --- a/mysql-test/suite/rpl/t/rpl_bug28777-2-slave.opt 1970-01-01 00:00:00
> +0000
>>>> +++ b/mysql-test/suite/rpl/t/rpl_bug28777-2-slave.opt 2009-02-11 08:57:59
> +0000
>>>> @@ -0,0 +1 @@
>>>> +--relay-log=$MYSQLTEST_VARDIR/tmp/slave-relay-bin --skip-log-bin
> --skip-log-slave-update
>>>>
>>>> === added file 'mysql-test/suite/rpl/t/rpl_bug28777-2.test'
>>>> --- a/mysql-test/suite/rpl/t/rpl_bug28777-2.test 1970-01-01 00:00:00
> +0000
>>>> +++ b/mysql-test/suite/rpl/t/rpl_bug28777-2.test 2009-02-11 08:57:59
> +0000
>>>> @@ -0,0 +1,46 @@
>>>> +# ==== Purpose ====
>>>> +#
>>>> +# Test that show bin log event can work with relay log file when binlog
> is close in slave.
>>>> +#
>>>> +# ==== Method ====
>>>> +#
>>>> +# Do 'show binlog events' for relay log file in slave.
>>>> +#
>>>> +
>>>> +source include/have_binlog_format_statement.inc;
>>>> +source include/master-slave-2.inc;
>>>> +
>>>> +
>>>> +--echo ==== Initialize ====
>>>> +
>>>> +--echo [on master]
>>>> +--connection master
>>>> +
>>>> +CREATE TABLE t1 (a INT);
>>>> +SHOW CREATE TABLE t1;
>>>> +
>>>> +
>>>> +INSERT INTO t1 VALUES (1);
>>>> +INSERT INTO t1 VALUES (2);
>>>> +INSERT INTO t1 VALUES (3);
>>>> +INSERT INTO t1 VALUES (4);
>>>> +INSERT INTO t1 VALUES (5);
>>>> +INSERT INTO t1 VALUES (6);
>>>> +SELECT * FROM t1 ORDER BY a;
>>>> +
>>>> +--echo [on slave]
>>>> +sync_slave_with_master;
>>>> +
>>>> +
>>>> +--echo ==== Verify results on slave ====
>>>> +--replace_column 2 # 5 #
>>>> +show binlog events in 'slave-relay-bin.000002';
>>>> +
>>>> +--echo ==== Clean up ====
>>>> +
>>>> +--echo [on master]
>>>> +connection master;
>>>> +DROP TABLE t1;
>>>> +
>>>> +#--echo [on slave]
>>>> +sync_slave_with_master;
>>>>
>>>> === added file 'mysql-test/suite/rpl/t/rpl_bug28777-slave.opt'
>>>> --- a/mysql-test/suite/rpl/t/rpl_bug28777-slave.opt 1970-01-01 00:00:00
> +0000
>>>> +++ b/mysql-test/suite/rpl/t/rpl_bug28777-slave.opt 2009-02-11 08:57:59
> +0000
>>>> @@ -0,0 +1 @@
>>>> +--relay-log=$MYSQLTEST_VARDIR/tmp/slave-relay-bin
>>>>
>>>> === added file 'mysql-test/suite/rpl/t/rpl_bug28777.test'
>>>> --- a/mysql-test/suite/rpl/t/rpl_bug28777.test 1970-01-01 00:00:00 +0000
>>>> +++ b/mysql-test/suite/rpl/t/rpl_bug28777.test 2009-02-11 08:57:59 +0000
>>>> @@ -0,0 +1,48 @@
>>>> +# ==== Purpose ====
>>>> +#
>>>> +# Test that show bin log event can work with relay log file and bin log
> file.
>>>> +#
>>>> +# ==== Method ====
>>>> +#
>>>> +# Do 'show binlog events' for relay log file and bin log file in slave.
>>>> +#
>>>> +
>>>> +source include/have_binlog_format_statement.inc;
>>>> +source include/master-slave.inc;
>>>> +
>>>> +
>>>> +--echo ==== Initialize ====
>>>> +
>>>> +--echo [on master]
>>>> +--connection master
>>>> +
>>>> +CREATE TABLE t1 (a INT);
>>>> +SHOW CREATE TABLE t1;
>>>> +
>>>> +
>>>> +INSERT INTO t1 VALUES (1);
>>>> +INSERT INTO t1 VALUES (2);
>>>> +INSERT INTO t1 VALUES (3);
>>>> +INSERT INTO t1 VALUES (4);
>>>> +INSERT INTO t1 VALUES (5);
>>>> +INSERT INTO t1 VALUES (6);
>>>> +SELECT * FROM t1 ORDER BY a;
>>>> +
>>>> +--echo [on slave]
>>>> +sync_slave_with_master;
>>>> +
>>>> +
>>>> +--echo ==== Verify results on slave ====
>>>> +--replace_column 2 # 5 #
>>>> +show binlog events in 'slave-bin.000001';
>>>> +--replace_column 2 # 5 #
>>>> +show binlog events in 'slave-relay-bin.000002';
>>>> +
>>>> +--echo ==== Clean up ====
>>>> +
>>>> +--echo [on master]
>>>> +connection master;
>>>> +DROP TABLE t1;
>>>> +
>>>> +#--echo [on slave]
>>>> +sync_slave_with_master;
>>>>
>>>> === modified file 'sql/sql_parse.cc'
>>>> --- a/sql/sql_parse.cc 2009-02-05 06:16:00 +0000
>>>> +++ b/sql/sql_parse.cc 2009-02-11 08:57:59 +0000
>>>> @@ -2321,7 +2321,7 @@ mysql_execute_command(THD *thd)
>>>> {
>>>> if (check_global_access(thd, REPL_SLAVE_ACL))
>>>> goto error;
>>>> - res = mysql_show_binlog_events(thd);
>>>> + res = mysql_show_binlog_events(thd,active_mi);
>>>> break;
>>>> }
>>>> #endif
>>>>
>>>> === modified file 'sql/sql_repl.cc'
>>>> --- a/sql/sql_repl.cc 2009-01-23 12:22:05 +0000
>>>> +++ b/sql/sql_repl.cc 2009-02-11 08:57:59 +0000
>>>> @@ -1389,7 +1389,7 @@ int cmp_master_pos(const char* log_file_
>>>> @retval FALSE success
>>>> @retval TRUE failure
>>>> */
>>>> -bool mysql_show_binlog_events(THD* thd)
>>>> +bool mysql_show_binlog_events(THD* thd, Master_info* mi)
>>>> {
>>>> Protocol *protocol= thd->protocol;
>>>> List<Item> field_list;
>>>> @@ -1414,7 +1414,7 @@ bool mysql_show_binlog_events(THD* thd)
>>>> */
>>>> ha_binlog_wait(thd);
>>>>
>>>> - if (mysql_bin_log.is_open())
>>>> + if (mysql_bin_log.is_open() || mi->rli.relay_log.is_open())
>>>> {
>>>> LEX_MASTER_INFO *lex_mi= &thd->lex->mi;
>>>> SELECT_LEX_UNIT *unit= &thd->lex->unit;
>>>> @@ -1438,10 +1438,18 @@ bool mysql_show_binlog_events(THD* thd)
>>>>
>>>> linfo.index_file_offset = 0;
>>>>
>>>> - if (mysql_bin_log.find_log_pos(&linfo, name, 1))
>>>> + if (!mysql_bin_log.is_open() ||
> mysql_bin_log.find_log_pos(&linfo, name, 1))
>>>> {
>>>> - errmsg = "Could not find target log";
>>>> - goto err;
>>>> + if (log_file_name)
>>>> + mi->rli.relay_log.make_log_name(search_file_name,
> log_file_name);
>>>> + else
>>>> + name=0; // Find first log
>>>> +
>>>> + if(mi->rli.relay_log.find_log_pos(&linfo, name, 1))
>>>> + {
>>>> + errmsg = "Could not find target log";
>>>> + goto err;
>>>> + }
>>>> }
>>>>
>>>> pthread_mutex_lock(&LOCK_thread_count);
>>>>
>>>> === modified file 'sql/sql_repl.h'
>>>> --- a/sql/sql_repl.h 2008-03-14 17:38:54 +0000
>>>> +++ b/sql/sql_repl.h 2009-02-11 08:57:59 +0000
>>>> @@ -39,7 +39,7 @@ extern my_bool opt_sporadic_binlog_dump_
>>>> int start_slave(THD* thd, Master_info* mi, bool net_report);
>>>> int stop_slave(THD* thd, Master_info* mi, bool net_report);
>>>> bool change_master(THD* thd, Master_info* mi);
>>>> -bool mysql_show_binlog_events(THD* thd);
>>>> +bool mysql_show_binlog_events(THD* thd, Master_info* mi);
>>>> int cmp_master_pos(const char* log_file_name1, ulonglong log_pos1,
>>>> const char* log_file_name2, ulonglong log_pos2);
>>>> int reset_slave(THD *thd, Master_info* mi);
>>>>
>>>>
>>>>
>>>>
>
>
>
