Skip to content

Commit 3ef5939

Browse files
ThirunarayananThirunarayanan
committed
MDEV-39261 MariaDB crash on startup in presence of indexed virtual columns
Problem: ======== A single InnoDB purge worker thread can process undo logs from different tables within the same batch. But get_purge_table(), open_purge_table() incorrectly assumes that a 1:1 relationship between a purge worker thread and a table within a single batch. Based on this wrong assumtion, InnoDB attempts to reuse TABLE objects cached in thd->open_tables for virtual column computation. 1) Purge worker opens Table A and caches the TABLE pointer in thd->open_tables. 2) Same purge worker moves to Table B in the same batch, get_purge_table() retrieves the cached pointer for Table A instead of opening Table B. 3) Because innobase::open() is ignored for Table B, the virtual column template is never initialized. 4) virtual column computation for Table B aborts the server Solution: ======== The purge coordinator thread now opens both InnoDB dict_table_t* and MariaDB TABLE* handles during batch preparation in trx_purge_attach_undo_recs(). purge_node_t::tables now stores only std::pair<dict_table_t*, TABLE*>. When worker thread needs TABLE* for virtual column computation, it fetches the TABLE* from purge_node_t->tables. trx_purge_table_open(): Modified to open TABLE* using the coordinator's MDL ticket open_purge_table(): Accept and use the MDL ticket from coordinator thread purge_sys_t::coordinator_thd: To track the coordinator thread in purge subsystem purge_node_t::end(): To prevent premature table closure when the coordinator acts as a worker and skips innobase_reset_background_thd() purge_sys_t::close_and_reopen(): Properly handles retry logic by clearing all table state and reopening the table. Uses mdl_map for maintaining mdl_tickets for each table id trx_purge_close_tables(): Now accepts mdl_map parameter to release MDL tickets from the coordinator's map after closing tables trx_purge(): MDL tickets are now stored in a local mdl_map instead of purge_node_t. Closes the coordinator's TABLE* objects after all workers are completed Declared open_purge_table() and close_thread_tables() in trx0purge.cc
1 parent 8229217 commit 3ef5939

File tree

16 files changed

+276
-153
lines changed

16 files changed

+276
-153
lines changed

‎mysql-test/suite/gcol/r/innodb_virtual_basic.result‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -48,9 +48,9 @@ INSERT INTO t VALUES (1290, 212, DEFAULT, "xmx");
4848
ROLLBACK;
4949
SELECT c FROM t;
5050
c
51-
NULL
5251
13
5352
29
53+
NULL
5454
SELECT * FROM t;
5555
a b c h
5656
10 3 13 mm
@@ -303,23 +303,23 @@ END|
303303
CALL UPDATE_t();
304304
SELECT c FROM t;
305305
c
306-
NULL
307306
19
308-
29
309307
2103
308+
29
309+
NULL
310310
CALL DELETE_insert_t();
311311
SELECT c FROM t;
312312
c
313-
NULL
314313
19
315-
29
316314
2103
315+
29
316+
NULL
317317
DROP INDEX idx ON t;
318318
CALL UPDATE_t();
319319
SELECT c FROM t;
320320
c
321-
2103
322321
19
322+
2103
323323
29
324324
NULL
325325
DROP PROCEDURE DELETE_insert_t;
@@ -523,10 +523,10 @@ UPDATE t SET h = "e" WHERE h="a";
523523
ROLLBACK;
524524
SELECT a, c, h FROM t;
525525
a c h
526-
NULL NULL d
527526
11 14 a
528527
18 19 b
529528
28 29 c
529+
NULL NULL d
530530
DROP TABLE t;
531531
CREATE TABLE `t1` (
532532
`col1` int(11) NOT NULL,

‎mysql-test/suite/gcol/t/innodb_virtual_basic.test‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ INSERT INTO t VALUES (128, 22, DEFAULT, "xx");
4242
INSERT INTO t VALUES (1290, 212, DEFAULT, "xmx");
4343
ROLLBACK;
4444

45+
--sorted_result
4546
SELECT c FROM t;
4647
SELECT * FROM t;
4748

@@ -356,13 +357,16 @@ END|
356357
delimiter ;|
357358

358359
CALL UPDATE_t();
360+
--sorted_result
359361
SELECT c FROM t;
360362

361363
CALL DELETE_insert_t();
364+
--sorted_result
362365
SELECT c FROM t;
363366

364367
DROP INDEX idx ON t;
365368
CALL UPDATE_t();
369+
--sorted_result
366370
SELECT c FROM t;
367371

368372
DROP PROCEDURE DELETE_insert_t;
@@ -537,6 +541,7 @@ START TRANSACTION;
537541
UPDATE t SET m =10 WHERE m = 1;
538542
UPDATE t SET h = "e" WHERE h="a";
539543
ROLLBACK;
544+
--sorted_result
540545
SELECT a, c, h FROM t;
541546

542547
DROP TABLE t;

‎mysql-test/suite/vcol/r/races.result‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,3 +14,17 @@ disconnect con1;
1414
connection default;
1515
drop table t1;
1616
set debug_sync='reset';
17+
#
18+
# MDEV-39261 MariaDB crash on startup in presence of
19+
# indexed virtual columns
20+
#
21+
# Create 33 tables with virtual index
22+
InnoDB 0 transactions not purged
23+
connect purge_control,localhost,root;
24+
START TRANSACTION WITH CONSISTENT SNAPSHOT;
25+
connection default;
26+
# Do update on all 33 tables
27+
# restart: --innodb_purge_threads=1 --debug_dbug=d,ib_purge_virtual_index_callback
28+
InnoDB 0 transactions not purged
29+
# Drop all 33 tables
30+
# restart

‎mysql-test/suite/vcol/t/races.test‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,59 @@ disconnect con1;
2020
connection default;
2121
drop table t1;
2222
set debug_sync='reset';
23+
24+
--echo #
25+
--echo # MDEV-39261 MariaDB crash on startup in presence of
26+
--echo # indexed virtual columns
27+
--echo #
28+
# To make purge thread to work on multiple tables on the same batch,
29+
# we need 33 tables because there are 32 pre-existing purge_node exists.
30+
31+
--echo # Create 33 tables with virtual index
32+
--disable_query_log
33+
let $i = 33;
34+
while ($i)
35+
{
36+
eval CREATE TABLE t$i(
37+
a INT PRIMARY KEY,
38+
b INT DEFAULT 1, INDEX(b),
39+
c INT GENERATED ALWAYS AS (a + b) VIRTUAL,
40+
INDEX(c)
41+
) ENGINE=InnoDB;
42+
eval INSERT INTO t$i(a) VALUES(1);
43+
dec $i;
44+
}
45+
--enable_query_log
46+
--source ../../innodb/include/wait_all_purged.inc
47+
--connect purge_control,localhost,root
48+
START TRANSACTION WITH CONSISTENT SNAPSHOT;
49+
50+
--connection default
51+
--echo # Do update on all 33 tables
52+
--disable_query_log
53+
let $i = 33;
54+
while ($i)
55+
{
56+
eval UPDATE t$i SET b = 11 WHERE a = 1;
57+
dec $i;
58+
}
59+
--enable_query_log
60+
61+
let $shutdown_timeout=0;
62+
let $restart_parameters=--innodb_purge_threads=1 --debug_dbug=d,ib_purge_virtual_index_callback;
63+
--source include/restart_mysqld.inc
64+
--source ../../innodb/include/wait_all_purged.inc
65+
66+
--echo # Drop all 33 tables
67+
--disable_query_log
68+
let $i = 33;
69+
while ($i)
70+
{
71+
eval DROP TABLE t$i;
72+
dec $i;
73+
}
74+
--enable_query_log
75+
76+
let $restart_parameters=;
77+
let $shutdown_timeout=;
78+
--source include/restart_mysqld.inc

‎sql/sql_base.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -820,7 +820,7 @@ static inline bool check_field_pointers(const TABLE *table)
820820
leave prelocked mode if needed.
821821
*/
822822

823-
int close_thread_tables(THD *thd)
823+
int close_thread_tables(THD *thd) noexcept
824824
{
825825
TABLE *table;
826826
int error= 0;

‎sql/sql_base.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ TABLE_LIST *find_table_in_list(TABLE_LIST *table,
163163
TABLE_LIST *TABLE_LIST::*link,
164164
const LEX_CSTRING *db_name,
165165
const LEX_CSTRING *table_name);
166-
int close_thread_tables(THD *thd);
166+
int close_thread_tables(THD *thd) noexcept;
167167
void switch_to_nullable_trigger_fields(List<Item> &items, TABLE *);
168168
void switch_defaults_to_nullable_trigger_fields(TABLE *table);
169169
bool fill_record_n_invoke_before_triggers(THD *thd, TABLE *table,

‎sql/sql_class.cc‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5024,10 +5024,10 @@ extern "C" const char *thd_priv_user(MYSQL_THD thd, size_t *length)
50245024
have only one table open at any given time.
50255025
*/
50265026
TABLE *open_purge_table(THD *thd, const char *db, size_t dblen,
5027-
const char *tb, size_t tblen)
5027+
const char *tb, size_t tblen,
5028+
MDL_ticket *mdl_ticket) noexcept
50285029
{
50295030
DBUG_ENTER("open_purge_table");
5030-
DBUG_ASSERT(thd->open_tables == NULL);
50315031
DBUG_ASSERT(thd->locked_tables_mode < LTM_PRELOCKED);
50325032

50335033
/* Purge already hold the MDL for the table */
@@ -5038,6 +5038,7 @@ TABLE *open_purge_table(THD *thd, const char *db, size_t dblen,
50385038

50395039
tl->init_one_table(&db_name, &table_name, 0, TL_READ);
50405040
tl->i_s_requested_object= OPEN_TABLE_ONLY;
5041+
tl->mdl_request.ticket= mdl_ticket;
50415042

50425043
bool error= open_table(thd, tl, &ot_ctx);
50435044

@@ -5050,13 +5051,6 @@ TABLE *open_purge_table(THD *thd, const char *db, size_t dblen,
50505051
DBUG_RETURN(error ? NULL : tl->table);
50515052
}
50525053

5053-
TABLE *get_purge_table(THD *thd)
5054-
{
5055-
/* see above, at most one table can be opened */
5056-
DBUG_ASSERT(thd->open_tables == NULL || thd->open_tables->next == NULL);
5057-
return thd->open_tables;
5058-
}
5059-
50605054
/** Find an open table in the list of prelocked tabled
50615055
50625056
Used for foreign key actions, for example, in UPDATE t1 SET a=1;
@@ -5217,10 +5211,13 @@ void destroy_background_thd(MYSQL_THD thd)
52175211

52185212
void reset_thd(MYSQL_THD thd)
52195213
{
5214+
const char *proc_info= thd->proc_info;
5215+
thd->proc_info="reset";
52205216
close_thread_tables(thd);
52215217
thd->release_transactional_locks();
52225218
thd->free_items();
52235219
free_root(thd->mem_root, MYF(MY_KEEP_PREALLOC));
5220+
thd->proc_info= proc_info;
52245221
}
52255222

52265223
/**

‎storage/innobase/dict/dict0dict.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -626,13 +626,13 @@ bool dict_table_t::parse_name(char (&db_name)[NAME_LEN + 1],
626626
dict_sys.unfreeze();
627627

628628
*db_name_len= filename_to_tablename(db_buf, db_name,
629-
MAX_DATABASE_NAME_LEN + 1, true);
629+
NAME_LEN + 1, true);
630630

631631
if (is_temp)
632632
return false;
633633

634634
*tbl_name_len= filename_to_tablename(tbl_buf, tbl_name,
635-
MAX_TABLE_NAME_LEN + 1, true);
635+
NAME_LEN + 1, true);
636636
return true;
637637
}
638638

‎storage/innobase/handler/ha_innodb.cc‎

Lines changed: 8 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -124,10 +124,10 @@ TABLE *find_fk_open_table(THD *thd, const char *db, size_t db_len,
124124
const char *table, size_t table_len);
125125
MYSQL_THD create_background_thd();
126126
void reset_thd(MYSQL_THD thd);
127-
TABLE *get_purge_table(THD *thd);
128127
TABLE *open_purge_table(THD *thd, const char *db, size_t dblen,
129-
const char *tb, size_t tblen);
130-
void close_thread_tables(THD* thd);
128+
const char *tb, size_t tblen,
129+
MDL_ticket *mdl_ticket) noexcept;
130+
int close_thread_tables(THD* thd) noexcept;
131131

132132
#ifdef MYSQL_DYNAMIC_PLUGIN
133133
#define tc_size 400
@@ -1785,10 +1785,7 @@ innobase_reset_background_thd(MYSQL_THD thd)
17851785
ut_ad(thd);
17861786
ut_ad(THDVAR(thd, background_thread));
17871787

1788-
/* background purge thread */
1789-
const char *proc_info= thd_proc_info(thd, "reset");
17901788
reset_thd(thd);
1791-
thd_proc_info(thd, proc_info);
17921789
}
17931790

17941791

@@ -8536,7 +8533,7 @@ ATTRIBUTE_COLD bool wsrep_append_table_key(MYSQL_THD thd, const dict_table_t &ta
85368533
{
85378534
char db_buf[NAME_LEN + 1];
85388535
char tbl_buf[NAME_LEN + 1];
8539-
ulint db_buf_len, tbl_buf_len;
8536+
size_t db_buf_len, tbl_buf_len;
85408537

85418538
if (!table.parse_name(db_buf, tbl_buf, &db_buf_len, &tbl_buf_len))
85428539
{
@@ -20071,33 +20068,18 @@ ha_innobase::multi_range_read_explain_info(
2007120068
for purge thread */
2007220069
static TABLE* innodb_find_table_for_vc(THD* thd, dict_table_t* table)
2007320070
{
20074-
TABLE *mysql_table;
20075-
const bool bg_thread = THDVAR(thd, background_thread);
20076-
20077-
if (bg_thread) {
20078-
if ((mysql_table = get_purge_table(thd))) {
20079-
return mysql_table;
20080-
}
20081-
} else {
20082-
if (table->vc_templ->mysql_table_query_id
20083-
== thd_get_query_id(thd)) {
20084-
return table->vc_templ->mysql_table;
20085-
}
20071+
if (table->vc_templ->mysql_table_query_id == thd_get_query_id(thd)) {
20072+
return table->vc_templ->mysql_table;
2008620073
}
2008720074

20075+
TABLE *mysql_table;
2008820076
char db_buf[NAME_LEN + 1];
2008920077
char tbl_buf[NAME_LEN + 1];
20090-
ulint db_buf_len, tbl_buf_len;
20078+
size_t db_buf_len, tbl_buf_len;
2009120079

2009220080
if (!table->parse_name(db_buf, tbl_buf, &db_buf_len, &tbl_buf_len)) {
2009320081
return NULL;
2009420082
}
20095-
20096-
if (bg_thread) {
20097-
return open_purge_table(thd, db_buf, db_buf_len,
20098-
tbl_buf, tbl_buf_len);
20099-
}
20100-
2010120083
mysql_table = find_fk_open_table(thd, db_buf, db_buf_len,
2010220084
tbl_buf, tbl_buf_len);
2010320085
table->vc_templ->mysql_table = mysql_table;

‎storage/innobase/include/dict0mem.h‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2551,6 +2551,17 @@ struct dict_table_t {
25512551
static dict_table_t *create(const span<const char> &name, fil_space_t *space,
25522552
ulint n_cols, ulint n_v_cols, ulint flags,
25532553
ulint flags2);
2554+
2555+
/** @return whether the table has any indexed virtual column */
2556+
bool has_virtual_index() const noexcept
2557+
{
2558+
if (UNIV_UNLIKELY(n_v_cols != 0))
2559+
for (dict_index_t *index = indexes.start;
2560+
index; index = UT_LIST_GET_NEXT(indexes, index))
2561+
if (index->has_virtual())
2562+
return true;
2563+
return false;
2564+
}
25542565
};
25552566

25562567
inline void dict_index_t::set_modified(mtr_t& mtr) const

0 commit comments

Comments
 (0)
⚡