Bug #71761 ANALYZE TABLE should remove its table from background stat processing queue
Submitted: 18 Feb 2014 15:03 Modified: 1 Dec 2016 8:18
Reporter: Laurynas Biveinis (OCA) Email Updates:
Status: Verified Impact on me:
None 
Category:MySQL Server: InnoDB storage engine Severity:S5 (Performance)
Version:5.6.16, 5.7, 8.0 OS:Any
Assigned to:
Tags: analyze, background stats, stats

[18 Feb 2014 15:03] Laurynas Biveinis
Description:
1. An InnoDB table receives many updates and is added to the background stats processing queue.
2. Updates stop, an ANALYZE TABLE request comes, and the table stats are recalculated.
3. Some time later, the background stats thread picks up this table and recalculates its stats even though they are already up to date.

How to repeat:
Run write workload on a table, stop it, ANALYZE TABLE, observe the background stats thread recalculating its stats again.

Suggested fix:
One way to fix it is to add dict_stats_recalc_pool_del(ib_table), guarded by dict_mutex_enter/exit_for_mysql(), to ha_innobase::info_low if(is_analyze || innobase_stats_on_metadata).
[27 Sep 2016 10:04] Laurynas Biveinis
Bug 71761 fix for 5.6

(*) I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it.

Contribution: bug71761-5.6.patch (application/octet-stream, text), 691 bytes.

[27 Sep 2016 10:04] Laurynas Biveinis
Bug 71761 fix for 5.7

(*) I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it.

Contribution: bug71761-5.7.patch (application/octet-stream, text), 691 bytes.

[27 Sep 2016 10:04] Laurynas Biveinis
Bug 71761 fix for 8.0

(*) I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it.

Contribution: bug71761-8.0.patch (application/octet-stream, text), 1.03 KiB.

[27 Sep 2016 10:06] Laurynas Biveinis
Is there anything blocking the verification of this bug?
[10 Nov 2016 4:55] Umesh Shastry
Hello Laurynas,

Thank you for the report and contribution.

Thanks,
Umesh
[14 Nov 2016 7:13] Vasil Dimov
This came into my radar now.

Yes, it is obvious that a manual ANALYZE TABLE should cancel any pending automatic ANALYZEs that were scheduled automatically.

I should have done this in the initial implementation.

The provided patch looks good. I will see that it gets pushed into the trees soon.

Notice that, due to a concurrent DML, a table might get added to the background queue immediately or shortly after we have removed it at the start of a manual ANALYZE TABLE but before the ANALYZE TABLE has completed. This is fine - if the table gets changed beyond the threshold while ANALYZE is running, that warrants a new re-calc. So it is ok to just remove the table from the queue at the beginning of a manual ANALYZE TABLE.
[1 Dec 2016 7:43] Laurynas Biveinis
The patch crashes on 8.0 

main.dd_is_compatibility_ci              w1 [ fail ]
...
2016-11-30T22:20:58.729262Z 4 [ERROR] InnoDB: Assertion failure:
dict0stats_bg.cc:186:!srv_read_only_mode
...
5   mysqld-debug                        0x000000010a9a1acc ut_dbg_assertion_failed(char const*, char const*, unsigned long) + 220
6   mysqld-debug                        0x000000010a61a08d dict_stats_recalc_pool_del(dict_table_t const*) + 77
7   mysqld-debug                        0x000000010a6e11a8 ha_innobase::info_low(unsigned int, bool) + 376
8   mysqld-debug                        0x000000010a6e2e7b ha_innobase::analyze(THD*, st_ha_check_opt*) + 59
9   mysqld-debug                        0x00000001095bea0a handler::ha_analyze(THD*, st_ha_check_opt*) + 154
10  mysqld-debug                        0x0000000109e10132 mysql_admin_table(THD*, TABLE_LIST*, st_ha_check_opt*, char const*, thr_lock_type, bool, bool, unsigned int, int (*)(THD*, TABLE_LIST*, st_ha_check_opt*), int (handler::*)(THD*, st_ha_check_opt*), int) + 7618
11  mysqld-debug                        0x0000000109e125ea Sql_cmd_analyze_table::execute(THD*) + 410
12  mysqld-debug                        0x0000000109ef1aea mysql_execute_command(THD*, bool) + 33674
13  mysqld-debug                        0x0000000109ee7d0d mysql_parse(THD*, Parser_state*) + 1661
14  mysqld-debug                        0x0000000109ee49e6 dispatch_command(THD*, COM_DATA const*, enum_server_command) + 4022
15  mysqld-debug                        0x0000000109ee6d12 do_command(THD*) + 1234
16  mysqld-debug                        0x000000010a35a4c3 handle_connection(void*) + 483
17  mysqld-debug                        0x000000010aa56d0c pfs_spawn_thread(void*) + 396
18  libsystem_pthread.dylib             0x00007fff9bc73aab _pthread_body + 180
19  libsystem_pthread.dylib             0x00007fff9bc739f7 _pthread_body + 0
20  libsystem_pthread.dylib             0x00007fff9bc73221 thread_start + 13

The following, on the top of the submitted patch, fixes it. It looks like the bug is present in lower version patches too.

diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc
index 99300c5..c81b5a4 100644
--- a/storage/innobase/handler/ha_innodb.cc
+++ b/storage/innobase/handler/ha_innodb.cc
@@ -14768,9 +14768,13 @@ ha_innobase::info_low(
                                        /* If this table is already queued for
                                        background analyze, remove it from the
                                        queue as we are about to do the same */
-                                       dict_mutex_enter_for_mysql();
-                                       dict_stats_recalc_pool_del(ib_table);
-                                       dict_mutex_exit_for_mysql();
+                                       if (!srv_read_only_mode) {
+
+                                               dict_mutex_enter_for_mysql();
+                                               dict_stats_recalc_pool_del(
+                                                       ib_table);
+                                               dict_mutex_exit_for_mysql();
+                                       }
 
                                        opt = DICT_STATS_RECALC_PERSISTENT;
                                } else {
[1 Dec 2016 8:18] Laurynas Biveinis
A testcase for 5.6/5.7 too

--source include/have_innodb.inc

CREATE TABLE t1 (a INT PRIMARY KEY) ENGINE=InnoDB;
INSERT INTO t1 VALUES (1);

--source include/restart_readonly_mysqld.inc

ANALYZE TABLE t1;

--source include/restart_mysqld.inc

DROP TABLE t1;
[14 Apr 9:08] Laurynas Biveinis
Bug 71761 fix for 8.0.1

(*) I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it.

Contribution: bug71761-8.0.1.patch (application/octet-stream, text), 2.63 KiB.

[4 Aug 19:30] Laurynas Biveinis
Bug 71761 fix for 8.0.2

(*) I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it.

Contribution: bug71761-8.0.2.patch (application/octet-stream, text), 2.63 KiB.