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