Hi Sachin,
On Mon, Jan 30, 2017 at 12:10 AM, Sachin Setiya <sachin.set...@mariadb.com>
wrote:
> HI Nirbhay!
>
> On Sun, Jan 29, 2017 at 11:42 PM, Nirbhay Choubey <nirb...@mariadb.com>
> wrote:
> > Hi Sachin!
> >
> > On Fri, Jan 27, 2017 at 12:53 AM, SachinSetiya <
> sachin.set...@mariadb.com>
> > 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
> >> comm...@mariadb.org
> >> https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits
> >
> >
> >
> > _______________________________________________
> > commits mailing list
> > comm...@mariadb.org
> > 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
_______________________________________________
Mailing list: https://launchpad.net/~maria-developers
Post to : maria-developers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~maria-developers
More help : https://help.launchpad.net/ListHelp
