From 502733803979e2109b6dcdcb3d8c5a0ddd6d2363 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Lindstr=C3=B6m?= Date: Thu, 27 Mar 2014 09:35:24 +0200 Subject: [PATCH] Fix bug https://code.launchpad.net/~laurynas-biveinis/percona-server/bug1295268 (Inadequate background LRU flushing for write workloads with InnoDB compression). If InnoDB compression is used and the workload has writes, the following situation is possible. The LRU flusher issues an LRU flush request for an instance. buf_do_LRU_batch decides to perform unzip_LRU eviction and this eviction might fully satisfy the request. Then buf_flush_LRU_tail checks the number of flushed pages in the last iteration, finds it to be zero, and wrongly decides not to flush that instance anymore. Fixed by maintaining unzip_LRU eviction counter in struct flush_counter_t variables, and checking it in buf_flush_LRU_tail when deciding whether to stop flushing the current instance. Added test cases for new configuration files to get mysql-test-run suite sys_vars to pass. Fix some small errors. --- .../r/innodb_mtflush_threads_basic.result | 21 +++++++++++ .../sys_vars/r/innodb_use_lz4_basic.result | 3 ++ .../r/innodb_use_mtflush_basic.result | 21 +++++++++++ .../sys_vars/r/innodb_use_trim_basic.result | 33 +++++++++++++++++ .../t/innodb_mtflush_threads_basic.test | 21 +++++++++++ .../sys_vars/t/innodb_use_lz4_basic.test | 5 +++ .../sys_vars/t/innodb_use_mtflush_basic.test | 22 ++++++++++++ .../sys_vars/t/innodb_use_trim_basic.test | 36 +++++++++++++++++++ storage/innobase/handler/ha_innodb.cc | 4 +-- storage/xtradb/buf/buf0flu.cc | 24 ++++++++----- storage/xtradb/handler/ha_innodb.cc | 4 +-- storage/xtradb/include/buf0flu.h | 2 ++ 12 files changed, 184 insertions(+), 12 deletions(-) create mode 100644 mysql-test/suite/sys_vars/r/innodb_mtflush_threads_basic.result create mode 100644 mysql-test/suite/sys_vars/r/innodb_use_lz4_basic.result create mode 100644 mysql-test/suite/sys_vars/r/innodb_use_mtflush_basic.result create mode 100644 mysql-test/suite/sys_vars/r/innodb_use_trim_basic.result create mode 100644 mysql-test/suite/sys_vars/t/innodb_mtflush_threads_basic.test create mode 100644 mysql-test/suite/sys_vars/t/innodb_use_lz4_basic.test create mode 100644 mysql-test/suite/sys_vars/t/innodb_use_mtflush_basic.test create mode 100644 mysql-test/suite/sys_vars/t/innodb_use_trim_basic.test diff --git a/mysql-test/suite/sys_vars/r/innodb_mtflush_threads_basic.result b/mysql-test/suite/sys_vars/r/innodb_mtflush_threads_basic.result new file mode 100644 index 00000000000..75a1cc5262e --- /dev/null +++ b/mysql-test/suite/sys_vars/r/innodb_mtflush_threads_basic.result @@ -0,0 +1,21 @@ +select @@global.innodb_mtflush_threads; +@@global.innodb_mtflush_threads +8 +select @@session.innodb_mtflush_threads; +ERROR HY000: Variable 'innodb_mtflush_threads' is a GLOBAL variable +show global variables like 'innodb_mtflush_threads'; +Variable_name Value +innodb_mtflush_threads 8 +show session variables like 'innodb_mtflush_threads'; +Variable_name Value +innodb_mtflush_threads 8 +select * from information_schema.global_variables where variable_name='innodb_mtflush_threads'; +VARIABLE_NAME VARIABLE_VALUE +INNODB_MTFLUSH_THREADS 8 +select * from information_schema.session_variables where variable_name='innodb_mtflush_threads'; +VARIABLE_NAME VARIABLE_VALUE +INNODB_MTFLUSH_THREADS 8 +set global innodb_mtflush_threads=1; +ERROR HY000: Variable 'innodb_mtflush_threads' is a read only variable +set session innodb_mtflush_threads=1; +ERROR HY000: Variable 'innodb_mtflush_threads' is a read only variable diff --git a/mysql-test/suite/sys_vars/r/innodb_use_lz4_basic.result b/mysql-test/suite/sys_vars/r/innodb_use_lz4_basic.result new file mode 100644 index 00000000000..4c3cfa524af --- /dev/null +++ b/mysql-test/suite/sys_vars/r/innodb_use_lz4_basic.result @@ -0,0 +1,3 @@ +select @@global.innodb_use_fallocate; +@@global.innodb_use_fallocate +0 diff --git a/mysql-test/suite/sys_vars/r/innodb_use_mtflush_basic.result b/mysql-test/suite/sys_vars/r/innodb_use_mtflush_basic.result new file mode 100644 index 00000000000..f77abba7ac9 --- /dev/null +++ b/mysql-test/suite/sys_vars/r/innodb_use_mtflush_basic.result @@ -0,0 +1,21 @@ +select @@global.innodb_use_mtflush; +@@global.innodb_use_mtflush +0 +select @@session.innodb_use_mtflush; +ERROR HY000: Variable 'innodb_use_mtflush' is a GLOBAL variable +show global variables like 'innodb_use_mtflush'; +Variable_name Value +innodb_use_mtflush OFF +show session variables like 'innodb_use_mtflush'; +Variable_name Value +innodb_use_mtflush OFF +select * from information_schema.global_variables where variable_name='innodb_use_mtflush'; +VARIABLE_NAME VARIABLE_VALUE +INNODB_USE_MTFLUSH OFF +select * from information_schema.session_variables where variable_name='innodb_use_mtflush'; +VARIABLE_NAME VARIABLE_VALUE +INNODB_USE_MTFLUSH OFF +set global innodb_use_mtflush=1; +ERROR HY000: Variable 'innodb_use_mtflush' is a read only variable +set session innodb_use_mtflush=1; +ERROR HY000: Variable 'innodb_use_mtflush' is a read only variable diff --git a/mysql-test/suite/sys_vars/r/innodb_use_trim_basic.result b/mysql-test/suite/sys_vars/r/innodb_use_trim_basic.result new file mode 100644 index 00000000000..63292f5d3c8 --- /dev/null +++ b/mysql-test/suite/sys_vars/r/innodb_use_trim_basic.result @@ -0,0 +1,33 @@ +SET @start_use_trim = @@global.innodb_use_trim; +SELECT @start_use_trim; +@start_use_trim +0 +SELECT COUNT(@@GLOBAL.innodb_use_trim); +COUNT(@@GLOBAL.innodb_use_trim) +1 +1 Expected +SET @@GLOBAL.innodb_use_trim=1; +SELECT COUNT(@@GLOBAL.innodb_use_trim); +COUNT(@@GLOBAL.innodb_use_trim) +1 +1 Expected +SELECT IF(@@GLOBAL.innodb_use_trim, 'ON', 'OFF') = VARIABLE_VALUE +FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES +WHERE VARIABLE_NAME='innodb_use_trim'; +IF(@@GLOBAL.innodb_use_trim, 'ON', 'OFF') = VARIABLE_VALUE +1 +1 Expected +SELECT COUNT(@@GLOBAL.innodb_use_trim); +COUNT(@@GLOBAL.innodb_use_trim) +1 +1 Expected +SELECT COUNT(VARIABLE_VALUE) +FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES +WHERE VARIABLE_NAME='innodb_use_trim'; +COUNT(VARIABLE_VALUE) +1 +1 Expected +SET @@global.innodb_use_trim = @start_use_trim; +SELECT @@global.innodb_use_trim; +@@global.innodb_use_trim +0 diff --git a/mysql-test/suite/sys_vars/t/innodb_mtflush_threads_basic.test b/mysql-test/suite/sys_vars/t/innodb_mtflush_threads_basic.test new file mode 100644 index 00000000000..c8412f969eb --- /dev/null +++ b/mysql-test/suite/sys_vars/t/innodb_mtflush_threads_basic.test @@ -0,0 +1,21 @@ +--source include/have_innodb.inc +# bool readonly + +# +# show values; +# +select @@global.innodb_mtflush_threads; +--error ER_INCORRECT_GLOBAL_LOCAL_VAR +select @@session.innodb_mtflush_threads; +show global variables like 'innodb_mtflush_threads'; +show session variables like 'innodb_mtflush_threads'; +select * from information_schema.global_variables where variable_name='innodb_mtflush_threads'; +select * from information_schema.session_variables where variable_name='innodb_mtflush_threads'; + +# +# show that it's read-only +# +--error ER_INCORRECT_GLOBAL_LOCAL_VAR +set global innodb_mtflush_threads=1; +--error ER_INCORRECT_GLOBAL_LOCAL_VAR +set session innodb_mtflush_threads=1; diff --git a/mysql-test/suite/sys_vars/t/innodb_use_lz4_basic.test b/mysql-test/suite/sys_vars/t/innodb_use_lz4_basic.test new file mode 100644 index 00000000000..aefa276dcee --- /dev/null +++ b/mysql-test/suite/sys_vars/t/innodb_use_lz4_basic.test @@ -0,0 +1,5 @@ +--source include/have_innodb.inc +# bool readonly +# not on all compilations +select @@global.innodb_use_fallocate; + diff --git a/mysql-test/suite/sys_vars/t/innodb_use_mtflush_basic.test b/mysql-test/suite/sys_vars/t/innodb_use_mtflush_basic.test new file mode 100644 index 00000000000..a9c40b9e522 --- /dev/null +++ b/mysql-test/suite/sys_vars/t/innodb_use_mtflush_basic.test @@ -0,0 +1,22 @@ +--source include/have_innodb.inc +# bool readonly + +# +# show values; +# +select @@global.innodb_use_mtflush; +--error ER_INCORRECT_GLOBAL_LOCAL_VAR +select @@session.innodb_use_mtflush; +show global variables like 'innodb_use_mtflush'; +show session variables like 'innodb_use_mtflush'; +select * from information_schema.global_variables where variable_name='innodb_use_mtflush'; +select * from information_schema.session_variables where variable_name='innodb_use_mtflush'; + +# +# show that it's read-only +# +--error ER_INCORRECT_GLOBAL_LOCAL_VAR +set global innodb_use_mtflush=1; +--error ER_INCORRECT_GLOBAL_LOCAL_VAR +set session innodb_use_mtflush=1; + diff --git a/mysql-test/suite/sys_vars/t/innodb_use_trim_basic.test b/mysql-test/suite/sys_vars/t/innodb_use_trim_basic.test new file mode 100644 index 00000000000..c1b0f142179 --- /dev/null +++ b/mysql-test/suite/sys_vars/t/innodb_use_trim_basic.test @@ -0,0 +1,36 @@ +--source include/have_innodb.inc + +SET @start_use_trim = @@global.innodb_use_trim; +SELECT @start_use_trim; + +SELECT COUNT(@@GLOBAL.innodb_use_trim); +--echo 1 Expected + +#################################################################### +# Check if Value can set # +#################################################################### + +SET @@GLOBAL.innodb_use_trim=1; + +SELECT COUNT(@@GLOBAL.innodb_use_trim); +--echo 1 Expected + +################################################################# +# Check if the value in GLOBAL Table matches value in variable # +################################################################# + +SELECT IF(@@GLOBAL.innodb_use_trim, 'ON', 'OFF') = VARIABLE_VALUE +FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES +WHERE VARIABLE_NAME='innodb_use_trim'; +--echo 1 Expected + +SELECT COUNT(@@GLOBAL.innodb_use_trim); +--echo 1 Expected + +SELECT COUNT(VARIABLE_VALUE) +FROM INFORMATION_SCHEMA.GLOBAL_VARIABLES +WHERE VARIABLE_NAME='innodb_use_trim'; +--echo 1 Expected + +SET @@global.innodb_use_trim = @start_use_trim; +SELECT @@global.innodb_use_trim; \ No newline at end of file diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc index b790ae76121..16e33c8901f 100644 --- a/storage/innobase/handler/ha_innodb.cc +++ b/storage/innobase/handler/ha_innodb.cc @@ -16801,7 +16801,7 @@ static MYSQL_SYSVAR_BOOL(use_lz4, srv_use_lz4, #endif /* HAVE_LZ4 */ static MYSQL_SYSVAR_LONG(mtflush_threads, srv_mtflush_threads, - PLUGIN_VAR_RQCMDARG, + PLUGIN_VAR_NOCMDARG | PLUGIN_VAR_READONLY, "Number of multi-threaded flush threads", NULL, NULL, MTFLUSH_DEFAULT_WORKER, /* Default setting */ @@ -16810,7 +16810,7 @@ static MYSQL_SYSVAR_LONG(mtflush_threads, srv_mtflush_threads, 0); static MYSQL_SYSVAR_BOOL(use_mtflush, srv_use_mtflush, - PLUGIN_VAR_OPCMDARG , + PLUGIN_VAR_NOCMDARG | PLUGIN_VAR_READONLY, "Use multi-threaded flush. Default FALSE.", NULL, NULL, FALSE); diff --git a/storage/xtradb/buf/buf0flu.cc b/storage/xtradb/buf/buf0flu.cc index 53ac9bb9cc7..f4ba0f10761 100644 --- a/storage/xtradb/buf/buf0flu.cc +++ b/storage/xtradb/buf/buf0flu.cc @@ -1549,6 +1549,7 @@ buf_flush_LRU_list_batch( n->flushed = 0; n->evicted = 0; + n->unzip_LRU_evicted = 0; ut_ad(mutex_own(&buf_pool->LRU_list_mutex)); @@ -1660,21 +1661,22 @@ buf_do_LRU_batch( flush_counters_t* n) /*!< out: flushed/evicted page counts */ { - ulint count = 0; - if (buf_LRU_evict_from_unzip_LRU(buf_pool)) { - count += buf_free_from_unzip_LRU_list_batch(buf_pool, max); + n->unzip_LRU_evicted + += buf_free_from_unzip_LRU_list_batch(buf_pool, max); + } else { + n->unzip_LRU_evicted = 0; } - if (max > count) { - buf_flush_LRU_list_batch(buf_pool, max - count, limited_scan, - n); + if (max > n->unzip_LRU_evicted) { + buf_flush_LRU_list_batch(buf_pool, max - n->unzip_LRU_evicted, + limited_scan, n); } else { n->evicted = 0; n->flushed = 0; } - n->flushed += count; + n->evicted += n->unzip_LRU_evicted; } /*******************************************************************//** @@ -2306,9 +2308,15 @@ buf_flush_LRU_tail(void) requested_pages[i] += lru_chunk_size; + /* If we failed to flush or evict this + instance, do not bother anymore. But take into + account that we might have zero flushed pages + because the flushing request was fully + satisfied by unzip_LRU evictions. */ if (requested_pages[i] >= scan_depth[i] || !(srv_cleaner_eviction_factor - ? n.evicted : n.flushed)) { + ? n.evicted + : (n.flushed + n.unzip_LRU_evicted))) { active_instance[i] = false; remaining_instances--; diff --git a/storage/xtradb/handler/ha_innodb.cc b/storage/xtradb/handler/ha_innodb.cc index 4436dc3d0e1..09416a990d7 100644 --- a/storage/xtradb/handler/ha_innodb.cc +++ b/storage/xtradb/handler/ha_innodb.cc @@ -17955,7 +17955,7 @@ static MYSQL_SYSVAR_BOOL(use_lz4, srv_use_lz4, #endif /* HAVE_LZ4 */ static MYSQL_SYSVAR_LONG(mtflush_threads, srv_mtflush_threads, - PLUGIN_VAR_RQCMDARG, + PLUGIN_VAR_NOCMDARG | PLUGIN_VAR_READONLY, "Number of multi-threaded flush threads", NULL, NULL, MTFLUSH_DEFAULT_WORKER, /* Default setting */ @@ -17964,7 +17964,7 @@ static MYSQL_SYSVAR_LONG(mtflush_threads, srv_mtflush_threads, 0); static MYSQL_SYSVAR_BOOL(use_mtflush, srv_use_mtflush, - PLUGIN_VAR_OPCMDARG , + PLUGIN_VAR_NOCMDARG | PLUGIN_VAR_READONLY, "Use multi-threaded flush. Default FALSE.", NULL, NULL, FALSE); diff --git a/storage/xtradb/include/buf0flu.h b/storage/xtradb/include/buf0flu.h index 528ec7b3f64..4cb1446036b 100644 --- a/storage/xtradb/include/buf0flu.h +++ b/storage/xtradb/include/buf0flu.h @@ -40,6 +40,8 @@ extern ibool buf_page_cleaner_is_active; struct flush_counters_t { ulint flushed; /*!< number of dirty pages flushed */ ulint evicted; /*!< number of clean pages evicted */ + ulint unzip_LRU_evicted;/*!< number of uncompressed page images + evicted */ };