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