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 ? > >> >> 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 Regards Sachin _______________________________________________ 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