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.

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.

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 */

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.

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