From 77b7a86206508a074eb6750ba628dfbef3f39e74 Mon Sep 17 00:00:00 2001 From: unknown Date: Mon, 20 Nov 2006 20:40:35 -0700 Subject: [PATCH 1/2] WL#3602 (SET GLOBAL READONLY) Bug#11733 (COMMITs should not happen if read-only is set) Bug#22009 (Can write to a read-only server under some circumstances) See the work log for details The change consist of a) acquiring the global read lock in SET GLOBAL READONLY b) honoring opt_readonly in ha_commit_trans(), c) honoring opt_readonly in mysql_lock_tables(). a) takes care of the server stability, b) makes the transactional tables safe (Bug 11733) c) makes the non transactional tables safe (Bug 22009) mysql-test/r/read_only.result: WL#3602 (SET GLOBAL READONLY) mysql-test/t/read_only.test: WL#3602 (SET GLOBAL READONLY) sql/handler.cc: WL#3602 (SET GLOBAL READONLY) sql/lock.cc: WL#3602 (SET GLOBAL READONLY) sql/set_var.cc: WL#3602 (SET GLOBAL READONLY) sql/set_var.h: WL#3602 (SET GLOBAL READONLY) mysql-test/r/read_only_innodb.result: WL#3602 (SET GLOBAL READONLY) mysql-test/t/read_only_innodb.test: WL#3602 (SET GLOBAL READONLY) --- mysql-test/r/read_only.result | 52 ++++++++++++- mysql-test/r/read_only_innodb.result | 18 +++++ mysql-test/t/read_only.test | 110 ++++++++++++++++++++++++++- mysql-test/t/read_only_innodb.test | 43 +++++++++++ sql/handler.cc | 9 +++ sql/lock.cc | 13 ++++ sql/set_var.cc | 66 +++++++++++++++- sql/set_var.h | 14 ++++ 8 files changed, 321 insertions(+), 4 deletions(-) create mode 100644 mysql-test/r/read_only_innodb.result create mode 100644 mysql-test/t/read_only_innodb.test diff --git a/mysql-test/r/read_only.result b/mysql-test/r/read_only.result index 1a1991a6255..337143ce414 100644 --- a/mysql-test/r/read_only.result +++ b/mysql-test/r/read_only.result @@ -39,6 +39,56 @@ delete t1 from t1,t3 where t1.a=t3.a; drop table t1; insert into t1 values(1); ERROR HY000: The MySQL server is running with the --read-only option so it cannot execute this statement +set global read_only=0; +lock table t1 write; +lock table t2 write; +set global read_only=1; +ERROR HY000: Can't execute the given command because you have active locked tables or an active transaction +unlock tables ; +set global read_only=1; +select @@global.read_only; +@@global.read_only +0 +unlock tables ; +select @@global.read_only; +@@global.read_only +1 +set global read_only=0; +lock table t1 read; +lock table t2 read; +set global read_only=1; +ERROR HY000: Can't execute the given command because you have active locked tables or an active transaction +unlock tables ; +set global read_only=1; +select @@global.read_only; +@@global.read_only +0 +unlock tables ; +select @@global.read_only; +@@global.read_only +1 +set global read_only=0; +BEGIN; +BEGIN; +set global read_only=1; +ERROR HY000: Can't execute the given command because you have active locked tables or an active transaction +ROLLBACK; +set global read_only=1; +select @@global.read_only; +@@global.read_only +1 +ROLLBACK; +set global read_only=0; +flush tables with read lock; +set global read_only=1; +unlock tables; +set global read_only=0; +flush tables with read lock; +set global read_only=1; +select @@global.read_only; +@@global.read_only +1 +unlock tables; +set global read_only=0; drop table t1,t2; drop user test@localhost; -set global read_only=0; diff --git a/mysql-test/r/read_only_innodb.result b/mysql-test/r/read_only_innodb.result new file mode 100644 index 00000000000..d028e3cc207 --- /dev/null +++ b/mysql-test/r/read_only_innodb.result @@ -0,0 +1,18 @@ +DROP TABLE IF EXISTS table_11733 ; +grant CREATE, SELECT, DROP on *.* to test@localhost; +set global read_only=0; +create table table_11733 (a int) engine=InnoDb; +BEGIN; +insert into table_11733 values(11733); +set global read_only=1; +select @@global.read_only; +@@global.read_only +1 +select * from table_11733 ; +a +11733 +COMMIT; +ERROR HY000: The MySQL server is running with the --read-only option so it cannot execute this statement +set global read_only=0; +drop table table_11733 ; +drop user test@localhost; diff --git a/mysql-test/t/read_only.test b/mysql-test/t/read_only.test index 175a5bba6fa..655e207e235 100644 --- a/mysql-test/t/read_only.test +++ b/mysql-test/t/read_only.test @@ -101,8 +101,114 @@ drop table t1; --error 1290 insert into t1 values(1); +# +# BUG#11733: COMMITs should not happen if read-only is set +# + +# LOCK TABLE ... WRITE / READ_ONLY +# - is an error in the same connection +# - is ok in a different connection + connection default; +set global read_only=0; +lock table t1 write; + +connection con1; +lock table t2 write; + +connection default; +--error ER_LOCK_OR_ACTIVE_TRANSACTION +set global read_only=1; +unlock tables ; +# The following call blocks until con1 releases the write lock. +# Blocking is expected. +send set global read_only=1; + +connection con1; +--sleep 1 +select @@global.read_only; +unlock tables ; +--sleep 1 +select @@global.read_only; + +connection default; +reap; + +# LOCK TABLE ... READ / READ_ONLY +# - is an error in the same connection +# - is ok in a different connection + +connection default; +set global read_only=0; +lock table t1 read; + +connection con1; +lock table t2 read; + +connection default; +--error ER_LOCK_OR_ACTIVE_TRANSACTION +set global read_only=1; +unlock tables ; +# The following call blocks until con1 releases the read lock. +# Blocking is a limitation, and could be improved. +send set global read_only=1; + +connection con1; +--sleep 1 +select @@global.read_only; +unlock tables ; +--sleep 1 +select @@global.read_only; + +connection default; +reap; + +# pending transaction / READ_ONLY +# - is an error in the same connection +# - is ok in a different connection + +connection default; +set global read_only=0; +BEGIN; + +connection con1; +BEGIN; + +connection default; +--error ER_LOCK_OR_ACTIVE_TRANSACTION +set global read_only=1; +ROLLBACK; +set global read_only=1; + +connection con1; +select @@global.read_only; +ROLLBACK; + +# Verify that FLUSH TABLES WITH READ LOCK do not block READ_ONLY +# - in the same SUPER connection +# - in another SUPER connection + +connection default; +set global read_only=0; +flush tables with read lock; +set global read_only=1; +unlock tables; + +connect (root2,localhost,root,,test); + +connection default; +set global read_only=0; +flush tables with read lock; + +connection root2; +set global read_only=1; + +connection default; +select @@global.read_only; +unlock tables; + +# Cleanup +connection default; +set global read_only=0; drop table t1,t2; drop user test@localhost; - -set global read_only=0; diff --git a/mysql-test/t/read_only_innodb.test b/mysql-test/t/read_only_innodb.test new file mode 100644 index 00000000000..76d9748aa60 --- /dev/null +++ b/mysql-test/t/read_only_innodb.test @@ -0,0 +1,43 @@ +# should work with embedded server after mysqltest is fixed +-- source include/not_embedded.inc +-- source include/have_innodb.inc + +# +# BUG#11733: COMMITs should not happen if read-only is set +# + +--disable_warnings +DROP TABLE IF EXISTS table_11733 ; +--enable_warnings + +# READ_ONLY does nothing to SUPER users +# so we use a non-SUPER one: + +grant CREATE, SELECT, DROP on *.* to test@localhost; + +connect (con1,localhost,test,,test); + +connection default; +set global read_only=0; + +# Any transactional engine will do +create table table_11733 (a int) engine=InnoDb; + +connection con1; +BEGIN; +insert into table_11733 values(11733); + +connection default; +set global read_only=1; + +connection con1; +select @@global.read_only; +select * from table_11733 ; +-- error ER_OPTION_PREVENTS_STATEMENT +COMMIT; + +connection default; +set global read_only=0; +drop table table_11733 ; +drop user test@localhost; + diff --git a/sql/handler.cc b/sql/handler.cc index 451f974a066..e97fb7ddee4 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -692,6 +692,15 @@ int ha_commit_trans(THD *thd, bool all) ha_rollback_trans(thd, all); DBUG_RETURN(1); } + + if (is_real_trans && opt_readonly) + { + my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--read-only"); + ha_rollback_trans(thd, all); + error= 1; + goto end; + } + DBUG_EXECUTE_IF("crash_commit_before", abort();); /* Close all cursors that can not survive COMMIT */ diff --git a/sql/lock.cc b/sql/lock.cc index f36ecf58620..9886149ba77 100644 --- a/sql/lock.cc +++ b/sql/lock.cc @@ -151,6 +151,19 @@ MYSQL_LOCK *mysql_lock_tables(THD *thd, TABLE **tables, uint count, } } + if (write_lock_used && opt_readonly) + { + /* + Someone has issued SET GLOBAL READ_ONLY=1 and we want a write lock. + We do not wait for READ_ONLY=0, and fail. + */ + reset_lock_data(sql_lock); + my_free((gptr) sql_lock, MYF(0)); + sql_lock=0; + my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--read-only"); + break; + } + thd->proc_info="System lock"; DBUG_PRINT("info", ("thd->proc_info %s", thd->proc_info)); if (lock_external(thd, tables, count)) diff --git a/sql/set_var.cc b/sql/set_var.cc index 5590e71c810..4a01ab41763 100644 --- a/sql/set_var.cc +++ b/sql/set_var.cc @@ -370,7 +370,7 @@ sys_var_thd_ulong sys_preload_buff_size("preload_buffer_size", &SV::preload_buff_size); sys_var_thd_ulong sys_read_buff_size("read_buffer_size", &SV::read_buff_size); -sys_var_bool_ptr sys_readonly("read_only", &opt_readonly); +sys_var_opt_readonly sys_readonly("read_only", &opt_readonly); sys_var_thd_ulong sys_read_rnd_buff_size("read_rnd_buffer_size", &SV::read_rnd_buff_size); sys_var_thd_ulong sys_div_precincrement("div_precision_increment", @@ -3880,6 +3880,70 @@ bool sys_var_trust_routine_creators::update(THD *thd, set_var *var) return sys_var_bool_ptr::update(thd, var); } +bool sys_var_opt_readonly::update(THD *thd, set_var *var) +{ + bool result; + + DBUG_ENTER("sys_var_opt_readonly::update"); + + /* Prevent self dead-lock */ + if (thd->locked_tables || thd->active_transaction()) + { + my_error(ER_LOCK_OR_ACTIVE_TRANSACTION, MYF(0)); + DBUG_RETURN(true); + } + + if (thd->global_read_lock) + { + /* + This connection already holds the global read lock. + This can be the case with: + - FLUSH TABLES WITH READ LOCK + - SET GLOBAL READ_ONLY = 1 + */ + result= sys_var_bool_ptr::update(thd, var); + DBUG_RETURN(result); + } + + /* + Perform a 'FLUSH TABLES WITH READ LOCK'. + This is a 3 step process: + - [1] lock_global_read_lock() + - [2] close_cached_tables() + - [3] make_global_read_lock_block_commit() + [1] prevents new connections from obtaining tables locked for write. + [2] waits until all existing connections close their tables. + [3] prevents transactions from being committed. + */ + + if (lock_global_read_lock(thd)) + DBUG_RETURN(true); + + /* + This call will be blocked by any connection holding a READ or WRITE lock. + Ideally, we want to wait only for pending WRITE locks, but since: + con 1> LOCK TABLE T FOR READ; + con 2> LOCK TABLE T FOR WRITE; (blocked by con 1) + con 3> SET GLOBAL READ ONLY=1; (blocked by con 2) + can cause to wait on a read lock, it's required for the client application + to unlock everything, and acceptable for the server to wait on all locks. + */ + if (close_cached_tables(thd, true, NULL, false)) + goto end_with_read_lock; + + if (result= make_global_read_lock_block_commit(thd)) + goto end_with_read_lock; + + /* Change the opt_readonly system variable, safe because the lock is held */ + result= sys_var_bool_ptr::update(thd, var); + +end_with_read_lock: + /* Release the lock */ + unlock_global_read_lock(thd); + DBUG_RETURN(result); +} + + /* even session variable here requires SUPER, because of -#o,file */ bool sys_var_thd_dbug::check(THD *thd, set_var *var) { diff --git a/sql/set_var.h b/sql/set_var.h index 01669b378e1..14792f4a7c8 100644 --- a/sql/set_var.h +++ b/sql/set_var.h @@ -905,6 +905,20 @@ public: }; +/** + Handler for setting the system variable --read-only. +*/ + +class sys_var_opt_readonly :public sys_var_bool_ptr +{ +public: + sys_var_opt_readonly(const char *name_arg, my_bool *value_arg) : + sys_var_bool_ptr(name_arg, value_arg) {}; + ~sys_var_opt_readonly() {}; + bool update(THD *thd, set_var *var); +}; + + class sys_var_thd_lc_time_names :public sys_var_thd { public: From d469888a951368a764f807839c330feec21489f4 Mon Sep 17 00:00:00 2001 From: unknown Date: Thu, 30 Nov 2006 18:43:33 -0700 Subject: [PATCH 2/2] WL#3602 Post review changes the --read-only option is not enforced for the slave thread in replication, or for the SUPER user. sql/handler.cc: Post review changes Allowing writes for the slave thread or for SUPER, in read-only mode sql/lock.cc: Post review changes Allowing writes for the slave thread or for SUPER, in read-only mode mysql-test/r/rpl_read_only.result: New test mysql-test/t/rpl_read_only-slave.opt: New test mysql-test/t/rpl_read_only.test: New test --- mysql-test/r/rpl_read_only.result | 113 +++++++++++++++++++++++++++ mysql-test/t/rpl_read_only-slave.opt | 1 + mysql-test/t/rpl_read_only.test | 105 +++++++++++++++++++++++++ sql/handler.cc | 6 +- sql/lock.cc | 6 +- 5 files changed, 229 insertions(+), 2 deletions(-) create mode 100644 mysql-test/r/rpl_read_only.result create mode 100644 mysql-test/t/rpl_read_only-slave.opt create mode 100644 mysql-test/t/rpl_read_only.test diff --git a/mysql-test/r/rpl_read_only.result b/mysql-test/r/rpl_read_only.result new file mode 100644 index 00000000000..0b06a3d414a --- /dev/null +++ b/mysql-test/r/rpl_read_only.result @@ -0,0 +1,113 @@ +stop slave; +drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9; +reset master; +reset slave; +drop table if exists t1,t2,t3,t4,t5,t6,t7,t8,t9; +start slave; +create table t1(a int) engine=InnoDB; +create table t2(a int) engine=MyISAM; +insert into t1 values(1001); +insert into t2 values(2001); +set global read_only=1; +select @@read_only; +@@read_only +1 +select * from t1; +a +1001 +select * from t2; +a +2001 +select @@read_only; +@@read_only +0 +select * from t1; +a +1001 +select * from t2; +a +2001 +set global read_only=0; +BEGIN; +insert into t1 values(1002); +insert into t2 values(2002); +BEGIN; +insert into t1 values(1003); +insert into t2 values(2003); +set global read_only=1; +COMMIT; +COMMIT; +ERROR HY000: The MySQL server is running with the --read-only option so it cannot execute this statement +set global read_only=0; +insert into t1 values(1004); +insert into t2 values(2004); +select * from t1; +a +1001 +1002 +1004 +select * from t2; +a +2001 +2002 +2003 +2004 +select * from t1; +a +1001 +1002 +1004 +select * from t2; +a +2001 +2002 +2003 +2004 +set global read_only=1; +select @@read_only; +@@read_only +1 +show create table t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `a` int(11) DEFAULT NULL +) ENGINE=InnoDB DEFAULT CHARSET=latin1 +show create table t2; +Table Create Table +t2 CREATE TABLE `t2` ( + `a` int(11) DEFAULT NULL +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +insert into t1 values(1005); +insert into t2 values(2005); +select * from t1; +a +1001 +1002 +1004 +1005 +select * from t2; +a +2001 +2002 +2003 +2004 +2005 +select * from t1; +a +1001 +1002 +1004 +1005 +select * from t2; +a +2001 +2002 +2003 +2004 +2005 +insert into t1 values(1006); +ERROR HY000: The MySQL server is running with the --read-only option so it cannot execute this statement +insert into t2 values(2006); +ERROR HY000: The MySQL server is running with the --read-only option so it cannot execute this statement +drop table t1; +drop table t2; diff --git a/mysql-test/t/rpl_read_only-slave.opt b/mysql-test/t/rpl_read_only-slave.opt new file mode 100644 index 00000000000..627becdbfb5 --- /dev/null +++ b/mysql-test/t/rpl_read_only-slave.opt @@ -0,0 +1 @@ +--innodb diff --git a/mysql-test/t/rpl_read_only.test b/mysql-test/t/rpl_read_only.test new file mode 100644 index 00000000000..659c3d10044 --- /dev/null +++ b/mysql-test/t/rpl_read_only.test @@ -0,0 +1,105 @@ +# Test case for BUG #11733 +-- source include/master-slave.inc +-- source include/have_innodb.inc + +# Setting the master readonly : +# - the variable @@readonly is not replicated on the slave + +connect (master2,127.0.0.1,test,,test,$MASTER_MYPORT,); +connect (slave2,127.0.0.1,test,,test,$SLAVE_MYPORT,); + +connection master1; + +create table t1(a int) engine=InnoDB; +create table t2(a int) engine=MyISAM; +insert into t1 values(1001); +insert into t2 values(2001); + +connection master; +set global read_only=1; + +connection master1; +select @@read_only; +select * from t1; +select * from t2; + +sync_slave_with_master; +select @@read_only; +select * from t1; +select * from t2; + +# - replication of transactions +connection master; +set global read_only=0; + +connection master1; +BEGIN; +insert into t1 values(1002); +insert into t2 values(2002); + +connection master2; +BEGIN; +insert into t1 values(1003); +insert into t2 values(2003); + +connection master; +set global read_only=1; + +connection master1; +## works even with read_only=1, because master1 is root +COMMIT; + +connection master2; +--error ER_OPTION_PREVENTS_STATEMENT +COMMIT; + +connection master; +set global read_only=0; + +connection master1; +insert into t1 values(1004); +insert into t2 values(2004); + +select * from t1; +select * from t2; + +sync_slave_with_master; +select * from t1; +select * from t2; + +# Setting the slave readonly : replication will pass +# +connection slave1; +set global read_only=1; + +connection slave; +select @@read_only; +# Make sure the replicated table is also transactional +show create table t1; +# Make sure the replicated table is not transactional +show create table t2; + +connection master; +insert into t1 values(1005); +insert into t2 values(2005); +select * from t1; +select * from t2; + +sync_slave_with_master; +connection slave; +select * from t1; +select * from t2; + +# Non root user can not write on the slave +connection slave2; +--error ER_OPTION_PREVENTS_STATEMENT +insert into t1 values(1006); +--error ER_OPTION_PREVENTS_STATEMENT +insert into t2 values(2006); + +## Cleanup +connection master; +drop table t1; +drop table t2; +sync_slave_with_master; + diff --git a/sql/handler.cc b/sql/handler.cc index e97fb7ddee4..2ba0be11ec4 100644 --- a/sql/handler.cc +++ b/sql/handler.cc @@ -693,7 +693,11 @@ int ha_commit_trans(THD *thd, bool all) DBUG_RETURN(1); } - if (is_real_trans && opt_readonly) + if ( is_real_trans + && opt_readonly + && ! (thd->security_ctx->master_access & SUPER_ACL) + && ! thd->slave_thread + ) { my_error(ER_OPTION_PREVENTS_STATEMENT, MYF(0), "--read-only"); ha_rollback_trans(thd, all); diff --git a/sql/lock.cc b/sql/lock.cc index 9886149ba77..be267f8d160 100644 --- a/sql/lock.cc +++ b/sql/lock.cc @@ -151,7 +151,11 @@ MYSQL_LOCK *mysql_lock_tables(THD *thd, TABLE **tables, uint count, } } - if (write_lock_used && opt_readonly) + if ( write_lock_used + && opt_readonly + && ! (thd->security_ctx->master_access & SUPER_ACL) + && ! thd->slave_thread + ) { /* Someone has issued SET GLOBAL READ_ONLY=1 and we want a write lock.