[MDEV-15384] buf_flush_LRU_list_batch() always reports n->flushed=0, n->evicted=0 Created: 2018-02-22  Updated: 2018-03-16  Resolved: 2018-03-13

Status: Closed
Project: MariaDB Server
Component/s: Storage Engine - InnoDB
Affects Version/s: 10.2.2, 10.3.0
Fix Version/s: 10.2.14, 10.3.6

Type: Bug Priority: Major
Reporter: Marko Mäkelä Assignee: Thirunarayanan Balathandayuthapani
Resolution: Fixed Votes: 0
Labels: regression-10.2

Issue Links:
Relates
relates to MDEV-6936 Buffer pool list scan optimization Closed
relates to MDEV-15053 Reduce buf_pool_t::mutex contention Closed
Sprint: 10.2.14

 Description   

The flush_counters_t that were introduced in MDEV-6936 were broken in the merge of InnoDB from MySQL 5.7.9 to MariaDB 10.2.2. After that change, the function buf_flush_LRU_list_batch() initializes all counts to 0 and will not update them.

I would fix this as follows. I still need to figure out how to add a regression test for this.

diff --git a/storage/innobase/buf/buf0flu.cc b/storage/innobase/buf/buf0flu.cc
 
index e938c27cfec..17f915a9d12 100644
 
--- a/storage/innobase/buf/buf0flu.cc
 
+++ b/storage/innobase/buf/buf0flu.cc
 
@@ -1654,8 +1654,6 @@ buf_flush_LRU_list_batch(
 
 {
 
 	buf_page_t*	bpage;
 
 	ulint		scanned = 0;
 
-	ulint		evict_count = 0;
 
-	ulint		count = 0;
 
 	ulint		free_len = UT_LIST_GET_LEN(buf_pool->free);
 
 	ulint		lru_len = UT_LIST_GET_LEN(buf_pool->LRU);
 
 	ulint		withdraw_depth = 0;
 
@@ -1671,7 +1669,7 @@ buf_flush_LRU_list_batch(
 
 	}
 
 
 	for (bpage = UT_LIST_GET_LAST(buf_pool->LRU);
 
-	     bpage != NULL && count + evict_count < max
 
+	     bpage != NULL && n->flushed + n->evicted < max
 
 	     && free_len < srv_LRU_scan_depth + withdraw_depth
 
 	     && lru_len > BUF_LRU_MIN_LEN;
 
 	     ++scanned,
 
@@ -1689,7 +1687,7 @@ buf_flush_LRU_list_batch(
 
 			clean and is not IO-fixed or buffer fixed. */
 
 			mutex_exit(block_mutex);
 
 			if (buf_LRU_free_page(bpage, true)) {
 
-				++evict_count;
 
+				++n->evicted;
 
 			}
 
 		} else if (buf_flush_ready_for_flush(bpage, BUF_FLUSH_LRU)) {
 
 			/* Block is ready for flush. Dispatch an IO
 
@@ -1697,7 +1695,7 @@ buf_flush_LRU_list_batch(
 
 			free list in IO completion routine. */
 
 			mutex_exit(block_mutex);
 
 			buf_flush_page_and_try_neighbors(
 
-				bpage, BUF_FLUSH_LRU, max, &count);
 
+				bpage, BUF_FLUSH_LRU, max, &n->flushed);
 
 		} else {
 
 			/* Can't evict or dispatch this block. Go to
 
 			previous. */
 
@@ -1721,12 +1719,12 @@ buf_flush_LRU_list_batch(
 
 
 	ut_ad(buf_pool_mutex_own(buf_pool));
 
 
-	if (evict_count) {
 
+	if (n->evicted) {
 
 		MONITOR_INC_VALUE_CUMULATIVE(
 
 			MONITOR_LRU_BATCH_EVICT_TOTAL_PAGE,
 
 			MONITOR_LRU_BATCH_EVICT_COUNT,
 
 			MONITOR_LRU_BATCH_EVICT_PAGES,
 
-			evict_count);
 
+			n->evicted);
 
 	}
 
 
 	if (scanned) {
 
@@ -2182,17 +2180,16 @@ buf_flush_lists(
 
 			help in the retry which will follow the
 
 			failure. */
 
 			success = false;
 
-
 
-			continue;
 
 		}
 
+
 
+		n_flushed += n.flushed;
 
 	}
 
 
 	if (n_flushed) {
 
 		buf_flush_stats(n_flushed, 0);
 
-	}
 
-
 
-	if (n_processed) {
 
-		*n_processed = n_flushed;
 
+		if (n_processed) {
 
+			*n_processed = n_flushed;
 
+		}
 
 	}
 
 
 	return(success);



 Comments   
Comment by Marko Mäkelä [ 2018-02-22 ]

Even with the patch, I cannot get a test to report any page flushes:

diff --git a/mysql-test/suite/innodb/t/purge_secondary.test b/mysql-test/suite/innodb/t/purge_secondary.test
 
index 47cfaec41ca..3ffaa9b9802 100644
 
--- a/mysql-test/suite/innodb/t/purge_secondary.test
 
+++ b/mysql-test/suite/innodb/t/purge_secondary.test
 
@@ -1,5 +1,8 @@
 
 --source include/have_innodb.inc
 
 
+SET GLOBAL innodb_monitor_enable = '%flush%';
 
+SET GLOBAL innodb_monitor_enable = DEFAULT;
 
+
 
 # Ensure that the history list length will actually be decremented by purge.
 
 SET @saved_frequency = @@GLOBAL.innodb_purge_rseg_truncate_frequency;
 
 SET GLOBAL innodb_purge_rseg_truncate_frequency = 1;
 
@@ -121,11 +124,21 @@ SELECT OTHER_INDEX_SIZE FROM INFORMATION_SCHEMA.INNODB_SYS_TABLESTATS
 
 WHERE NAME='test/t1';
 
 
 --echo # Note: The OTHER_INDEX_SIZE does not cover any SPATIAL INDEX.
 
---echo # To test that all indexes were emptied, replace DROP TABLE
 
---echo # with the following, and examine the root pages in t1.ibd:
 
---echo # FLUSH TABLES t1 FOR EXPORT;
 
---echo # UNLOCK TABLES;
 
+--echo # To test that all indexes were emptied, remove DROP TABLE
 
+--echo # and examine the root pages in t1.ibd:
 
+FLUSH TABLES t1 FOR EXPORT;
 
+UNLOCK TABLES;
 
 
 DROP TABLE t1;
 
 
 SET GLOBAL innodb_purge_rseg_truncate_frequency = @saved_frequency;
 
+
 
+SELECT * FROM information_schema.global_status
 
+WHERE LOWER(variable_name) LIKE 'innodb%flush%';
 
+
 
+SELECT name, max_count, min_count, count,
 
+	max_count_reset, min_count_reset, count_reset, status
 
+FROM information_schema.innodb_metrics
 
+WHERE name LIKE '%flush%';
 
+SET GLOBAL innodb_monitor_disable = '%flush%';
 
+SET GLOBAL innodb_monitor_disable = DEFAULT;

Please revise this to make this work.
I noticed the problem when working on MDEV-15053, on the MySQL 8.0 BUG#23102834 INNODB: ZERO FLUSH FROM LRU.
Preferrably I would test this in some existing tests, instead of adding a separate big test for this, like the above Oracle change did.
Also, we might want to check INNODB_ENCRYPTION_ROTATION_PAGES_FLUSHED in some of the encryption tests.

Comment by Marko Mäkelä [ 2018-03-12 ]

OK to push.

Generated at Thu Feb 08 08:20:53 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.