From e5c4c0842d0a11b9919efcda09377083a4a0d69a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Thu, 19 Dec 2024 14:05:16 +0200 Subject: [PATCH 1/4] MDEV-35443: opt_search_plan_for_table() may degrade to full table scan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit opt_calc_index_goodness(): Correct an inaccurate condition. We can very well use a clustered index of a table that is subject to online rebuild. But we must not choose an index that has not been committed (it is a secondary index that was not fully created) or that is corrupted or not a normal B-tree index. opt_search_plan_for_table(): Remove some redundant code, now that opt_calc_index_goodness() checks against corrupted indexes. The test case allows this code to be exercised. The main observation in the following: ./mtr --rr innodb.stats_persistent rr replay var/log/mysqld.1.rr/latest-trace should be that when opt_search_plan_for_table() is being invoked by dict_stats_update_persistent() on the being-altered statistics table in the 2nd call after ha_innobase::inplace_alter_table(), and the fix in opt_calc_index_goodness() is absent, it would choose the code path if (n_fields == 0), that is, a full table scan, instead of searching for the record. The GDB commands to execute in "rr replay" would be as follows: break ha_innobase::inplace_alter_table continue break opt_search_plan_for_table continue continue next next … Reviewed by: Vladislav Lesin --- mysql-test/suite/innodb/r/stats_persistent.result | 9 +++++++++ mysql-test/suite/innodb/t/stats_persistent.test | 11 +++++++++++ storage/innobase/include/dict0mem.h | 8 ++++++++ storage/innobase/pars/pars0opt.cc | 13 +++++-------- 4 files changed, 33 insertions(+), 8 deletions(-) diff --git a/mysql-test/suite/innodb/r/stats_persistent.result b/mysql-test/suite/innodb/r/stats_persistent.result index 7e9c038d6f7..2752811a07c 100644 --- a/mysql-test/suite/innodb/r/stats_persistent.result +++ b/mysql-test/suite/innodb/r/stats_persistent.result @@ -6,14 +6,23 @@ SET DEBUG_SYNC='dict_stats_update_persistent SIGNAL stop WAIT_FOR go'; ANALYZE TABLE t1; connect con1, localhost, root; SET DEBUG_SYNC='now WAIT_FOR stop'; +connect con2, localhost, root; +SET DEBUG_SYNC='innodb_inplace_alter_table_enter SIGNAL astop WAIT_FOR ago'; +ALTER TABLE mysql.innodb_index_stats FORCE; +connection con1; SELECT SUM(DATA_LENGTH+INDEX_LENGTH) FROM information_schema.TABLES WHERE ENGINE='InnoDB'; SUM(DATA_LENGTH+INDEX_LENGTH) SUM +SET DEBUG_SYNC='now WAIT_FOR astop'; SET DEBUG_SYNC='now SIGNAL go'; disconnect con1; connection default; Table Op Msg_type Msg_text test.t1 analyze status Engine-independent statistics collected test.t1 analyze status OK +SET DEBUG_SYNC='now SIGNAL ago'; +connection con2; +disconnect con2; +connection default; SET DEBUG_SYNC= 'RESET'; DROP TABLE t1; diff --git a/mysql-test/suite/innodb/t/stats_persistent.test b/mysql-test/suite/innodb/t/stats_persistent.test index 8561298c4d3..870808993f4 100644 --- a/mysql-test/suite/innodb/t/stats_persistent.test +++ b/mysql-test/suite/innodb/t/stats_persistent.test @@ -14,14 +14,25 @@ SET DEBUG_SYNC='dict_stats_update_persistent SIGNAL stop WAIT_FOR go'; --connect(con1, localhost, root) SET DEBUG_SYNC='now WAIT_FOR stop'; +--connect(con2, localhost, root) +SET DEBUG_SYNC='innodb_inplace_alter_table_enter SIGNAL astop WAIT_FOR ago'; +send ALTER TABLE mysql.innodb_index_stats FORCE; + +--connection con1 --replace_column 1 SUM SELECT SUM(DATA_LENGTH+INDEX_LENGTH) FROM information_schema.TABLES WHERE ENGINE='InnoDB'; +SET DEBUG_SYNC='now WAIT_FOR astop'; SET DEBUG_SYNC='now SIGNAL go'; --disconnect con1 --connection default --reap +SET DEBUG_SYNC='now SIGNAL ago'; +--connection con2 +reap; +--disconnect con2 +--connection default SET DEBUG_SYNC= 'RESET'; DROP TABLE t1; diff --git a/storage/innobase/include/dict0mem.h b/storage/innobase/include/dict0mem.h index 5cebfe6710d..5ec2273ef65 100644 --- a/storage/innobase/include/dict0mem.h +++ b/storage/innobase/include/dict0mem.h @@ -1203,6 +1203,14 @@ public: /** @return whether this is the change buffer */ bool is_ibuf() const { return UNIV_UNLIKELY(type & DICT_IBUF); } + /** @return whether this is a normal, non-virtual B-tree index + (not the change buffer, not SPATIAL or FULLTEXT) */ + bool is_normal_btree() const noexcept { + return UNIV_LIKELY(!(type & (DICT_IBUF | DICT_SPATIAL + | DICT_FTS | DICT_CORRUPT + | DICT_VIRTUAL))); + } + /** @return whether the index includes virtual columns */ bool has_virtual() const { return type & DICT_VIRTUAL; } diff --git a/storage/innobase/pars/pars0opt.cc b/storage/innobase/pars/pars0opt.cc index e1a913b0179..a5ab15b74d4 100644 --- a/storage/innobase/pars/pars0opt.cc +++ b/storage/innobase/pars/pars0opt.cc @@ -340,9 +340,7 @@ opt_calc_index_goodness( ulint op; ulint j; - /* At least for now we don't support using FTS indexes for queries - done through InnoDB's own SQL parser. */ - if (dict_index_is_online_ddl(index) || (index->type & DICT_FTS)) { + if (!index->is_normal_btree() || !index->is_committed()) { return(0); } @@ -572,11 +570,10 @@ opt_search_plan_for_table( /* Calculate goodness for each index of the table */ index = dict_table_get_first_index(table); - best_index = index; /* Eliminate compiler warning */ + best_index = index; best_goodness = 0; - /* should be do ... until ? comment by Jani */ - while (index) { + do { goodness = opt_calc_index_goodness(index, sel_node, i, index_plan, &last_op); if (goodness > best_goodness) { @@ -590,8 +587,8 @@ opt_search_plan_for_table( best_last_op = last_op; } - dict_table_next_uncorrupted_index(index); - } + index = dict_table_get_next_index(index); + } while (index); plan->index = best_index; From f2ffcd949b797dd04c6277e5a8939279dd0a107c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marko=20M=C3=A4kel=C3=A4?= Date: Thu, 19 Dec 2024 14:18:55 +0200 Subject: [PATCH 2/4] MDEV-35657: Add work-arounds for clang 11 --- extra/mariabackup/fil_cur.cc | 1 + extra/mariabackup/xtrabackup.cc | 1 + 2 files changed, 2 insertions(+) diff --git a/extra/mariabackup/fil_cur.cc b/extra/mariabackup/fil_cur.cc index 2383e3a03a7..4d0e99332a2 100644 --- a/extra/mariabackup/fil_cur.cc +++ b/extra/mariabackup/fil_cur.cc @@ -187,6 +187,7 @@ xb_fil_cur_open( } #else err = fstat(cursor->file.m_file, &cursor->statinfo); + MSAN_STAT_WORKAROUND(&cursor->statinfo); #endif if (max_file_size < (ulonglong)cursor->statinfo.st_size) { cursor->statinfo.st_size = (ulonglong)max_file_size; diff --git a/extra/mariabackup/xtrabackup.cc b/extra/mariabackup/xtrabackup.cc index d8015a0e331..a59e8f0858f 100644 --- a/extra/mariabackup/xtrabackup.cc +++ b/extra/mariabackup/xtrabackup.cc @@ -3648,6 +3648,7 @@ next_file: return(-1); } + MSAN_STAT_WORKAROUND(&statinfo); info->size = statinfo.st_size; if (S_ISDIR(statinfo.st_mode)) { From d878d80bc4a3ee6f044c0dd68601d86cd435332e Mon Sep 17 00:00:00 2001 From: Eric Herman Date: Thu, 19 Dec 2024 16:40:18 +0100 Subject: [PATCH 3/4] MDEV-35695: mtr failure suggests wrong url When running the ./mtr tests and getting failures, rather than provide a dead-link to mysql.com, this points developers to the Jira instance. Signed-off-by: Eric Herman --- mysql-test/lib/mtr_report.pm | 5 ++--- mysql-test/lib/v1/mtr_report.pl | 5 ++--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/mysql-test/lib/mtr_report.pm b/mysql-test/lib/mtr_report.pm index 308dbb77a3e..ff239d676a9 100644 --- a/mysql-test/lib/mtr_report.pm +++ b/mysql-test/lib/mtr_report.pm @@ -454,9 +454,8 @@ sub mtr_report_stats ($$$$) { # Print info about reporting the error print "The log files in var/log may give you some hint of what went wrong.\n\n", - "If you want to report this error, please read first ", - "the documentation\n", - "at http://dev.mysql.com/doc/mysql/en/mysql-test-suite.html\n\n"; + "If you want to report this error, MariaDB's bug tracker is found at\n", + "https://jira.mariadb.org\n\n"; } else diff --git a/mysql-test/lib/v1/mtr_report.pl b/mysql-test/lib/v1/mtr_report.pl index 8964b0f8077..6f8677110ba 100644 --- a/mysql-test/lib/v1/mtr_report.pl +++ b/mysql-test/lib/v1/mtr_report.pl @@ -198,9 +198,8 @@ sub mtr_report_stats ($) { print "The log files in var/log may give you some hint\n", "of what went wrong.\n", - "If you want to report this error, please read first ", - "the documentation at\n", - "http://dev.mysql.com/doc/mysql/en/mysql-test-suite.html\n"; + "If you want to report this error, MariaDB's bug tracker is found at\n", + "https://jira.mariadb.org\n" } if (!$::opt_extern) { From 24e5d564005904bc589fc869ea2f913eec8e6c3a Mon Sep 17 00:00:00 2001 From: Oleg Smirnov Date: Fri, 20 Dec 2024 14:58:33 +0700 Subject: [PATCH 4/4] MDEV-35680 Table number > MAX_TABLES causes overflow of table_map at main.join test Fix a regression introduced by commit d98ac851 (MDEV-29935, MDEV-26247) causing MAX_TABLES overflow in `setup_table_map()`. The check for MAX_TABLES was moved outside of the loop that increments table numbers, allowing overflows during loop iterations. Since setup_table_map() operates on a 64-bit bitmap, table numbers exceeding 64 triggered the UBSAN check. This commit returns the overflow check within the loop and adds a debug assertion to `setup_table_map()` to ensure no bitmap overrun occurs. --- sql/sql_base.cc | 14 +++++++++----- sql/sql_base.h | 1 + 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/sql/sql_base.cc b/sql/sql_base.cc index bcab54ac1ec..5c03ba3d42d 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -7862,11 +7862,15 @@ bool setup_tables(THD *thd, Name_resolution_context *context, DBUG_RETURN(1); } tablenr++; - } - if (tablenr > MAX_TABLES) - { - my_error(ER_TOO_MANY_TABLES,MYF(0), static_cast(MAX_TABLES)); - DBUG_RETURN(1); + /* + Test MAX_TABLES overflow here inside the loop as setup_table_map() + called in each iteration is sensitive for this + */ + if (tablenr > MAX_TABLES) + { + my_error(ER_TOO_MANY_TABLES, MYF(0), static_cast(MAX_TABLES)); + DBUG_RETURN(1); + } } if (select_insert && !is_insert_tables_num_set) { diff --git a/sql/sql_base.h b/sql/sql_base.h index 894c8213e66..90c47e69d94 100644 --- a/sql/sql_base.h +++ b/sql/sql_base.h @@ -353,6 +353,7 @@ inline void setup_table_map(TABLE *table, TABLE_LIST *table_list, uint tablenr) table->maybe_null= embedding->outer_join; embedding= embedding->embedding; } + DBUG_ASSERT(tablenr <= MAX_TABLES); table->tablenr= tablenr; table->map= (table_map) 1 << tablenr; table->force_index= table_list->force_index;