Bug #12410 mysqldump/InnoDB too permissive with LOCK TABLE foo READ LOCAL
Submitted: 6 Aug 2005 6:20 Modified: 1 Sep 2005 14:46
Reporter: Timothy Smith Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server: InnoDB storage engine Severity:S2 (Serious)
Version:4.1 OS:Any (any)
Assigned to: Heikki Tuuri

[6 Aug 2005 6:20] Timothy Smith
Description:
This problem came up because mysqldump was not locking InnoDB tables, despite the --lock-tables option being specified.

It turns out that mysqldump uses LOCK TABLES foo READ LOCAL; for MyISAM tables, this ensures that all tables are dumped consistently, but still lets other sessions insert into the tables.

On InnoDB, though, it is too lax.  It prevents the current session from updating the table, but it allows any other session to do any updates on the table.  In addition, those updates are visible to the session that issues the LOCK ... LOCAL.

How to repeat:

set autocommit = 1;
drop table if exists t;
create table t (id int) engine = innodb;
insert into t values (1);
lock tables t read local;
insert into t values (2); -- should get an error
select * from t;

-- Now suspend that session, and in another session

insert into t values (2);  -- this works

-- this succeeds immediately, but it should block
update t set id = 10 where id = 1;

-- Now, back in the first session

select * from t; -- Shows 10, 2; it should just show 1.

Here's an example from a recent MySQL 4.1 bk snapshot on FreeBSD 5.4:

Welcome to the MySQL monitor.  Commands end with ; or \g.
Your MySQL connection id is 7 to server version: 4.1.14-debug-log

Type 'help;' or '\h' for help. Type '\c' to clear the buffer.

mysql> set autocommit = 1;
Query OK, 0 rows affected (0.00 sec)

mysql> drop table if exists t;
Query OK, 0 rows affected, 1 warning (0.00 sec)

mysql> create table t (id int) engine = innodb;
Query OK, 0 rows affected (0.01 sec)

mysql> insert into t values (1);
Query OK, 1 row affected (0.00 sec)

mysql> lock tables t read local;
Query OK, 0 rows affected (0.00 sec)

mysql> insert into t values (2); -- should get an error
ERROR 1099 (HY000): Table 't' was locked with a READ lock and can't be updated
mysql> select * from t;
+------+
| id   |
+------+
|    1 |
+------+
1 row in set (0.01 sec)

mysql> 
zsh: suspended  mysql test
18:16 ~/m/41/m$ mysql -A test
Welcome to the MySQL monitor.  Commands end with ; or \g.
Your MySQL connection id is 8 to server version: 4.1.14-debug-log

Type 'help;' or '\h' for help. Type '\c' to clear the buffer.

mysql> -- Now suspend that session, and in another session
mysql> 
mysql> insert into t values (2);  -- this works
Query OK, 1 row affected (0.00 sec)

mysql> 
mysql> -- this succeeds immediately, but it should block
mysql> update t set id = 10 where id = 1;
Query OK, 1 row affected (0.01 sec)
Rows matched: 1  Changed: 1  Warnings: 0

mysql> Bye
18:17 ~/m/41/m$ fg
[1]  + continued  mysql test
-- Now, back in the first session
mysql> 
mysql> select * from t; -- Shows 10, 2; it should just show 1.
+------+
| id   |
+------+
|   10 |
|    2 |
+------+
2 rows in set (0.00 sec)

mysql> Bye

Suggested fix:
I don't know.  One fix for the presenting problem is to have mysqldump start a transaction at the same time it runs LOCK TABLES.  But the underlying problem remains.
[7 Aug 2005 6:30] Heikki Tuuri
Tim,

in READ LOCAL, InnoDB does use an S-lock on the rows it reads.

ha_innodb.cc, ::store_lock():

        if ((lock_type == TL_READ && thd->in_lock_tables) ||
            (lock_type == TL_READ_HIGH_PRIORITY && thd->in_lock_tables) ||
            lock_type == TL_READ_WITH_SHARED_LOCKS ||
            lock_type == TL_READ_NO_INSERT ||
            (thd->lex->sql_command != SQLCOM_SELECT
             && lock_type != TL_IGNORE)) {

                /* The OR cases above are in this order:
                1) MySQL is doing LOCK TABLES ... READ LOCAL, or
                2) (we do not know when TL_READ_HIGH_PRIORITY is used), or
                3) this is a SELECT ... IN SHARE MODE, or
                4) we are doing a complex SQL statement like
                INSERT INTO ... SELECT ... and the logical logging (MySQL
                binlog) requires the use of a locking read, or
                MySQL is doing LOCK TABLES ... READ.
                5) we let InnoDB do locking reads for all SQL statements that
                are not simple SELECTs; note that select_lock_type in this
                case may get strengthened in ::external_lock() to LOCK_X. */

...
                        prebuilt->select_lock_type = LOCK_S;
                        prebuilt->stored_select_lock_type = LOCK_S;

Thus, a single table is dumped in a consistent state, which is the state at the end of the transaction.

The bug is that mysqldump does not use BEGIN ... COMMIT to wrap the whole dump operation. Then after dumping each individual table, S-locks on rows are released. Then each individual table can correspond to a different timepoint!

Possible fixes:
1) we could convert the MySQL lock TL_READ to TL_READ_NO_INSERT in ::store_lock(); then READ LOCAL would be equivalent to READ for InnoDB tables;

or

2) mysqldump could use BEGIN ... COMMIT to wrap the whole dump operation.

Note that InnoDB tables should normally be dumped with the --single-transaction option. Then there are no locks whatsoever, and the dump is consistent across different InnoDB tables. The READ LOCAL option should be used if we want a dump of mixed MyISAM and InnoDB tables.

Hmm... we have to think about this.

Regards,

Heikki
[8 Aug 2005 22:07] Timothy Smith
Heikki,

As I see it, the problem is that the meaning of READ LOCAL for MyISAM is quite different from the meaning it currently has for InnoDB.  I think either READ LOCAL should be disallowed for InnoDB tables, or it should somehow behave in the same way.  Perhaps it is OK to be more restrictive with InnoDB (have READ LOCAL behave as READ), and give a warning.

With MyISAM, the LOCK TABLES foo READ LOCAL works across multiple transactions, allows other sessions to INSERT only (no updates or deletes), and the session that holds the lock does not see those changes.

With InnoDB, it does not work across multiple transactions.  Also, the share locks aren't needed; inside a single transaction, the a normal SELECT is fine.  The goal is not to prevent others from updating the tables, but only to prevent the current session from seeing those updates.  Well, that's what InnoDB always does with a simple SELECT.

So, if mysqldump were to use BEGIN ... END around the dumps of all tables in the database, then no LOCK TABLES statement at all is needed (for InnoDB tables, that is).  I still don't like the idea of LOCK TABLES ... READ LOCAL having very different semantics between InnoDB and MyISAM.  I'd much prefer that it simply not work on InnoDB, or perhaps have stronger locking semantics and issue a warning (and of course document it clearly in the LOCK TABLES section of the manual).

So, to move forward with this (assuming I understood things correctly), we should certainly change mysqldump to dump all tables in a database within a single transaction.  That will solve the immediate problem for the customer.

And, we should decide how to resolve the semantic difference between MyISAM and InnoDB for READ LOCAL locks.  The choices I see are:

1) throw an error when trying to lock an InnoDB table,

2) convert the READ LOCAL lock to a READ lock, and issue a warning, or

3) somehow make the READ LOCAL S-locks persist across multiple transactions

Maybe there are some other options.  My preference is #1 - it's not needed for InnoDB tables, it's probably very hard to implement the same semantics, and disallowing it will be the least confusing to the user.

My problem with #2 is that it makes InnoDB do more locking than MyISAM, when there is no need for it.  It may damage InnoDB's credibility ("If InnoDB can't allow inserts here, where else does it have locking problems?"), it's a hack that may come back to bite us later on, etc.  READ LOCAL makes sense for non-transactional storage engines.  It makes no sense for InnoDB, in my opinion.

My problem with #3 is that it's likely to be a lot of work, inefficient, and it's useless anyways.

Regards,

Timothy
[17 Aug 2005 14:29] Heikki Tuuri
Tim,

if someone has time to fix this in mysqldump, that would be very good.

I do not think that converting LOCK TABLES ... READ LOCAL to LOCK TABLES READ is a big sin. Normally, a database engine is allowed to use stricter locking if it does not have the exact same level of locking.

Hmm... I am changing this to a mysqldump bug report, and unassigning myself from this. Anyone is free to assign this bug back to me if he thinks I should change the InnoDB internal behavior.

Regards,

Heikki
[18 Aug 2005 1:24] Timothy Smith
Heikki,

I'm not sure that mysqldump can be fixed without changing the InnoDB behavior.  If mysqldump tries to "start transaction with consistent snapshot" and then "lock tables", there is a gap there where the MyISAM tables might get updated and be out of sync with the consistent snapshot of the InnoDB tables.  Also, LOCK TABLES will perform an implicit commit, so the transaction disappears anyways.

If mysqldump tries to lock tables first, there is a probem because START TRANSACTION will unlock tables implicitly.

Also, I still feel that the current READ LOCAL behavior of InnoDB is wrong.  Changing it to behave just like READ would be acceptable to me, but leaving it as it is now does not seem OK.  I will let the MySQL architecture people decide that, though.

Regards,

Timothy
[19 Aug 2005 9:11] Heikki Tuuri
Tim,

hmm... actually now I see that READ LOCAL causes MyISAM tables to be dumped in the state corresponding to the START of the dump, while InnoDB tables are dumped at the state corresponding to the END of the dump!

This is clearly a bug, and I think the only way to fix this is to convert READ LOCAL inside InnoDB to a strict READ lock.

I am assigning this bug back to myself.

Regards,

Heikki
[31 Aug 2005 11:10] Bugs System
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/internals/29095
[31 Aug 2005 11:12] Heikki Tuuri
Fixed in 4.1.15 and 5.0.13.

Dear docs team: also update the manual to say that for InnoDB tables,  LOCK TABLE ... READ LOCAL is converted to LOCK TABLE ... READ starting from the above versions.
[1 Sep 2005 14:46] Paul Dubois
Noted in 4.1.15, 5.0.13 changelogs and
in LOCK TABLES section.