Hi Luis,
Thank you for the work, I have two concerns!
First, I'm wondering if it's possible to avoid changing InnoDB codes,
which would ask for reviewers from the InnoDB team, and what's more,
means that your fix probably only works for InnoDB, and we probably need
to do the similar fixes for other engines, such as Falcon or others. So
I think it's better to do it in the server side, and I'm suggesting to
temporarily disable binlog when the statement should be ignored by the
db filtering rules when executing the statement by the storage engines.
I think this way we can make it work for all storage engines.
Second, when in MIXED mode, and the statement should be ignored by the
db filtering rules cannot be logged (cannot be logged in either
STATEMENT nor ROW format, for example one statement that modifies two
engines, one can only log it in STATEMENT, and the other can only log it
in ROW format), it will still throw a warning or error after your patch.
I think this case should be handled too. I am also thinking about
temporarily disabling binlog for the engines in this case.
Please don't hesitate to argument if you disagree!
Luis Soares wrote:
> #At file:///home/lsoares/Workspace/mysql-server/bugfix/42829/5.1-bt/ based on
> revid:matthias.leich@stripped
>
> 2840 Luis Soares 2009-04-23
> BUG#42829: binlogging enabled for all schemas regardless of
> binlog-db-db / binlog-ignore-db
>
> InnoDB will return an error if statement based replication is used
> along with transaction isolation level READ-COMMITTED (or stricter),
> even if the statement in question is filtered out according to the
> binlog-do-db rules set. In this case, an error should not be printed.
>
> This patch addresses this issue by extending the existing check in
> external_lock to take into account the filter rules before deciding to
> print an error. Furthermore, it also changes decide_logging_format to
> take into consideration whether the statement is filtered out from
> binlog before decision is made.
> @ mysql-test/suite/binlog/r/binlog_mix_do_db.result
> Result file.
> @ mysql-test/suite/binlog/r/binlog_stm_do_db.result
> Result file.
> @ mysql-test/suite/binlog/t/binlog_mix_do_db.test
> Added test case for mixed format to check that statements are
> turned into unsafe and logged in rows and that the additional
> clause added in decide logging format produces no side-effects.
> @ mysql-test/suite/binlog/t/binlog_stm_do_db.test
> Test case for statement based logging. This format triggered
> the bug reported.
> @ sql/sql_base.cc
> Changed the check on decide_logging_format to take into account
> whether statement is filtered or not (and if it is in mixed mode)
> before making the decision on log format (error checking).
> @ sql/sql_class.cc
> Added the thd_binlog_filter_ok to INNODB_COMPATIBILITY_HOOKS set.
> @ storage/innobase/handler/ha_innodb.cc
> Extended check in external_lock to take into consideration the
> filtering when deciding to throw an error.
> @ storage/innobase/handler/ha_innodb.h
> Added declaration of new hook.
>
> added:
> mysql-test/suite/binlog/r/binlog_mix_do_db.result
> mysql-test/suite/binlog/r/binlog_stm_do_db.result
> mysql-test/suite/binlog/t/binlog_mix_do_db-master.opt
> mysql-test/suite/binlog/t/binlog_mix_do_db.test
> mysql-test/suite/binlog/t/binlog_stm_do_db-master.opt
> mysql-test/suite/binlog/t/binlog_stm_do_db.test
> modified:
> sql/sql_base.cc
> sql/sql_class.cc
> storage/innobase/handler/ha_innodb.cc
> storage/innobase/handler/ha_innodb.h
> === added file 'mysql-test/suite/binlog/r/binlog_mix_do_db.result'
> --- a/mysql-test/suite/binlog/r/binlog_mix_do_db.result 1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/binlog/r/binlog_mix_do_db.result 2009-04-23 11:31:32 +0000
> @@ -0,0 +1,42 @@
> +SET @old_isolation_level= @@tx_isolation;
> +SET tx_isolation= 'READ-COMMITTED';
> +CREATE DATABASE db_not_filtered;
> +use db_not_filtered;
> +CREATE TABLE t1 (x int, y int) engine=InnoDB;
> +CREATE TABLE t2 (x int, y int) engine=InnoDB;
> +CREATE DATABASE db_filtered;
> +use db_filtered;
> +CREATE TABLE t1 (x int, y int) engine=InnoDB;
> +CREATE TABLE t2 (x int, y int) engine=InnoDB;
> +INSERT INTO t2 VALUES (1,2);
> +INSERT INTO t1 SELECT * FROM t2;
> +INSERT INTO db_not_filtered.t1 VALUES (1, 2);
> +INSERT INTO db_not_filtered.t1 VALUES (UUID(), 2);
> +use db_not_filtered;
> +INSERT INTO t2 VALUES (1,2);
> +INSERT INTO t1 SELECT * FROM t2;
> +DROP DATABASE db_not_filtered;
> +DROP DATABASE db_filtered;
> +show binlog events from <binlog_start>;
> +Log_name Pos Event_type Server_id End_log_pos Info
> +master-bin.000001 # Query # # CREATE DATABASE db_not_filtered
> +master-bin.000001 # Query # # use `db_not_filtered`; CREATE TABLE t1 (x int, y int)
> engine=InnoDB
> +master-bin.000001 # Query # # use `db_not_filtered`; CREATE TABLE t2 (x int, y int)
> engine=InnoDB
> +master-bin.000001 # Query # # use `db_filtered`; BEGIN
> +master-bin.000001 # Table_map # # table_id: # (db_not_filtered.t1)
> +master-bin.000001 # Write_rows # # table_id: # flags: STMT_END_F
> +master-bin.000001 # Xid # # COMMIT /* XID */
> +master-bin.000001 # Query # # use `db_filtered`; BEGIN
> +master-bin.000001 # Table_map # # table_id: # (db_not_filtered.t1)
> +master-bin.000001 # Write_rows # # table_id: # flags: STMT_END_F
> +master-bin.000001 # Xid # # COMMIT /* XID */
> +master-bin.000001 # Query # # use `db_not_filtered`; BEGIN
> +master-bin.000001 # Table_map # # table_id: # (db_not_filtered.t2)
> +master-bin.000001 # Write_rows # # table_id: # flags: STMT_END_F
> +master-bin.000001 # Xid # # COMMIT /* XID */
> +master-bin.000001 # Query # # use `db_not_filtered`; BEGIN
> +master-bin.000001 # Table_map # # table_id: # (db_not_filtered.t1)
> +master-bin.000001 # Write_rows # # table_id: # flags: STMT_END_F
> +master-bin.000001 # Xid # # COMMIT /* XID */
> +master-bin.000001 # Query # # DROP DATABASE db_not_filtered
> +SET @@tx_isolation= @old_isolation_level;
>
> === added file 'mysql-test/suite/binlog/r/binlog_stm_do_db.result'
> --- a/mysql-test/suite/binlog/r/binlog_stm_do_db.result 1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/binlog/r/binlog_stm_do_db.result 2009-04-23 11:31:32 +0000
> @@ -0,0 +1,26 @@
> +SET @old_isolation_level= @@tx_isolation;
> +SET tx_isolation= 'READ-COMMITTED';
> +CREATE DATABASE db_not_filtered;
> +use db_not_filtered;
> +CREATE TABLE t1 (x int, y int) engine=InnoDB;
> +CREATE TABLE t2 (x int, y int) engine=InnoDB;
> +CREATE DATABASE db_filtered;
> +use db_filtered;
> +CREATE TABLE t1 (x int, y int) engine=InnoDB;
> +CREATE TABLE t2 (x int, y int) engine=InnoDB;
> +INSERT INTO t2 VALUES (1,2);
> +INSERT INTO t1 SELECT * FROM t2;
> +use db_not_filtered;
> +INSERT INTO t2 VALUES (1,2);
> +ERROR HY000: Binary logging not possible. Message: Transaction level
> 'READ-COMMITTED' in InnoDB is not safe for binlog mode 'STATEMENT'
> +INSERT INTO t1 SELECT * FROM t2;
> +ERROR HY000: Binary logging not possible. Message: Transaction level
> 'READ-COMMITTED' in InnoDB is not safe for binlog mode 'STATEMENT'
> +DROP DATABASE db_not_filtered;
> +DROP DATABASE db_filtered;
> +show binlog events from <binlog_start>;
> +Log_name Pos Event_type Server_id End_log_pos Info
> +master-bin.000001 # Query # # CREATE DATABASE db_not_filtered
> +master-bin.000001 # Query # # use `db_not_filtered`; CREATE TABLE t1 (x int, y int)
> engine=InnoDB
> +master-bin.000001 # Query # # use `db_not_filtered`; CREATE TABLE t2 (x int, y int)
> engine=InnoDB
> +master-bin.000001 # Query # # DROP DATABASE db_not_filtered
> +SET @@tx_isolation= @old_isolation_level;
>
> === added file 'mysql-test/suite/binlog/t/binlog_mix_do_db-master.opt'
> --- a/mysql-test/suite/binlog/t/binlog_mix_do_db-master.opt 1970-01-01 00:00:00
> +0000
> +++ b/mysql-test/suite/binlog/t/binlog_mix_do_db-master.opt 2009-04-23 11:31:32
> +0000
> @@ -0,0 +1 @@
> +--binlog-do-db=db_not_filtered
>
> === added file 'mysql-test/suite/binlog/t/binlog_mix_do_db.test'
> --- a/mysql-test/suite/binlog/t/binlog_mix_do_db.test 1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/binlog/t/binlog_mix_do_db.test 2009-04-23 11:31:32 +0000
> @@ -0,0 +1,53 @@
> +# BUG#42829: binlogging enabled for all schemas regardless of binlog-db-db /
> +# binlog-ignore-db
> +#
> +# DESCRIPTION
> +# ===========
> +#
> +# The test is implemented as follows:
> +# i) set tx_isolation to read-committed.
> +# ii) create two databases (one filtered other not - using binlog-do-db)
> +# iii) Create statements that are to be filtered on filtered db
> +# - At this point before the patch an error would be thrown.
> +# iv) Create statements while using db_filtered that change db_not_filtered
> +# tables. (These should be marked as unsafe and make it into the binlog)
> +# v) Insert into tables in the not filtered database and check that
> +# no error exists because these are logged as row events.
> +
> +source include/have_log_bin.inc;
> +source include/have_innodb.inc;
> +source include/have_binlog_format_mixed.inc;
> +
> +SET @old_isolation_level= @@tx_isolation;
> +SET tx_isolation= 'READ-COMMITTED';
> +
> +let $engine= InnoDB;
> +
> +CREATE DATABASE db_not_filtered;
> +use db_not_filtered;
> +eval CREATE TABLE t1 (x int, y int) engine=$engine;
> +eval CREATE TABLE t2 (x int, y int) engine=$engine;
> +
> +CREATE DATABASE db_filtered;
> +use db_filtered;
> +eval CREATE TABLE t1 (x int, y int) engine=$engine;
> +eval CREATE TABLE t2 (x int, y int) engine=$engine;
> +
> +INSERT INTO t2 VALUES (1,2);
> +INSERT INTO t1 SELECT * FROM t2;
> +
> +INSERT INTO db_not_filtered.t1 VALUES (1, 2);
> +disable_warnings;
> +INSERT INTO db_not_filtered.t1 VALUES (UUID(), 2);
> +enable_warnings;
> +
> +use db_not_filtered;
> +INSERT INTO t2 VALUES (1,2);
> +INSERT INTO t1 SELECT * FROM t2;
> +
> +DROP DATABASE db_not_filtered;
> +DROP DATABASE db_filtered;
> +
> +source include/show_binlog_events.inc;
> +
> +SET @@tx_isolation= @old_isolation_level;
>
> === added file 'mysql-test/suite/binlog/t/binlog_stm_do_db-master.opt'
> --- a/mysql-test/suite/binlog/t/binlog_stm_do_db-master.opt 1970-01-01 00:00:00
> +0000
> +++ b/mysql-test/suite/binlog/t/binlog_stm_do_db-master.opt 2009-04-23 11:31:32
> +0000
> @@ -0,0 +1 @@
> +--binlog-do-db=db_not_filtered
>
> === added file 'mysql-test/suite/binlog/t/binlog_stm_do_db.test'
> --- a/mysql-test/suite/binlog/t/binlog_stm_do_db.test 1970-01-01 00:00:00 +0000
> +++ b/mysql-test/suite/binlog/t/binlog_stm_do_db.test 2009-04-23 11:31:32 +0000
> @@ -0,0 +1,52 @@
> +#
> +# BUG#42829: binlogging enabled for all schemas regardless of binlog-db-db /
> +# binlog-ignore-db
> +#
> +# DESCRIPTION
> +# ===========
> +#
> +# The test is implemented as follows:
> +# i) set tx_isolation to read-committed.
> +# ii) create two databases (one filtered other not - using binlog-do-db)
> +# iii) Create statements that are to be filtered on filtered db
> +# - At this point before the patch an error would be thrown.
> +# iv) do the same thing for not the filtered database and check that
> +# events throw an error:
> +#
> +# - Error: 1598 SQLSTATE: HY000 (ER_BINLOG_LOGGING_IMPOSSIBLE)
> +#
> +
> +source include/have_log_bin.inc;
> +source include/have_innodb.inc;
> +source include/have_binlog_format_statement.inc;
> +
> +SET @old_isolation_level= @@tx_isolation;
> +SET tx_isolation= 'READ-COMMITTED';
> +
> +let $engine= InnoDB;
> +
> +CREATE DATABASE db_not_filtered;
> +use db_not_filtered;
> +eval CREATE TABLE t1 (x int, y int) engine=$engine;
> +eval CREATE TABLE t2 (x int, y int) engine=$engine;
> +
> +CREATE DATABASE db_filtered;
> +use db_filtered;
> +eval CREATE TABLE t1 (x int, y int) engine=$engine;
> +eval CREATE TABLE t2 (x int, y int) engine=$engine;
> +
> +INSERT INTO t2 VALUES (1,2);
> +INSERT INTO t1 SELECT * FROM t2;
> +
> +use db_not_filtered;
> +--error 1598
> +INSERT INTO t2 VALUES (1,2);
> +--error 1598
> +INSERT INTO t1 SELECT * FROM t2;
> +
> +DROP DATABASE db_not_filtered;
> +DROP DATABASE db_filtered;
> +
> +source include/show_binlog_events.inc;
> +
> +SET @@tx_isolation= @old_isolation_level;
>
> === modified file 'sql/sql_base.cc'
> --- a/sql/sql_base.cc 2009-03-27 12:55:14 +0000
> +++ b/sql/sql_base.cc 2009-04-23 11:31:32 +0000
> @@ -5089,7 +5089,19 @@ static void mark_real_tables_as_free_for
>
> int decide_logging_format(THD *thd, TABLE_LIST *tables)
> {
> - if (mysql_bin_log.is_open() && (thd->options & OPTION_BIN_LOG))
> + /*
> + Only deciding if:
> +
> + 1. binlog is open and option bin log is set
> +
> + 2. AND statement is not filtered out from binlog OR binlog format
> + is mixed (meaning that filter rules may present different results
> + based on the logging format choosen later - see BUG#43457 for
> + details).
> + */
> + if (mysql_bin_log.is_open() && (thd->options & OPTION_BIN_LOG)
> &&
> + (thd->variables.binlog_format == BINLOG_FORMAT_MIXED ||
> + binlog_filter->db_ok(thd->db)))
> {
> /*
> Compute the starting vectors for the computations by creating a
>
> === modified file 'sql/sql_class.cc'
> --- a/sql/sql_class.cc 2009-03-24 13:58:52 +0000
> +++ b/sql/sql_class.cc 2009-04-23 11:31:32 +0000
> @@ -2894,6 +2894,11 @@ extern "C" void thd_mark_transaction_to_
> {
> mark_transaction_to_rollback(thd, all);
> }
> +
> +extern "C" bool thd_binlog_filter_ok(const MYSQL_THD thd)
> +{
> + return binlog_filter->db_ok(thd->db);
> +}
> #endif // INNODB_COMPATIBILITY_HOOKS */
>
> /****************************************************************************
>
> === modified file 'storage/innobase/handler/ha_innodb.cc'
> --- a/storage/innobase/handler/ha_innodb.cc 2009-03-24 13:58:52 +0000
> +++ b/storage/innobase/handler/ha_innodb.cc 2009-04-23 11:31:32 +0000
> @@ -6810,8 +6810,10 @@ ha_innobase::external_lock(
> {
> ulong const binlog_format= thd_binlog_format(thd);
> ulong const tx_isolation = thd_tx_isolation(current_thd);
> + bool const is_binlog_filter_ok= thd_binlog_filter_ok(thd);
> if (tx_isolation <= ISO_READ_COMMITTED &&
> - binlog_format == BINLOG_FORMAT_STMT)
> + binlog_format == BINLOG_FORMAT_STMT &&
> + is_binlog_filter_ok)
> {
> char buf[256];
> my_snprintf(buf, sizeof(buf),
>
> === modified file 'storage/innobase/handler/ha_innodb.h'
> --- a/storage/innobase/handler/ha_innodb.h 2008-12-14 20:59:50 +0000
> +++ b/storage/innobase/handler/ha_innodb.h 2009-04-23 11:31:32 +0000
> @@ -252,4 +252,11 @@ int thd_binlog_format(const MYSQL_THD th
> @param all TRUE <=> rollback main transaction.
> */
> void thd_mark_transaction_to_rollback(MYSQL_THD thd, bool all);
> +
> +/**
> + Check if binary logging is filtered for thread's current db.
> + @param thd Thread handle
> + @retval 1 the query is not filtered, 0 otherwise.
> +*/
> +bool thd_binlog_filter_ok(const MYSQL_THD thd);
> }
>
