[Maria-developers] Fwd: [Commits] d1124c9: MDEV-10812 WSREP causes responses being sent to protocol commands that must not send a response

27 Jan 2017

Hi  Nirbhay!

On Wed, Jan 25, 2017 at 7:53 PM, Nirbhay Choubey <nirbhay@mariadb.com> wrote:
...
Hi Sachin,
My comments below.
On Sun, Jan 22, 2017 at 7:18 AM, SachinSetiya <sachin.setiya@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
commits@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

    

Sachin Setiya

tags

participants (1)