diff --git a/mysql-test/include/mtr_warnings.sql b/mysql-test/include/mtr_warnings.sql index 2a9728c0b8a..82c7aa8cd75 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"), 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 d34841092cd..1ec4e38f015 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -1,5 +1,5 @@ /* Copyright (c) 2000, 2018, Oracle and/or its affiliates. - Copyright (c) 2009, 2021, MariaDB + Copyright (c) 2009, 2022, MariaDB This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by @@ -1937,10 +1937,17 @@ class Grant_tables public: Grant_tables() : p_user_table(&m_user_table_json) { } - int open_and_lock(THD *thd, int which_tables, enum thr_lock_type lock_type) + /** + An auxiliary to build a list of involved tables. + + @retval 0 Success + @retval -1 A my_error reported error + */ + int build_table_list(THD *thd, TABLE_LIST** ptr_first, + int which_tables, enum thr_lock_type lock_type, + TABLE_LIST *tables) { - DBUG_ENTER("Grant_tables::open_and_lock"); - TABLE_LIST tables[USER_TABLE+1], *first= NULL; + DBUG_ENTER("Grant_tables::build_table_list"); DBUG_ASSERT(which_tables); /* At least one table must be opened. */ /* @@ -1965,12 +1972,23 @@ class Grant_tables tl->updating= lock_type >= TL_FIRST_WRITE; if (i >= FIRST_OPTIONAL_TABLE) tl->open_strategy= TABLE_LIST::OPEN_IF_EXISTS; - tl->next_global= tl->next_local= first; - first= tl; + tl->next_global= tl->next_local= *ptr_first; + *ptr_first= tl; } else tl->table= NULL; } + DBUG_RETURN(0); + } + + int open_and_lock(THD *thd, int which_tables, enum thr_lock_type lock_type) + { + DBUG_ENTER("Grant_tables::open_and_lock"); + + TABLE_LIST tables[USER_TABLE+1], *first= NULL; + + if (build_table_list(thd, &first, which_tables, lock_type, tables)) + DBUG_RETURN(-1); uint counter; int res= really_open(thd, first, &counter); @@ -2041,6 +2059,48 @@ class Grant_tables inline const Roles_mapping_table& roles_mapping_table() const { return m_roles_mapping_table; } +#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, TABLE_LIST* tables, int which_tables= 0, + enum thr_lock_type lock_type= TL_IGNORE) + { + DBUG_ENTER("Grant_tables::rpl_ignore_tables"); + + if (!(thd->slave_thread && !thd->spcont)) + DBUG_RETURN(0); + + TABLE_LIST all_tables[USER_TABLE+1]; + + if (!tables) + { + int rc __attribute__((unused))= + build_table_list(thd, &tables, which_tables, lock_type, all_tables); + + DBUG_ASSERT(!rc); // Grant_tables must be already initialized + DBUG_ASSERT(tables); + } + + if (tables->lock_type >= TL_FIRST_WRITE) + { + /* + 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, tables)) + DBUG_RETURN(1); + } + DBUG_RETURN(0); + } +#endif + private: /* Before any operation is possible on grant tables, they must be opened. @@ -2054,16 +2114,9 @@ class Grant_tables { DBUG_ENTER("Grant_tables::really_open:"); #ifdef HAVE_REPLICATION - if (tables->lock_type >= TL_FIRST_WRITE && - thd->slave_thread && !thd->spcont) + if (rpl_ignore_tables(thd, tables)) { - /* - 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, tables)) - DBUG_RETURN(1); + DBUG_RETURN(1); } #endif if (open_tables(thd, &tables, counter, MYSQL_LOCK_IGNORE_TIMEOUT)) @@ -4074,6 +4127,17 @@ 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_LIST* first= NULL; + if (tables.rpl_ignore_tables(thd, first, Table_roles_mapping, TL_WRITE)) + DBUG_RETURN(0); +#endif DBUG_RETURN(check_alter_user(thd, host, user) || check_user_can_set_role(thd, user, host, NULL, role, NULL)); } diff --git a/storage/innobase/btr/btr0btr.cc b/storage/innobase/btr/btr0btr.cc index 9ce96f66e64..772ac99a5d5 100644 --- a/storage/innobase/btr/btr0btr.cc +++ b/storage/innobase/btr/btr0btr.cc @@ -530,8 +530,7 @@ btr_page_alloc_low( mtr->u_lock_register(savepoint); root->page.lock.u_lock(); #ifdef BTR_CUR_HASH_ADAPT - if (root->index) - mtr_t::defer_drop_ahi(root, MTR_MEMO_PAGE_SX_FIX); + btr_search_drop_page_hash_index(root, true); #endif } @@ -645,8 +644,7 @@ dberr_t btr_page_free(dict_index_t* index, buf_block_t* block, mtr_t* mtr, mtr->u_lock_register(savepoint); root->page.lock.u_lock(); #ifdef BTR_CUR_HASH_ADAPT - if (root->index) - mtr_t::defer_drop_ahi(root, MTR_MEMO_PAGE_SX_FIX); + btr_search_drop_page_hash_index(root, true); #endif } err= fseg_free_page(&root->page.frame[blob || diff --git a/storage/innobase/btr/btr0sea.cc b/storage/innobase/btr/btr0sea.cc index 2ca4417db37..5b35c7f4f97 100644 --- a/storage/innobase/btr/btr0sea.cc +++ b/storage/innobase/btr/btr0sea.cc @@ -1292,10 +1292,6 @@ void btr_search_drop_page_hash_index(buf_block_t* block, rec_offs* offsets; 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); - if (!block->index) { return; } diff --git a/storage/innobase/buf/buf0buf.cc b/storage/innobase/buf/buf0buf.cc index c26bbda9473..eaf0f955a1f 100644 --- a/storage/innobase/buf/buf0buf.cc +++ b/storage/innobase/buf/buf0buf.cc @@ -2818,12 +2818,10 @@ get_latch: goto page_id_mismatch; } get_latch_valid: -#ifdef BTR_CUR_HASH_ADAPT - if (block->index) { - mtr_t::defer_drop_ahi(block, fix_type); - } -#endif /* BTR_CUR_HASH_ADAPT */ mtr->memo_push(block, fix_type); +#ifdef BTR_CUR_HASH_ADAPT + btr_search_drop_page_hash_index(block, true); +#endif /* BTR_CUR_HASH_ADAPT */ break; case RW_SX_LATCH: fix_type = MTR_MEMO_PAGE_SX_FIX; diff --git a/storage/innobase/dict/dict0crea.cc b/storage/innobase/dict/dict0crea.cc index 4b175229971..88eff936c27 100644 --- a/storage/innobase/dict/dict0crea.cc +++ b/storage/innobase/dict/dict0crea.cc @@ -1100,7 +1100,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; diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index 3cc4ba1be49..c9d9f663050 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -14845,9 +14845,11 @@ ha_innobase::info_low( stats.index_file_length = ulonglong(stat_sum_of_other_index_sizes) * size; + space->s_lock(); stats.delete_length = 1024 * fsp_get_available_space_in_free_extents( *space); + space->s_unlock(); } stats.check_time = 0; stats.mrr_length_per_rec= (uint)ref_length + 8; // 8 = max(sizeof(void *)); diff --git a/storage/innobase/include/mtr0mtr.h b/storage/innobase/include/mtr0mtr.h index edf583aa466..ea8973c2d3b 100644 --- a/storage/innobase/include/mtr0mtr.h +++ b/storage/innobase/include/mtr0mtr.h @@ -622,12 +622,6 @@ public: PAGE_FLUSH_SYNC }; -#ifdef BTR_CUR_HASH_ADAPT - /** If a stale adaptive hash index exists on the block, drop it. */ - ATTRIBUTE_COLD - static void defer_drop_ahi(buf_block_t *block, mtr_memo_type_t fix_type); -#endif - private: /** Log a write of a byte string to a page. @param block buffer page diff --git a/storage/innobase/mtr/mtr0mtr.cc b/storage/innobase/mtr/mtr0mtr.cc index 45c02f48ec8..062eb650871 100644 --- a/storage/innobase/mtr/mtr0mtr.cc +++ b/storage/innobase/mtr/mtr0mtr.cc @@ -1288,43 +1288,6 @@ bool mtr_t::memo_contains(const fil_space_t& space, bool shared) return true; } -#ifdef BTR_CUR_HASH_ADAPT -/** If a stale adaptive hash index exists on the block, drop it. */ -ATTRIBUTE_COLD -void mtr_t::defer_drop_ahi(buf_block_t *block, mtr_memo_type_t fix_type) -{ - switch (fix_type) { - default: - ut_ad(fix_type == MTR_MEMO_BUF_FIX); - /* We do not drop the adaptive hash index, because safely doing so - would require acquiring exclusive block->page.lock, which could - lead to hangs in some 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. */ - block->page.lock.s_unlock(); - block->page.lock.x_lock(); - if (dict_index_t *index= block->index) - if (index->freed()) - btr_search_drop_page_hash_index(block); - block->page.lock.x_unlock(); - block->page.lock.s_lock(); - ut_ad(!block->page.is_read_fixed()); - break; - case MTR_MEMO_PAGE_SX_FIX: - block->page.lock.u_x_upgrade(); - if (dict_index_t *index= block->index) - if (index->freed()) - btr_search_drop_page_hash_index(block); - block->page.lock.x_u_downgrade(); - break; - case MTR_MEMO_PAGE_X_FIX: - btr_search_drop_page_hash_index(block); - } -} -#endif /* BTR_CUR_HASH_ADAPT */ - /** Upgrade U-latched pages to X */ struct UpgradeX { @@ -1379,8 +1342,7 @@ void mtr_t::page_lock(buf_block_t *block, ulint rw_latch) ut_d(const auto state= block->page.state()); ut_ad(state > buf_page_t::FREED); ut_ad(state > buf_page_t::WRITE_FIX || state < buf_page_t::READ_FIX); - switch (rw_latch) - { + switch (rw_latch) { case RW_NO_LATCH: fix_type= MTR_MEMO_BUF_FIX; goto done; @@ -1406,10 +1368,8 @@ void mtr_t::page_lock(buf_block_t *block, ulint rw_latch) } #ifdef BTR_CUR_HASH_ADAPT - if (dict_index_t *index= block->index) - if (index->freed()) - defer_drop_ahi(block, fix_type); -#endif /* BTR_CUR_HASH_ADAPT */ + btr_search_drop_page_hash_index(block, true); +#endif done: ut_ad(state < buf_page_t::UNFIXED || diff --git a/storage/innobase/os/os0file.cc b/storage/innobase/os/os0file.cc index 2660f28d682..2367b7b0596 100644 --- a/storage/innobase/os/os0file.cc +++ b/storage/innobase/os/os0file.cc @@ -118,7 +118,7 @@ public: size_t pending_io_count() { - return (size_t)m_max_aio - m_cache.size(); + return m_cache.pos(); } tpool::task_group* get_task_group() diff --git a/tpool/tpool_structs.h b/tpool/tpool_structs.h index 7b0fb857695..b49204f2d75 100644 --- a/tpool/tpool_structs.h +++ b/tpool/tpool_structs.h @@ -40,79 +40,121 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02111 - 1301 USA*/ namespace tpool { -enum cache_notification_mode -{ - NOTIFY_ONE, - NOTIFY_ALL -}; - /** Generic "pointer" cache of a fixed size with fast put/get operations. - Compared to STL containers, is faster/does not - do allocations. However, put() operation will wait - if there is no free items. + Compared to STL containers,e.g stack or queue + is faster/does not do allocations. + + However, get() operation will wait if there is no free items. + + We assume that put() will only put back the elements that + were retrieved previously with get(). */ template class cache { + /** Protects updates of m_pos and m_cache members */ std::mutex m_mtx; + + /** + Notify waiting threads about "cache full" or "cache not empty" conditions + @see get() and wait() + */ std::condition_variable m_cv; - std::vector m_base; + + /** Cached items vector.Does not change after construction */ + std::vector m_base; + + /** + Pointers to cached items. Protected by m_mtx. Does not grow after + construction. Elements in position [0,m_pos-1] are "borrowed", + elements in position [m_pos,capacity()-1] are "free" + */ std::vector m_cache; - cache_notification_mode m_notification_mode; + + /** Number of threads waiting for "cache full" condition (s. wait()) + Protected by m_mtx */ int m_waiters; + /** Current cache size. Protected by m_mtx*/ + size_t m_pos; + +private: + + inline size_t capacity() + { + return m_base.size(); + } + + /** + @return true if cache is full (no items are borrowed) + */ bool is_full() { - return m_cache.size() == m_base.size(); + return m_pos == 0; + } + + /** + @return true if cache is empty (all items are borrowed) + */ + bool is_empty() + { + return m_pos == capacity(); } public: - cache(size_t count, cache_notification_mode mode= tpool::cache_notification_mode::NOTIFY_ALL): - m_mtx(), m_cv(), m_base(count),m_cache(count), m_notification_mode(mode),m_waiters() + /** + Constructor + @param size - maximum number of items in cache + */ + cache(size_t size) : m_mtx(), m_cv(), m_base(size), m_cache(size), + m_waiters(), m_pos(0) { - for(size_t i = 0 ; i < count; i++) - m_cache[i]=&m_base[i]; + for(size_t i= 0 ; i < size; i++) + m_cache[i]= &m_base[i]; } - T* get(bool blocking=true) + /** + Retrieve an item from cache. Waits for free item, if cache is + currently empty. + @return borrowed item + */ + T* get() { std::unique_lock lk(m_mtx); - if (blocking) - { - while(m_cache.empty()) - m_cv.wait(lk); - } - else - { - if(m_cache.empty()) - return nullptr; - } - T* ret = m_cache.back(); - m_cache.pop_back(); - return ret; + while(is_empty()) + m_cv.wait(lk); + assert(m_pos < capacity()); + // return last element + return m_cache[m_pos++]; } - + /** + Put back an item to cache. + @param item - item to put back + */ void put(T *ele) { std::unique_lock lk(m_mtx); - m_cache.push_back(ele); - if (m_notification_mode == NOTIFY_ONE) - m_cv.notify_one(); - else if(m_cache.size() == 1) - m_cv.notify_all(); // Signal cache is not empty - else if(m_waiters && is_full()) - m_cv.notify_all(); // Signal cache is full + assert(!is_full()); + // put element to the logical end of the array + m_cache[--m_pos] = ele; + + /* Notify waiters when the cache becomes + not empty, or when it becomes full */ + if (m_pos == 1 || (m_waiters && is_full())) + m_cv.notify_all(); } + /** Check if pointer represents cached element */ bool contains(T* ele) { - return ele >= &m_base[0] && ele <= &m_base[m_base.size() -1]; + // No locking required, m_base does not change after construction. + return ele >= &m_base[0] && ele <= &m_base[capacity() - 1]; } - /* Wait until cache is full.*/ + /** Wait until cache is full.*/ void wait() { std::unique_lock lk(m_mtx); @@ -122,9 +164,13 @@ public: m_waiters--; } - TPOOL_SUPPRESS_TSAN size_t size() + /** + @return approximate number of "borrowed" items. + A "dirty" read, not used in any critical functionality. + */ + TPOOL_SUPPRESS_TSAN size_t pos() { - return m_cache.size(); + return m_pos; } };