maria-developers team mailing list archive

[Alexey Botchkov <holyfoot@xxxxxxxxxxxx>]

> Hm... If the default connection signals "locked" before you start
> waiting for it now - will your test still work?

Well the DEBUG_SYNC manual seems to be explicit about it:
"  A signal remains in effect until it is overwritten. If conn1 signals
  'opened' before conn2 reaches 'now', conn2 will still find the 'opened'
  signal. It does not wait in this case.
"

> > +SET DEBUG_SYNC= "sp_create_routine_started SIGNAL release";
> > +--error 0,ER_BAD_DB_ERROR

> why? Do you mean that even with your sync points the specific execution
> order if not guaranteed?

Not sure i understand your question properly.
But i mean, getting the BAD_DB_ERROR is the execution result i desire here.


Best regards.
HF


03.05.2016 21:38, Sergei Golubchik wrote:
> Hi, Alexey!
>
> convention below: when I start a comment with "btw" it means that you
> don't need to change anything, it's just a thought I had when looking at
> that line in the patch.
>
> On May 03, Alexey Botchkov wrote:
> > revision-id: c75cbc19806223be8e750c27ba0bd4b17ef61f54 (mariadb-10.1.13-24-gc75cbc1)
> > parent(s): 94cd0f6c9b3b04db67501ef29d470f32527ceda2
> > committer: Alexey Botchkov
> > timestamp: 2016-05-03 13:20:12 +0400
> > message:
> >
> > MDEV-717 LP:1003679 - Wrong binlog order on concurrent DROP schema and
> > CREATE function.
> >
> >          Test case added. That required setting some DEBUG_SYNC points.
> >
> > diff --git a/mysql-test/suite/binlog/t/binlog_mdev717.test b/mysql-test/suite/binlog/t/binlog_mdev717.test
> > new file mode 100644
> > index 0000000..5e8cce6
> > --- /dev/null
> > +++ b/mysql-test/suite/binlog/t/binlog_mdev717.test
> > @@ -0,0 +1,49 @@
> > +# MDEV-717 LP:1003679 - Wrong binlog order on concurrent DROP schema and CREATE function.
> > +
> > +--source include/have_log_bin.inc
> > +RESET MASTER;
> > +
> > +disable_warnings;
> > +DROP DATABASE IF EXISTS mysqltest;
> > +enable_warnings;
> btw, generally this isn't necessary anymore, after-test checks verify that
> every test cleans up after itself.
>
> > +connect(con1,localhost,root,,);
> btw, this works also without extra commas:
>
>     connect(con1,localhost,root);
>
> > +connection default;
> > +
> > +CREATE DATABASE mysqltest;
> > +SET DEBUG_SYNC= "wait_drop_db_name_locked SIGNAL locked WAIT_FOR release";
> > +--send DROP DATABASE mysqltest;
> > +connection con1;
> > +SET DEBUG_SYNC= "now WAIT_FOR locked";
> Hm... If the default connection signals "locked" before you start
> waiting for it now - will your test still work? You can test it by
> adding a sleep right after "connection con1". (don't commit the sleep,
> of course, it's just to way to verify whether the test case is valid)
>
> Alternatively, in many places tests wait for a connection to reach the
> certain debug_sync point with, like:
>
>    let $wait_condition= select count(*) = 1 from information_schema.processlist
>                         where state like "debug sync point: wait_drop_db_name_locked%";
>    source include/wait_condition.inc;
>
> > +SET DEBUG_SYNC= "sp_create_routine_started SIGNAL release";
> > +--error 0,ER_BAD_DB_ERROR
> why? Do you mean that even with your sync points the specific execution
> order if not guaranteed?
>
> > +CREATE FUNCTION mysqltest.f1() RETURNS INT RETURN 1;
> > +connection default;
> > +--reap
> > +
> > +CREATE DATABASE mysqltest;
> > +SET DEBUG_SYNC= "wait_drop_db_name_locked SIGNAL locked1 WAIT_FOR release1";
> > +--send DROP DATABASE mysqltest;
> > +connection con1;
> > +SET DEBUG_SYNC= "now WAIT_FOR locked1";
> > +SET DEBUG_SYNC= "create_event_started SIGNAL release1";
> > +--error 0,ER_BAD_DB_ERROR
> > +CREATE EVENT mysqltest.e1 ON SCHEDULE EVERY 15 MINUTE DO BEGIN END;
> > +connection default;
> > +--reap
> > +
> > +CREATE DATABASE mysqltest;
> > +CREATE EVENT mysqltest.e1 ON SCHEDULE EVERY 15 MINUTE DO BEGIN END;
> > +SET DEBUG_SYNC= "wait_drop_db_name_locked SIGNAL locked1 WAIT_FOR release1";
> > +--send DROP DATABASE mysqltest;
> > +connection con1;
> > +SET DEBUG_SYNC= "now WAIT_FOR locked1";
> > +SET DEBUG_SYNC= "update_event_started SIGNAL release1";
> > +--error 0,ER_BAD_DB_ERROR
> > +ALTER EVENT mysqltest.e1 ON SCHEDULE EVERY 20 MINUTE DO BEGIN END;
> > +connection default;
> > +--reap
> > +
> > +SET DEBUG_SYNC= "RESET";
> > +--source include/show_binlog_events.inc
> > +
> > diff --git a/sql/events.cc b/sql/events.cc
> > index 77dbb8b..3c50d6e 100644
> > diff --git a/sql/sp.cc b/sql/sp.cc
> > index a518b52..c2e3fd2 100644
> > --- a/sql/sp.cc
> > +++ b/sql/sp.cc
> > @@ -1043,6 +1043,8 @@ sp_create_routine(THD *thd, stored_procedure_type type, sp_head *sp)
> >     DBUG_ASSERT(type == TYPE_ENUM_PROCEDURE ||
> >                 type == TYPE_ENUM_FUNCTION);
> >   
> > +  DEBUG_SYNC(thd, "sp_create_routine_started");
> > +
> could you use "before_wait_locked_pname" sync point instead?
> It's at the beginning of lock_object_name().
>
> >     /* Grab an exclusive MDL lock. */
> >     if (lock_object_name(thd, mdl_type, sp->m_db.str, sp->m_name.str))
> >     {
> > diff --git a/sql/sql_db.cc b/sql/sql_db.cc
> > index 2ba67cb..fdb49f2 100644
> > --- a/sql/sql_db.cc
> > +++ b/sql/sql_db.cc
> > @@ -837,6 +837,8 @@ mysql_rm_db_internal(THD *thd,char *db, bool if_exists, bool silent)
> >     if (lock_schema_name(thd, dbnorm))
> >       DBUG_RETURN(true);
> >   
> > +  DEBUG_SYNC(thd, "wait_drop_db_name_locked");
> > +
> Could you use "after_wait_locked_schema_name" sync point instead?
> It's the one at the end of lock_schema_name().
>
> >     length= build_table_filename(path, sizeof(path) - 1, db, "", "", 0);
> >     strmov(path+length, MY_DB_OPT_FILE);		// Append db option file name
> >     del_dbopt(path);				// Remove dboption hash entry
> Regards,
> Sergei
> Chief Architect MariaDB
> and security@xxxxxxxxxxx
>
> _______________________________________________
> Mailing list: https://launchpad.net/~maria-developers
> Post to     : maria-developers@xxxxxxxxxxxxxxxxxxx
> Unsubscribe : https://launchpad.net/~maria-developers
> More help   : https://help.launchpad.net/ListHelp