List:Commits« Previous MessageNext Message »
From:Mats Kindahl Date:July 10 2008 9:15pm
Subject:Re: bzr commit into mysql-5.1 branch (ingo.struewing:2624) Bug#35807
View as plain text  
Hi Ingo!

I have some minor suggestions for changing the test, otherwise the patch
looks fine.

Just my few cents,
Mats Kindahl

Ingo Struewing wrote:
> #At file:///home2/mydev/bzrroot/mysql-5.1-bug35807/
> 
>  2624 Ingo Struewing	2008-07-09
>       Bug#35807 - INSTALL PLUGIN replicates row-based, but not stmt-based
>       
>       INSTALL PLUGIN and UNINSTALL PLUGIN worked with statement-based and
>       mixed-mode replication only, but not with row-based replication.
>       
>       There is no statement-based replication of these statements.
>       But there was row-based replication of the inserts and deletes
>       to and from the mysql.plugin table.
>       
>       The fix is to suppress binlogging during insert and delete to
>       and from the mysql.plugin table.
> added:
>   mysql-test/suite/rpl/r/rpl_plugin_load.result
>   mysql-test/suite/rpl/t/rpl_plugin_load-master.opt
>   mysql-test/suite/rpl/t/rpl_plugin_load-slave.opt
>   mysql-test/suite/rpl/t/rpl_plugin_load.test
> modified:
>   sql/sql_plugin.cc
> 
> per-file messages:
>   mysql-test/suite/rpl/r/rpl_plugin_load.result
>     Bug#35807 - INSTALL PLUGIN replicates row-based, but not stmt-based
>     New test result.
>   mysql-test/suite/rpl/t/rpl_plugin_load-master.opt
>     Bug#35807 - INSTALL PLUGIN replicates row-based, but not stmt-based
>     New master options.
>   mysql-test/suite/rpl/t/rpl_plugin_load-slave.opt
>     Bug#35807 - INSTALL PLUGIN replicates row-based, but not stmt-based
>     New slave options.
>   mysql-test/suite/rpl/t/rpl_plugin_load.test
>     Bug#35807 - INSTALL PLUGIN replicates row-based, but not stmt-based
>     New test case.
>   sql/sql_plugin.cc
>     Bug#35807 - INSTALL PLUGIN replicates row-based, but not stmt-based
>     Suppress binlogging during insert and delete to/from the
> === added file 'mysql-test/suite/rpl/r/rpl_plugin_load.result'
> --- a/mysql-test/suite/rpl/r/rpl_plugin_load.result	1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/rpl/r/rpl_plugin_load.result	2008-07-09 17:50:39 +0000
> @@ -0,0 +1,32 @@
> +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;
> +Show the binlog so far
> +SHOW BINLOG EVENTS;
> +Log_name	Pos	Event_type	Server_id	End_log_pos	Info
> +master-bin.000001	4	Format_desc	1	106	Server ver: 5.1.28-debug-log, Binlog ver: 4
> +Verify that example engine is not installed.
> +SELECT * FROM INFORMATION_SCHEMA.ENGINES WHERE ENGINE='EXAMPLE';
> +ENGINE	SUPPORT	COMMENT	TRANSACTIONS	XA	SAVEPOINTS
> +Install example engine.
> +INSTALL PLUGIN example SONAME 'ha_example.so';
> +Verify that example engine is installed.
> +SELECT * FROM INFORMATION_SCHEMA.ENGINES WHERE ENGINE='EXAMPLE';
> +ENGINE	SUPPORT	COMMENT	TRANSACTIONS	XA	SAVEPOINTS
> +EXAMPLE	YES	Example storage engine	NO	NO	NO
> +connection slave: Verify that example engine is not installed.
> +SELECT * FROM INFORMATION_SCHEMA.ENGINES WHERE ENGINE='EXAMPLE';
> +ENGINE	SUPPORT	COMMENT	TRANSACTIONS	XA	SAVEPOINTS
> +connection master: Uninstall example engine.
> +UNINSTALL PLUGIN example;
> +Verify that example engine is not installed.
> +SELECT * FROM INFORMATION_SCHEMA.ENGINES WHERE ENGINE='EXAMPLE';
> +ENGINE	SUPPORT	COMMENT	TRANSACTIONS	XA	SAVEPOINTS
> +Double-check that (UN)INSTALL PLUGIN did not modify the binlog
> +SHOW BINLOG EVENTS;
> +Log_name	Pos	Event_type	Server_id	End_log_pos	Info
> +slave-bin.000001	4	Format_desc	2	106	Server ver: 5.1.28-debug-log, Binlog ver: 4
> +slave-bin.000001	106	Query	2	216	use `test`; drop table if exists
> t1,t2,t3,t4,t5,t6,t7,t8,t9
> 
> === added file 'mysql-test/suite/rpl/t/rpl_plugin_load-master.opt'
> --- a/mysql-test/suite/rpl/t/rpl_plugin_load-master.opt	1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_plugin_load-master.opt	2008-07-09 17:50:39 +0000
> @@ -0,0 +1 @@
> +$EXAMPLE_PLUGIN_OPT
> 
> === added file 'mysql-test/suite/rpl/t/rpl_plugin_load-slave.opt'
> --- a/mysql-test/suite/rpl/t/rpl_plugin_load-slave.opt	1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_plugin_load-slave.opt	2008-07-09 17:50:39 +0000
> @@ -0,0 +1 @@
> +$EXAMPLE_PLUGIN_OPT
> 
> === added file 'mysql-test/suite/rpl/t/rpl_plugin_load.test'
> --- a/mysql-test/suite/rpl/t/rpl_plugin_load.test	1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/rpl/t/rpl_plugin_load.test	2008-07-09 17:50:39 +0000
> @@ -0,0 +1,48 @@
> +#
> +# Bug#35807 - INSTALL PLUGIN replicates row-based, but not stmt-based
> +#
> +# The test verifies that INSTALL PLUGIN and UNINSTALL PLUGIN
> +# work with replication.
> +#
> +# The test tries to install and uninstall a plugin on master,
> +# and verifies that it does not affect the slave,
> +# and that it does not add anything to the binlog.
> +
> +--source include/not_embedded.inc
> +--source include/have_log_bin.inc
> +# Dynamic loading of Example does not work on Windows currently.
> +--source include/not_windows.inc
> +--source include/have_example_plugin.inc
> +
> +# Initialize replication.
> +--source include/master-slave.inc
> +--echo Show the binlog so far
> +SHOW BINLOG EVENTS;

This risks causing a result mismatch as soon as it is executed in a
slightly different environment, for example, when the server changes
version. Normally, the include/show_binlog_events.inc is used, which
does a replace on the things that can vary, but I don't think that this
test needs to print the binary log. It is sufficient to demonstrate that
the plug-in is not installed on the slave and the slave does not stop
(e.g., because it cannot find the file).

If you want to verify that *nothing* is written to the binary log, you
can use the following code to see that you get zero.

let $before_pos = query_get_value("SHOW MASTER STATUS", Position, 1);
INSTALL PLUGIN example SONAME 'ha_example.so';
let $after_pos = query_get_value("SHOW MASTER STATUS", Position, 1);
eval SELECT $after_pos - $before_pos AS Delta;

Note that this is robust and will not cause a result mismatch in the
event that the binlog position changes.

I think you can remove the SHOW BINLOG EVENTS, use the code above to
verify that nothing is written, and verify that the plug-in is not
installed on the slave and the slave is still running.

> +--echo Verify that example engine is not installed.
> +SELECT * FROM INFORMATION_SCHEMA.ENGINES WHERE ENGINE='EXAMPLE';
> +--echo Install example engine.
> +INSTALL PLUGIN example SONAME 'ha_example.so';
> +--echo Verify that example engine is installed.
> +SELECT * FROM INFORMATION_SCHEMA.ENGINES WHERE ENGINE='EXAMPLE';
> +# Wait for slave to catch up with master.
> +sync_slave_with_master;
> +#
> +    --echo connection slave: Verify that example engine is not installed.
> +    connection slave;
> +    SELECT * FROM INFORMATION_SCHEMA.ENGINES WHERE ENGINE='EXAMPLE';
> +#
> +--echo connection master: Uninstall example engine.
> +connection master;
> +UNINSTALL PLUGIN example;
> +--echo Verify that example engine is not installed.
> +SELECT * FROM INFORMATION_SCHEMA.ENGINES WHERE ENGINE='EXAMPLE';
> +# Wait for slave to catch up with master.
> +sync_slave_with_master;
> +# Wait for binlog entries to be flushed.
> +--sleep 1
> +--echo Double-check that (UN)INSTALL PLUGIN did not modify the binlog
> +SHOW BINLOG EVENTS;
> +#
> +# Cleanup
> +--source include/master-slave-end.inc
> +
> 
> === modified file 'sql/sql_plugin.cc'
> --- a/sql/sql_plugin.cc	2008-05-30 10:21:45 +0000
> +++ b/sql/sql_plugin.cc	2008-07-09 17:50:39 +0000
> @@ -1662,11 +1662,18 @@ bool mysql_install_plugin(THD *thd, cons
>      goto deinit;
>    }
>  
> +  /*
> +    We do not replicate the INSTALL PLUGIN statement. Disable binlogging
> +    of the insert into the plugin table, so that it is not replicated in
> +    row based mode.
> +  */
> +  tmp_disable_binlog(thd);
>    table->use_all_columns();
>    restore_record(table, s->default_values);
>    table->field[0]->store(name->str, name->length, system_charset_info);
>    table->field[1]->store(dl->str, dl->length, files_charset_info);
>    error= table->file->ha_write_row(table->record[0]);
> +  reenable_binlog(thd);
>    if (error)
>    {
>      table->file->print_error(error, MYF(0));
> @@ -1731,7 +1738,15 @@ bool mysql_uninstall_plugin(THD *thd, co
>                                          HA_READ_KEY_EXACT))
>    {
>      int error;
> -    if ((error= table->file->ha_delete_row(table->record[0])))
> +    /*
> +      We do not replicate the UNINSTALL PLUGIN statement. Disable binlogging
> +      of the delete from the plugin table, so that it is not replicated in
> +      row based mode.
> +    */
> +    tmp_disable_binlog(thd);
> +    error= table->file->ha_delete_row(table->record[0]);
> +    reenable_binlog(thd);
> +    if (error)
>      {
>        table->file->print_error(error, MYF(0));
>        DBUG_RETURN(TRUE);
> 
> 


-- 
Mats Kindahl
Lead Software Developer
Replication Team
MySQL AB, www.mysql.com

Thread
bzr commit into mysql-5.1 branch (ingo.struewing:2624) Bug#35807Ingo Struewing9 Jul
  • Re: bzr commit into mysql-5.1 branch (ingo.struewing:2624) Bug#35807Mats Kindahl10 Jul
    • Re: bzr commit into mysql-5.1 branch (ingo.struewing:2624) Bug#35807Ingo Strüwing11 Jul
      • Re: bzr commit into mysql-5.1 branch (ingo.struewing:2624) Bug#35807Mats Kindahl11 Jul
    • Re: bzr commit into mysql-5.1 branch (ingo.struewing:2624) Bug#35807Ingo Strüwing11 Jul
      • Re: bzr commit into mysql-5.1 branch (ingo.struewing:2624) Bug#35807Mats Kindahl13 Jul