Hi Alfranio,
On Sat, 2010-09-25 at 10:59 +0100, Alfranio Correia wrote:
> Hi Luis,
>
> The patch looks great but I have a few concerns.
>
> There is a change in behavior and for that reason the patch should not
> be pushed into 5.1-bugteam. Geert Vanderkelen seems to agree with this.
Yes, I think we should not push to 5.1 (or 5.5).
> If the patch will not be pushed into 5.1 and 5.5, I think we should not
> fix the bug and should just create a build for Ericsson. The filters
> should be changed and completely redesigned as you already know. Maybe
> this is the time for this because some of the changes necessary to have
> great filters are also necessary to have the multi-thread slave.
Yes, they should be redesigned/replaced/improved on... But until then, I
say we fix such little things as this one on what we have today. If it
was a big effort, then I would agree to wait for a new filtering
approach (maybe based on WL#4008, WL#5287 and/or WL#2387). Since it's
not, then why not have this in anyway?
> Let's have a call next week to discuss further details on my idea.
OK.
> See some comments in-line that you need to address.
>
> Cheers.
>
> On 09/20/2010 04:42 PM, Luis Soares wrote:
> > #At file:///home/lsoares/Workspace/bzr/work/bugfixing/46110/mysql-5.1-bugteam/
> based on revid:alfranio.correia@stripped
> >
> > 3514 Luis Soares 2010-09-20
> > BUG#46110: --replicate-wild-do-table invalidates
> > --replicate-do-db matching rule
> >
> > When using --replicate-do-db and --replicate-ignore-db with
> > --replicate-wild-do-table, some database level statements, eg
> > DROP DATABASE or CREATE DATABASE, might not be replayed even if
> > the DATABASE name was inline with the rules set in
> > --replicate-do-db and/or --replicate-ignore-db. The following
> > example would not replicate correctly:
> >
> > SLAVE, configured with:
> > --replicate-do-db=dbx --replicate-wild-do-table=db%.t1
> >
> > MASTER, dba issues:
> > MASTER> CREATE DATABASE dbx;
> >
> > Although, at first sight, there is nothing in the filter rules
> > that might prevent this statement to be replayed, the slave would
> > not execute it. In fact, "replicate-wild-do-table=db%.t1"
> > invalidates the "OK" from "replicate-do-db=dbx", making the event
> > to be skipped/filtered out.
> >
> > We fix this, by making --replicate-do/ignore-db rules take
> > precedence over replicate-wild-do-table for *database* level
> > statements only:
> >
> > - CREATE/DROP/ALTER DATABASE ...
> >
> > As such, when replicating such statements, we only look into
> > replicate-wild-do-table if and only if replicate-do/ignore-db
> > rules are void. Otherwise, --replicate-do/ignore-db rules
> > dictate the fate of the statement: either filtered out or
> > executed.
> >
> > added:
> > mysql-test/suite/rpl/r/rpl_filter_bug.result
> > mysql-test/suite/rpl/t/rpl_filter_bug-slave.opt
> > mysql-test/suite/rpl/t/rpl_filter_bug.test
> > modified:
> > sql/sql_parse.cc
> > === added file 'mysql-test/suite/rpl/r/rpl_filter_bug.result'
> > --- a/mysql-test/suite/rpl/r/rpl_filter_bug.result 1970-01-01 00:00:00 +0000
> > +++ b/mysql-test/suite/rpl/r/rpl_filter_bug.result 2010-09-20 15:42:12 +0000
> > @@ -0,0 +1,6 @@
> > +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;
> >
> > === added file 'mysql-test/suite/rpl/t/rpl_filter_bug-slave.opt'
> > --- a/mysql-test/suite/rpl/t/rpl_filter_bug-slave.opt 1970-01-01 00:00:00 +0000
> > +++ b/mysql-test/suite/rpl/t/rpl_filter_bug-slave.opt 2010-09-20 15:42:12 +0000
> > @@ -0,0 +1 @@
> > +--replicate-do-db=dbx --replicate-wild-do-table=db%.t1
> >
> > === added file 'mysql-test/suite/rpl/t/rpl_filter_bug.test'
> > --- a/mysql-test/suite/rpl/t/rpl_filter_bug.test 1970-01-01 00:00:00 +0000
> > +++ b/mysql-test/suite/rpl/t/rpl_filter_bug.test 2010-09-20 15:42:12 +0000
> > @@ -0,0 +1,76 @@
> > +#
> > +# BUG#46110: --replicate-wild-do-table invalidates --replicate-do-db matching
> rule
> > +#
> > +
> > +-- source include/master-slave.inc
> > +
> > +## Checking CREATE DATABASE
> > +
> > +-- disable_query_log
> > +CREATE DATABASE dbx DEFAULT CHARACTER SET=latin1;
> > +-- enable_query_log
> > +
> > +if (!`SELECT count(*) = 1 FROM information_schema.SCHEMATA WHERE schema_name =
> 'dbx'`)
> > +{
> > + -- echo unexpected default character set for database: dbx
>
> I don't think this is the correct message.
Erhmm.... Copy and paste issue! Sorry! Same applies to the ones below.
> > + -- source include/show_rpl_debug_info.inc
> > + -- die
> > +}
> > +
> > +-- sync_slave_with_master
> > +
> > +if (!`SELECT count(*) = 1 FROM information_schema.SCHEMATA WHERE schema_name =
> 'dbx'`)
> > +{
> > + -- echo unexpected default character set for database: dbx
>
> I don't think this is the correct message.
>
> > + -- source include/show_rpl_debug_info.inc
> > + -- die
> > +}
> > +
> > +## Checking ALTER DATABASE
> > +
> > +-- connection master
> > +
> > +-- disable_query_log
> > +ALTER DATABASE dbx DEFAULT CHARACTER SET=latin5;
> > +-- enable_query_log
> > +
> > +if (!`SELECT DEFAULT_CHARACTER_SET_NAME='latin5' FROM
> information_schema.SCHEMATA WHERE schema_name = 'dbx'`)
> > +{
> > + -- echo unexpected default character set for database: dbx
> > + -- source include/show_rpl_debug_info.inc
> > + -- die
> > +}
> > +
> > +-- sync_slave_with_master
> > +
> > +if (!`SELECT DEFAULT_CHARACTER_SET_NAME='latin5' FROM
> information_schema.SCHEMATA WHERE schema_name = 'dbx'`)
> > +{
> > + -- echo unexpected default character set for database: dbx
> > + -- source include/show_rpl_debug_info.inc
> > + -- die
> > +}
> > +
> > +## Checking DROP DATABASE
> > +
> > +-- connection master
> > +-- disable_query_log
> > +DROP DATABASE dbx;
> > +-- enable_query_log
> > +
> > +if (!`SELECT count(*) = 0 FROM information_schema.SCHEMATA WHERE schema_name =
> 'dbx'`)
> > +{
> > + -- echo unexpected default character set for database: dbx
>
> I don't think this is the correct message.
>
> > + -- source include/show_rpl_debug_info.inc
> > + -- die
> > +}
> > +
> > +-- sync_slave_with_master
> > +
> > +if (!`SELECT count(*) = 0 FROM information_schema.SCHEMATA WHERE schema_name =
> 'dbx'`)
> > +{
> > + -- echo unexpected default character set for database: dbx
>
> I don't think this is the correct message.
>
> > + -- source include/show_rpl_debug_info.inc
> > + -- die
> > +}
> > +
> > +-- source include/master-slave-end.inc
> >
> > === modified file 'sql/sql_parse.cc'
> > --- a/sql/sql_parse.cc 2010-08-18 04:56:06 +0000
> > +++ b/sql/sql_parse.cc 2010-09-20 15:42:12 +0000
> > @@ -217,6 +217,34 @@ inline bool all_tables_not_ok(THD *thd,
> > return rpl_filter->is_on()&& tables&&
> !thd->spcont&&
> > !rpl_filter->tables_ok(thd->db, tables);
> > }
> > +
> > +inline bool db_stmt_db_ok(THD *thd, char* db)
> > +{
> > + DBUG_ENTER("db_stmt_db_ok");
> > +
> > + if (!thd->slave_thread)
> > + DBUG_RETURN(TRUE);
> > +
> > + /*
> > +
> > + Checking whether the event for the given database, db, should
> > + be ignored or not, is done by checking whether there are
> > + active rules in ignore_db or in do_db containers. If there
> > + are, then check if there is a match, if not then check the
> > + wild_do rules.
> > +
> > + This means that when using this function replicate-do-db and
> > + replicate-ignore-db take precedence over wild do rules.
> > +
> > + */
Hmm... ok, I can make this a function description and move it
outside the function body.
> > + bool filtered= (!(rpl_filter->get_do_db()->is_empty()&&
> > + rpl_filter->get_ignore_db()->is_empty())) ? /*
> filters exist in ignore/do_db ? */
> > + !rpl_filter->db_ok(db) : /*
> then just check them */
> > + !rpl_filter->db_ok_with_wild_table(db); /* if
> not, then check wild do table */
> > +
> > + DBUG_RETURN(!filtered);
> > +}
> > #endif
> >
> >
> > @@ -3646,9 +3674,7 @@ end_with_restore_list:
> > above was not called. So we have to check rules again here.
> > */
> > #ifdef HAVE_REPLICATION
> > - if (thd->slave_thread&&
> > - (!rpl_filter->db_ok(lex->name.str) ||
> > - !rpl_filter->db_ok_with_wild_table(lex->name.str)))
> > + if (!db_stmt_db_ok(thd, lex->name.str))
> > {
> > my_message(ER_SLAVE_IGNORED_TABLE, ER(ER_SLAVE_IGNORED_TABLE), MYF(0));
> > break;
> > @@ -3681,9 +3707,7 @@ end_with_restore_list:
> > above was not called. So we have to check rules again here.
> > */
> > #ifdef HAVE_REPLICATION
> > - if (thd->slave_thread&&
> > - (!rpl_filter->db_ok(lex->name.str) ||
> > - !rpl_filter->db_ok_with_wild_table(lex->name.str)))
> > + if (!db_stmt_db_ok(thd, lex->name.str))
> > {
> > my_message(ER_SLAVE_IGNORED_TABLE, ER(ER_SLAVE_IGNORED_TABLE), MYF(0));
> > break;
> > @@ -3710,9 +3734,7 @@ end_with_restore_list:
> > break;
> > }
> > #ifdef HAVE_REPLICATION
> > - if (thd->slave_thread&&
> > - (!rpl_filter->db_ok(db->str) ||
> > - !rpl_filter->db_ok_with_wild_table(db->str)))
> > + if (!db_stmt_db_ok(thd, lex->name.str))
> > {
> > res= 1;
> > my_message(ER_SLAVE_IGNORED_TABLE, ER(ER_SLAVE_IGNORED_TABLE), MYF(0));
> > @@ -3764,9 +3786,7 @@ end_with_restore_list:
> > above was not called. So we have to check rules again here.
> > */
> > #ifdef HAVE_REPLICATION
> > - if (thd->slave_thread&&
> > - (!rpl_filter->db_ok(db->str) ||
> > - !rpl_filter->db_ok_with_wild_table(db->str)))
> > + if (!db_stmt_db_ok(thd, lex->name.str))
> > {
> > my_message(ER_SLAVE_IGNORED_TABLE, ER(ER_SLAVE_IGNORED_TABLE), MYF(0));
> > break;
> >
> >
> >
> >
> >
>
Regards,
--
Luís Soares
