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.
  
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