Skip to content

Commit 60d1668

Browse files
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(). These handles are stored in a new purge_table_ctx_t structure within each purge_node_t->tables map, keyed by table_id. When worker thread needs TABLE* for virtual column computation, it fetches the TABLE* from purge_node_t->tables. purge_table_ctx_t(): Structure to hold dict_table_t*, MDL_ticket* and TABLE* for each table during batch processing. TABLE* is being opened only when virtual index exists for the table. innobase_open_purge_table(), innobase_close_thread_table(): wrapper to make open_purge_table(), close_thread_tables() accessible from InnoDB purge subsystem 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(): Skip innobase_reset_background_thd() for the coordinator thread to prevent the premature table closure trx_purge(): Closes the coordinator's TABLE* objects after all workers are completed
1 parent 6ffb219 commit 60d1668

File tree

15 files changed

+291
-131
lines changed

15 files changed

+291
-131
lines changed

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: 3 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;

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: 21 additions & 23 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
@@ -8536,7 +8536,7 @@ ATTRIBUTE_COLD bool wsrep_append_table_key(MYSQL_THD thd, const dict_table_t &ta
85368536
{
85378537
char db_buf[NAME_LEN + 1];
85388538
char tbl_buf[NAME_LEN + 1];
8539-
ulint db_buf_len, tbl_buf_len;
8539+
size_t db_buf_len, tbl_buf_len;
85408540

85418541
if (!table.parse_name(db_buf, tbl_buf, &db_buf_len, &tbl_buf_len))
85428542
{
@@ -20071,33 +20071,18 @@ ha_innobase::multi_range_read_explain_info(
2007120071
for purge thread */
2007220072
static TABLE* innodb_find_table_for_vc(THD* thd, dict_table_t* table)
2007320073
{
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-
}
20074+
if (table->vc_templ->mysql_table_query_id == thd_get_query_id(thd)) {
20075+
return table->vc_templ->mysql_table;
2008620076
}
2008720077

20078+
TABLE *mysql_table;
2008820079
char db_buf[NAME_LEN + 1];
2008920080
char tbl_buf[NAME_LEN + 1];
20090-
ulint db_buf_len, tbl_buf_len;
20081+
size_t db_buf_len, tbl_buf_len;
2009120082

2009220083
if (!table->parse_name(db_buf, tbl_buf, &db_buf_len, &tbl_buf_len)) {
2009320084
return NULL;
2009420085
}
20095-
20096-
if (bg_thread) {
20097-
return open_purge_table(thd, db_buf, db_buf_len,
20098-
tbl_buf, tbl_buf_len);
20099-
}
20100-
2010120086
mysql_table = find_fk_open_table(thd, db_buf, db_buf_len,
2010220087
tbl_buf, tbl_buf_len);
2010320088
table->vc_templ->mysql_table = mysql_table;
@@ -21306,3 +21291,16 @@ void alter_stats_rebuild(dict_table_t *table, THD *thd)
2130621291
" table rebuild: %s", ut_strerr(ret));
2130721292
DBUG_VOID_RETURN;
2130821293
}
21294+
21295+
TABLE* innobase_open_purge_table(THD *thd, const char *db_buf,
21296+
size_t db_len, const char *tbl_buf,
21297+
size_t tbl_len,
21298+
MDL_ticket *mdl_ticket) noexcept
21299+
{
21300+
return open_purge_table(thd, db_buf, db_len, tbl_buf, tbl_len, mdl_ticket);
21301+
}
21302+
21303+
int innobase_close_thread_tables(THD *thd) noexcept
21304+
{
21305+
return close_thread_tables(thd);
21306+
}

storage/innobase/include/dict0mem.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2551,6 +2551,18 @@ 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
2557+
{
2558+
for (dict_index_t *index = indexes.start;
2559+
index; index = UT_LIST_GET_NEXT(indexes, index))
2560+
{
2561+
if (index->has_virtual())
2562+
return true;
2563+
}
2564+
return false;
2565+
}
25542566
};
25552567

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

storage/innobase/include/row0mysql.h

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ Created 9/17/2000 Heikki Tuuri
4040
struct row_prebuilt_t;
4141
class ha_innobase;
4242
class ha_handler_stats;
43+
class MDL_ticket;
4344

4445
/*******************************************************************//**
4546
Frees the blob heap in prebuilt when no longer needed. */
@@ -830,6 +831,26 @@ void
830831
innobase_rename_vc_templ(
831832
dict_table_t* table);
832833

834+
/** Open a table for purge operations with MariaDB TABLE handle.
835+
This is a wrapper to make open_purge_table() accessible from InnoDB
836+
purge subsystem.
837+
@param thd thread handle
838+
@param db_buf database name buffer
839+
@param db_len database name length
840+
@param tbl_buf table name buffer
841+
@param tbl_len table name length
842+
@return MariaDB TABLE handle, or NULL if not opened */
843+
TABLE* innobase_open_purge_table(THD *thd, const char *db_buf,
844+
size_t db_len, const char *tbl_buf,
845+
size_t tbl_len,
846+
MDL_ticket *mdl_ticket) noexcept;
847+
848+
/** Close all tables opened by the thread.
849+
This is a wrapper to make close_thread_tables() accessible from
850+
InnoDB purge subsytem.
851+
@param thd thread handle */
852+
int innobase_close_thread_tables(THD *thd) noexcept;
853+
833854
#define ROW_PREBUILT_FETCH_MAGIC_N 465765687
834855

835856
#define ROW_MYSQL_WHOLE_ROW 0

storage/innobase/include/row0purge.h

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,29 @@ row_purge_step(
6767
que_thr_t* thr) /*!< in: query thread */
6868
MY_ATTRIBUTE((nonnull, warn_unused_result));
6969

70+
/** Table context for purge operations. Holds InnoDB table handle,
71+
MDL ticket, and MariaDB TABLE* for a table_id with indexed virtual
72+
columns. This information is fetched by the purge
73+
coordinator thread during batch preparation. Purge worker threads
74+
can retrieve the TABLE* when needed for virtual column computation. */
75+
struct purge_table_ctx_t
76+
{
77+
/** InnoDB table handle */
78+
dict_table_t *table;
79+
80+
/** MDL ticket for the table */
81+
MDL_ticket *mdl_ticket;
82+
83+
/** MariaDB TABLE handle (for virtual column computation) */
84+
TABLE *maria_table;
85+
86+
purge_table_ctx_t()
87+
: table(nullptr), mdl_ticket(nullptr), maria_table(nullptr) {}
88+
89+
purge_table_ctx_t(dict_table_t *t, MDL_ticket *m, TABLE *mt)
90+
: table(t), mdl_ticket(m), maria_table(mt) {}
91+
};
92+
7093
/** Purge worker context */
7194
struct purge_node_t
7295
{
@@ -111,8 +134,9 @@ struct purge_node_t
111134
/** Undo recs to purge */
112135
std::queue<trx_purge_rec_t> undo_recs;
113136

114-
/** map of table identifiers to table handles and meta-data locks */
115-
std::unordered_map<table_id_t, std::pair<dict_table_t*,MDL_ticket*>> tables;
137+
/** map of table identifiers to purge table context which has
138+
set by purge co-ordinator thread */
139+
std::unordered_map<table_id_t, purge_table_ctx_t> tables;
116140

117141
/** Constructor */
118142
explicit purge_node_t(que_thr_t *parent) :

0 commit comments

Comments
 (0)