From f88511647aa2498a1ffa449f305ed2f0f73de14c Mon Sep 17 00:00:00 2001 From: Aleksey Midenkov Date: Wed, 29 Jun 2022 22:53:29 +0300 Subject: [PATCH] MDEV-28567 Assertion `0' in open_tables upon function-related operation DBUG_ASSERT(0) was added by MDEV-17554 (auto-create history partitions) as an experimental measure. Testing has shown this conditional branch of can_recover_from_failed_open() can be possible due to MDL deadlock. The fix replaces DBUG_ASSERT with more specific one for !OT_ADD_HISTORY_PARTITION. Test case was synchronized to reproduce deadlock always and commented with execution path of MDL locking for Good (no deadlock) and Bad (deadlock). The logging was done with the help of preceding patch for debug MDL tracing. --- mysql-test/main/view_debug.result | 38 ++++++++++ mysql-test/main/view_debug.test | 121 +++++++++++++++++++++++++++++- sql/sql_base.cc | 5 +- sql/sql_base.h | 5 ++ 4 files changed, 165 insertions(+), 4 deletions(-) diff --git a/mysql-test/main/view_debug.result b/mysql-test/main/view_debug.result index cfb70b5e74c..591beba5ef3 100644 --- a/mysql-test/main/view_debug.result +++ b/mysql-test/main/view_debug.result @@ -21,5 +21,43 @@ connection con1; 3 3 SET DEBUG_SYNC= 'RESET'; +disconnect con1; +disconnect con2; +connection default; drop procedure proc; drop view v1,v2; +# +# MDEV-28567 Assertion `0' in open_tables upon function-related operation +# +CREATE TABLE t1 (a INT); +CREATE TABLE t2 (b INT); +CREATE TRIGGER tr1 BEFORE INSERT ON t1 FOR EACH ROW UPDATE t2 SET b = 0; +CREATE TRIGGER tr2 BEFORE INSERT ON t2 FOR EACH ROW UPDATE t1 SET a = 6; +CREATE VIEW v1 AS SELECT * FROM t1; +SET AUTOCOMMIT=OFF; +SELECT * FROM t1; +a +connect con1,localhost,root,,test; +DROP TRIGGER tr1; +connection default; +INSERT INTO t2 SELECT * FROM t2; +SELECT f() FROM t2; +ERROR 42000: FUNCTION test.f does not exist +connect con2,localhost,root,,test; +set debug_sync= 'after_open_table_mdl_shared signal s1'; +ALTER VIEW v1 AS SELECT f() FROM t1; +connection con1; +CREATE FUNCTION f() RETURNS INT RETURN 1; +connection default; +set debug_sync= 'now wait_for s1'; +SELECT * FROM ( SELECT * FROM v1 ) sq; +a +COMMIT; +connection con2; +disconnect con1; +disconnect con2; +connection default; +DROP VIEW v1; +DROP FUNCTION f; +DROP TABLE t1, t2; +set debug_sync= 'reset'; diff --git a/mysql-test/main/view_debug.test b/mysql-test/main/view_debug.test index bf8074ddeb1..acbd9cc740f 100644 --- a/mysql-test/main/view_debug.test +++ b/mysql-test/main/view_debug.test @@ -31,7 +31,124 @@ SET DEBUG_SYNC= 'now SIGNAL created'; --connection con1 --reap SET DEBUG_SYNC= 'RESET'; - - +--disconnect con1 +--disconnect con2 +--connection default drop procedure proc; drop view v1,v2; + +--echo # +--echo # MDEV-28567 Assertion `0' in open_tables upon function-related operation +--echo # +# To get MDL trace run this case like this: +# mtr --mysqld=--debug=d,mdl,query:i:o,/tmp/mdl.log ... +# Cleanup trace like this: +# sed -i -re '/(mysql|performance_schema|sys|mtr)\// d; /MDL_BACKUP_|MDL_INTENTION_/ d; /\/(t2|tr1|tr2)/ d' /tmp/mdl.log + +CREATE TABLE t1 (a INT); +CREATE TABLE t2 (b INT); +CREATE TRIGGER tr1 BEFORE INSERT ON t1 FOR EACH ROW UPDATE t2 SET b = 0; +CREATE TRIGGER tr2 BEFORE INSERT ON t2 FOR EACH ROW UPDATE t1 SET a = 6; +CREATE VIEW v1 AS SELECT * FROM t1; + +SET AUTOCOMMIT=OFF; +SELECT * FROM t1; +# T@6 +# Seized: test/t1 (MDL_SHARED_READ) + +--connect (con1,localhost,root,,test) +--send + DROP TRIGGER tr1; +# T@7 +# Seized: test/t1 (MDL_SHARED_NO_WRITE) +# Waiting: test/t1 (MDL_EXCLUSIVE) +# Waiting: test/t1 (MDL_SHARED_WRITE) +# Deadlock: test/t1 (MDL_SHARED_WRITE) + +--connection default +--error 0, ER_LOCK_DEADLOCK +INSERT INTO t2 SELECT * FROM t2; +# T@6 +# Released: test/t1 (MDL_SHARED_READ) +# T@7 +# Acquired: test/t1 (MDL_EXCLUSIVE) (good) +--error ER_SP_DOES_NOT_EXIST +SELECT f() FROM t2; +# T@6 +# Seized: test/f (MDL_SHARED) +# T@7 +# Released: test/t1 (MDL_EXCLUSIVE) +# Good1: continue T@6 below +# Bad1: continue T@8 below + +# Now we hold test/f, the below code creates concurrent +# waiting of 3 threads for test/f which leads to deadlock (Bad) + +# To achive Good comment out 'now wait_for s1' below and run multiple times. + +--connect (con2,localhost,root,,test) +set debug_sync= 'after_open_table_mdl_shared signal s1'; +--send + ALTER VIEW v1 AS SELECT f() FROM t1; +# T@8 +# Good2: Waiting: test/v1 (MDL_EXCLUSIVE) +# Good2-3: continue T@7 below +# Good5: Acquired: test/v1 (MDL_EXCLUSIVE) +# Good5: Seized: test/v1 (MDL_EXCLUSIVE) +# Good5-6: continue T@7 below +# Good7: Seized: test/t1 (MDL_SHARED_READ) +# Good7: Waiting: test/f (MDL_SHARED) +# Good7-8: continue T@7 below +# Good9: Acquired: test/f (MDL_SHARED) +# Good9: Released: test/f (MDL_SHARED) +# Good9: Released: test/t1 (MDL_SHARED_READ) +# Good9: Released: test/v1 (MDL_EXCLUSIVE) +# Good9: command finished without error +# Bad1: Seized: test/v1 (MDL_EXCLUSIVE) +# Bad1: Seized: test/v1 (MDL_EXCLUSIVE) +# Bad1: Seized: test/t1 (MDL_SHARED_READ) +# Bad1-2: continue T@6 below +# Bad4: Waiting: test/f (MDL_SHARED) +# Bad4: Deadlock: test/f (MDL_SHARED) +# Bad4: command finished with error + +--connection con1 +--reap +--send +CREATE FUNCTION f() RETURNS INT RETURN 1; +# T@7 +# Good3: Waiting: test/f (MDL_EXCLUSIVE) +# Good3-4: continue T@6 below +# Good6: Acquired: test/f (MDL_EXCLUSIVE) +# Good6-7: continue T@8 above +# Good8: Released: test/f (MDL_EXCLUSIVE) +# Good8-9: continue T@8 above +# Bad3: Waiting: test/f (MDL_EXCLUSIVE) +# Bad3-4: continue T@8 above + +--connection default +set debug_sync= 'now wait_for s1'; +SELECT * FROM ( SELECT * FROM v1 ) sq; +# T@6 +# Good1: Seized: test/v1 (MDL_SHARED_READ) +# Good1-2: continue T@8 above +# Good4: Seized: test/t1 (MDL_SHARED_READ) +# Bad2: Waiting: test/v1 (MDL_SHARED_READ) +# Bad2-3: continue T@7 above + +# Cleanup +COMMIT; +# Good4: Released: test/t1 (MDL_SHARED_READ) +# Good4: Released: test/v1 (MDL_SHARED_READ) +# Good4: Released: test/f (MDL_SHARED) +# Good4-5: continue T@8 above + +--connection con2 +--reap +--disconnect con1 +--disconnect con2 +--connection default +DROP VIEW v1; +DROP FUNCTION f; +DROP TABLE t1, t2; +set debug_sync= 'reset'; diff --git a/sql/sql_base.cc b/sql/sql_base.cc index d9e04261dc8..ed73f1cb88c 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -4678,10 +4678,11 @@ restart: if (unlikely(error)) { + /* F.ex. deadlock happened */ if (ot_ctx.can_recover_from_failed_open()) { - // FIXME: is this really used? - DBUG_ASSERT(0); + DBUG_ASSERT(ot_ctx.get_action() != + Open_table_context::OT_ADD_HISTORY_PARTITION); close_tables_for_reopen(thd, start, ot_ctx.start_of_statement_svp()); if (ot_ctx.recover_from_failed_open()) diff --git a/sql/sql_base.h b/sql/sql_base.h index 06d75596827..c86a652c33a 100644 --- a/sql/sql_base.h +++ b/sql/sql_base.h @@ -561,6 +561,11 @@ public: return m_timeout; } + enum_open_table_action get_action() const + { + return m_action; + } + uint get_flags() const { return m_flags; } /**