MySQL Bugs: #34182: SELECT ... FOR UPDATE does not lock when in subquery

Bug #34182	SELECT ... FOR UPDATE does not lock when in subquery
Submitted:	31 Jan 2008 0:19	Modified:	15 May 2009 15:56
Reporter:	Hakan Küçükyılmaz	Email Updates:
Status:	Closed	Impact on me:	None
Category:	MySQL Server: Falcon storage engine	Severity:	S3 (Non-critical)
Version:	mysql-6.0-falcon-team	OS:	Any
Assigned to:	Kevin Lewis	CPU Architecture:	Any
Tags:	F_TRANSACTION

Description:
A SELECT ... FOR UPDATE does not block when in subquery.

How to repeat:
Excerpt from falcon_deadlock.test.

-- Connection1
SET @@storage_engine = 'Falcon';

DROP TABLE IF EXISTS t1;
DROP TABLE IF EXISTS t2;

SET @@autocommit = 0;

CREATE TABLE t1 (
  id integer,
  x integer
);

CREATE TABLE t2 (
  b integer,
  a integer
);

INSERT INTO t1 VALUES (0, 0), (300, 300);
INSERT INTO t2 VALUES (0, 10), (1, 20), (2, 30);
COMMIT;

UPDATE t2 SET a = 100 WHERE b = (SELECT x FROM t1 WHERE id = b FOR UPDATE);

-- Connection2
SET @@autocommit = 0;
-- The following query should hang because connection1 is locking the record,
-- but it does not.
UPDATE t1 SET x = 2 WHERE id = 0;

-- Note, when the SELECT ... FOR UPDATE is run on it's own like:
SELECT x FROM t1 WHERE id = 0 FOR UPDATE
-- then connection2's UPDATE is blocked.

Suggested fix:
Let SELECT ... FOR UPDATE also work in subqueries.

Thank you for the report.

Verified as described.

Triage team, Please re-evaluate this CHECKED tag.  It should probably be RC because it affects the interactions between transactions and
could cause unexpected results in a "write committed" transaction.

Sergey, Please take a look at this.  Falcon uses a server flag (lockForUpdate = (lock_type == TL_WRITE)) to determine whether to call Table::fetchForUpdate() or RecordVersion::fetchVersion();  Maybe this is incorrect in the case of a subquery.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/62061

2946 Sergey Vojtovich	2008-12-19
      BUG#34182 - SELECT ... FOR UPDATE does not lock when in subquery
      
      Executing SELECT ... FOR UPDATE multiple times may release
      row locks set by previous queries. Affects not only subqueries,
      as was initially discovered.

Test case changes look ok for me.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/62258

2946 Sergey Vojtovich	2008-12-23
      BUG#34182 - SELECT ... FOR UPDATE does not lock when in subquery
      
      Fixed that SELECT ... FOR UPDATE/UPDATE/DELETE statements
      may release previously set record locks for records that do
      not match current WHERE condition.

I am experimenting with adding a second lock record onto the record chain when a single transaction attempts to lock a record twice.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/65353

3004 Sergey Vojtovich	2009-02-05
      Test case fixes. It should pass, when BUG#34182 is fixed.

When I initially described this issue to Jim Starkey, he thought that there was not a valid reason for the second lock to occur, since it was already locked int he first place.  Indeed, that is the way Falcon has implemented record locking, with the concept that you only need to lock it once.  

The UPDATE used for this bug is highly questionable in its utility;

UPDATE t2 SET a = 100 WHERE b = (SELECT x FROM t1 WHERE id = b FOR UPDATE);

Why would anyone put a FOR UPDATE in the subselect statement?  However, it needs to be supported.  During the execution of this statement, I find that the two records in table t1 are locked and unlocked during the execution of the UPDATE after having been locked initially during the subselect.  The record in t1 that matches ( id = b  ), which is (1, 1), should remain locked.  But since Falcon uses the previous lock instead of adding multiple layers of locks, the following lock/unlock during the UPDATE clears the lock that should stay.

This scenario can also be simulated by a more common example;

#First Client
DROP TABLE t1;
CREATE TABLE t1 (id INTEGER primary key, x INTEGER) ENGINE falcon;
INSERT INTO t1 VALUES (1, 1), (2, 2);
SELECT * FROM t1 WHERE id = 1 FOR UPDATE;
SAVEPOINT SP1;
UPDATE t1 SET x = 222 WHERE x = 2;

# Second client - This UPDATE should NOT be allowed!
# But it works because the previous UPDATE at SP1 locked 
# ALL records AND unlocked the records it did NOT UPDATE.
UPDATE t1 SET x = 111 WHERE id = 1;  

So we see that Falcon must support multiple levels of locks.

The following test script shows why we need to be able to save multiple lock records at multiple savepoints.  The Rollback to savepoint could happen to any savepoint.  

#First Client
DROP TABLE t1;
CREATE TABLE t1 (id INTEGER primary key, x INTEGER) ENGINE falcon;
INSERT INTO t1 VALUES (1, 1), (2, 2), (3, 3);
BEGIN;
SELECT * FROM t1 WHERE id = 1 FOR UPDATE;
SAVEPOINT SP1;
SELECT * FROM t1 WHERE id <= 2 FOR UPDATE;
SAVEPOINT SP2;
UPDATE t1 SET x = 333 WHERE x = 3;  

# Second client - Neither of these updates should work!
# They do work because the previous UPDATE at SP2 locked 
# all records and then unlocked the two records it did not update.
# One of them was locked before SP1 and the other before SP2
BEGIN;
UPDATE t1 SET x = 111 WHERE id = 1;  
UPDATE t1 SET x = 222 WHERE id = 2;  
ROLLBACK;

#First Client -  This aught TO unlock records 2 AND 3, but NOT  1.
ROLLBACK TO SP1;

# Second client - The first two UPDATEs ON  id=3&2 should succeed, 
# but the last ON id=1 should not
BEGIN;
UPDATE t1 SET x = 333 WHERE id = 3;  
UPDATE t1 SET x = 222 WHERE id = 2;  
UPDATE t1 SET x = 111 WHERE id = 1;  

Now, one could argue that if there is a savepoint at before SP1 and another before SP2 and another at a higher savepoint that gets rolled back, you really don't need to have the higher two lock records.  You just need to avoid unlocking the first one before SP1 until the whole transaction is rolled back or committed. 

This is my current struggle.  Either be able to add multiple lock records without too much complexity, or prevent locks from being unlocked before their time.

The previous comments suggested a new way to allow locks to be taken at multiple savepoints and retained during the life of that savepoint.  That is, instead of adding a new lock record at each savepoint, continue locking as it is done now, but unlock only records that have lock records at the current savepoint.  

This approach seems to work fine and it is much simpler than using multiple lock records.  After about a week of trying to get that to work, I finally gave up because it added unnecessary complexity into the code.  There are enough places where the prior record chain is traversed that special conditions need to be added in too many places.  

A new integer can be sent into unlockRecords which holds the current verbMark or savepoint.  LockRecords that are at a higher savepoint are not unlocked.  Then, if a commit, rollback, releaseSavepoint, or rollbackToSavepoint occurs that would release that lockRecord, it will get released just like it did before.  

A new testcase will be committed that shows this.

A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/66090

3015 Kevin Lewis	2009-02-12
      Bug#34182 - Falcon needs to be able to keep locks at lower savepoints when they are explicitly released or uinlocked at hight savepoints  Locks should be retained during the life of that savepoint unless they are explicitely unlocked before the next savepoint. 
      
      It is not necessary to add a new lock record at each savepoint.  The existing lock will do the job as long as it is not unlocked at a later savepoint.  The best solution to this problem is to only unlock records that were locked in the current savepoint, or verb.
      
      A new integer is sent into unlockRecords which holds the current verbMark or
      savepoint.  LockRecords that are at a higher savepoint are not unlocked.  Then, if a
      commit, rollback, releaseSavepoint, or rollbackToSavepoint occurs that would release that
      lockRecord, it will get released just like it did before.  
      
      A new testcase is added  that shows the affect of locks attained at multiple savepoints.

Pushed into 6.0.10-alpha (revid:alik@sun.com-20090211182317-uagkyj01fk30p1f8) (version source revid:svoj@mysql.com-20090205132327-42kc5us3a8q6g6f7) (merge vers: 6.0.10-alpha) (pib:6)

One some minor questions about the test case. Sent as a reply to the commit email.

Pushed into 6.0.11-alpha (revid:alik@sun.com-20090302140208-lfdejjbcyezlhhjt) (version source revid:vvaintroub@mysql.com-20090214213452-djoygjmxbmbgp8hj) (merge vers: 6.0.10-alpha) (pib:6)

A note has been added to the 6.0.11 changelog: 

A subquery using SELECT ... FOR UPDATE on a Falcon table fails to lock table correctly during the UPDATE