Bug #59749 Enabling concurrent reads while creating non-primary unique index gives failures
Submitted: 26 Jan 2011 17:07 Modified: 7 Dec 2011 16:05
Reporter: Jon Olav Hauglid Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server: InnoDB storage engine Severity:S3 (Non-critical)
Version:5.1, 5.5 OS:Any
Assigned to: Jimmy Yang CPU Architecture:Any
Triage: Triaged: D3 (Medium) / R1 (None/Negligible) / E2 (Low)

[26 Jan 2011 17:07] Jon Olav Hauglid
Description:
In the patch for Bug#42230 we fixed the server code so that concurrent
reads will be allowed during index creation if the storage engine supports it.

For InnoDB concurrent reads are currently allowed for non-primary non-unique
indexes. But according to Marko it should also work for non-primary unique
indexes. This can be enabled by the following patch (mysql-trunk):

=== modified file 'storage/innobase/handler/ha_innodb.cc'
--- storage/innobase/handler/ha_innodb.cc       2011-01-26 13:57:04 +0000
+++ storage/innobase/handler/ha_innodb.cc       2011-01-26 16:50:17 +0000
@@ -2794,6 +2794,7 @@ innobase_alter_table_flags(
                | HA_INPLACE_ADD_INDEX_NO_WRITE
                | HA_INPLACE_DROP_INDEX_NO_READ_WRITE
                | HA_INPLACE_ADD_UNIQUE_INDEX_NO_READ_WRITE
+               | HA_INPLACE_ADD_UNIQUE_INDEX_NO_WRITE
                | HA_INPLACE_DROP_UNIQUE_INDEX_NO_READ_WRITE
                | HA_INPLACE_ADD_PK_INDEX_NO_READ_WRITE);
 }

However, adding this flag causes test failures in:
  innodb.innodb-index
  innodb.innodb-index_ucs2
Both give:
  [ERROR] InnoDB could not find index PRIMARY key no 0 for
  table test/t1 through its index translation table

Reporting this as a bug as suggested by Marko.

Note that if support for HA_INPLACE_ADD_UNIQUE_INDEX_NO_WRITE is added,
a currently disabled test case in innodb_mysql_sync.test can be enabled.

How to repeat:
Apply patch above to mysql-trunk and run MTR.

Suggested fix:
N/A.
[31 Jan 2011 4:02] Jimmy Yang
The problem seems due to an "alter table t1 add unique index (b)" did not call ha_innobase::close and ha_innobase::open afterwards to reload the InnoDB dictionary info, so that innobase_build_index_translation() is not called to rebuild the index translation table.

Our original design assumption is that  ha_innobase::open will be called for each create index/drop index call, so that in ::add_index we will invalidate the index translation table by resetting "share->idx_trans_tbl.index_count" to zero in ha_innobase::add_index or ha_innobase::final_drop_index. However, we do not reset "share->idx_trans_tbl.index_mapping" so that we do not need to reallocate memory.

So the question is that why ha_innobase::close and ha_innobase::open is not called after we creating index, as we rely on  ha_innobase::open->innobase_build_index_translation to rebuild index.

Some debugging info, the translation table has following value:

(gdb) p share->idx_trans_tbl
$11 = {index_count = 0, array_size = 1, index_mapping = 0x90d2880}

Note that index_count is zero, however, the array does contain one member, and index_mapping is non-null. 

The reason is that index_count is being reset to zero in ha_innobase::add_index or ha_innobase::final_drop_index when we add a new index or drop a index. And we reset index_count to 0, to tell the code to recreate the translation table when we reopen the table.

      /* We will need to rebuild index translation table. Set
        valid index entry count in the translation table to zero */
        share->idx_trans_tbl.index_count = 0;

We do not reset the array_size and index_mapping value because the memory can be re-used.

Our assumption is that ha_innobase::close and ha_innobase::open will be called for each add index and drop index, so that innobase_build_index_translation() will be called to rebuild the translation table if necessary. 

In this case, following call build the index, but did not call ha_innobase::close and ha_innobase::open  afterwards, so that innobase_build_index_translation() is not called:

#0  ha_innobase::add_index (this=0x9135128, table=0x91134b8, 
    key_info=0x9104c40, num_of_keys=1)
    at /home/jy/work2/mysql-trunk/storage/innobase/handler/handler0alter.cc:865
#1  0x082a5000 in mysql_alter_table (thd=0x9043998, new_db=0x9103fb8 "test", 
    new_name=0x9103bd8 "t1", create_info=0xb1c2672c, table_list=0x9103c00, 
    alter_info=0xb1c267b4, order_num=0, order=0x0, ignore=false)
    at /home/jy/work2/mysql-trunk/sql/sql_table.cc:6676
#2  0x084b665e in Sql_cmd_alter_table::execute (this=0x9104020, thd=0x9043998)
    at /home/jy/work2/mysql-trunk/sql/sql_alter.cc:107
#3  0x0821ca69 in mysql_execute_command (thd=0x9043998)
    at /home/jy/work2/mysql-trunk/sql/sql_parse.cc:4357
#4  0x0821f496 in mysql_parse (thd=0x9043998, 
    rawbuf=0x9103b40 "alter table t1 add unique index (b)", length=35, 
    parser_state=0xb1c27a9c)
    at /home/jy/work2/mysql-trunk/sql/sql_parse.cc:5550
#5  0x082130ca in dispatch_command (command=COM_QUERY, thd=0x9043998, 
    packet=0x90fbb11 "alter table t1 add unique index (b)", packet_length=35)
    at /home/jy/work2/mysql-trunk/sql/sql_parse.cc:1078
#6  0x08212453 in do_command (thd=0x9043998)
    at /home/jy/work2/mysql-trunk/sql/sql_parse.cc:815
#7  0x082ef4fd in do_handle_one_connection (thd_arg=0x9043998)
    at /home/jy/work2/mysql-trunk/sql/sql_connect.cc:748
#8  0x082ef10e in handle_one_connection (arg=0x9043998)

We will need to see if this is a correct or special case, that ha_innobase::open will not be called after create index. Since in such case, index translation table might not be built.
[31 Jan 2011 9:02] Jon Olav Hauglid
The failing test can be simplified to just:

create table t1(a int not null, b int, c char(10), d varchar(20), primary key (a)) engine = innodb;
insert into t1 values (1,1,'ab','ab'),(2,2,'ac','ac'),(3,2,'ad','ad'),(4,4,'afe','afe');
--error ER_DUP_ENTRY
alter table t1 add unique index (b);
select * from t1;  # This triggers sql_print_error() in ha_innodb.cc. 

So this could be a problem with cleanup after ALTER TABLE fails. I'll investigate some more.
[31 Jan 2011 11:43] Jimmy Yang
Ok, Jon, I see the case, the create index failed in row_merge_build_indexes(), and since the create index is failed, the table is not reloaded. We had reset idx_trans_tbl.index_count to zero too early. It should be reset only when all the steps in create index is successful. Will check in the patch.
[31 Jan 2011 12:02] 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/commits/130036

3697 Jimmy Yang	2011-01-31
      Fix Bug #59749 Enabling concurrent reads while creating non-primary
      unique index gives failures
[31 Jan 2011 12:07] Marko Mäkelä
Looks good to me. Previously, we invalidated the idx_trans_tbl too early, before actually completing the index creation.
[15 Feb 2011 6:32] Jimmy Yang
Fix checked in mysql-5.1-innodb and ported to mysql-5.5-innodb and mysql-trunk-innodb