MySQL Bugs: #42230: during add index, cannot do queries on storage engines that implement add

Bug #42230	during add index, cannot do queries on storage engines that implement add_index
Submitted:	20 Jan 2009 22:02	Modified:	29 Jan 2011 23:05
Reporter:	Zardosht Kasheff (OCA)	Email Updates:
Status:	Closed	Impact on me:	None
Category:	MySQL Server	Severity:	S3 (Non-critical)
Version:	5.1.30	OS:	Any
Assigned to:	Jon Olav Hauglid	CPU Architecture:	Any
Tags:	add_index, Contribution

Description:
For storage engines that do not implement add_index, one can do queries on a table while a new index is being created. For storage engines that cannot, such as the latest InnoDB downloaded off of their website, one cannot do queries.

How to repeat:
download the latest InnoDB from their website and compile it with MySQL. Add an index to some table. While the index is being added, try to do some query on the table. The query hangs until the index creation is completed.

Suggested fix:
The query should still execute.

Thank you for the bug report. Verified on Windows 64-bit:

c:\5.1\bin>mysqld --standalone --console
090120 20:30:32  InnoDB: highest supported file format is Barracuda.
090120 20:30:33 InnoDB Plugin 1.0.2 started; log sequence number 6970293
090120 20:30:35 [Note] Event Scheduler: Loaded 0 events
090120 20:30:35 [Note] mysqld: ready for connections.
Version: '5.1.31-nt'  socket: ''  port: 3306  Source distribution

c:\5.1\bin>mysql -uroot w1
Welcome to the MySQL monitor.  Commands end with ; or \g.
Your MySQL connection id is 2
Server version: 5.1.31-nt Source distribution

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

mysql> select * from tb limit 1;
+----+----------------------------------------------------+
| id | col1                                               |
+----+----------------------------------------------------+
|  1 | aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa |
+----+----------------------------------------------------+
1 row in set (0.02 sec)

mysql> select * from tb limit 1;
+----+----------------------------------------------------+
| id | col1                                               |
+----+----------------------------------------------------+
|  1 | aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa |
+----+----------------------------------------------------+
1 row in set (2 min 39.87 sec)

mysql> show variables like "innodb_version";
+----------------+-------+
| Variable_name  | Value |
+----------------+-------+
| innodb_version | 1.0.2 |
+----------------+-------+
1 row in set (0.00 sec)

mysql> show create table tb;
+-------+-----------------------------------------------------------
--------------------------------------------------------------------
| Table | Create Table

+-------+-----------------------------------------------------------
--------------------------------------------------------------------
| tb    | CREATE TABLE `tb` (
  `id` bigint(20) unsigned NOT NULL AUTO_INCREMENT,
  `col1` varchar(50) DEFAULT NULL,
  UNIQUE KEY `id` (`id`),
  KEY `col1` (`col1`)
) ENGINE=InnoDB AUTO_INCREMENT=2490316 DEFAULT CHARSET=latin1 |
+-------+-----------------------------------------------------------
--------------------------------------------------------------------
1 row in set (0.00 sec)

Regression was caused by fix for BUG#24395. Unless there is other code relying on the incorrect behaviour (there seem to be none), it will be a one line fix. An effort and risk to fix are rather low.

The problem is that the code around ENABLE/DISABLE KEYS acquires exclusive table lock unconditionally, whereas the lock should be acquired only if ENABLE/DISABLE KEYS are requested.

6.0 doesn't seem to be affected.

Out of curiousity, what is the one line fix?

I think the following patch solves the issue. Based of Sergey's comment

Attachment: 55528.diff (application/octet-stream, text), 1.14 KiB.

patch as suggested by Davi Arnaut on internals alias

Attachment: 55528-version2.diff (application/octet-stream, text), 484 bytes.

I just added 55528-version2.diff, based off of discussion on internals alias. Please disregard 55528.diff and consider this patch. The relevant discussion on the internals alias is http://lists.mysql.com/internals/37995

Bug #55528 was marked as duplicate of this one.

Back almost two years ago I concluded: "Unless there is other code relying on the incorrect behaviour (there seem to be none), it will be a one line fix."

The one-line fix I was talking about is:
=== modified file 'sql/sql_table.cc'
--- sql/sql_table.cc	2010-10-01 13:59:07 +0000
+++ sql/sql_table.cc	2010-10-19 07:16:30 +0000
@@ -6343,7 +6343,7 @@
                                     alter_info->keys_onoff,
                                     alter_info->error_if_not_empty);
   }
-  else
+  else if (alter_info->flags & ALTER_KEYS_ONOFF)
   {
     if (!table->s->tmp_table &&
         wait_while_table_is_used(thd, table, HA_EXTRA_FORCE_REOPEN))

Unfortunately my assumption was wrong and there is code relying on this incorrect behaviour.

The story is rather tricky. Some relevant events, hopefully in proper order:
- "fast" alter table was implemented (probably WL#1563) with proper locking context;
- fix for BUG#24395 introduced exclusive lock;
- "fast" alter table code was re-shuffled and lost it's locking context. But it still works, because of fix for BUG#24395;
- mdl was introduced and we're further loosing add/drop index locking context;
- WL#3020 was created, but seem to have never been implemented.

Before applying this one-line fix, we must restore proper add/drop index mdl context.

Even having this all fixed, InnoDB may have to be adjusted as well.

To be clear, this one line fix works in 5.1, correct? It is in 5.5 where this additional work needs to be taken into account?

The one-line fix seem to open race condition hole even in 5.1.

The one-line fix seem to open race condition hole even in 5.1.

What is the race condition? Is it in InnoDB or in all storage engines?

Sergey,

> Unfortunately my assumption was wrong and there is code relying
> on this incorrect behaviour.

Which code is this?

> Before applying this one-line fix, we must restore proper add/drop
> index mdl context.

What do you mean?

Another thing I would like to understand is: what does it take to fix this in 5.1 v. 5.5? The metadata lock is new to 5.5, so I imagine the work to fix this in 5.1 is less than 5.5, correct?

Before discussing fix and such, I would like to see a concise description of the problem. There are only superficial conjectures  here..

Locking context that was lost seem to be relevant to WL#3020 (online alter table), sorry for confusion. Server seem to conform to design described in WL#1563 (fast alter table).

This extra problem seem to be InnoDB specific. With the one-line fix innodb-index test is failing (an error in error log). No concurrency involved, for some reason InnoDB seem to expect table reopen before ::add_index(). It is pretty suspicious expectation - I'd foresee more problems with concurrency.

So the question, that I think run-time should answer, is if locks and reopens are done properly. If answer is positive - the bug should go to InnoDB.

Q&A
---
In 5.1, why is this fix not sufficient?
Because relevant logics in 5.1 is in no way different from 5.5. MDL just makes things slightly more difficult to track.

Why is open_n_lock_single_table not sufficient for locking?
According to specification (WL#1563) it seem to be sufficient.

Which code is this?
Likely InnoDB.

What do you mean?
Disregard.

Thanks Sergey, I'll take it further with the InnoDB team.

Response from Marko when asked about index management operations under a shared metadata lock:

"For dropping or creating secondary indexes, a shared lock is fine. InnoDB already acquires internal InnoDB table locks according to these rules." [..] "It is safe for dropping indexes or creating secondary indexes, but not for creating a primary key, which requires a table rebuild. It probably would not crash when you create a primary key, but it could lead to a long lock wait or a lock wait timeout, until the table has been rebuilt."

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/125036

3395 Jon Olav Hauglid	2010-11-25
      Bug #42230 during add index, cannot do queries on storage engines
                 that implement add_index
      
      The problem was that ALTER TABLE blocked reads on an InnoDB table
      while adding a secondary index, even if this was not needed. It is
      only needed for the final step where the .frm file is updated.
      
      The reason queries were blocked, was that ALTER TABLE upgraded the
      metadata lock from MDL_SHARED_NO_WRITE (which blocks writes) to
      MDL_EXCLUSIVE (which blocks all accesses) before index creation.
      
      The way the server handles index creation, is that storage engines
      publish their capabilities to the server and the server determines
      which of the following three ways this can be handled: 1) build a
      new version of the table; 2) change the existing table but with
      exclusive metadata lock; 3) change the existing table but without
      metadata lock upgrade.
      
      For InnoDB and secondary index creation, option 3) should have been
      selected. However this failed for two reasons. First, InnoDB did
      not publish this capability properly. This patch fixes the problem
      by adding HA_ONLINE_ADD_INDEX to InnoDB's list of alter table flags.
      (The already published HA_ONLINE_ADD_INDEX_NO_WRITES flag only
      indicates that InnoDB support option 2).
      
      Second, the ALTER TABLE code failed to made proper use of the
      information supplied by the storage engine. A variable
      need_lock_for_indexes was set accordingly, but was not later used.
      This patch fixes this problem by only doing metadata lock upgrade
      before index creation/deletion if this variable has been set.
      
      Test case added to innodb_mysql_sync.test.

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/127171

3209 Jon Olav Hauglid	2010-12-17
      Bug #42230 during add index, cannot do queries on storage engines
                 that implement add_index
      
      The problem was that ALTER TABLE blocked reads on an InnoDB table
      while adding a secondary index, even if this was not needed. It is
      only needed for the final step where the .frm file is updated.
      
      The reason queries were blocked, was that ALTER TABLE upgraded the
      metadata lock from MDL_SHARED_NO_WRITE (which blocks writes) to
      MDL_EXCLUSIVE (which blocks all accesses) before index creation.
      
      The way the server handles index creation, is that storage engines
      publish their capabilities to the server and the server determines
      which of the following three ways this can be handled: 1) build a
      new version of the table; 2) change the existing table but with
      exclusive metadata lock; 3) change the existing table but without
      metadata lock upgrade.
      
      For InnoDB and secondary index creation, option 3) should have been
      selected. However this failed for two reasons. First, InnoDB did
      not publish this capability properly.
      
      Second, the ALTER TABLE code failed to made proper use of the
      information supplied by the storage engine. A variable
      need_lock_for_indexes was set accordingly, but was not later used.
      This patch fixes this problem by only doing metadata lock upgrade
      before index creation/deletion if this variable has been set.
      
      Test case added to innodb_mysql_sync.test.

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/127333

3217 Jon Olav Hauglid	2010-12-20
      Bug #42230 during add index, cannot do queries on storage engines
                 that implement add_index
      
      The problem was that ALTER TABLE blocked reads on an InnoDB table
      while adding a secondary index, even if this was not needed. It is
      only needed for the final step where the .frm file is updated.
      
      The reason queries were blocked, was that ALTER TABLE upgraded the
      metadata lock from MDL_SHARED_NO_WRITE (which blocks writes) to
      MDL_EXCLUSIVE (which blocks all accesses) before index creation.
      
      The way the server handles index creation, is that storage engines
      publish their capabilities to the server and the server determines
      which of the following three ways this can be handled: 1) build a
      new version of the table; 2) change the existing table but with
      exclusive metadata lock; 3) change the existing table but without
      metadata lock upgrade.
      
      For InnoDB and secondary index creation, option 3) should have been
      selected. However this failed for two reasons. First, InnoDB did
      not publish this capability properly.
      
      Second, the ALTER TABLE code failed to made proper use of the
      information supplied by the storage engine. A variable
      need_lock_for_indexes was set accordingly, but was not later used.
      This patch fixes this problem by only doing metadata lock upgrade
      before index creation/deletion if this variable has been set.
      
      This patch also changes some of the related terminology used 
      in the code. Specifically the use of "fast" and "online" with
      respect to ALTER TABLE. "Fast" was used to indicate that an
      ALTER TABLE operation could be done without involving a
      temporary table. "Fast" has been renamed "in-place" to more
      accurately describe the behavior.
      
      "Online" meant that the operation could be done without taking
      a table lock. However, in the current implementation writes
      are always prohibited during ALTER TABLE and an exclusive
      metadata lock is held while updating the .frm, so ALTER TABLE
      was not completely online. This patch replaces "online" with 
      "in-place", with additional comments indicating if concurrent
      reads are allowed during index creation/deletion or not.
      
      An important part of this update of terminology is renaming
      of the handler flags used by handlers to indicate if index
      creation/deletion can be done in-place and if concurrent reads
      are allowed. For example, the HA_ONLINE_ADD_INDEX_NO_WRITES
      flag has been renamed HA_INPLACE_ADD_INDEX_NO_READ_WRITE, while
      HA_ONLINE_ADD_INDEX is now HA_INPLACE_ADD_INDEX_NO_WRITE.
      Note that this is a rename to clarify current behavior, the
      flag values have not changed and no flags have been removed or
      added.
      
      Test case added to innodb_mysql_sync.test.

The patch looks OK as far as InnoDB is concerned, and the description looks OK too. I would like to request another test case to innodb_mysql_sync.test: Attempt to create a primary key and ensure that access to the table is blocked. Just in case, do the primary key creation both implicitly and explicitly.

I would suggest that you adapt this (commented) excerpt from innodb-index.test and interleave it with the DML connection. The first two ALTER statements should lock the table exclusively, and the last one should not (it is just creating a unique secondary index on a table that already has a user-supplied primary key).

-- creates a table with gen_clust_index (DB_ROW_ID as the 'primary key')
create table t1(a int not null, b int, c char(10) not null, d varchar(20)) engine = innodb;
-- creates a clustered index (PRIMARY KEY) on c, because it is declared NOT NULL
alter table t1 add unique index (c), add index (d);
-- drops the old clustered index and creates a new one (copies the table too)
alter table t1 add primary key (a), drop index c;
-- this should create a secondary index (non-blocking)
create unique index c on t1 (c);

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/128494

3237 Jon Olav Hauglid	2011-01-12
      Bug #42230 during add index, cannot do queries on storage engines
                 that implement add_index
      
      The problem was that ALTER TABLE blocked reads on an InnoDB table
      while adding a secondary index, even if this was not needed. It is
      only needed for the final step where the .frm file is updated.
      
      The reason queries were blocked, was that ALTER TABLE upgraded the
      metadata lock from MDL_SHARED_NO_WRITE (which blocks writes) to
      MDL_EXCLUSIVE (which blocks all accesses) before index creation.
      
      The way the server handles index creation, is that storage engines
      publish their capabilities to the server and the server determines
      which of the following three ways this can be handled: 1) build a
      new version of the table; 2) change the existing table but with
      exclusive metadata lock; 3) change the existing table but without
      metadata lock upgrade.
      
      For InnoDB and secondary index creation, option 3) should have been
      selected. However this failed for two reasons. First, InnoDB did
      not publish this capability properly.
      
      Second, the ALTER TABLE code failed to made proper use of the
      information supplied by the storage engine. A variable
      need_lock_for_indexes was set accordingly, but was not later used.
      This patch fixes this problem by only doing metadata lock upgrade
      before index creation/deletion if this variable has been set.
      
      This patch also changes some of the related terminology used 
      in the code. Specifically the use of "fast" and "online" with
      respect to ALTER TABLE. "Fast" was used to indicate that an
      ALTER TABLE operation could be done without involving a
      temporary table. "Fast" has been renamed "in-place" to more
      accurately describe the behavior.
      
      "Online" meant that the operation could be done without taking
      a table lock. However, in the current implementation writes
      are always prohibited during ALTER TABLE and an exclusive
      metadata lock is held while updating the .frm, so ALTER TABLE
      is not completely online. This patch replaces "online" with 
      "in-place", with additional comments indicating if concurrent
      reads are allowed during index creation/deletion or not.
      
      An important part of this update of terminology is renaming
      of the handler flags used by handlers to indicate if index
      creation/deletion can be done in-place and if concurrent reads
      are allowed. For example, the HA_ONLINE_ADD_INDEX_NO_WRITES
      flag has been renamed HA_INPLACE_ADD_INDEX_NO_READ_WRITE, while
      HA_ONLINE_ADD_INDEX is now HA_INPLACE_ADD_INDEX_NO_WRITE.
      Note that this is a rename to clarify current behavior, the
      flag values have not changed and no flags have been removed or
      added.
      
      Test case added to innodb_mysql_sync.test.

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/129650

3277 Jon Olav Hauglid	2011-01-26
      Bug #42230 during add index, cannot do queries on storage engines
                 that implement add_index
      
      The problem was that ALTER TABLE blocked reads on an InnoDB table
      while adding a secondary index, even if this was not needed. It is
      only needed for the final step where the .frm file is updated.
      
      The reason queries were blocked, was that ALTER TABLE upgraded the
      metadata lock from MDL_SHARED_NO_WRITE (which blocks writes) to
      MDL_EXCLUSIVE (which blocks all accesses) before index creation.
      
      The way the server handles index creation, is that storage engines
      publish their capabilities to the server and the server determines
      which of the following three ways this can be handled: 1) build a
      new version of the table; 2) change the existing table but with
      exclusive metadata lock; 3) change the existing table but without
      metadata lock upgrade.
      
      For InnoDB and secondary index creation, option 3) should have been
      selected. However this failed for two reasons. First, InnoDB did
      not publish this capability properly.
      
      Second, the ALTER TABLE code failed to made proper use of the
      information supplied by the storage engine. A variable
      need_lock_for_indexes was set accordingly, but was not later used.
      This patch fixes this problem by only doing metadata lock upgrade
      before index creation/deletion if this variable has been set.
      
      This patch also changes some of the related terminology used 
      in the code. Specifically the use of "fast" and "online" with
      respect to ALTER TABLE. "Fast" was used to indicate that an
      ALTER TABLE operation could be done without involving a
      temporary table. "Fast" has been renamed "in-place" to more
      accurately describe the behavior.
      
      "Online" meant that the operation could be done without taking
      a table lock. However, in the current implementation writes
      are always prohibited during ALTER TABLE and an exclusive
      metadata lock is held while updating the .frm, so ALTER TABLE
      is not completely online. This patch replaces "online" with 
      "in-place", with additional comments indicating if concurrent
      reads are allowed during index creation/deletion or not.
      
      An important part of this update of terminology is renaming
      of the handler flags used by handlers to indicate if index
      creation/deletion can be done in-place and if concurrent reads
      are allowed. For example, the HA_ONLINE_ADD_INDEX_NO_WRITES
      flag has been renamed to HA_INPLACE_ADD_INDEX_NO_READ_WRITE,
      while HA_ONLINE_ADD_INDEX is now HA_INPLACE_ADD_INDEX_NO_WRITE.
      Note that this is a rename to clarify current behavior, the
      flag values have not changed and no flags have been removed or
      added.
      
      Test case added to innodb_mysql_sync.test.

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/129664

3278 Jon Olav Hauglid	2011-01-26
      Bug #42230 during add index, cannot do queries on storage engines
                 that implement add_index
      
      The problem was that ALTER TABLE blocked reads on an InnoDB table
      while adding a secondary index, even if this was not needed. It is
      only needed for the final step where the .frm file is updated.
      
      The reason queries were blocked, was that ALTER TABLE upgraded the
      metadata lock from MDL_SHARED_NO_WRITE (which blocks writes) to
      MDL_EXCLUSIVE (which blocks all accesses) before index creation.
      
      The way the server handles index creation, is that storage engines
      publish their capabilities to the server and the server determines
      which of the following three ways this can be handled: 1) build a
      new version of the table; 2) change the existing table but with
      exclusive metadata lock; 3) change the existing table but without
      metadata lock upgrade.
      
      For InnoDB and secondary index creation, option 3) should have been
      selected. However this failed for two reasons. First, InnoDB did
      not publish this capability properly.
      
      Second, the ALTER TABLE code failed to made proper use of the
      information supplied by the storage engine. A variable
      need_lock_for_indexes was set accordingly, but was not later used.
      This patch fixes this problem by only doing metadata lock upgrade
      before index creation/deletion if this variable has been set.
      
      This patch also changes some of the related terminology used 
      in the code. Specifically the use of "fast" and "online" with
      respect to ALTER TABLE. "Fast" was used to indicate that an
      ALTER TABLE operation could be done without involving a
      temporary table. "Fast" has been renamed "in-place" to more
      accurately describe the behavior.
      
      "Online" meant that the operation could be done without taking
      a table lock. However, in the current implementation writes
      are always prohibited during ALTER TABLE and an exclusive
      metadata lock is held while updating the .frm, so ALTER TABLE
      is not completely online. This patch replaces "online" with 
      "in-place", with additional comments indicating if concurrent
      reads are allowed during index creation/deletion or not.
      
      An important part of this update of terminology is renaming
      of the handler flags used by handlers to indicate if index
      creation/deletion can be done in-place and if concurrent reads
      are allowed. For example, the HA_ONLINE_ADD_INDEX_NO_WRITES
      flag has been renamed to HA_INPLACE_ADD_INDEX_NO_READ_WRITE,
      while HA_ONLINE_ADD_INDEX is now HA_INPLACE_ADD_INDEX_NO_WRITE.
      Note that this is a rename to clarify current behavior, the
      flag values have not changed and no flags have been removed or
      added.
      
      Test case added to innodb_mysql_sync.test.

Bug#49730 was closed as a duplicate of this bug.

Pushed into mysql-trunk 5.6.2 (revid:jon.hauglid@oracle.com-20110126135704-m2mgl5x1zjz7ojif) (version source revid:jon.hauglid@oracle.com-20110126135704-m2mgl5x1zjz7ojif) (merge vers: 5.6.2) (pib:24)

Pushed into mysql-5.5 5.5.10 (revid:jon.hauglid@oracle.com-20110126132329-r1wyh235a3pi2n71) (version source revid:jon.hauglid@oracle.com-20110126132329-r1wyh235a3pi2n71) (merge vers: 5.5.10) (pib:24)

Bug #60388 was marked as duplicate of this one.

Noted in 5.5.10, 5.6.2 changelogs.

InnoDB now allows concurrent reads on a table while creating
nonprimary nonunique indexes. (This was found to create problems and
was reverted in 5.5.12.)