Hi Nirbhay!
On Wed, Jan 25, 2017 at 7:53 PM, Nirbhay Choubey <nirb...@mariadb.com> wrote:
> Hi Sachin,
>
> My comments below.
>
> On Sun, Jan 22, 2017 at 7:18 AM, SachinSetiya <sachin.set...@mariadb.com>
> wrote:
>>
>> revision-id: d1124c98cbede6366e4b4d27a23dcf5b9d207cf0
>> (mariadb-10.1.21-2-gd1124c9)
>> parent(s): a14638581b4c8ef175e68dccff07967d819b3b7e
>> author: Sachin Setiya
>> committer: Sachin Setiya
>> timestamp: 2017-01-22 17:47:29 +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 () which, by spec, must not send a
>
>
> s/close prepared statement ()/close prepared statement and close connection/
>
>> response.
>>
>> Solution:- We will not jump to dispatch_end: when there is deadlock and
>> query is either COM_QUIT or COM_STMT_CLOSE.
>
>
> Or, I would say, in dispatch_command, we handle COM_QUIT and COM_STMT_CLOSE
> commands even in case of error.
Changed.
>
>>
>>
>> Patch Credit:- Jaka Močnik
>>
>> ---
>> .../galera/r/galera_prepared_statement.result | 18 ++++++++++++++++
>> .../suite/galera/t/galera_prepared_statement.test | 25
>> ++++++++++++++++++++++
>> sql/sql_parse.cc | 6 +++++-
>> 3 files changed, 48 insertions(+), 1 deletion(-)
>>
>> diff --git a/mysql-test/suite/galera/r/galera_prepared_statement.result
>> b/mysql-test/suite/galera/r/galera_prepared_statement.result
>> index de5ac9c..3425648 100644
>> --- a/mysql-test/suite/galera/r/galera_prepared_statement.result
>> +++ b/mysql-test/suite/galera/r/galera_prepared_statement.result
>> @@ -27,7 +27,25 @@ ALTER TABLE t1 ADD COLUMN f2 INTEGER;
>> ALTER TABLE t1 DROP COLUMN f1;
>> EXECUTE st1;
>> ERROR 22007: Incorrect integer value: 'abc' for column 'f2' at row 1
>> +SET TRANSACTION ISOLATION LEVEL SERIALIZABLE;
>> +CREATE TABLE t5(a int primary key);
>> +INSERT INTO t5 VALUES(1),(2),(3);
>> +START TRANSACTION ;
>> +PREPARE st5 FROM 'SELECT * FROM t5';
>> +EXECUTE st5;
>> +a
>> +1
>> +2
>> +3
>> +update t5 set a=a+100;
>> +EXECUTE st5;
>> +a
>> +101
>> +102
>> +103
>> +update t5 set a=a+100;
>> DROP TABLE t1;
>> DROP TABLE t2;
>> DROP TABLE t3;
>> DROP TABLE t4;
>> +DROP TABLE t5;
>>
>> diff --git a/mysql-test/suite/galera/t/galera_prepared_statement.test
>> b/mysql-test/suite/galera/t/galera_prepared_statement.test
>> index 3bee097..debc00e 100644
>> --- a/mysql-test/suite/galera/t/galera_prepared_statement.test
>> +++ b/mysql-test/suite/galera/t/galera_prepared_statement.test
>> @@ -38,8 +38,33 @@ ALTER TABLE t1 DROP COLUMN f1;
>> --error ER_TRUNCATED_WRONG_VALUE_FOR_FIELD
>> EXECUTE st1;
>>
>> +# MDEV-10812
>> +# On Closing Prepare Stamemnt, When wsrep_conflict_state is ABORTED
>> +# It causes wrong response to be sent on Clients.
>> +
>> +# First Create a Deadlock
>> --connection node_1
>> +SET TRANSACTION ISOLATION LEVEL SERIALIZABLE;
>> +CREATE TABLE t5(a int primary key);
>> +INSERT INTO t5 VALUES(1),(2),(3);
>> +START TRANSACTION ;
>> +PREPARE st5 FROM 'SELECT * FROM t5';
>> +EXECUTE st5;
>> +update t5 set a=a+100;
>> +EXECUTE st5;
>> +
>> +--sleep 2
>> +--connection node_2
>> +update t5 set a=a+100;
>> +
>> +--sleep 2
>> +--connection node_1
>> +# here we have deadlock
>> +--disconnect node_1
>> +
>> +--connection node_2
>> DROP TABLE t1;
>> DROP TABLE t2;
>> DROP TABLE t3;
>> DROP TABLE t4;
>> +DROP TABLE t5;
>
>
>
> The test always hits command != COM_QUIT assert if I revert :
>
> + if (thd->wsrep_conflict_state == ABORTED &&
> + command != COM_STMT_CLOSE && command != COM_QUIT)
>
> So, I modified your test a bit (simplified) to not use PS.
>
> --echo #
> --echo # MDEV-10812: On closing prepare stamemnt, 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;
>
> I think it should be ok.
>
>
Yes, also Because Initial prepared statement test was written because,
I thought we will hit
COM_STMT_CLOSE, but is simple not possible using mtr.
Test changed.
>>
>> diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc
>> index 033e88a..d45b196 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 STMT_CLOSE execute even if wsrep aborted */
>
Changed.
>
> 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,8 @@ bool dispatch_command(enum enum_server_command
>> command, THD *thd,
>>
>> if (WSREP(thd))
>> {
>> + DBUG_ASSERT((command != COM_QUIT && command != COM_STMT_CLOSE)
>> + || thd->get_stmt_da()->is_disabled());
>
>
> Please add a comment explaining this assert.
>
Added.
> Thanks,
> 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
>
>
http://lists.askmonty.org/pipermail/commits/2017-January/010534.html
--
Regards
Sachin Setiya
Software Engineer at MariaDB
--
Regards
Sachin Setiya
Software Engineer at MariaDB
_______________________________________________
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
