maria-developers team mailing list archive

[andrei.elkin@xxxxxxxxxx]

Sachin, howdy.

> revision-id: 647c22692ad1fe466eb14b749520cc520807d4fa (mariadb-10.1.38-114-g647c22692ad)
> parent(s): 6c5e4c9bc0d9ac30f7ec7ee334630bacb58687ba
> author: Sachin
> committer: Sachin
> timestamp: 2019-04-23 17:58:03 +0530
> message:
>
> MDEV-17588 replicate-do filters cause errors when creating filtered-out tables on master with syntax unsupported on slave
>
> Prototype Commit.
In next commit could you please brief about the problem and attempted
solution.

>
> Delay throwing of error in parsing stage untill we know the result from rpl
> filter.

I think you were on the right track implementing this deferred
error/warning issuing. Thanks for attempting it!

There's a new THD::"flag" for that purpose. I suggest we place it
inside struct LEX_MASTER_INFO to set pretty similarly to how the patch does.

Secondly, let blocks in sql_yacc.yy that call error/warning functions that gets deferred
(because of the slave thread) be invoked (if they will be) in
mysql_execute_command() in verbatim.
That means to turn the blocks into functions and call them at once or
later.
Your patch presents two such blocks, I point to them below.
For each block I suggest we
introduce a flag. The block flag is set when the block gets visited (but
not executed 'cos of slave). The "flag" therefore is going to be an
array (or enum?).

>
> ---
>  mysql-test/suite/rpl/r/rpl_mdev_17588.result    | 26 ++++++++++++++++++++++
>  mysql-test/suite/rpl/t/rpl_mdev_17588-slave.opt |  1 +
>  mysql-test/suite/rpl/t/rpl_mdev_17588.test      | 29 +++++++++++++++++++++++++
>  sql/sql_class.h                                 |  1 +
>  sql/sql_parse.cc                                |  7 ++++++
>  sql/sql_yacc.yy                                 | 12 +++++++---
>  6 files changed, 73 insertions(+), 3 deletions(-)
>
> diff --git a/mysql-test/suite/rpl/r/rpl_mdev_17588.result b/mysql-test/suite/rpl/r/rpl_mdev_17588.result
> new file mode 100644
> index 00000000000..976efde2bdd
> --- /dev/null
> +++ b/mysql-test/suite/rpl/r/rpl_mdev_17588.result
> @@ -0,0 +1,26 @@
> +include/master-slave.inc
> +[connection master]
> +set sql_log_bin= 0;
> +install soname 'ha_tokudb';
> +set sql_log_bin= 1;
> +create table t1 (a int) engine=TokuDB;
> +create table t2 (a int);
> +create table t3 (a int) engine=TokuDB;
> +include/wait_for_slave_sql_error.inc [errno=1286]
> +show create table t1;
> +ERROR 42S02: Table 'test.t1' doesn't exist
> +show create table t2;
> +Table	Create Table
> +t2	CREATE TABLE `t2` (
> +  `a` int(11) DEFAULT NULL
> +) ENGINE=MyISAM DEFAULT CHARSET=latin1
> +show create table t3;
> +ERROR 42S02: Table 'test.t3' doesn't exist
> +SET GLOBAL SQL_SLAVE_SKIP_COUNTER=1;
> +START SLAVE;
> +drop table t1, t2, t3;
> +set sql_log_bin= 0;
> +uninstall soname 'ha_tokudb';
> +set sql_log_bin= 1;
> +CALL mtr.add_suppression('Slave: Unknown storage engine .* Error_code: 1286');
> +include/rpl_end.inc
> diff --git a/mysql-test/suite/rpl/t/rpl_mdev_17588-slave.opt b/mysql-test/suite/rpl/t/rpl_mdev_17588-slave.opt
> new file mode 100644
> index 00000000000..19497afd22a
> --- /dev/null
> +++ b/mysql-test/suite/rpl/t/rpl_mdev_17588-slave.opt
> @@ -0,0 +1 @@
> +--replicate-do-table=test.t2 --replicate-do-table=test.t3 --sql-mode='NO_ENGINE_SUBSTITUTION'
> diff --git a/mysql-test/suite/rpl/t/rpl_mdev_17588.test b/mysql-test/suite/rpl/t/rpl_mdev_17588.test
> new file mode 100644
> index 00000000000..c636077e4d1
> --- /dev/null
> +++ b/mysql-test/suite/rpl/t/rpl_mdev_17588.test
> @@ -0,0 +1,29 @@
> +--source include/master-slave.inc
> +
> +--connection master
> +set sql_log_bin= 0;
> +install soname 'ha_tokudb';
> +set sql_log_bin= 1;
> +
> +create table t1 (a int) engine=TokuDB;
> +create table t2 (a int);
> +create table t3 (a int) engine=TokuDB;
> +
> +--connection slave
> +let $slave_sql_errno= 1286;
> +source include/wait_for_slave_sql_error.inc;
> +--error ER_NO_SUCH_TABLE
> +show create table t1;
> +show create table t2;
> +--error ER_NO_SUCH_TABLE
> +show create table t3;
> +SET GLOBAL SQL_SLAVE_SKIP_COUNTER=1;
> +START SLAVE;
> +--connection master
> +drop table t1, t2, t3;
> +set sql_log_bin= 0;
> +uninstall soname 'ha_tokudb';
> +set sql_log_bin= 1;
> +--sync_slave_with_master
> +CALL mtr.add_suppression('Slave: Unknown storage engine .* Error_code: 1286');
> +--source include/rpl_end.inc
> diff --git a/sql/sql_class.h b/sql/sql_class.h
> index 6640e02147a..4361700ee6c 100644
> --- a/sql/sql_class.h
> +++ b/sql/sql_class.h
> @@ -2802,6 +2802,7 @@ class THD :public Statement,
>    uint8      password; /* 0, 1 or 2 */
>    uint8      failed_com_change_user;
>    bool       slave_thread;
> +  char       *rpl_unavailable_storage_plugin= NULL;
>    bool       extra_port;                        /* If extra connection */
>  
>    bool	     no_errors;
> diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
> index 6649c60f827..5f8c833c76f 100644
> --- a/sql/sql_parse.cc
> +++ b/sql/sql_parse.cc
> @@ -2656,6 +2656,13 @@ mysql_execute_command(THD *thd)
>        /* we warn the slave SQL thread */
>        my_message(ER_SLAVE_IGNORED_TABLE, ER_THD(thd, ER_SLAVE_IGNORED_TABLE),
>                   MYF(0));
> +      thd->rpl_unavailable_storage_plugin= NULL;
> +      DBUG_RETURN(0);
> +    }

> +    if (thd->rpl_unavailable_storage_plugin)
> +    {
> +      my_error(ER_UNKNOWN_STORAGE_ENGINE, MYF(0), thd->rpl_unavailable_storage_plugin);
> +      thd->rpl_unavailable_storage_plugin= NULL;

Here the check would go

       for each thd->lex.mi.flag[i]
         when set
            call "flag_func[i]";

I index flag[] but I don't insist on implementing it like that. Here is the function's body

            {
              if (thd->variables.sql_mode & MODE_NO_ENGINE_SUBSTITUTION)
                my_yyabort_error((ER_UNKNOWN_STORAGE_ENGINE, MYF(0), $1.str));
              $$= 0;
              push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN,
                                  ER_UNKNOWN_STORAGE_ENGINE,
                                  ER_THD(thd, ER_UNKNOWN_STORAGE_ENGINE),
                                  $1.str);
            }

and a flag function also might need some parameters like $1.str in above.

>        DBUG_RETURN(0);
>      }
>      /* 
> diff --git a/sql/sql_yacc.yy b/sql/sql_yacc.yy
> index 5111f0690ab..0756028c34a 100644
> --- a/sql/sql_yacc.yy
> +++ b/sql/sql_yacc.yy
> @@ -2476,7 +2476,7 @@ create:
>              LEX *lex= thd->lex;
>              lex->current_select= &lex->select_lex; 
>              if ((lex->create_info.used_fields & HA_CREATE_USED_ENGINE) &&
> -                !lex->create_info.db_type)
> +                !lex->create_info.db_type && !thd->rpl_unavailable_storage_plugin)
>              {
>                lex->create_info.use_default_db_type(thd);

This is the 2nd block which depends on the 1st one.

            if ((lex->create_info.used_fields & HA_CREATE_USED_ENGINE) &&
                !lex->create_info.db_type)
            {
              lex->create_info.use_default_db_type(thd);
              if (!thd->slave_thread || !lex->mi.flag[0])
               print_warn_using_other_handler(...)
            }

where print_warn_using_other_handler() is
            {
               push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN,
                                   ER_WARN_USING_OTHER_HANDLER,
                                   ER_THD(thd, ER_WARN_USING_OTHER_HANDLER),
                                   hton_name(lex->create_info.db_type)->str,
                                   $5->table.str);
             }

To call it deferredly we seem need two args to memorize (reserve
fields too) in Lex->mi.

I hope my suggestions make sense and feasible.

Cheers,

Andrei


>                push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN,


> @@ -5791,9 +5791,15 @@ storage_engines:
>              else
>              {
>                if (thd->variables.sql_mode & MODE_NO_ENGINE_SUBSTITUTION)
> -                my_yyabort_error((ER_UNKNOWN_STORAGE_ENGINE, MYF(0), $1.str));
> +              {
> +                if (!thd->slave_thread)
> +                  my_yyabort_error((ER_UNKNOWN_STORAGE_ENGINE, MYF(0), $1.str));
> +                else
> +                  thd->rpl_unavailable_storage_plugin= $1.str;
> +              }
>                $$= 0;
> -              push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN,
> +              if (!thd->rpl_unavailable_storage_plugin)
> +                push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN,
>                                    ER_UNKNOWN_STORAGE_ENGINE,
>                                    ER_THD(thd, ER_UNKNOWN_STORAGE_ENGINE),
>                                    $1.str);
> _______________________________________________
> commits mailing list
> commits@xxxxxxxxxxx
> https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits