Re: [Maria-developers] [Commits] d44c14e: MDEV-10812 WSREP causes responses being sent to protocol commands that must not send a response
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
HI Nirbhay! On Sun, Jan 29, 2017 at 11:42 PM, Nirbhay Choubey <nirbhay@mariadb.com> wrote:
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. Changed, But does this rule also apply to First line ?
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
_______________________________________________ commits mailing list commits@mariadb.org https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits
Pushed to bb-10.1-sachin. https://buildbot.askmonty.org/buildbot/grid?category=main&branch=bb-10.1-sachin Regards Sachin
Hi Sachin, On Mon, Jan 30, 2017 at 12:10 AM, Sachin Setiya <sachin.setiya@mariadb.com> wrote:
HI Nirbhay!
On Sun, Jan 29, 2017 at 11:42 PM, Nirbhay Choubey <nirbhay@mariadb.com> wrote:
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. Changed, But does this rule also apply to First line ?
Yes, I normally break it too.
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
_______________________________________________ commits mailing list commits@mariadb.org https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits
Pushed to bb-10.1-sachin. https://buildbot.askmonty.org/buildbot/grid?category=main& branch=bb-10.1-sachin
Best, Nirbhay
participants (2)
-
Nirbhay Choubey -
Sachin Setiya