[MDEV-23991] dict_table_stats_lock() has unnecessarily long scope Created: 2020-10-20  Updated: 2021-07-26  Resolved: 2020-10-28

Status: Closed
Project: MariaDB Server
Component/s: Storage Engine - InnoDB
Affects Version/s: 10.0, 10.1, 10.2, 10.3, 10.4, 10.5
Fix Version/s: 10.2.35, 10.3.26, 10.4.16, 10.5.7

Type: Bug Priority: Blocker
Reporter: Marko Mäkelä Assignee: Eugene Kosov (Inactive)
Resolution: Fixed Votes: 0
Labels: upstream-fixed

Issue Links:
Blocks
blocks MDEV-23989 Merge new release of InnoDB 5.7.32 to... Closed
Problem/Incident
causes MDEV-24275 InnoDB persistent stats analyze force... Closed

 Description   

The following test demonstrates that while InnoDB is collecting statistics are collected on a large table, any access from SQL to that table will be unnecessarily blocked by dict_table_stats_lock():

diff a/mysql-test/suite/innodb/t/stats_persistent.test b/mysql-test/suite/innodb/t/stats_persistent.test
 
new file mode 100644
 
--- /dev/null
 
+++ b/mysql-test/suite/innodb/t/stats_persistent.test
 
@@ -0,0 +1,26 @@
 
+--source include/have_innodb.inc
 
+--source include/have_debug.inc
 
+--source include/have_debug_sync.inc
 
+--source include/count_sessions.inc
 
+
 
+CREATE TABLE t1(a int) ENGINE=INNODB STATS_PERSISTENT=1;
 
+
 
+SET DEBUG_SYNC='dict_stats_update_persistent SIGNAL stop WAIT_FOR go';
 
+send ANALYZE TABLE t1;
 
+
 
+--connect(con1, localhost, root)
 
+SET DEBUG_SYNC='now WAIT_FOR stop';
 
+
 
+SELECT * FROM information_schema.TABLES WHERE ENGINE='InnoDB';
 
+
 
+SET DEBUG_SYNC='now SIGNAL go';
 
+--disconnect con1
 
+
 
+--connection default
 
+--reap
 
+SET DEBUG_SYNC= 'RESET';
 
+DROP TABLE t1;
 
+
 
+--source include/wait_until_count_sessions.inc
 
diff a/storage/innobase/dict/dict0stats.cc b/storage/innobase/dict/dict0stats.cc
 
--- a/storage/innobase/dict/dict0stats.cc
 
+++ b/storage/innobase/dict/dict0stats.cc
 
@@ -2207,6 +2207,8 @@ dict_stats_update_persistent(
 
 
 	dict_table_stats_lock(table, RW_X_LATCH);
 
 
+	DEBUG_SYNC_C("dict_stats_update_persistent");
 
+
 
 	/* analyze the clustered index first */
 
 
 	index = dict_table_get_first_index(table);

We can allow multiple threads to collect statistics for the same table in parallel, but only one thread at a time may be allowed to update the statistics. To prevent unnecessary work for multiple threads collecting statistics for the same table, we might introduce a counter of pending statistics-gathering operations. MySQL 5.7.32 preferred to introduce a lazily initialized mutex, which would not be easy to merge to 10.4.

 Comments   
Comment by Marko Mäkelä [ 2020-10-20 ]

I think that we should properly separate the statistics collection and the updates. If we do that, updating and accessing the statistics in dict_table_t and dict_index_t should be quick enough to be protected by dict_sys->mutex. Updating the persistent statistics tables will be holding it already.

Collecting that statistics can be protected by normal dict_index_t::lock and buf_block_t::lock. I do not think that multiple concurrent threads collecting statistics on the same index are going to be an issue.

Comment by Marko Mäkelä [ 2020-10-27 ]

The fix so far looks good, except that in a few places we seem to be violating the latching order, by acquiring dict_sys->mutex while holding an index->lock. Be sure to release the latch via mtr_t::commit() or similar, to fix this. Run tests with ./mtr --mysqld=--loose-innodb-debug-sync to catch this.

Generated at Thu Feb 08 09:26:37 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.