#At file:///export/home/tmp/mysql/opt-bug41029-opt-back/ based on revid:epotemkin@stripped
3182 Olav Sandstaa 2010-05-21
Fix for Bug#41029 MRR: SELECT FOR UPDATE fails to lock gaps (InnoDB table)
This problem occurs due to MRR is creating a second handler
object. This handler object is a "clone" of the first/initial handler
object and the MRR code use this second handler object for executing
some of the operations.
The second handler object is created in DsMRR_impl::dsmrr_init() (in
handler.cc) by "cloning" the initial handler object:
if (!(new_h2= h->clone(thd->mem_root)) ||
new_h2->ha_external_lock(thd, F_RDLCK))
The important thing about the above code is that on this cloned
handler object a call to ha_external_lock() is done with a request to
use a read lock (F_RDLCK). When this handler object (h2) is used in
DsMrr_impl::dsmrr_fill_buffer() for doing the initial search for the
first record in the range interval, the index search done by InnoDB
will be done without setting any exclusive lock on the search
interval. This allows a second transaction to succeed to insert data
within the "read interval" of the first transaction.
The fix for this problem is to use the same lock type when calling
new_h2->ha_external_lock() as was done on the original handler
object. To do this we store (in handler::ha_external_lock()) the
original lock type in a new member variable in the handler object
(m_lock_type) and use this lock type when "cloning" the handler object
in DsMRR_impl::dsmrr_init().
Note to reviewers (and myself):
1. The included test case should be will be included in the innodb_mrr
test before the fix is pushed but due to innodb_mrr test being unstable
I have included as a separate test case.
2. The final commit/push should also remove the test and result files for
this bug from the optimizer_unfixed_bugs suite.
@ mysql-test/r/bug41029.result
Result file for test of Bug#41029 MRR: SELECT FOR UPDATE fails to lock gaps (InnoDB table)
@ mysql-test/t/bug41029-master.opt
Option file for reducing the default lock timeout in order to reduce the time for running the test.
@ mysql-test/t/bug41029.test
Test for Bug#41029 MRR: SELECT FOR UPDATE fails to lock gaps (InnoDB table)
@ sql/handler.cc
When MRR is used a second handler object is cloned based on the original
handler object. This second handler object is used for the initial navigation
of the underlying storage engine's data. When this second handler object
was created a call to ha_external_lock() was done with F_RDLCK as the
lock type. Since F_RDLCK was always used this made InnoDB
not put gap lock on the search interval and allowed concurrent transactions
to insert data into the search interval of transasactions running SELECT
FOR UPDATE statements in REPEATABLE READ mode.
To ensure the second handler object created when MRR is used is getting the
same lock type as the initial handler the following changes are implemented:
1. Extend handler::ha_external_lock() to save the lock type in a member variable
(m_lock_type).
2. Extend DsMRR_impl::dsmrr_init() so that it uses the same lock type when
cloning a second handler object as used for the initial handler object
(instead of always using F_RDLCK).
@ sql/handler.h
Add a new member variable to the handler class to store the lock type set by
the call to handler::ha_external_lock(). This lock type is needed by the MRR
implementation when creating a second handler object by cloning the initial
handler.
added:
mysql-test/r/bug41029.result
mysql-test/t/bug41029-master.opt
mysql-test/t/bug41029.test
modified:
sql/handler.cc
sql/handler.h
=== added file 'mysql-test/r/bug41029.result'
=== added file 'mysql-test/r/bug41029.result'
--- a/mysql-test/r/bug41029.result 1970-01-01 00:00:00 +0000
+++ b/mysql-test/r/bug41029.result 2010-05-21 13:57:37 +0000
@@ -0,0 +1,30 @@
+#
+# Bug#41029 "MRR: SELECT FOR UPDATE fails to lock gaps (InnoDB table)"
+#
+SET AUTOCOMMIT=0;
+CREATE TABLE t1 (
+dummy INT PRIMARY KEY,
+a INT UNIQUE,
+b INT
+) ENGINE=InnoDB;
+INSERT INTO t1 VALUES (1,1,1),(3,3,3),(5,5,5);
+COMMIT;
+SET SESSION TRANSACTION ISOLATION LEVEL REPEATABLE READ;
+SELECT @@tx_isolation;
+@@tx_isolation
+REPEATABLE-READ
+START TRANSACTION;
+EXPLAIN SELECT * FROM t1 WHERE a > 2 FOR UPDATE;
+id select_type table type possible_keys key key_len ref rows Extra
+1 SIMPLE t1 range a a 5 NULL 1 Using where; Using MRR
+SELECT * FROM t1 WHERE a > 2 FOR UPDATE;
+dummy a b
+3 3 3
+5 5 5
+SET AUTOCOMMIT=0;
+START TRANSACTION;
+INSERT INTO t1 VALUES (2,2,2);
+ERROR HY000: Lock wait timeout exceeded; try restarting transaction
+ROLLBACK;
+ROLLBACK;
+DROP TABLE t1;
=== added file 'mysql-test/t/bug41029-master.opt'
--- a/mysql-test/t/bug41029-master.opt 1970-01-01 00:00:00 +0000
+++ b/mysql-test/t/bug41029-master.opt 2010-05-21 13:57:37 +0000
@@ -0,0 +1,1 @@
+--loose-innodb_lock_wait_timeout=3
=== added file 'mysql-test/t/bug41029.test'
--- a/mysql-test/t/bug41029.test 1970-01-01 00:00:00 +0000
+++ b/mysql-test/t/bug41029.test 2010-05-21 13:57:37 +0000
@@ -0,0 +1,53 @@
+--echo #
+--echo # Bug#41029 "MRR: SELECT FOR UPDATE fails to lock gaps (InnoDB table)"
+--echo #
+
+--source include/have_innodb.inc
+
+# This test verifies that a SELECT FOR UPDATE statement executed in
+# REPEATABLE READ isolation will lock the entire read interval by verifying
+# that a second transaction trying to update data within this interval will
+# be blocked.
+
+connect (con1,localhost,root,,);
+connect (con2,localhost,root,,);
+
+connection con1;
+
+SET AUTOCOMMIT=0;
+
+CREATE TABLE t1 (
+ dummy INT PRIMARY KEY,
+ a INT UNIQUE,
+ b INT
+) ENGINE=InnoDB;
+
+INSERT INTO t1 VALUES (1,1,1),(3,3,3),(5,5,5);
+COMMIT;
+
+SET SESSION TRANSACTION ISOLATION LEVEL REPEATABLE READ;
+SELECT @@tx_isolation;
+START TRANSACTION;
+
+EXPLAIN SELECT * FROM t1 WHERE a > 2 FOR UPDATE;
+
+SELECT * FROM t1 WHERE a > 2 FOR UPDATE;
+
+connection con2;
+
+SET AUTOCOMMIT=0;
+START TRANSACTION;
+
+--error ER_LOCK_WAIT_TIMEOUT
+INSERT INTO t1 VALUES (2,2,2);
+ROLLBACK;
+
+connection con1;
+
+ROLLBACK;
+DROP TABLE t1;
+
+connection default;
+disconnect con1;
+disconnect con2;
+
=== modified file 'sql/handler.cc'
--- a/sql/handler.cc 2010-05-14 10:58:39 +0000
+++ b/sql/handler.cc 2010-05-21 13:57:37 +0000
@@ -4464,7 +4464,7 @@
uint mrr_keyno= h->active_index;
if (!(new_h2= h->clone(thd->mem_root)) ||
- new_h2->ha_external_lock(thd, F_RDLCK))
+ new_h2->ha_external_lock(thd, h->m_lock_type))
{
delete new_h2;
DBUG_RETURN(1);
@@ -5505,8 +5505,16 @@
int error= external_lock(thd, lock_type);
if (error == 0)
+ {
cached_table_flags= table_flags();
+ /*
+ The lock type is needed by MRR when creating a clone of this handler
+ object.
+ */
+ m_lock_type= lock_type;
+ }
+
if (MYSQL_HANDLER_RDLOCK_DONE_ENABLED() ||
MYSQL_HANDLER_WRLOCK_DONE_ENABLED() ||
MYSQL_HANDLER_UNLOCK_DONE_ENABLED())
=== modified file 'sql/handler.h'
--- a/sql/handler.h 2010-05-14 11:22:27 +0000
+++ b/sql/handler.h 2010-05-21 13:57:37 +0000
@@ -1487,6 +1487,14 @@
*/
PSI_table *m_psi;
+ /**
+ The lock type set by when calling::ha_external_lock(). This is
+ propagated down to the storage engine. The reason for also storing
+ this here is when doing MRR we need to create/clone a second handler
+ object. This cloned handler object needs to know about the lock_type used.
+ */
+ int m_lock_type;
+
handler(handlerton *ht_arg, TABLE_SHARE *share_arg)
:table_share(share_arg), table(0),
estimation_rows_to_insert(0), ht(ht_arg),
@@ -1498,7 +1506,7 @@
pushed_cond(0), pushed_idx_cond(NULL), pushed_idx_cond_keyno(MAX_KEY),
next_insert_id(0), insert_id_for_cur_row(0),
auto_inc_intervals_count(0),
- m_psi(NULL)
+ m_psi(NULL), m_lock_type(F_UNLCK)
{}
virtual ~handler(void)
{
Attachment: [text/bzr-bundle] bzr/olav@sun.com-20100521135737-2pv0d5cf8fmbf52e.bundle
