diff options
author | Marko Mäkelä <marko.makela@mariadb.com> | 2020-08-20 18:38:38 +0530 |
---|---|---|
committer | Marko Mäkelä <marko.makela@mariadb.com> | 2020-08-20 18:38:38 +0530 |
commit | 65b23c87f9edc9fd290b8d6cb8483cd6dc7a7d28 (patch) | |
tree | 433dd4e01c5dd3367adedd744f8409aa3aed2195 | |
parent | c6137adbb7faf95d85f1a9d2a2b41c07e17f9f3b (diff) | |
download | mariadb-git-10.3-enterprise-MENT-581.tar.gz |
MDEV-23475 InnoDB performance regression for write-heavy workloads10.3-enterprise-MENT-581
In commit fe39d02f51b96536dccca7ff89faf05e13548877 (MDEV-20638)
we removed some wake-up signaling of the master thread that should
have been there, to ensure a steady log checkpointing workload.
Common sense suggests that the commit omitted some necessary calls
to srv_inc_activity_count(). But, an attempt to add the call to
trx_flush_log_if_needed_low() as well as to reinstate the function
innobase_active_small() did not restore the performance for the
case where sync_binlog=1 is set.
Therefore, we will revert the entire commit in MariaDB Server 10.2.
In MariaDB Server 10.5, adding a srv_inc_activity_count() call to
trx_flush_log_if_needed_low() did restore the performance, so we
will not revert MDEV-20638 across all versions.
-rw-r--r-- | extra/mariabackup/xtrabackup.cc | 6 | ||||
-rw-r--r-- | storage/innobase/buf/buf0flu.cc | 4 | ||||
-rw-r--r-- | storage/innobase/handler/ha_innodb.cc | 48 | ||||
-rw-r--r-- | storage/innobase/handler/handler0alter.cc | 5 | ||||
-rw-r--r-- | storage/innobase/include/srv0srv.h | 25 | ||||
-rw-r--r-- | storage/innobase/row/row0mysql.cc | 2 | ||||
-rw-r--r-- | storage/innobase/srv/srv0srv.cc | 100 | ||||
-rw-r--r-- | storage/innobase/srv/srv0start.cc | 4 | ||||
-rw-r--r-- | storage/innobase/trx/trx0roll.cc | 3 | ||||
-rw-r--r-- | storage/innobase/trx/trx0trx.cc | 6 |
10 files changed, 169 insertions, 34 deletions
diff --git a/extra/mariabackup/xtrabackup.cc b/extra/mariabackup/xtrabackup.cc index f15dc425601..ed957081561 100644 --- a/extra/mariabackup/xtrabackup.cc +++ b/extra/mariabackup/xtrabackup.cc @@ -259,6 +259,12 @@ my_bool innobase_locks_unsafe_for_binlog; my_bool innobase_rollback_on_timeout; my_bool innobase_create_status_file; +/* The following counter is used to convey information to InnoDB +about server activity: in selects it is not sensible to call +srv_active_wake_master_thread after each fetch or search, we only do +it every INNOBASE_WAKE_INTERVAL'th step. */ + +#define INNOBASE_WAKE_INTERVAL 32 ulong innobase_active_counter = 0; #ifndef _WIN32 diff --git a/storage/innobase/buf/buf0flu.cc b/storage/innobase/buf/buf0flu.cc index e1acc7b9095..6f56a09eda2 100644 --- a/storage/innobase/buf/buf0flu.cc +++ b/storage/innobase/buf/buf0flu.cc @@ -3116,7 +3116,7 @@ DECLARE_THREAD(buf_flush_page_cleaner_coordinator)(void*) /* The page_cleaner skips sleep if the server is idle and there are no pending IOs in the buffer pool and there is work to do. */ - if (srv_check_activity(&last_activity) + if (srv_check_activity(last_activity) || buf_get_n_pending_read_ios() || n_flushed == 0) { @@ -3208,7 +3208,7 @@ DECLARE_THREAD(buf_flush_page_cleaner_coordinator)(void*) n_flushed = n_flushed_lru + n_flushed_list; - } else if (srv_check_activity(&last_activity)) { + } else if (srv_check_activity(last_activity)) { ulint n_to_flush; lsn_t lsn_limit = 0; diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index b6f39a573ab..64d3fef0fbe 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -449,6 +449,14 @@ TYPELIB innodb_flush_method_typelib = { NULL }; +/* The following counter is used to convey information to InnoDB +about server activity: in case of normal DML ops it is not +sensible to call srv_active_wake_master_thread after each +operation, we only do it every INNOBASE_WAKE_INTERVAL'th step. */ + +#define INNOBASE_WAKE_INTERVAL 32 +static ulong innobase_active_counter = 0; + /** Allowed values of innodb_change_buffering */ static const char* innodb_change_buffering_names[] = { "none", /* IBUF_USE_NONE */ @@ -1846,6 +1854,23 @@ thd_to_trx_id( #endif /* WITH_WSREP */ /********************************************************************//** +Increments innobase_active_counter and every INNOBASE_WAKE_INTERVALth +time calls srv_active_wake_master_thread. This function should be used +when a single database operation may introduce a small need for +server utility activity, like checkpointing. */ +inline +void +innobase_active_small(void) +/*=======================*/ +{ + innobase_active_counter++; + + if ((innobase_active_counter % INNOBASE_WAKE_INTERVAL) == 0) { + srv_active_wake_master_thread(); + } +} + +/********************************************************************//** Converts an InnoDB error code to a MySQL error code and also tells to MySQL about a possible transaction rollback inside InnoDB caused by a lock wait timeout or a deadlock. @@ -6460,6 +6485,11 @@ ha_innobase::close() MONITOR_INC(MONITOR_TABLE_CLOSE); + /* Tell InnoDB server that there might be work for + utility threads: */ + + srv_active_wake_master_thread(); + DBUG_RETURN(0); } @@ -8170,6 +8200,8 @@ report_error: } func_exit: + innobase_active_small(); + DBUG_RETURN(error_result); } @@ -8856,6 +8888,11 @@ func_exit: error, m_prebuilt->table->flags, m_user_thd); } + /* Tell InnoDB server that there might be work for + utility threads: */ + + innobase_active_small(); + #ifdef WITH_WSREP if (error == DB_SUCCESS && trx->is_wsrep() && wsrep_thd_exec_mode(m_user_thd) == LOCAL_STATE && @@ -8914,6 +8951,11 @@ ha_innobase::delete_row( innobase_srv_conc_exit_innodb(m_prebuilt); + /* Tell the InnoDB server that there might be work for + utility threads: */ + + innobase_active_small(); + #ifdef WITH_WSREP if (error == DB_SUCCESS && trx->is_wsrep() && wsrep_thd_exec_mode(m_user_thd) == LOCAL_STATE @@ -12701,6 +12743,7 @@ create_table_info_t::create_table_update_dict() if (m_flags2 & DICT_TF2_FTS) { if (!innobase_fts_load_stopword(innobase_table, NULL, m_thd)) { dict_table_close(innobase_table, FALSE, FALSE); + srv_active_wake_master_thread(); trx_free(m_trx); DBUG_RETURN(-1); } @@ -12850,6 +12893,11 @@ ha_innobase::create( error = info.create_table_update_dict(); + /* Tell the InnoDB server that there might be work for + utility threads: */ + + srv_active_wake_master_thread(); + DBUG_RETURN(error); } diff --git a/storage/innobase/handler/handler0alter.cc b/storage/innobase/handler/handler0alter.cc index c5693afe50b..ac36cff6173 100644 --- a/storage/innobase/handler/handler0alter.cc +++ b/storage/innobase/handler/handler0alter.cc @@ -9791,6 +9791,11 @@ foreign_fail: log_append_on_checkpoint(NULL); + /* Tell the InnoDB server that there might be work for + utility threads: */ + + srv_active_wake_master_thread(); + if (fail) { for (inplace_alter_handler_ctx** pctx = ctx_array; *pctx; pctx++) { diff --git a/storage/innobase/include/srv0srv.h b/storage/innobase/include/srv0srv.h index 93755ad7520..6c575733710 100644 --- a/storage/innobase/include/srv0srv.h +++ b/storage/innobase/include/srv0srv.h @@ -788,6 +788,19 @@ srv_reset_io_thread_op_info(); /** Wake up the purge threads if there is work to do. */ void srv_wake_purge_thread_if_not_active(); +/** Wake up the InnoDB master thread if it was suspended (not sleeping). */ +void +srv_active_wake_master_thread_low(); + +#define srv_active_wake_master_thread() \ + do { \ + if (!srv_read_only_mode) { \ + srv_active_wake_master_thread_low(); \ + } \ + } while (0) +/** Wake up the master thread if it is suspended or being suspended. */ +void +srv_wake_master_thread(); /******************************************************************//** Outputs to a file the output of the InnoDB Monitor. @@ -816,13 +829,13 @@ reading this value as it is only used in heuristics. ulint srv_get_activity_count(void); /*========================*/ - -/** Check if there has been any activity. -@param[in,out] activity_count recent activity count to be returned -if there is a change +/*******************************************************************//** +Check if there has been any activity. @return FALSE if no change in activity counter. */ -bool srv_check_activity(ulint *activity_count); - +ibool +srv_check_activity( +/*===============*/ + ulint old_activity_count); /*!< old activity count */ /******************************************************************//** Increment the server activity counter. */ void diff --git a/storage/innobase/row/row0mysql.cc b/storage/innobase/row/row0mysql.cc index be221d5cfcd..5dcd596fc98 100644 --- a/storage/innobase/row/row0mysql.cc +++ b/storage/innobase/row/row0mysql.cc @@ -3817,7 +3817,7 @@ funct_exit_all_freed: trx->op_info = ""; - srv_inc_activity_count(); + srv_wake_master_thread(); DBUG_RETURN(err); } diff --git a/storage/innobase/srv/srv0srv.cc b/storage/innobase/srv/srv0srv.cc index 6af3b89f15c..c5b08cd5eb8 100644 --- a/storage/innobase/srv/srv0srv.cc +++ b/storage/innobase/srv/srv0srv.cc @@ -1923,6 +1923,33 @@ srv_get_active_thread_type(void) return(ret); } +/** Wake up the InnoDB master thread if it was suspended (not sleeping). */ +void +srv_active_wake_master_thread_low() +{ + ut_ad(!srv_read_only_mode); + ut_ad(!srv_sys_mutex_own()); + + srv_inc_activity_count(); + + if (my_atomic_loadlint(&srv_sys.n_threads_active[SRV_MASTER]) == 0) { + srv_slot_t* slot; + + srv_sys_mutex_enter(); + + slot = &srv_sys.sys_threads[SRV_MASTER_SLOT]; + + /* Only if the master thread has been started. */ + + if (slot->in_use) { + ut_a(srv_slot_get_type(slot) == SRV_MASTER); + os_event_set(slot->event); + } + + srv_sys_mutex_exit(); + } +} + /** Wake up the purge threads if there is work to do. */ void srv_wake_purge_thread_if_not_active() @@ -1937,6 +1964,14 @@ srv_wake_purge_thread_if_not_active() } } +/** Wake up the master thread if it is suspended or being suspended. */ +void +srv_wake_master_thread() +{ + srv_inc_activity_count(); + srv_release_threads(SRV_MASTER, 1); +} + /*******************************************************************//** Get current server activity count. We don't hold srv_sys::mutex while reading this value as it is only used in heuristics. @@ -1948,20 +1983,15 @@ srv_get_activity_count(void) return(srv_sys.activity_count); } -/** Check if there has been any activity. -@param[in,out] activity_count recent activity count to be returned -if there is a change +/*******************************************************************//** +Check if there has been any activity. @return FALSE if no change in activity counter. */ -bool srv_check_activity(ulint *activity_count) +ibool +srv_check_activity( +/*===============*/ + ulint old_activity_count) /*!< in: old activity count */ { - ulint new_activity_count= srv_sys.activity_count; - if (new_activity_count != *activity_count) - { - *activity_count= new_activity_count; - return true; - } - - return false; + return(srv_sys.activity_count != old_activity_count); } /********************************************************************//** @@ -2362,30 +2392,52 @@ DECLARE_THREAD(srv_master_thread)( slot = srv_reserve_slot(SRV_MASTER); ut_a(slot == srv_sys.sys_threads); +loop: while (srv_shutdown_state <= SRV_SHUTDOWN_INITIATED) { srv_master_sleep(); MONITOR_INC(MONITOR_MASTER_THREAD_SLEEP); - if (srv_check_activity(&old_activity_count)) { + if (srv_check_activity(old_activity_count)) { + old_activity_count = srv_get_activity_count(); srv_master_do_active_tasks(); } else { srv_master_do_idle_tasks(); } } - ut_ad(srv_shutdown_state == SRV_SHUTDOWN_EXIT_THREADS - || srv_shutdown_state == SRV_SHUTDOWN_CLEANUP); - - if (srv_shutdown_state == SRV_SHUTDOWN_CLEANUP - && srv_fast_shutdown < 2) { - srv_shutdown(srv_fast_shutdown == 0); + switch (srv_shutdown_state) { + case SRV_SHUTDOWN_NONE: + case SRV_SHUTDOWN_INITIATED: + break; + case SRV_SHUTDOWN_FLUSH_PHASE: + case SRV_SHUTDOWN_LAST_PHASE: + ut_ad(0); + /* fall through */ + case SRV_SHUTDOWN_EXIT_THREADS: + /* srv_init_abort() must have been invoked */ + case SRV_SHUTDOWN_CLEANUP: + if (srv_shutdown_state == SRV_SHUTDOWN_CLEANUP + && srv_fast_shutdown < 2) { + srv_shutdown(srv_fast_shutdown == 0); + } + srv_suspend_thread(slot); + my_thread_end(); + os_thread_exit(); } + srv_main_thread_op_info = "suspending"; + srv_suspend_thread(slot); - my_thread_end(); - os_thread_exit(); - OS_THREAD_DUMMY_RETURN; + + /* DO NOT CHANGE THIS STRING. innobase_start_or_create_for_mysql() + waits for database activity to die down when converting < 4.1.x + databases, and relies on this string being exactly as it is. InnoDB + manual also mentions this string in several places. */ + srv_main_thread_op_info = "waiting for server activity"; + + srv_resume_thread(slot); + goto loop; } /** @return whether purge should exit due to shutdown */ @@ -2553,13 +2605,15 @@ static ulint srv_do_purge(ulint* n_total_purged ++n_use_threads; } - } else if (srv_check_activity(&old_activity_count) + } else if (srv_check_activity(old_activity_count) && n_use_threads > 1) { /* History length same or smaller since last snapshot, use fewer threads. */ --n_use_threads; + + old_activity_count = srv_get_activity_count(); } /* Ensure that the purge threads are less than what diff --git a/storage/innobase/srv/srv0start.cc b/storage/innobase/srv/srv0start.cc index 859e5a3eec2..318ab49cc9c 100644 --- a/storage/innobase/srv/srv0start.cc +++ b/storage/innobase/srv/srv0start.cc @@ -1178,7 +1178,7 @@ srv_shutdown_all_bg_threads() if (srv_start_state_is_set(SRV_START_STATE_MASTER)) { /* c. We wake the master thread so that it exits */ - srv_inc_activity_count(); + srv_wake_master_thread(); } if (srv_start_state_is_set(SRV_START_STATE_PURGE)) { @@ -2526,7 +2526,7 @@ void srv_shutdown_bg_undo_sources() fts_optimize_shutdown(); dict_stats_shutdown(); while (row_get_background_drop_list_len_low()) { - srv_inc_activity_count(); + srv_wake_master_thread(); os_thread_yield(); } srv_undo_sources = false; diff --git a/storage/innobase/trx/trx0roll.cc b/storage/innobase/trx/trx0roll.cc index c61a67ec05e..1be444adf07 100644 --- a/storage/innobase/trx/trx0roll.cc +++ b/storage/innobase/trx/trx0roll.cc @@ -164,6 +164,9 @@ trx_rollback_to_savepoint_low( mem_heap_free(heap); + /* There might be work for utility threads.*/ + srv_active_wake_master_thread(); + MONITOR_DEC(MONITOR_TRX_ACTIVE); } diff --git a/storage/innobase/trx/trx0trx.cc b/storage/innobase/trx/trx0trx.cc index e1efc7c895f..1d4202b3033 100644 --- a/storage/innobase/trx/trx0trx.cc +++ b/storage/innobase/trx/trx0trx.cc @@ -1470,6 +1470,12 @@ trx_commit_in_memory( } trx->commit_lsn = lsn; + + /* Tell server some activity has happened, since the trx + does changes something. Background utility threads like + master thread, purge thread or page_cleaner thread might + have some work to do. */ + srv_active_wake_master_thread(); } ut_ad(!trx->rsegs.m_noredo.undo); |