commit 08465c82ded73a43e6c5b83390d5033e9837d126 Author: Laurynas Biveinis Date: Thu Mar 2 15:47:56 2017 +0200 Fix bugs 84437 and 84350 (FLUSH LOGS disabled by read_only and super_read_only) There are couple of distinct issues and fixes: 1) On debug build, a failing FLUSH LOGS attempts to send OK to connection after having sent error reply before. This is caused by missing error propagation between where the error happened (ha_commit_trans) and where the OK is sent (at the top level switch statement in mysql_execute_statement). Fix by making previously void methods return bool for success/failure, and check for errors along the way as necessary. I decided not to annotate the touched methods with warn_unused_result, as that forces changes elsewhere. This fix has an important side effect in that more failures to write mysql.gtid_executed table on binlog rotation will cause server abort with the default binlog_error_action=ABORT_SERVER. This appears to be a correct change by reviewing WL#6559, which introduced mysql.gtid_executed table, binlog.binlog_gtids_table_gcov testcase. 2) The FLUSH LOGS failure itself is fixed by allowing FLUSH statement, in FLUSH LOGS and FLUSH BINARY LOGS forms, in check_readonly. diff --git a/mysql-test/suite/rpl_gtid/r/rpl_gtid_read_only_flush_logs.result b/mysql-test/suite/rpl_gtid/r/rpl_gtid_read_only_flush_logs.result new file mode 100644 index 00000000000..e19a67ab9bf --- /dev/null +++ b/mysql-test/suite/rpl_gtid/r/rpl_gtid_read_only_flush_logs.result @@ -0,0 +1,38 @@ +include/master-slave.inc +Warnings: +Note #### Sending passwords in plain text without SSL/TLS is extremely insecure. +Note #### Storing MySQL user name or password information in the master info repository is not secure and is therefore not recommended. Please consider using the USER and PASSWORD connection options for START SLAVE; see the 'START SLAVE Syntax' in the MySQL Manual for more information. +[connection master] +CREATE TABLE t1 (a INT PRIMARY KEY); +INSERT INTO t1 VALUES (1); +include/sync_slave_sql_with_master.inc +# +# Bug 84350: Error 1290 executing flush logs in read-only slave +# +CREATE USER 'reload_user'@'localhost'; +GRANT RELOAD ON *.* TO 'reload_user'@'localhost'; +SET @saved_read_only= @@GLOBAL.read_only; +SET GLOBAL read_only= ON; +# slave reload_user connection +FLUSH LOGS; +INSERT INTO t1 VALUES (2); +include/sync_slave_sql_with_master.inc +FLUSH BINARY LOGS; +# +# Bug 84437: super-read-only does not allow FLUSH LOGS on 5.7 +# +# slave root connection +SET @saved_super_read_only= @@GLOBAL.super_read_only; +SET GLOBAL super_read_only=ON; +INSERT INTO t1 VALUES (3); +include/sync_slave_sql_with_master.inc +FLUSH LOGS; +INSERT INTO t1 VALUES (4); +include/sync_slave_sql_with_master.inc +FLUSH BINARY LOGS; +DROP TABLE t1; +include/sync_slave_sql_with_master.inc +SET GLOBAL super_read_only= @saved_super_read_only; +SET GLOBAL read_only= @saved_read_only; +DROP USER reload_user@localhost; +include/rpl_end.inc diff --git a/mysql-test/suite/rpl_gtid/t/rpl_gtid_read_only_flush_logs-master.opt b/mysql-test/suite/rpl_gtid/t/rpl_gtid_read_only_flush_logs-master.opt new file mode 100644 index 00000000000..0acf1412626 --- /dev/null +++ b/mysql-test/suite/rpl_gtid/t/rpl_gtid_read_only_flush_logs-master.opt @@ -0,0 +1 @@ +--gtid-mode=ON --enforce-gtid-consistency=ON --log-slave-updates diff --git a/mysql-test/suite/rpl_gtid/t/rpl_gtid_read_only_flush_logs-slave.opt b/mysql-test/suite/rpl_gtid/t/rpl_gtid_read_only_flush_logs-slave.opt new file mode 100644 index 00000000000..0acf1412626 --- /dev/null +++ b/mysql-test/suite/rpl_gtid/t/rpl_gtid_read_only_flush_logs-slave.opt @@ -0,0 +1 @@ +--gtid-mode=ON --enforce-gtid-consistency=ON --log-slave-updates diff --git a/mysql-test/suite/rpl_gtid/t/rpl_gtid_read_only_flush_logs.test b/mysql-test/suite/rpl_gtid/t/rpl_gtid_read_only_flush_logs.test new file mode 100644 index 00000000000..2d71202d9d7 --- /dev/null +++ b/mysql-test/suite/rpl_gtid/t/rpl_gtid_read_only_flush_logs.test @@ -0,0 +1,68 @@ +--source include/have_gtid.inc +--source include/master-slave.inc +--source include/count_sessions.inc + +CREATE TABLE t1 (a INT PRIMARY KEY); +INSERT INTO t1 VALUES (1); + +--source include/sync_slave_sql_with_master.inc + +--echo # +--echo # Bug 84350: Error 1290 executing flush logs in read-only slave +--echo # + +CREATE USER 'reload_user'@'localhost'; +GRANT RELOAD ON *.* TO 'reload_user'@'localhost'; + +SET @saved_read_only= @@GLOBAL.read_only; +SET GLOBAL read_only= ON; + +--connect(con1,127.0.0.1,reload_user,,test,$SLAVE_MYPORT,) +--echo # slave reload_user connection + +FLUSH LOGS; + +--connection master +INSERT INTO t1 VALUES (2); +--source include/sync_slave_sql_with_master.inc +--connection con1 + +FLUSH BINARY LOGS; + +--disconnect con1 + +--echo # +--echo # Bug 84437: super-read-only does not allow FLUSH LOGS on 5.7 +--echo # + +--connection slave +--echo # slave root connection + +SET @saved_super_read_only= @@GLOBAL.super_read_only; +SET GLOBAL super_read_only=ON; + +--connection master +INSERT INTO t1 VALUES (3); +--source include/sync_slave_sql_with_master.inc + +FLUSH LOGS; + +--connection master +INSERT INTO t1 VALUES (4); +--source include/sync_slave_sql_with_master.inc + +FLUSH BINARY LOGS; + +--connection master +DROP TABLE t1; + +--source include/sync_slave_sql_with_master.inc + +SET GLOBAL super_read_only= @saved_super_read_only; +SET GLOBAL read_only= @saved_read_only; + +DROP USER reload_user@localhost; + +--source include/wait_until_count_sessions.inc +--connection master +--source include/rpl_end.inc diff --git a/sql/auth/sql_authorization.cc b/sql/auth/sql_authorization.cc index 5ebf24e6c1e..5e2053600b8 100644 --- a/sql/auth/sql_authorization.cc +++ b/sql/auth/sql_authorization.cc @@ -1784,7 +1784,9 @@ bool check_readonly(THD *thd, bool err_if_readonly) sql_command == SQLCOM_CHANGE_MASTER || sql_command == SQLCOM_START_GROUP_REPLICATION || sql_command == SQLCOM_STOP_GROUP_REPLICATION || - sql_command == SQLCOM_CHANGE_REPLICATION_FILTER) + sql_command == SQLCOM_CHANGE_REPLICATION_FILTER || + (sql_command == SQLCOM_FLUSH && + thd->lex->type & (REFRESH_LOG | REFRESH_BINARY_LOG))) DBUG_RETURN(FALSE); bool is_super= thd->security_context()->check_access(SUPER_ACL); diff --git a/sql/rpl_gtid_persist.cc b/sql/rpl_gtid_persist.cc index f0e4ac1a759..f7226ce15dc 100644 --- a/sql/rpl_gtid_persist.cc +++ b/sql/rpl_gtid_persist.cc @@ -195,12 +195,12 @@ bool Gtid_table_access_context::init(THD **thd, TABLE **table, bool is_write) } -void Gtid_table_access_context::deinit(THD *thd, TABLE *table, +bool Gtid_table_access_context::deinit(THD *thd, TABLE *table, bool error, bool need_commit) { DBUG_ENTER("Gtid_table_access_context::deinit"); - this->close_table(thd, table, &m_backup, 0 != error, need_commit); + bool res= this->close_table(thd, table, &m_backup, 0 != error, need_commit); /* If Gtid is inserted through Attachable_trx_rw its has been done @@ -219,7 +219,7 @@ void Gtid_table_access_context::deinit(THD *thd, TABLE *table, if (m_drop_thd_object) this->drop_thd(m_drop_thd_object); - DBUG_VOID_RETURN; + DBUG_RETURN(res); } @@ -448,7 +448,9 @@ int Gtid_table_persistor::save(const Gtid_set *gtid_set) ret= error= save(table, gtid_set); end: - table_access_ctx.deinit(thd, table, 0 != error, true); + const int deinit_ret= table_access_ctx.deinit(thd, table, 0 != error, true); + if (!ret) + ret= deinit_ret; /* Notify compression thread to compress gtid_executed table. */ if (error == 0 && DBUG_EVALUATE_IF("dont_compress_gtid_table", 0, 1)) diff --git a/sql/rpl_gtid_persist.h b/sql/rpl_gtid_persist.h index 6b02d1495db..77d753aa52d 100644 --- a/sql/rpl_gtid_persist.h +++ b/sql/rpl_gtid_persist.h @@ -61,8 +61,12 @@ public: @param table Table to be closed @param error If there was an error while updating the table @param need_commit Need to commit current transaction if it is true + + @return + @retval true failed + @retval false success */ - void deinit(THD *thd, TABLE *table, bool error, bool need_commit); + bool deinit(THD *thd, TABLE *table, bool error, bool need_commit); /** Prepares before opening table. - set flags diff --git a/sql/rpl_table_access.cc b/sql/rpl_table_access.cc index 3d3409651ec..07faaf0338d 100644 --- a/sql/rpl_table_access.cc +++ b/sql/rpl_table_access.cc @@ -93,37 +93,42 @@ bool System_table_access::open_table(THD* thd, const LEX_STRING dbstr, } -void System_table_access::close_table(THD *thd, TABLE* table, +bool System_table_access::close_table(THD *thd, TABLE* table, Open_tables_backup *backup, bool error, bool need_commit) { Query_tables_list query_tables_list_backup; + bool res= false; DBUG_ENTER("System_table_access::close_table"); if (table) { if (error) - ha_rollback_trans(thd, false); + res= ha_rollback_trans(thd, false); else { /* To make the commit not to block with global read lock set "ignore_global_read_lock" flag to true. */ - ha_commit_trans(thd, false, true); + res= ha_commit_trans(thd, false, true); } if (need_commit) { if (error) - ha_rollback_trans(thd, true); + { + if (ha_rollback_trans(thd, true)) + res= true; + } else { /* To make the commit not to block with global read lock set "ignore_global_read_lock" flag to true. */ - ha_commit_trans(thd, true, true); + if (ha_commit_trans(thd, true, true)) + res= true; } } /* @@ -137,7 +142,7 @@ void System_table_access::close_table(THD *thd, TABLE* table, thd->restore_backup_open_tables_state(backup); } - DBUG_VOID_RETURN; + DBUG_RETURN(res); } diff --git a/sql/rpl_table_access.h b/sql/rpl_table_access.h index 670c2853404..f002d476189 100644 --- a/sql/rpl_table_access.h +++ b/sql/rpl_table_access.h @@ -91,8 +91,12 @@ public: committed. In this case, the changes were not done on behalf of any user transaction and if not finished, there would be pending changes. + + @return + @retval true failed + @retval false success */ - void close_table(THD *thd, TABLE* table, Open_tables_backup *backup, + bool close_table(THD *thd, TABLE* table, Open_tables_backup *backup, bool error, bool need_commit); /** Creates a new thread in the bootstrap process or in the mysqld startup,