diff --git a/mysql-test/main/xa.result b/mysql-test/main/xa.result index 75f32a1a0aa..faabf1cf695 100644 --- a/mysql-test/main/xa.result +++ b/mysql-test/main/xa.result @@ -544,6 +544,54 @@ formatID gtrid_length bqual_length data drop table asd; disconnect con_tmp; connection default; +# MDEV-30978: a read-only server should still allow the commit of +# read-only XA transactions +set @sav_read_only=@@global.read_only; +set global read_only=1; +xa start '1'; +select 0; +0 +0 +xa end '1'; +xa prepare '1'; +xa commit '1'; +xa start '2'; +select 0; +0 +0 +xa end '2'; +xa prepare '2'; +xa rollback '2'; +# Read-only disconnect case +connect con1_ro,localhost,root,,; +xa start '3'; +select 0; +0 +0 +xa end '3'; +xa prepare '3'; +disconnect con1_ro; +connection default; +xa recover; +formatID gtrid_length bqual_length data +1 1 0 3 +xa commit '3'; +ERROR XA100: XA_RBROLLBACK: Transaction branch was rolled back +connect con2_ro,localhost,root,,; +xa start '4'; +select 0; +0 +0 +xa end '4'; +xa prepare '4'; +disconnect con2_ro; +connection default; +xa recover; +formatID gtrid_length bqual_length data +1 1 0 4 +xa rollback '4'; +ERROR XA100: XA_RBROLLBACK: Transaction branch was rolled back +set @@global.read_only=@sav_read_only; # # End of 10.5 tests # diff --git a/mysql-test/main/xa.test b/mysql-test/main/xa.test index 408c7e01c9d..e1ca39be9ab 100644 --- a/mysql-test/main/xa.test +++ b/mysql-test/main/xa.test @@ -695,6 +695,58 @@ disconnect con_tmp; --source include/wait_until_disconnected.inc connection default; +--echo # MDEV-30978: a read-only server should still allow the commit of +--echo # read-only XA transactions +set @sav_read_only=@@global.read_only; +set global read_only=1; + +# Commit case +xa start '1'; +select 0; +xa end '1'; +xa prepare '1'; +xa commit '1'; + +# Rollback case +xa start '2'; +select 0; +xa end '2'; +xa prepare '2'; +xa rollback '2'; + +--echo # Read-only disconnect case + +--source include/count_sessions.inc +connect (con1_ro,localhost,root,,); +xa start '3'; +select 0; +xa end '3'; +xa prepare '3'; +disconnect con1_ro; + +connection default; +--source include/wait_until_count_sessions.inc +xa recover; +--error ER_XA_RBROLLBACK +xa commit '3'; + +--source include/count_sessions.inc +connect (con2_ro,localhost,root,,); +xa start '4'; +select 0; +xa end '4'; +xa prepare '4'; +disconnect con2_ro; + +connection default; +--source include/wait_until_count_sessions.inc +xa recover; +--error ER_XA_RBROLLBACK +xa rollback '4'; + +set @@global.read_only=@sav_read_only; + + --echo # --echo # End of 10.5 tests --echo # diff --git a/mysql-test/suite/rpl/r/rpl_read_only.result b/mysql-test/suite/rpl/r/rpl_read_only.result index dbcd58a224f..a68b6f6d53e 100644 --- a/mysql-test/suite/rpl/r/rpl_read_only.result +++ b/mysql-test/suite/rpl/r/rpl_read_only.result @@ -139,6 +139,26 @@ insert into t1 values(1006); ERROR HY000: The MariaDB server is running with the --read-only option so it cannot execute this statement insert into t2 values(2006); ERROR HY000: The MariaDB server is running with the --read-only option so it cannot execute this statement +# +# MDEV-30978: On slave XA COMMIT/XA ROLLBACK fail to return an error in read-only mode +# +# Where a read-only server permits writes through replication, it +# should not permit user connections to commit/rollback XA transactions +# prepared via replication. This test ensure this behavior is prohibited +# +connection master; +xa start '1'; +insert into t1 values (1007); +xa end '1'; +xa prepare '1'; +connection slave; +connection slave2; +xa commit '1'; +ERROR HY000: The MariaDB server is running with the --read-only option so it cannot execute this statement +xa rollback '1'; +ERROR HY000: The MariaDB server is running with the --read-only option so it cannot execute this statement +connection master; +xa rollback '1'; connection master; drop user test; drop table t1; diff --git a/mysql-test/suite/rpl/t/rpl_read_only.test b/mysql-test/suite/rpl/t/rpl_read_only.test index c4781bbbb3b..6f53221bfa2 100644 --- a/mysql-test/suite/rpl/t/rpl_read_only.test +++ b/mysql-test/suite/rpl/t/rpl_read_only.test @@ -117,6 +117,33 @@ insert into t1 values(1006); --error ER_OPTION_PREVENTS_STATEMENT insert into t2 values(2006); + +--echo # +--echo # MDEV-30978: On slave XA COMMIT/XA ROLLBACK fail to return an error in read-only mode +--echo # +--echo # Where a read-only server permits writes through replication, it +--echo # should not permit user connections to commit/rollback XA transactions +--echo # prepared via replication. This test ensure this behavior is prohibited +--echo # + +# Note: slave's read_only=1 is set prior to this test case + +connection master; +xa start '1'; +insert into t1 values (1007); +xa end '1'; +xa prepare '1'; +sync_slave_with_master; + +connection slave2; +--error ER_OPTION_PREVENTS_STATEMENT +xa commit '1'; +--error ER_OPTION_PREVENTS_STATEMENT +xa rollback '1'; + +connection master; +xa rollback '1'; + ## Cleanup connection master; drop user test; diff --git a/sql/handler.cc b/sql/handler.cc index 7f591b8456c..49e9b0bcbea 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -1668,10 +1668,7 @@ int ha_commit_trans(THD *thd, bool all) DEBUG_SYNC(thd, "ha_commit_trans_after_acquire_commit_lock"); } - if (rw_trans && - opt_readonly && - !(thd->security_ctx->master_access & PRIV_IGNORE_READ_ONLY) && - !thd->slave_thread) + if (rw_trans && thd->is_read_only_ctx()) { my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--read-only"); goto err; diff --git a/sql/sql_class.h b/sql/sql_class.h index c373c0f6a43..f0f5ac47179 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -2787,6 +2787,16 @@ public: return m_binlog_filter_state; } + /** + Checks if a user connection is read-only + */ + inline bool is_read_only_ctx() + { + return opt_readonly && + !(security_ctx->master_access & PRIV_IGNORE_READ_ONLY) && + !slave_thread; + } + private: /** Indicate if the current statement should be discarded diff --git a/sql/xa.cc b/sql/xa.cc index b83dcbf2085..35834165b5b 100644 --- a/sql/xa.cc +++ b/sql/xa.cc @@ -601,6 +601,16 @@ bool trans_xa_commit(THD *thd) if (auto xs= xid_cache_search(thd, thd->lex->xid)) { bool xid_deleted= false; + MDL_request mdl_request; + bool rw_trans= (xs->rm_error != ER_XA_RBROLLBACK); + + if (rw_trans && thd->is_read_only_ctx()) + { + my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--read-only"); + res= 1; + goto _end_external_xid; + } + res= xa_trans_rolled_back(xs); /* Acquire metadata lock which will ensure that COMMIT is blocked @@ -609,7 +619,6 @@ bool trans_xa_commit(THD *thd) We allow FLUSHer to COMMIT; we assume FLUSHer knows what it does. */ - MDL_request mdl_request; MDL_REQUEST_INIT(&mdl_request, MDL_key::BACKUP, "", "", MDL_BACKUP_COMMIT, MDL_EXPLICIT); if (thd->mdl_context.acquire_lock(&mdl_request, @@ -659,7 +668,11 @@ bool trans_xa_commit(THD *thd) DBUG_RETURN(res); } - if (xa_trans_rolled_back(xid_state.xid_cache_element)) + if (thd->transaction->all.is_trx_read_write() && thd->is_read_only_ctx()) + { + my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--read-only"); + DBUG_RETURN(TRUE); + } else if (xa_trans_rolled_back(xid_state.xid_cache_element)) { xa_trans_force_rollback(thd); DBUG_RETURN(thd->is_error()); @@ -777,6 +790,15 @@ bool trans_xa_rollback(THD *thd) bool res; bool xid_deleted= false; MDL_request mdl_request; + bool rw_trans= (xs->rm_error != ER_XA_RBROLLBACK); + + if (rw_trans && thd->is_read_only_ctx()) + { + my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--read-only"); + res= 1; + goto _end_external_xid; + } + MDL_REQUEST_INIT(&mdl_request, MDL_key::BACKUP, "", "", MDL_BACKUP_COMMIT, MDL_EXPLICIT); if (thd->mdl_context.acquire_lock(&mdl_request, @@ -822,7 +844,11 @@ bool trans_xa_rollback(THD *thd) DBUG_RETURN(thd->get_stmt_da()->is_error()); } - if (xid_state.xid_cache_element->xa_state == XA_ACTIVE) + if (thd->transaction->all.is_trx_read_write() && thd->is_read_only_ctx()) + { + my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--read-only"); + DBUG_RETURN(TRUE); + } else if (xid_state.xid_cache_element->xa_state == XA_ACTIVE) { xid_state.er_xaer_rmfail(); DBUG_RETURN(TRUE);