From 8963d64ee87e92a07f1175292f5010c3a0d03090 Mon Sep 17 00:00:00 2001 From: Brandon Nesterenko Date: Tue, 26 Apr 2022 19:51:42 -0600 Subject: MDEV-28294: set default role bypasses Replicate_Wild_Ignore_Table: mysql.% Problem: ======== When replicating SET DEFAULT ROLE, the pre-update check (i.e. that in set_var_default_role::check()) tries to validate the existence of the given rules/user even when the targeted tables are ignored. When previously issued CREATE USER/ROLE commands are ignored by the replica because of the replication filtering rules, this results in an error because the targeted data does not exist. Solution: ======== Before checking that the given roles/user exist of a SET DEFAULT ROLE command, first ensure that the mysql.user and mysql.roles_mapping tables are not excluded by replication filters. Reviewed By: ============ Andrei Elkin Sergei Golubchik --- .../rpl/r/rpl_filter_set_var_missing_data.result | 55 ++++++++++++++ .../rpl/t/rpl_filter_set_var_missing_data.test | 83 ++++++++++++++++++++++ sql/sql_acl.cc | 50 +++++++++---- 3 files changed, 176 insertions(+), 12 deletions(-) create mode 100644 mysql-test/suite/rpl/r/rpl_filter_set_var_missing_data.result create mode 100644 mysql-test/suite/rpl/t/rpl_filter_set_var_missing_data.test diff --git a/mysql-test/suite/rpl/r/rpl_filter_set_var_missing_data.result b/mysql-test/suite/rpl/r/rpl_filter_set_var_missing_data.result new file mode 100644 index 00000000000..e232edae1ed --- /dev/null +++ b/mysql-test/suite/rpl/r/rpl_filter_set_var_missing_data.result @@ -0,0 +1,55 @@ +include/master-slave.inc +[connection master] +# +# Set replica to ignore system mysql tables +connection slave; +include/stop_slave.inc +SET @@GLOBAL.replicate_wild_ignore_table="mysql.%"; +include/start_slave.inc +# +# Execute grant-based commands on primary which modify mysql system +# tables +connection master; +CREATE ROLE journalist; +CREATE USER testuser@localhost IDENTIFIED by ''; +GRANT journalist to testuser@localhost; +# +# Execute SET commands which use the previous user/role data +SET DEFAULT ROLE journalist for testuser@localhost; +SET PASSWORD for testuser@localhost= PASSWORD('123'); +include/save_master_gtid.inc +# +# Verify primary's grant tables have the correct user/role data +select count(*)=1 from mysql.user where User='testuser'; +count(*)=1 +1 +select count(*)=1 from mysql.roles_mapping where User='testuser'; +count(*)=1 +1 +# +# Ensure that the replica receives all of the primary's events without +# error +connection slave; +include/sync_with_master_gtid.inc +Last_SQL_Error = +Last_SQL_Errno = 0 +# +# Verify that the replica did not execute the master's commands +select count(*)=0 from mysql.user where User='testuser'; +count(*)=0 +1 +select count(*)=0 from mysql.roles_mapping where User='testuser'; +count(*)=0 +1 +# +# Clean up +connection master; +DROP ROLE journalist; +DROP USER testuser@localhost; +include/save_master_gtid.inc +connection slave; +include/sync_with_master_gtid.inc +include/stop_slave.inc +SET @@GLOBAL.replicate_wild_ignore_table=""; +include/start_slave.inc +include/rpl_end.inc diff --git a/mysql-test/suite/rpl/t/rpl_filter_set_var_missing_data.test b/mysql-test/suite/rpl/t/rpl_filter_set_var_missing_data.test new file mode 100644 index 00000000000..25efb6ed662 --- /dev/null +++ b/mysql-test/suite/rpl/t/rpl_filter_set_var_missing_data.test @@ -0,0 +1,83 @@ +# +# Purpose: +# This test ensures that the SET DEFAULT ROLE and SET PASSWORD commands can +# be ignored by replica filter rules. MDEV-28294 exposed a bug in which +# SET DEFAULT ROLE would check for the existence of the given roles/user even +# when the targeted tables are ignored, resulting in errors if the targeted +# data does not exist. More specifically, when previously issued +# CREATE USER/ROLE commands are ignored by the replica because of the +# replication filtering rules, SET DEFAULT ROLE would result in an error +# because the targeted data does not exist. +# +# Methodology: +# Using a replica configured with replicate_wild_ignore_table="mysql.%", +# execute SET DEFAULT ROLE and SET PASSWORD on the primary and ensure that the +# replica neither errors nor executes the commands which the primary sends. +# +# References: +# MDEV-28294: set default role bypasses Replicate_Wild_Ignore_Table: mysql.% +# + +source include/master-slave.inc; +source include/have_binlog_format_mixed.inc; + +--echo # +--echo # Set replica to ignore system mysql tables +connection slave; +let $old_filter= query_get_value(SHOW SLAVE STATUS, Replicate_Wild_Ignore_Table, 1); +source include/stop_slave.inc; +SET @@GLOBAL.replicate_wild_ignore_table="mysql.%"; +source include/start_slave.inc; + +--echo # +--echo # Execute grant-based commands on primary which modify mysql system +--echo # tables +connection master; +CREATE ROLE journalist; +CREATE USER testuser@localhost IDENTIFIED by ''; +GRANT journalist to testuser@localhost; + +--echo # +--echo # Execute SET commands which use the previous user/role data +SET DEFAULT ROLE journalist for testuser@localhost; +SET PASSWORD for testuser@localhost= PASSWORD('123'); +--source include/save_master_gtid.inc + +--echo # +--echo # Verify primary's grant tables have the correct user/role data +select count(*)=1 from mysql.user where User='testuser'; +select count(*)=1 from mysql.roles_mapping where User='testuser'; + +--echo # +--echo # Ensure that the replica receives all of the primary's events without +--echo # error +connection slave; +--source include/sync_with_master_gtid.inc +let $error= query_get_value(SHOW SLAVE STATUS, Last_SQL_Error, 1); +--echo Last_SQL_Error = $error +let $errno= query_get_value(SHOW SLAVE STATUS, Last_SQL_Errno, 1); +--echo Last_SQL_Errno = $errno + +--echo # +--echo # Verify that the replica did not execute the master's commands +select count(*)=0 from mysql.user where User='testuser'; +select count(*)=0 from mysql.roles_mapping where User='testuser'; + +--echo # +--echo # Clean up + +# The master has to drop the role/user combination while the slave still has +# its filters active; otherwise, the slave would try to drop users/roles that +# were never replicated. +--connection master +DROP ROLE journalist; +DROP USER testuser@localhost; +--source include/save_master_gtid.inc + +--connection slave +--source include/sync_with_master_gtid.inc +source include/stop_slave.inc; +--eval SET @@GLOBAL.replicate_wild_ignore_table="$old_filter" +source include/start_slave.inc; + +--source include/rpl_end.inc diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 3a605f8e7b7..930c107cde2 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -1249,18 +1249,8 @@ class Grant_tables DBUG_ENTER("Grant_tables::open_and_lock"); DBUG_ASSERT(first_table_in_list); #ifdef HAVE_REPLICATION - if (first_table_in_list->tl.lock_type >= TL_WRITE_ALLOW_WRITE && - thd->slave_thread && !thd->spcont) - { - /* - GRANT and REVOKE are applied the slave in/exclusion rules as they are - some kind of updates to the mysql.% tables. - */ - Rpl_filter *rpl_filter= thd->system_thread_info.rpl_sql_info->rpl_filter; - if (rpl_filter->is_on() && - !rpl_filter->tables_ok(0, &first_table_in_list->tl)) - DBUG_RETURN(1); - } + if (int ignore_ret= rpl_ignore_tables(thd)) + DBUG_RETURN(ignore_ret); #endif if (open_and_lock_tables(thd, &first_table_in_list->tl, FALSE, MYSQL_LOCK_IGNORE_TIMEOUT)) @@ -1290,6 +1280,32 @@ class Grant_tables DBUG_RETURN(0); } +#ifdef HAVE_REPLICATION + /* Checks if the tables targeted by a grant command should be ignored because + of the configured replication filters + + @retval 1 Tables are excluded for replication + @retval 0 tables are included for replication + */ + int rpl_ignore_tables(THD *thd) + { + DBUG_ENTER("Grant_tables::rpl_ignore_tables"); + if (first_table_in_list->tl.lock_type >= TL_WRITE_ALLOW_WRITE && + thd->slave_thread && !thd->spcont) + { + /* + GRANT and REVOKE are applied the slave in/exclusion rules as they are + some kind of updates to the mysql.% tables. + */ + Rpl_filter *rpl_filter= thd->system_thread_info.rpl_sql_info->rpl_filter; + if (rpl_filter->is_on() && + !rpl_filter->tables_ok(0, &first_table_in_list->tl)) + DBUG_RETURN(1); + } + DBUG_RETURN(0); + } +#endif + inline const User_table& user_table() const { return m_user_table; @@ -3469,6 +3485,16 @@ int acl_check_set_default_role(THD *thd, const char *host, const char *user, const char *role) { DBUG_ENTER("acl_check_set_default_role"); +#ifdef HAVE_REPLICATION + /* + If the roles_mapping table is excluded by the replication filter, we return + successful without validating the user/role data because the command will + be ignored in a later call to `acl_set_default_role()` for a graceful exit. + */ + Grant_tables tables(Table_roles_mapping, TL_WRITE); + if (tables.rpl_ignore_tables(thd)) + DBUG_RETURN(0); +#endif DBUG_RETURN(check_alter_user(thd, host, user) || check_user_can_set_role(thd, user, host, NULL, role, NULL)); } -- cgit v1.2.1 From 0b805733105cd5b5f8b16146ec804e56018b43d9 Mon Sep 17 00:00:00 2001 From: Thirunarayanan Balathandayuthapani Date: Mon, 22 Aug 2022 18:36:30 +0530 Subject: MDEV-29319 Assertion failure size_in_header >= space.free_limit in fsp_get_available_space_in_free_extents() - Race condition between fsp_get_available_space_in_free_extents() and fsp_try_extend_data_file() while accessing space.free_limit. Before calling fsp_get_available_space_in_free_extents(), take shared lock on space->latch. --- storage/innobase/handler/ha_innodb.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index e1b645c1cc3..1e9c9f03ed1 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -14202,7 +14202,7 @@ been acquired by the caller who holds it for the calculation, @param[in] space tablespace object from fil_space_acquire() @return available space in KiB */ static uintmax_t -fsp_get_available_space_in_free_extents(const fil_space_t& space) +fsp_get_available_space_in_free_extents(fil_space_t& space) { ulint size_in_header = space.size_in_header; if (size_in_header < FSP_EXTENT_SIZE) { @@ -14394,9 +14394,11 @@ ha_innobase::info_low( stats.index_file_length = ulonglong(stat_sum_of_other_index_sizes) * size; + rw_lock_s_lock(&space->latch); stats.delete_length = 1024 * fsp_get_available_space_in_free_extents( *space); + rw_lock_s_unlock(&space->latch); } stats.check_time = 0; stats.mrr_length_per_rec= (uint)ref_length + 8; // 8 = max(sizeof(void *)); -- cgit v1.2.1 From dd737d071e1508b4b1441ee7079f5e93de9d2fed Mon Sep 17 00:00:00 2001 From: Thirunarayanan Balathandayuthapani Date: Wed, 17 Aug 2022 18:09:06 +0530 Subject: MDEV-29291 Assertion `!table->fts' failed in dict_table_can_be_evicted on SHUTDOWN - InnoDB fts table initially added to LRU table cache while creating the table. Later, table was marked as non-evicted when we add the table to fts optimizer list. Before marking the table as non-evicted, master thread can try to evict the fts table. --- storage/innobase/dict/dict0crea.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/storage/innobase/dict/dict0crea.cc b/storage/innobase/dict/dict0crea.cc index eba70aca6d1..f0754295aef 100644 --- a/storage/innobase/dict/dict0crea.cc +++ b/storage/innobase/dict/dict0crea.cc @@ -1271,7 +1271,7 @@ dict_create_table_step( if (node->state == TABLE_ADD_TO_CACHE) { DBUG_EXECUTE_IF("ib_ddl_crash_during_create", DBUG_SUICIDE();); - node->table->can_be_evicted = true; + node->table->can_be_evicted = !node->table->fts; node->table->add_to_cache(); err = DB_SUCCESS; -- cgit v1.2.1 From 61f456e772cc3d907a3b7881dc4dfb7edc1401d5 Mon Sep 17 00:00:00 2001 From: Thirunarayanan Balathandayuthapani Date: Wed, 24 Aug 2022 12:27:15 +0530 Subject: MDEV-29319 Assertion failure size_in_header >= space.free_limit in fsp_get_available_space_in_free_extents() - Don't remove the constant parameter in fsp_get_available_space_in_free_extents() --- storage/innobase/handler/ha_innodb.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index 1e9c9f03ed1..9ce1915b2cd 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -14202,7 +14202,7 @@ been acquired by the caller who holds it for the calculation, @param[in] space tablespace object from fil_space_acquire() @return available space in KiB */ static uintmax_t -fsp_get_available_space_in_free_extents(fil_space_t& space) +fsp_get_available_space_in_free_extents(const fil_space_t& space) { ulint size_in_header = space.size_in_header; if (size_in_header < FSP_EXTENT_SIZE) { -- cgit v1.2.1 From f2a53b6158a9c641e5dde679e45c7ff721902e77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Wed, 24 Aug 2022 15:00:47 +0300 Subject: btr_search_drop_page_hash_index(): Remove a racey debug check --- storage/innobase/btr/btr0sea.cc | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/storage/innobase/btr/btr0sea.cc b/storage/innobase/btr/btr0sea.cc index 8cc30de0dd8..4b4bba9a941 100644 --- a/storage/innobase/btr/btr0sea.cc +++ b/storage/innobase/btr/btr0sea.cc @@ -2,7 +2,7 @@ Copyright (c) 1996, 2016, Oracle and/or its affiliates. All Rights Reserved. Copyright (c) 2008, Google Inc. -Copyright (c) 2017, 2021, MariaDB Corporation. +Copyright (c) 2017, 2022, MariaDB Corporation. Portions of this file contain modifications contributed and copyrighted by Google, Inc. Those modifications are gratefully acknowledged and are described @@ -1123,9 +1123,6 @@ void btr_search_drop_page_hash_index(buf_block_t* block, rw_lock_t* latch; retry: - /* This debug check uses a dirty read that could theoretically cause - false positives while buf_pool_clear_hash_index() is executing. */ - assert_block_ahi_valid(block); ut_ad(!btr_search_own_any(RW_LOCK_S)); ut_ad(!btr_search_own_any(RW_LOCK_X)); -- cgit v1.2.1 From 2f6a728075a08c70103fa559180d0efc39f86fd4 Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Wed, 10 Aug 2022 13:27:01 +0200 Subject: update a global_suppressions() list followup for "remove invalid options from warning messages" --- mysql-test/include/mtr_warnings.sql | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mysql-test/include/mtr_warnings.sql b/mysql-test/include/mtr_warnings.sql index fa1ea95e5e1..9ce81703450 100644 --- a/mysql-test/include/mtr_warnings.sql +++ b/mysql-test/include/mtr_warnings.sql @@ -174,7 +174,7 @@ INSERT INTO global_suppressions VALUES /* Added 2009-08-XX after fixing Bug #42408 */ - ("Although a path was specified for the .* option, log tables are used"), + ("Although a .* file was specified, log tables are used. To enable logging to files "), ("Backup: Operation aborted"), ("Restore: Operation aborted"), ("Restore: The grant .* was skipped because the user does not exist"), -- cgit v1.2.1 From d1a80c42ee5b9c845ca72288d3bc58b47f5632a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Thu, 25 Aug 2022 15:14:38 +0300 Subject: MDEV-29384 Hangs caused by innodb_adaptive_hash_index=ON buf_defer_drop_ahi(): Remove. Ever since commit c7f8cfc9e733517cff4aaa6f6eaca625a3afc098 (MDEV-27700) it is safe to invoke btr_search_drop_page_hash_index(block, true) to remove an orphan adaptive hash index. Any attempt to upgrade page latches is prone to deadlocks. Recently, we observed a few hangs that involved nothing more than a small table consisting of one clustered index page, one secondary index page and some undo pages. --- storage/innobase/buf/buf0buf.cc | 41 +---------------------------------------- 1 file changed, 1 insertion(+), 40 deletions(-) diff --git a/storage/innobase/buf/buf0buf.cc b/storage/innobase/buf/buf0buf.cc index 8658b3a4a89..eb57489172c 100644 --- a/storage/innobase/buf/buf0buf.cc +++ b/storage/innobase/buf/buf0buf.cc @@ -3917,42 +3917,6 @@ buf_wait_for_read( } } -#ifdef BTR_CUR_HASH_ADAPT -/** If a stale adaptive hash index exists on the block, drop it. -Multiple executions of btr_search_drop_page_hash_index() on the -same block must be prevented by exclusive page latch. */ -ATTRIBUTE_COLD -static void buf_defer_drop_ahi(buf_block_t *block, mtr_memo_type_t fix_type) -{ - switch (fix_type) { - case MTR_MEMO_BUF_FIX: - /* We do not drop the adaptive hash index, because safely doing - so would require acquiring block->lock, and that is not safe - to acquire in some RW_NO_LATCH access paths. Those code paths - should have no business accessing the adaptive hash index anyway. */ - break; - case MTR_MEMO_PAGE_S_FIX: - /* Temporarily release our S-latch. */ - rw_lock_s_unlock(&block->lock); - rw_lock_x_lock(&block->lock); - btr_search_drop_page_hash_index(block, true); - rw_lock_x_unlock(&block->lock); - rw_lock_s_lock(&block->lock); - break; - case MTR_MEMO_PAGE_SX_FIX: - rw_lock_sx_unlock(&block->lock); - rw_lock_x_lock(&block->lock); - btr_search_drop_page_hash_index(block, true); - rw_lock_x_unlock(&block->lock); - rw_lock_sx_lock(&block->lock); - break; - default: - ut_ad(fix_type == MTR_MEMO_PAGE_X_FIX); - btr_search_drop_page_hash_index(block); - } -} -#endif /* BTR_CUR_HASH_ADAPT */ - /** Lock the page with the given latch type. @param[in,out] block block to be locked @param[in] rw_latch RW_S_LATCH, RW_X_LATCH, RW_NO_LATCH @@ -3988,10 +3952,7 @@ static buf_block_t* buf_page_mtr_lock(buf_block_t *block, } #ifdef BTR_CUR_HASH_ADAPT - { - if (block->index) - buf_defer_drop_ahi(block, fix_type); - } + btr_search_drop_page_hash_index(block, true); #endif /* BTR_CUR_HASH_ADAPT */ done: -- cgit v1.2.1