-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
MDEV-10962 Deadlock with 3 concurrent DELETEs by unique key
PROBLEM:
A deadlock was possible when a transaction tried to "upgrade" an already
held Record Lock to Next Key Lock.
SOLUTION:
This patch is based on observations that:
(1) a Next Key Lock is equivalent to Record Lock combined with Gap Lock
(2) a GAP Lock never has to wait for any other lock
In case we request a Next Key Lock, we check if we already own a Record
Lock of equal or stronger mode, and if so, then we change the requested
lock type to GAP Lock, which we either already have, or can be granted
immediately, as GAP locks don't conflict with any other lock types.
(We don't consider Insert Intention Locks a Gap Lock in above statements).
The reason of why we don't upgrage Record Lock to Next Key Lock is the
following.
Imagine a transaction which does something like this:
for each row {
request lock in LOCK_X|LOCK_REC_NOT_GAP mode
request lock in LOCK_S mode
}
If we upgraded lock from Record Lock to Next Key lock, there would be
created only two lock_t structs for each page, one for
LOCK_X|LOCK_REC_NOT_GAP mode and one for LOCK_S mode, and then used
their bitmaps to mark all records from the same page.
The situation would look like this:
request lock in LOCK_X|LOCK_REC_NOT_GAP mode on row 1:
// -> creates new lock_t for LOCK_X|LOCK_REC_NOT_GAP mode and sets bit for
// 1
request lock in LOCK_S mode on row 1:
// -> notices that we already have LOCK_X|LOCK_REC_NOT_GAP on the row 1,
// so it upgrades it to X
request lock in LOCK_X|LOCK_REC_NOT_GAP mode on row 2:
// -> creates a new lock_t for LOCK_X|LOCK_REC_NOT_GAP mode (because we
// don't have any after we've upgraded!) and sets bit for 2
request lock in LOCK_S mode on row 2:
// -> notices that we already have LOCK_X|LOCK_REC_NOT_GAP on the row 2,
// so it upgrades it to X
...etc...etc..
Each iteration of the loop creates a new lock_t struct, and in the end we
have a lot (one for each record!) of LOCK_X locks, each with single bit
set in the bitmap. Soon we run out of space for lock_t structs.
If we create LOCK_GAP instead of lock upgrading, the above scenario works
like the following:
// -> creates new lock_t for LOCK_X|LOCK_REC_NOT_GAP mode and sets bit for
// 1
request lock in LOCK_S mode on row 1:
// -> notices that we already have LOCK_X|LOCK_REC_NOT_GAP on the row 1,
// so it creates LOCK_S|LOCK_GAP only and sets bit for 1
request lock in LOCK_X|LOCK_REC_NOT_GAP mode on row 2:
// -> reuses the lock_t for LOCK_X|LOCK_REC_NOT_GAP by setting bit for 2
request lock in LOCK_S mode on row 2:
// -> notices that we already have LOCK_X|LOCK_REC_NOT_GAP on the row 2,
// so it reuses LOCK_S|LOCK_GAP setting bit for 2
In the end we have just two locks per page, one for each mode:
LOCK_X|LOCK_REC_NOT_GAP and LOCK_S|LOCK_GAP.
Another benefit of this solution is that it avoids not-entirely
const-correct, (and otherwise looking risky) "upgrading".
The fix was ported from
mysql/mysql-server@bfba840
mysql/mysql-server@75cefdb
Reviewed by: Marko Mäkelä- Loading branch information
1 parent
19cdddf
commit 1bfd3cc
Showing
4 changed files
with
318 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| # | ||
| # Bug #23755664 DEADLOCK WITH 3 CONCURRENT DELETES BY UNIQUE KEY | ||
| # | ||
| connection default; | ||
| CREATE TABLE `t`( | ||
| `id` INT, | ||
| `a` INT DEFAULT NULL, | ||
| PRIMARY KEY(`id`), | ||
| UNIQUE KEY `u`(`a`) | ||
| ) ENGINE=InnoDB; | ||
| INSERT INTO t (`id`,`a`) VALUES | ||
| (1,1), | ||
| (2,9999), | ||
| (3,10000); | ||
| connect deleter,localhost,root,,; | ||
| connect holder,localhost,root,,; | ||
| connect waiter,localhost,root,,; | ||
| connection deleter; | ||
| SET DEBUG_SYNC = | ||
| 'lock_sec_rec_read_check_and_lock_has_locked | ||
| SIGNAL deleter_has_locked | ||
| WAIT_FOR waiter_has_locked'; | ||
| DELETE FROM t WHERE a = 9999; | ||
| connection holder; | ||
| SET DEBUG_SYNC= | ||
| 'now WAIT_FOR deleter_has_locked'; | ||
| SET DEBUG_SYNC= | ||
| 'lock_sec_rec_read_check_and_lock_has_locked SIGNAL holder_has_locked'; | ||
| DELETE FROM t WHERE a = 9999; | ||
| connection waiter; | ||
| SET DEBUG_SYNC= | ||
| 'now WAIT_FOR holder_has_locked'; | ||
| SET DEBUG_SYNC= | ||
| 'lock_sec_rec_read_check_and_lock_has_locked SIGNAL waiter_has_locked'; | ||
| DELETE FROM t WHERE a = 9999; | ||
| connection deleter; | ||
| connection holder; | ||
| connection waiter; | ||
| connection default; | ||
| disconnect deleter; | ||
| disconnect holder; | ||
| disconnect waiter; | ||
| DROP TABLE `t`; | ||
| SET DEBUG_SYNC='reset'; | ||
| CREATE TABLE `t`( | ||
| `id` INT NOT NULL PRIMARY KEY | ||
| ) ENGINE=InnoDB; | ||
| INSERT INTO t (`id`) VALUES (1), (2); | ||
| connect holder,localhost,root,,; | ||
| connect waiter,localhost,root,,; | ||
| connection holder; | ||
| BEGIN; | ||
| SELECT id FROM t WHERE id=1 FOR UPDATE; | ||
| id | ||
| 1 | ||
| SELECT id FROM t WHERE id=2 FOR UPDATE; | ||
| id | ||
| 2 | ||
| connection waiter; | ||
| SET DEBUG_SYNC= | ||
| 'lock_wait_suspend_thread_enter SIGNAL waiter_will_wait'; | ||
| SELECT id FROM t WHERE id = 1 FOR UPDATE; | ||
| connection holder; | ||
| SET DEBUG_SYNC= | ||
| 'now WAIT_FOR waiter_will_wait'; | ||
| SELECT * FROM t FOR UPDATE; | ||
| id | ||
| 1 | ||
| 2 | ||
| COMMIT; | ||
| connection waiter; | ||
| id | ||
| 1 | ||
| connection default; | ||
| disconnect holder; | ||
| disconnect waiter; | ||
| DROP TABLE `t`; | ||
| SET DEBUG_SYNC='reset'; |
143 changes: 143 additions & 0 deletions
143
mysql-test/suite/innodb/t/deadlock_on_lock_upgrade.test
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,143 @@ | ||
| --echo # | ||
| --echo # Bug #23755664 DEADLOCK WITH 3 CONCURRENT DELETES BY UNIQUE KEY | ||
| --echo # | ||
|
|
||
| --source include/have_innodb.inc | ||
| --source include/have_debug.inc | ||
| --source include/have_debug_sync.inc | ||
| --source include/count_sessions.inc | ||
|
|
||
| --connection default | ||
| # There are various scenarious in which a transaction already holds "half" | ||
| # of a record lock (for example, a lock on the record but not on the gap) | ||
| # and wishes to "upgrade it" to a full lock (i.e. on both gap and record). | ||
| # This is often a cause for a deadlock, if there is another transaction | ||
| # which is already waiting for the lock being blocked by us: | ||
| # 1. our granted lock for one half | ||
| # 2. her waiting lock for the same half | ||
| # 3. our waiting lock for the whole | ||
|
|
||
| # | ||
| # SCENARIO 1 | ||
| # | ||
| # In this scenario, three different threads try to delete the same row, | ||
| # identified by a secondary index key. | ||
| # This kind of operation (besides LOCK_IX on a table) requires | ||
| # an LOCK_REC_NOT_GAP|LOCK_REC|LOCK_X lock on a secondary index | ||
| # 1. `deleter` is the first to get the required lock | ||
| # 2. `holder` enqueues a waiting lock | ||
| # 3. `waiter` enqueues right after `holder` | ||
| # 4. `deleter` commits, releasing the lock, and granting it to `holder` | ||
| # 5. `holder` now observes that the row was deleted, so it needs to | ||
| # "seal the gap", by obtaining a LOCK_X|LOCK_REC, but.. | ||
| # 6. this causes a deadlock between `holder` and `waiter` | ||
| # | ||
| # This scenario does not fail if MDEV-10962 is not fixed because of MDEV-30225 | ||
| # fix, as the 'holder' does not "seal the gap" after 'deleter' was committed, | ||
| # because it was initially sealed, as row_search_mvcc() requests next-key lock | ||
| # after MDEV-30225 fix in the case when it requested not-gap lock before the | ||
| # fix. | ||
| # | ||
| # But let the scenario be in the tests, because it can fail if MDEV-30225 | ||
| # related code is changed | ||
|
|
||
| CREATE TABLE `t`( | ||
| `id` INT, | ||
| `a` INT DEFAULT NULL, | ||
| PRIMARY KEY(`id`), | ||
| UNIQUE KEY `u`(`a`) | ||
| ) ENGINE=InnoDB; | ||
|
|
||
| INSERT INTO t (`id`,`a`) VALUES | ||
| (1,1), | ||
| (2,9999), | ||
| (3,10000); | ||
|
|
||
| --connect(deleter,localhost,root,,) | ||
| --connect(holder,localhost,root,,) | ||
| --connect(waiter,localhost,root,,) | ||
|
|
||
|
|
||
| --connection deleter | ||
| SET DEBUG_SYNC = | ||
| 'lock_sec_rec_read_check_and_lock_has_locked | ||
| SIGNAL deleter_has_locked | ||
| WAIT_FOR waiter_has_locked'; | ||
| --send DELETE FROM t WHERE a = 9999 | ||
|
|
||
| --connection holder | ||
| SET DEBUG_SYNC= | ||
| 'now WAIT_FOR deleter_has_locked'; | ||
| SET DEBUG_SYNC= | ||
| 'lock_sec_rec_read_check_and_lock_has_locked SIGNAL holder_has_locked'; | ||
| --send DELETE FROM t WHERE a = 9999 | ||
|
|
||
| --connection waiter | ||
| SET DEBUG_SYNC= | ||
| 'now WAIT_FOR holder_has_locked'; | ||
| SET DEBUG_SYNC= | ||
| 'lock_sec_rec_read_check_and_lock_has_locked SIGNAL waiter_has_locked'; | ||
| --send DELETE FROM t WHERE a = 9999 | ||
|
|
||
| --connection deleter | ||
| --reap | ||
|
|
||
| --connection holder | ||
| --reap | ||
|
|
||
| --connection waiter | ||
| --reap | ||
|
|
||
| --connection default | ||
|
|
||
| --disconnect deleter | ||
| --disconnect holder | ||
| --disconnect waiter | ||
|
|
||
| DROP TABLE `t`; | ||
| SET DEBUG_SYNC='reset'; | ||
|
|
||
| # SCENARIO 2 | ||
| # | ||
| # Here, we form a situation in which con1 has LOCK_REC_NOT_GAP on rows 1 and 2 | ||
| # con2 waits for lock on row 1, and then con1 wants to upgrade the lock on row 1, | ||
| # which might cause a deadlock, unless con1 properly notices that even though the | ||
| # lock on row 1 can not be upgraded, a separate LOCK_GAP can be obtaied easily. | ||
|
|
||
| CREATE TABLE `t`( | ||
| `id` INT NOT NULL PRIMARY KEY | ||
| ) ENGINE=InnoDB; | ||
|
|
||
| INSERT INTO t (`id`) VALUES (1), (2); | ||
|
|
||
| --connect(holder,localhost,root,,) | ||
| --connect(waiter,localhost,root,,) | ||
|
|
||
| --connection holder | ||
| BEGIN; | ||
| SELECT id FROM t WHERE id=1 FOR UPDATE; | ||
| SELECT id FROM t WHERE id=2 FOR UPDATE; | ||
|
|
||
| --connection waiter | ||
| SET DEBUG_SYNC= | ||
| 'lock_wait_suspend_thread_enter SIGNAL waiter_will_wait'; | ||
| --send SELECT id FROM t WHERE id = 1 FOR UPDATE | ||
|
|
||
| --connection holder | ||
| SET DEBUG_SYNC= | ||
| 'now WAIT_FOR waiter_will_wait'; | ||
| SELECT * FROM t FOR UPDATE; | ||
| COMMIT; | ||
|
|
||
| --connection waiter | ||
| --reap | ||
|
|
||
| --connection default | ||
|
|
||
| --disconnect holder | ||
| --disconnect waiter | ||
|
|
||
| DROP TABLE `t`; | ||
| SET DEBUG_SYNC='reset'; | ||
|
|
||
| --source include/wait_until_count_sessions.inc |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters