Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

10.6 mdev 34877 #3723

Open
wants to merge 2 commits into
base: 10.6
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
129 changes: 129 additions & 0 deletions mysql-test/suite/innodb/r/avoid_deadlock_with_blocked.result
Original file line number Diff line number Diff line change
@@ -0,0 +1,129 @@
connect stop_purge,localhost,root;
START TRANSACTION WITH CONSISTENT SNAPSHOT;
connect con1,localhost,root,,;
connect con2,localhost,root,,;
connect con3,localhost,root,,;
connection default;
CREATE TABLE t1 (id INT PRIMARY KEY) ENGINE=InnoDB STATS_PERSISTENT=0;
INSERT INTO t1 (id) VALUES (1);
connection con1;
BEGIN;
SELECT * FROM t1 LOCK IN SHARE MODE;
id
1
connection con2;
BEGIN;
SET DEBUG_SYNC = 'lock_wait_before_suspend SIGNAL con2_will_wait';
SELECT * FROM t1 FOR UPDATE;
connection con1;
SET DEBUG_SYNC = 'now WAIT_FOR con2_will_wait';
SELECT * FROM t1 FOR UPDATE;
id
1
COMMIT;
connection con2;
id
1
COMMIT;
connection con1;
BEGIN;
SELECT * FROM t1 WHERE id=1 FOR UPDATE;
id
1
connection con2;
BEGIN;
SET DEBUG_SYNC = 'lock_wait_start SIGNAL con2_will_wait';
SELECT * FROM t1 LOCK IN SHARE MODE;
connection con1;
SET DEBUG_SYNC = 'now WAIT_FOR con2_will_wait';
INSERT INTO t1 VALUES (0);
ROLLBACK;
connection con2;
ERROR 40001: Deadlock found when trying to get lock; try restarting transaction
COMMIT;
connection con1;
BEGIN;
SELECT * FROM t1 LOCK IN SHARE MODE;
id
1
connection con2;
BEGIN;
SELECT * FROM t1 WHERE id=1 LOCK IN SHARE MODE;
id
1
connection default;
connection con3;
SET DEBUG_SYNC = 'lock_wait_before_suspend SIGNAL con3_will_wait';
SELECT * FROM t1 FOR UPDATE;
connection con1;
SET DEBUG_SYNC = 'now WAIT_FOR con3_will_wait';
SET DEBUG_SYNC = 'lock_wait_start SIGNAL con1_will_wait';
INSERT INTO t1 VALUES (0);
connection con2;
SET DEBUG_SYNC = 'now WAIT_FOR con1_will_wait';
COMMIT;
connection con1;
ROLLBACK;
connection con3;
ERROR 40001: Deadlock found when trying to get lock; try restarting transaction
connection con1;
BEGIN;
SELECT * FROM t1 LOCK IN SHARE MODE;
id
1
connection con2;
BEGIN;
SELECT * FROM t1 WHERE id=1 LOCK IN SHARE MODE;
id
1
connection default;
connection con3;
SET DEBUG_SYNC = 'lock_wait_before_suspend SIGNAL con3_will_wait';
SELECT * FROM t1 FOR UPDATE;
connection con1;
SET DEBUG_SYNC = 'now WAIT_FOR con3_will_wait';
SET DEBUG_SYNC = 'lock_wait_before_suspend SIGNAL con1_will_wait';
SELECT * FROM t1 WHERE id=1 FOR UPDATE;
connection con2;
SET DEBUG_SYNC = 'now WAIT_FOR con1_will_wait';
COMMIT;
connection con1;
id
1
COMMIT;
connection con3;
id
1
COMMIT;
connection con1;
BEGIN;
SELECT * FROM t1 LOCK IN SHARE MODE;
id
1
connection con2;
SET DEBUG_SYNC = 'lock_wait_before_suspend SIGNAL con2_will_wait';
SELECT * FROM t1 FOR UPDATE;
connection con3;
SET DEBUG_SYNC = 'now WAIT_FOR con2_will_wait';
SET DEBUG_SYNC = 'lock_wait_before_suspend SIGNAL con3_will_wait';
SELECT * FROM t1 FOR UPDATE;
connection con1;
SET DEBUG_SYNC = 'now WAIT_FOR con3_will_wait';
SELECT * FROM t1 WHERE id=1 FOR UPDATE;
id
1
COMMIT;
connection con2;
id
1
COMMIT;
connection con3;
id
1
COMMIT;
connection default;
disconnect con1;
disconnect con2;
disconnect con3;
disconnect stop_purge;
DROP TABLE t1;
194 changes: 194 additions & 0 deletions mysql-test/suite/innodb/t/avoid_deadlock_with_blocked.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,194 @@
--source include/have_innodb.inc
--source include/have_debug_sync.inc
--source include/count_sessions.inc

--disable_query_log
call mtr.add_suppression("InnoDB: Transaction was aborted due to ");
--enable_query_log

connect stop_purge,localhost,root;
START TRANSACTION WITH CONSISTENT SNAPSHOT;

--connect (con1,localhost,root,,)
--connect (con2,localhost,root,,)
--connect (con3,localhost,root,,)

--connection default
CREATE TABLE t1 (id INT PRIMARY KEY) ENGINE=InnoDB STATS_PERSISTENT=0;
INSERT INTO t1 (id) VALUES (1);
# Simplest scenario:
# <con1, S, granted>,
# <con1, S, granted>, <con2, X, waiting for con1>,
# Before MDEV-34877:
# <con1, S, granted>, <con2, X, waiting for con1>, <con1, X, waiting for con1>
# After MDEV-34877:
# <con1, S, granted>, <con1, X, granted>, <con2, X, waiting for con1>
# Expected: instead of deadlocking, the con1's request should ingore con2's

--connection con1
BEGIN;
SELECT * FROM t1 LOCK IN SHARE MODE;

--connection con2
BEGIN;
SET DEBUG_SYNC = 'lock_wait_before_suspend SIGNAL con2_will_wait';
--send SELECT * FROM t1 FOR UPDATE

--connection con1
SET DEBUG_SYNC = 'now WAIT_FOR con2_will_wait';
SELECT * FROM t1 FOR UPDATE;
COMMIT;

--connection con2
--reap
COMMIT;

# A variant of the above scenario:
# <con1, X REC_NOT_GAP, granted>,
# <con1, X REC_NOT_GAP, granted>, <con2, S, waiting for con1>,
# <con1, X REC_NOT_GAP, granted>, <con2, S, waiting for con1>, <con1, INSERT INTENTION, waiting for con1>
# Expected: a deadlock, as INSERT INTENTION should not overtake locks on gap, to not slice them
Comment on lines +46 to +50
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice explanation. Can you please also echo some comment line(s) to result files ? It helps mapping the result file lines to the test case in case of a failure. Something like adding --echo before a comment line.

--echo # Test-2: A variant of the first scenario:

--connection con1
BEGIN;
SELECT * FROM t1 WHERE id=1 FOR UPDATE;

--connection con2
BEGIN;
SET DEBUG_SYNC = 'lock_wait_start SIGNAL con2_will_wait';
--send SELECT * FROM t1 LOCK IN SHARE MODE

--connection con1
SET DEBUG_SYNC = 'now WAIT_FOR con2_will_wait';
INSERT INTO t1 VALUES (0);
ROLLBACK;

--connection con2
--error ER_LOCK_DEADLOCK
--reap
COMMIT;

# More complicated scenario:
# <con1, S, granted>,
# <con1, S, granted>, <con2, S REC_NOT_GAP, granted>,
# <con1, S, granted>, <con2, S REC_NOT_GAP, granted>, <con3, X, waiting for con2>
# <con1, S, granted>, <con2, S REC_NOT_GAP, granted>, <con3, X, waiting for con1>, <con1, INSERT_INTENTION, waiting for con3>
# <con1, S, granted>, <con3, X, waiting for con1>, <con1, INSERT_INTENTION, waiting for con3>
# Expected: a deadlock, as INSERT INTENTION should not overtake locks on gap, to not slice them

--connection con1
BEGIN;
SELECT * FROM t1 LOCK IN SHARE MODE;

--connection con2
BEGIN;
SELECT * FROM t1 WHERE id=1 LOCK IN SHARE MODE;

--connection default

--connection con3
SET DEBUG_SYNC = 'lock_wait_before_suspend SIGNAL con3_will_wait';
--send SELECT * FROM t1 FOR UPDATE

--connection con1
SET DEBUG_SYNC = 'now WAIT_FOR con3_will_wait';
SET DEBUG_SYNC = 'lock_wait_start SIGNAL con1_will_wait';
--send INSERT INTO t1 VALUES (0)

--connection con2
SET DEBUG_SYNC = 'now WAIT_FOR con1_will_wait';
COMMIT;

--connection con1
--reap
ROLLBACK;


--connection con3
--error ER_LOCK_DEADLOCK
--reap
Comment on lines +88 to +108
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very nice test ensuring II (INSERT INTENTION) lock doesn't try to bypass other locks and deadlock is expected here.

One thing about deadlock detection is we never want to guarantee the deadlock victim that would be chosen in case of a deadlock. What do you think ? Is it fine to assume con3 as deadlock victim here or should we try to handle the case if con1 is chosen as victim ?


# More complicated scenario.
# <con1, S, granted>,
# <con1, S, granted>, <con2, S REC_NOT_GAP, granted>,
# <con1, S, granted>, <con2, S REC_NOT_GAP, granted>, <con3, X, waiting for con1>
# <con1, S, granted>, <con2, S REC_NOT_GAP, granted>, <con3, X, waiting for con1>, <con1, X REC_NOT_GAP, waiting for con2>
# Before MDEV-34877:
# <con1, S, granted>, <con3, X, waiting for con1>, <con1, X REC_NOT_GAP, waiting for con3>
# After MDEV-34877:
# <con1, S, granted>, <con1, X REC_NOT_GAP, granted>, <con3, X, waiting for con1>


--connection con1
BEGIN;
SELECT * FROM t1 LOCK IN SHARE MODE;

--connection con2
BEGIN;
SELECT * FROM t1 WHERE id=1 LOCK IN SHARE MODE;

--connection default

--connection con3
SET DEBUG_SYNC = 'lock_wait_before_suspend SIGNAL con3_will_wait';
--send SELECT * FROM t1 FOR UPDATE

--connection con1
SET DEBUG_SYNC = 'now WAIT_FOR con3_will_wait';
SET DEBUG_SYNC = 'lock_wait_before_suspend SIGNAL con1_will_wait';
--send SELECT * FROM t1 WHERE id=1 FOR UPDATE

--connection con2
SET DEBUG_SYNC = 'now WAIT_FOR con1_will_wait';
COMMIT;

--connection con1
--reap
COMMIT;

--connection con3
--reap
COMMIT;

# A secenario, where con1 has to bypass two transactions:
# <con1, S, granted>
# <con1, S, granted> <con2, X, waiting>
# <con1, S, granted> <con2, X, waiting> <con3, X, waiting>
# Before MDEV-34877:
# <con1, S, granted> <con2, X, waiting> <con3, X, waiting> <con1, X REC_NOT_GAP, waiting for con2>
# After MDEV-34877:
# <con1, S, granted> <con1, X REC_NOT_GAP, granted> <con2, X, waiting> <con3, X, waiting>
Comment on lines +152 to +159
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After reviewing the code, I checked another similar scenario.
<con1, S, granted> <con2, X, waiting> <con3, S, waiting><con1, X waiting>

con1

BEGIN;
SELECT * FROM t1 LOCK IN SHARE MODE;

con2

BEGIN;
SELECT * FROM t1 FOR UPDATE;  // Waits

con3

BEGIN;
SELECT * FROM t1 LOCK IN SHARE MODE;  // Waits

con1

SELECT * FROM t1 FOR UPDATE;

con3

ERROR 1213 (40001): Deadlock found when trying to get lock; try restarting transaction 

Now, don't we want to avoid deadlock in such cases and allow con1 to directly acquire the X lock ? If we need to allow this we have to relax lock_t::can_be_bypassed() to bypass an S lock and also to not care if the bypassed lock waiting transaction is current transaction or not.

Even if we don't want to attempt to address this deadlock, I think it makes sense to add this testcase to capture the behaviour for such cases.

--connection con1
BEGIN;
SELECT * FROM t1 LOCK IN SHARE MODE;

--connection con2
SET DEBUG_SYNC = 'lock_wait_before_suspend SIGNAL con2_will_wait';
--send SELECT * FROM t1 FOR UPDATE

--connection con3
SET DEBUG_SYNC = 'now WAIT_FOR con2_will_wait';
SET DEBUG_SYNC = 'lock_wait_before_suspend SIGNAL con3_will_wait';
--send SELECT * FROM t1 FOR UPDATE

--connection con1
SET DEBUG_SYNC = 'now WAIT_FOR con3_will_wait';
SELECT * FROM t1 WHERE id=1 FOR UPDATE;
COMMIT;

--connection con2
--reap
COMMIT;

--connection con3
--reap
COMMIT;

--connection default
--disconnect con1
--disconnect con2
--disconnect con3
--disconnect stop_purge

DROP TABLE t1;

--source include/wait_until_count_sessions.inc
32 changes: 32 additions & 0 deletions storage/innobase/include/hash0hash.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,38 @@ struct hash_cell_t
{
remove(search(next, [&element](const T *p){return p==&element;}), next);
}

/** Delete an element.
@tparam T type of the element
@param remove the being-removed element
@param next the next-element pointer in T */
template<typename T>
void remove(const T &remove, T *T::*next)
{
T *prev;
for (prev= static_cast<T *>(node); prev && prev->*next != &remove;
prev= prev->*next);
ut_a(prev);
prev->*next= remove.*next;
}
vlad-lesin marked this conversation as resolved.
Show resolved Hide resolved

/** Insert an element after another.
@tparam T type of the element
@param after the element after which to insert
@param insert the being-inserted element
@param next the next-element pointer in T */
template <typename T> void insert_after(T &after, T &insert, T *T::*next)
{
#ifdef UNIV_DEBUG
for (const T *c= static_cast<const T *>(node); c; c= c->*next)
if (c == &after)
goto found;
ut_error;
found:
#endif
insert.*next= after.*next;
after.*next= &insert;
}
};

/** Hash table with singly-linked overflow lists */
Expand Down
Loading