maria-developers team mailing list archive

[Nirbhay Choubey <nirbhay@xxxxxxxxxxx>]

Hi Sachin,

On Mon, Jan 30, 2017 at 12:10 AM, Sachin Setiya <sachin.setiya@xxxxxxxxxxx>
wrote:

> HI Nirbhay!
>
> On Sun, Jan 29, 2017 at 11:42 PM, Nirbhay Choubey <nirbhay@xxxxxxxxxxx>
> wrote:
> > Hi Sachin!
> >
> > On Fri, Jan 27, 2017 at 12:53 AM, SachinSetiya <
> sachin.setiya@xxxxxxxxxxx>
> > wrote:
> >>
> >> revision-id: d44c14e1e0dea312779ba0a8633583ee94284295
> >> (mariadb-10.1.21-2-gd44c14e)
> >> parent(s): a14638581b4c8ef175e68dccff07967d819b3b7e
> >> author: Sachin Setiya
> >> committer: Sachin Setiya
> >> timestamp: 2017-01-27 11:23:17 +0530
> >> message:
> >>
> >> MDEV-10812 WSREP causes responses being sent to protocol commands that
> >> must not send a response
> >>
> >> Problem:- When using wsrep (w/ galera) and issuing commands that can
> cause
> >> deadlocks, deadlock exception errors are sent in responses to commands
> >> such as close prepared statement and close connection which, by spec,
> must
> >> not send a
> >
> >
> > In commit message, we normally limit line width to 60 chars.
> Changed, But does this rule also apply to First line ?
>

Yes, I normally break it too.


> >
> >>
> >> response.
> >>
> >> Solution:- In dispatch_command, we will handle COM_QUIT and
> COM_STMT_CLOSE
> >> commands even in case of error.
> >
> >
> > I think you forgot to add the patch credit you had in previous commits.
> >
> >>
> >>
> >> ---
> >>  mysql-test/suite/galera/r/galera_mdev_10812.result | 11 +++++++++
> >>  mysql-test/suite/galera/t/galera_mdev_10812.test   | 27
> >> ++++++++++++++++++++++
> >>  sql/sql_parse.cc                                   | 10 +++++++-
> >>  3 files changed, 47 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/mysql-test/suite/galera/r/galera_mdev_10812.result
> >> b/mysql-test/suite/galera/r/galera_mdev_10812.result
> >> new file mode 100644
> >> index 0000000..fba1000
> >> --- /dev/null
> >> +++ b/mysql-test/suite/galera/r/galera_mdev_10812.result
> >> @@ -0,0 +1,11 @@
> >> +#
> >> +# MDEV-10812: On COM_STMT_CLOSE/COM_QUIT, when wsrep_conflict_state
> >> +# is ABORTED, it causes wrong response to be sent to the client
> >> +#
> >> +SET TRANSACTION ISOLATION LEVEL SERIALIZABLE;
> >> +CREATE TABLE t1(a INT PRIMARY KEY);
> >> +INSERT INTO t1 VALUES(1),(2),(3);
> >> +START TRANSACTION ;
> >> +UPDATE t1 SET a=a+100;
> >> +UPDATE t1 SET a=a+100;
> >> +DROP TABLE t1;
> >> diff --git a/mysql-test/suite/galera/t/galera_mdev_10812.test
> >> b/mysql-test/suite/galera/t/galera_mdev_10812.test
> >> new file mode 100644
> >> index 0000000..4539ab6
> >> --- /dev/null
> >> +++ b/mysql-test/suite/galera/t/galera_mdev_10812.test
> >> @@ -0,0 +1,27 @@
> >> +--source include/galera_cluster.inc
> >> +--source include/have_innodb.inc
> >> +
> >> +--echo #
> >> +--echo # MDEV-10812: On COM_STMT_CLOSE/COM_QUIT, when
> >> wsrep_conflict_state
> >> +--echo # is ABORTED, it causes wrong response to be sent to the client
> >> +--echo #
> >> +
> >> +#  First create a deadlock
> >> +--connect node_1a, 127.0.0.1, root, , test, $NODE_MYPORT_1
> >> +SET TRANSACTION ISOLATION LEVEL SERIALIZABLE;
> >> +CREATE TABLE t1(a INT PRIMARY KEY);
> >> +INSERT INTO t1 VALUES(1),(2),(3);
> >> +START TRANSACTION ;
> >> +UPDATE t1 SET a=a+100;
> >> +
> >> +--sleep 2
> >> +--connection node_2
> >> +UPDATE t1 SET a=a+100;
> >> +
> >> +--sleep 2
> >> +--connection node_1a
> >> +# here we get deadlock error
> >> +--disconnect node_1a
> >> +
> >> +--connection node_2
> >> +DROP TABLE t1;
> >> diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
> >> index 033e88a..cb0bd41 100644
> >> --- a/sql/sql_parse.cc
> >> +++ b/sql/sql_parse.cc
> >> @@ -1265,7 +1265,9 @@ bool dispatch_command(enum enum_server_command
> >> command, THD *thd,
> >>      {
> >>        wsrep_client_rollback(thd);
> >>      }
> >> -    if (thd->wsrep_conflict_state== ABORTED)
> >> +    /* We let COM_QUIT and COM_STMT_CLOSE to execute even if wsrep
> >> aborted. */
> >> +    if (thd->wsrep_conflict_state == ABORTED &&
> >> +        command != COM_STMT_CLOSE && command != COM_QUIT)
> >>      {
> >>        my_error(ER_LOCK_DEADLOCK, MYF(0), "wsrep aborted transaction");
> >>        WSREP_DEBUG("Deadlock error for: %s", thd->query());
> >> @@ -1918,6 +1920,12 @@ bool dispatch_command(enum enum_server_command
> >> command, THD *thd,
> >>
> >>    if (WSREP(thd))
> >>    {
> >> +    /*
> >> +      MDEV-10812
> >> +      In the case of COM_QUIT/COM_CLOSE thread status should be
> disabled.
> >
> >
> > s/COM_CLOSE/COM_STMT_CLOSE/
> >
> >>
> >> +    */
> >> +    DBUG_ASSERT((command != COM_QUIT && command != COM_STMT_CLOSE)
> >> +                || thd->get_stmt_da()->is_disabled());
> >
> >
> > Could you check the indentation of the above line?
> >
> > Considering above changes, I have no more suggestions. Ok to push.
> >
> > Best,
> > Nirbhay
> >
> >>
> >>      /* wsrep BF abort in query exec phase */
> >>      mysql_mutex_lock(&thd->LOCK_wsrep_thd);
> >>      do_end_of_statement= thd->wsrep_conflict_state != REPLAYING &&
> >> _______________________________________________
> >> commits mailing list
> >> commits@xxxxxxxxxxx
> >> https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits
> >
> >
> >
> > _______________________________________________
> > commits mailing list
> > commits@xxxxxxxxxxx
> > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits
>
> Pushed to bb-10.1-sachin.
> https://buildbot.askmonty.org/buildbot/grid?category=main&;
> branch=bb-10.1-sachin
>
>
Best,
Nirbhay