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

HI Nirbhay!

On Sun, Jan 29, 2017 at 11:42 PM, Nirbhay Choubey <nirbhay@mariadb.com> wrote:
> Hi Sachin!
>
> On Fri, Jan 27, 2017 at 12:53 AM, SachinSetiya <sachin.setiya@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
>> commits@mariadb.org
>> https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits
>
>
>
> _______________________________________________
> commits mailing list
> commits@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