List:Commits« Previous MessageNext Message »
From:Luís Soares Date:September 27 2010 10:41am
Subject:Re: bzr commit into mysql-5.1-bugteam branch (luis.soares:3514)
Bug#46110
View as plain text  
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

Thread
bzr commit into mysql-5.1-bugteam branch (luis.soares:3514) Bug#46110Luis Soares20 Sep
  • Re: bzr commit into mysql-5.1-bugteam branch (luis.soares:3514) Bug#46110Alfranio Correia25 Sep
    • Re: bzr commit into mysql-5.1-bugteam branch (luis.soares:3514) Bug#46110Alfranio Correia25 Sep
    • Re: bzr commit into mysql-5.1-bugteam branch (luis.soares:3514)Bug#46110Luís Soares27 Sep
      • Re: bzr commit into mysql-5.1-bugteam branch (luis.soares:3514) Bug#46110Alfranio Correia27 Sep
        • Re: bzr commit into mysql-5.1-bugteam branch (luis.soares:3514)Bug#46110Luís Soares6 Oct
  • Re: bzr commit into mysql-5.1-bugteam branch (luis.soares:3514)Bug#46110Daogang Qu28 Sep
    • Re: bzr commit into mysql-5.1-bugteam branch (luis.soares:3514)Bug#46110Luís Soares28 Sep
      • Re: bzr commit into mysql-5.1-bugteam branch (luis.soares:3514)Bug#46110Daogang Qu29 Sep
    • Re: bzr commit into mysql-5.1-bugteam branch (luis.soares:3514)Bug#46110Daogang Qu28 Sep