Details
-
Bug
-
Status: Stalled (View Workflow)
-
Major
-
Resolution: Unresolved
-
10.6
-
None
Description
With the following sync point added:
diff --git a/storage/innobase/row/row0ins.cc b/storage/innobase/row/row0ins.cc
|
--- a/storage/innobase/row/row0ins.cc (revision 05cd10b74cffe882d04c6dd3333ef0beec2b41c2)
|
+++ b/storage/innobase/row/row0ins.cc (date 1669546784733)
|
@@ -1944,6 +1944,7 @@
|
dict_table_t* referenced_table
|
= foreign->referenced_table;
|
|
+ DEBUG_SYNC_C("row_ins_check_fk_ref_table_loaded");
|
if (referenced_table == NULL) {
|
|
ref_table = dict_table_open_on_name( |
The following test fails to insert a row in a child table, however the opposite is expected:
--source include/have_innodb.inc
|
|
create table t1 (a int, key(a), b int) engine=innodb; |
create table t2 (a int key references t1(a), b int) engine=innodb; |
insert t1 values (1, 1); |
set autocommit =0; |
set global innodb_evict_tables_on_commit_debug=1; |
|
|
begin; |
insert t1 values (20000, 1); |
|
--connect bug, localhost, root
|
set debug_sync="row_ins_check_fk_ref_table_loaded signal open wait_for go"; |
send
|
insert t2 values(1,1); |
|
--connect purge, localhost, root
|
FLUSH TABLES;
|
|
--connection default
|
set debug_sync="now wait_for open"; |
commit; |
set debug_sync="now signal go"; |
--connection bug
|
reap;
|
--connection default
|
|
# Cleanup
|
set debug_sync= reset; |
drop table t2, t1; |
actual result, rev. 05cd10b74c |
mysqltest: At line 26: query 'reap' failed: ER_NO_REFERENCED_ROW_2 (1452):
|
Cannot add or update a child row: a foreign key constraint fails (`test`.`t2`, CONSTRAINT
|
`t2_ibfk_1` FOREIGN KEY (`a`) REFERENCES `t1` (`a`))
|
The effect may be much more severe with real eviction, but I didn't succeed to simulate it with just innodb_evict_tables_on_commit_debug
Attachments
Issue Links
- is caused by
-
MDEV-22180 Planner opens unnecessary tables when updated table is referenced by foreign keys
-
- Closed
-
- relates to
-
MDEV-29181 Potential corruption on Foreign key update on a table with vcol index
-
- Stalled
-
-
MDEV-22180 Planner opens unnecessary tables when updated table is referenced by foreign keys
-
- Closed
-
-
MDEV-26217 Failing assertion: list.count > 0 in ut_list_remove or Assertion `lock->trx == this' failed in dberr_t trx_t::drop_table
-
- Closed
-
-
MDEV-30021 FK actions are ignored after tc_purge and COMMIT
-
- Stalled
-
The test is constructed similarly to MDEV-30021, however the effect and the supposed fix are different.
The difference between real eviction and innodb_evict_tables_on_commit_debug is that the latter also acquires lock_sys in addition to dict_sys.
If lock_sys is not acquired, then any consequence is possible, like crash or data corruption.
This problem is similar to the one fixed in
MDEV-26217, where the after-commit debug eviction was also augmented with lock_sys acquisition, but dict_sys_t::evict_table_LRU was probably forgotten.There are a few ways of fixing it:
1. Add lock_sys acquisition to dict_sys_t::evict_table_LRU and adjust FK checks
this will follow
MDEV-26217which actually works around the missing related tables prelocking.dict_sys::delete() call should be protected with lock_sys, together with table locks check (lock_table_has_locks).
The good news is that it's enough to lock when n_ref_count is 0, and if the table has foreign/referential relations.
n_ref_count is already protected by dict_sys lock. There can be other cases when lock_sys is required, but I'm not aware of them.
Then, the referential constraint algorithm should be modified. Now lock_table (as of
MDEV-26217) checks referential table pointer insidedict_foreign_t under lock_sys and atomically adds a table lock. If a table was NULL though, it reports a deadlock, and this effect can also be seen.
We hardly can call a table eviction a deadlock. though.
row_ins_check_foreign_constraint locking should be modified as following:
1. Try calling lock_table. It'll acquire lock_sys, so either acquire the lock, or fk_table will be NULL
2. If fk_table is NULL, the error returned was DEADLOCK. Try to call dict_table_open_on_name.
3. If succeeds, we'll have n_ref_count > 0. Call lock_table again.
Then the race with table eviction will be eliminated
2. Make correct MDL protection.
What's worth to mention, the MDL prelocking itself is not suffitient in this regard. handler::open should also be
called to acquire table data dictionary (dict_table_t). This does not happen, for example, when referenced constraint
is read-only (RESTRICT/NO ACTION), so it's currently also vulnerable.
And this is how this bug differs from MDEV-30021, by the way: even the correct prelocking could break there, as FLUSH
just deleted the relations from datadict.
Opening a stub for read-only references was an optimization in scope of user-reported
MDEV-22180. The reported analysisdoesn't look correct, as only first table opening leads to file reads and parsing. I guess that only cold opens were counted there,
and the fix should be reverted.
Additionally, all referenced tables should be prelocked, in order to call ha_open and increase tables' reference counts.
I think it's true that only query-affected tables could be prelocked, but it shouldn't affect the performance in a long run,
except for the common benefit of eliminating a lock.
Anyway, event if it could be in question, an optimization could be done: don't load frm and don't parse it for referential tables, but call ha_open.