From: Jon Olav Hauglid Date: November 25 2010 2:50pm Subject: bzr commit into mysql-trunk-bugfixing branch (jon.hauglid:3395) Bug#42230 List-Archive: http://lists.mysql.com/commits/125036 X-Bug: 42230 Message-Id: <201011251451.oAP8GCbG020319@rcsinet13.oracle.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============2254448847006676657==" --===============2254448847006676657== MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Content-Disposition: inline #At file:///export/home/x/mysql-trunk-bugfixing-altertable/ based on revid:magnus.blaudd@stripped 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. modified: mysql-test/r/innodb_mysql_sync.result mysql-test/t/innodb_mysql_sync.test sql/sql_table.cc storage/innobase/handler/ha_innodb.cc storage/innobase/handler/handler0alter.cc === modified file 'mysql-test/r/innodb_mysql_sync.result' --- a/mysql-test/r/innodb_mysql_sync.result 2010-06-25 07:07:18 +0000 +++ b/mysql-test/r/innodb_mysql_sync.result 2010-11-25 14:50:08 +0000 @@ -66,3 +66,23 @@ SELECT ((@id := id) - id) FROM t2; KILL @id; SET DEBUG_SYNC= "now SIGNAL killed"; DROP TABLE t1, t2; +SET DEBUG_SYNC= "RESET"; +# +# Bug#42230 during add index, cannot do queries on storage engines +# that implement add_index +# +DROP DATABASE IF EXISTS db1; +CREATE DATABASE db1; +CREATE TABLE db1.t1(id INT NOT NULL AUTO_INCREMENT PRIMARY KEY, value INT) engine=innodb; +INSERT INTO db1.t1(value) VALUES (1), (2); +SET DEBUG_SYNC= "alter_table_manage_keys SIGNAL manage WAIT_FOR query"; +ALTER TABLE db1.t1 ADD INDEX(value); +SET DEBUG_SYNC= "now WAIT_FOR manage"; +USE db1; +SELECT * FROM t1; +id value +1 1 +2 2 +SET DEBUG_SYNC= "now SIGNAL query"; +DROP DATABASE db1; +SET DEBUG_SYNC= "RESET"; === modified file 'mysql-test/t/innodb_mysql_sync.test' --- a/mysql-test/t/innodb_mysql_sync.test 2010-06-25 07:07:18 +0000 +++ b/mysql-test/t/innodb_mysql_sync.test 2010-11-25 14:50:08 +0000 @@ -104,6 +104,37 @@ SELECT ((@id := id) - id) FROM t2; KILL @id; SET DEBUG_SYNC= "now SIGNAL killed"; DROP TABLE t1, t2; +disconnect con1; +SET DEBUG_SYNC= "RESET"; + + +--echo # +--echo # Bug#42230 during add index, cannot do queries on storage engines +--echo # that implement add_index +--echo # + +--disable_warnings +DROP DATABASE IF EXISTS db1; +--enable_warnings + +CREATE DATABASE db1; +CREATE TABLE db1.t1(id INT NOT NULL AUTO_INCREMENT PRIMARY KEY, value INT) engine=innodb; +INSERT INTO db1.t1(value) VALUES (1), (2); +SET DEBUG_SYNC= "alter_table_manage_keys SIGNAL manage WAIT_FOR query"; +--send ALTER TABLE db1.t1 ADD INDEX(value) + +connect(con1,localhost,root); +SET DEBUG_SYNC= "now WAIT_FOR manage"; +# Neither of these two statements should be blocked +USE db1; +SELECT * FROM t1; +SET DEBUG_SYNC= "now SIGNAL query"; + +connection default; +--reap +DROP DATABASE db1; +disconnect con1; +SET DEBUG_SYNC= "RESET"; # Check that all connections opened by test cases in this file are really === modified file 'sql/sql_table.cc' --- a/sql/sql_table.cc 2010-11-19 09:15:30 +0000 +++ b/sql/sql_table.cc 2010-11-25 14:50:08 +0000 @@ -6606,10 +6606,11 @@ bool mysql_alter_table(THD *thd,char *ne } else { - if (!table->s->tmp_table && + if (!table->s->tmp_table && need_lock_for_indexes && wait_while_table_is_used(thd, table, HA_EXTRA_FORCE_REOPEN)) goto err_new_table_cleanup; thd_proc_info(thd, "manage keys"); + DEBUG_SYNC(thd, "alter_table_manage_keys"); alter_table_manage_keys(table, table->file->indexes_are_disabled(), alter_info->keys_onoff); error= trans_commit_stmt(thd); === modified file 'storage/innobase/handler/ha_innodb.cc' --- a/storage/innobase/handler/ha_innodb.cc 2010-11-17 11:30:54 +0000 +++ b/storage/innobase/handler/ha_innodb.cc 2010-11-25 14:50:08 +0000 @@ -2758,6 +2758,7 @@ innobase_alter_table_flags( uint flags) { return(HA_ONLINE_ADD_INDEX_NO_WRITES + | HA_ONLINE_ADD_INDEX | HA_ONLINE_DROP_INDEX_NO_WRITES | HA_ONLINE_ADD_UNIQUE_INDEX_NO_WRITES | HA_ONLINE_DROP_UNIQUE_INDEX_NO_WRITES === modified file 'storage/innobase/handler/handler0alter.cc' --- a/storage/innobase/handler/handler0alter.cc 2010-11-11 06:08:37 +0000 +++ b/storage/innobase/handler/handler0alter.cc 2010-11-25 14:50:08 +0000 @@ -1009,7 +1009,7 @@ convert_error: user_thd); } - ut_a(innodb_table->n_ref_count == 1); + ut_a(!new_primary || innodb_table->n_ref_count == 1); mem_heap_free(heap); trx_commit_for_mysql(trx); --===============2254448847006676657== MIME-Version: 1.0 Content-Type: text/bzr-bundle; charset="us-ascii"; name="bzr/jon.hauglid@stripped" Content-Transfer-Encoding: 7bit Content-Disposition: inline # Bazaar merge directive format 2 (Bazaar 0.90) # revision_id: jon.hauglid@stripped # target_branch: file:///export/home/x/mysql-trunk-bugfixing-\ # altertable/ # testament_sha1: 1a42715900b2d95923da60d99824a55ac7d0f0af # timestamp: 2010-11-25 15:50:11 +0100 # source_branch: file:///export/home/x/mysql-5.5-bugteam/ # base_revision_id: magnus.blaudd@stripped\ # p1lrmhuqar6uyxrf # # Begin bundle IyBCYXphYXIgcmV2aXNpb24gYnVuZGxlIHY0CiMKQlpoOTFBWSZTWfBHvRwABXJfgECwe/f//3// 3+C////0YAz5c589Xd3rorT0KE4Y7s32p66Hl99h76zQUUtfcb27nhJQmmknlT/SNJnoqeo81R6n ptUGgNAPUAHpDQHqAlU9BSeFNmpPU9qmj1Nqep6gAANAAAAA9QEohoIp4FPSmYUfqmQaaNDQAaAa NAaAAkSJpoip+mR6mBNMpHhR5E/VMCPUGnlADQB6QbUkGkn6SZojIeoA0AAAbUaAAAACSQEDQmmJ MKZpPQieTUxTQ9TRpo09T1GhoDyTbZBnZO9UoHSrZ3PoT9qR0rVr6kxeTJ7w3HfWZ7T82G/WxdNn YPvwZWZk5ZIjsKiqdsHMlKO54OLlQ31SSkEI53R6HiFXNPzuMC2lzcG1WbSl+FXBtc87f2zfZnLf y1/WSVVqOAu1hrD3Vr7vwLTR9B+JTyo6UbUNtsbbGwzfuIlnd3qzSOcZrsjRzX32zsW+lKUoUoyT bk0pK4abtixV5iuspO8eIPIDnMJJ/VSWsiOkYSkZbU6/ZH7mvCE9xRnn6eoOEi2xhMOntcqBIGbC SJqS9tEa2yudg3vH6vM6pwVbqTK3H2A4PyPMcKlV81j0WFowUYo2DCxKOMT52M+jQvKJ1k4j07j1 ypJ0/eEuhxN2XdQ8u1VUK2N17BSUrWqQiUVSMVsSllgt8moI/CCsqbOfGMVc278hthjKgi0OV8Mm JQjSWRDa0XSc8qv88xo1O3DXAsOZs8nGxqD/Mw7D3GfEcS+IccjiYs3xHot6kfTTVf5MnsQuxZXx wnf3BY00Jg0PubGiIlQ4pB4okNMuGdKqAie4X41rEWOoQkbhfQIyBBu9xbOs9PQh1dQeoj7hrZZA cRdfQZUDoJHwPH+CkOE8zX3vFu+Qq9xxt2emAlmOWCoLpsxZkNlmiMMNrnxe8svxgq8xhpqsZCOT kBVuXATA45EUWQOVES24UEY2BDFDNy48USZoCR7RKoVZUMCIJIoJpMzPCY44jQWR3VyrICkfg8pH eToWDpCTFTDgmiEygxySmtFuHBIqc6s4iqJEee0unMefgwqrip4S4kZmTwj5jcOIu7YEGBDO+CFh vOTiNp5otvIjOM0poDLLksC6+iCbQTgZAUrIVbTjwnfdZrCTbPEekwOWIlT2Y7Co6dZHVYpgqgc4 qj0cT5A7KN33Xw0lELpPpGZhdRjNLW3ogLQ1NSw7jevRWbjrgTEnDl29Zy7csB7Juw7RuoQoK3sy NRKW5ZyUw7TluxIICrfIk9PHw4CKbLHASOivMCWFBtJ/LzE4TgLdtsKGLV9NiSabQqN5YdROF9eZ Wbezx6Fyhda7Dm6mFBYg8eYF4wZHF/EiX1GZqRzNPXG2G229A/l96mQoUcFTCV5EZPizQ4jFmIoX i3EQhEs+nLZ9VmnC9JVJBiSHXdXKB+qzJiyHDqOrKCtH56jFYoOIbB/I2V5F3XVtkLUYFNZgVFIm o2r4rEtSss7F8iSkyEYwkCrjIlcNicSzS5g3erUVxCkiu7dSiYT3FQnGhZXAeZZGq0Tqq6SM2GcG BucSIzLWGqgX1i+3Ww2VmBMwxFkoEbiAxRTMCtyUiBUZFZcMe7mbKV1qgoJLabxWEosdU4SCeZMi OL9hKPLfoX7BxbtuzvMjcKE7ygowlg19UQwUTURYTGBjOhOhMuJlpCsuNo2E2sMCbirj3REdTbZC Yq4lr68K1lEWRJZ2K6sknN2CmBbc6hnPmbartghPYQiY2Sgw6OwWr1CPZfhaNVhMGQ2+xEIi1l/Y jTvWkwDpPC8EX+tCZ4rxD/QksH4n9951C/IXgFCxSRacrTQ2PwQqAvLaYkj7aBBYVLBcEVR9jxEk UKgmqr9i+Qx8ROE38wkkYvHwGKySHn0HQB4lMNKDvoXr5kQicRDYuIBakVxSYLCYPGoqh4syJtCc BaMTIBuSzL1cQS79h9DGV04jgG9IeInTywcE4wHLMWD0wOgKyYUaYKTQcFgWqdYOczKjAJkhSsVw YimJQYRAMLwliPSI3khGJUYjDwYsFYlW4M0OgWmo8LfiyqaZqkVDx9ZEmwXloFBi1IYoVhExhbSn ySHna8vKanDF22puxeo3JyQ4YboX5X+XPEhYAbvfEQKyx5Um48jUc2EkX1xIryYKFUVGda/PI+WC GZBnc7E+EtqER8/bMJoI/axqtDQu11IJlBhySeRgUqPqswOgsHLUDxWZZoBORLFSPxGIrJjn8lqh XHaegureiqshBJohMamc4kwevuqQGnCD/iXHvsFcg4HxHvAs7wy7cD49+42ViMZeFBYjSj0QGo/V NIO9YKiOhboCrKkADI6eA47UmFeXr1nhmdQ7eSIi3ZjEhRMsEvC0UsCBEkgtFSWkRm04rqNBjIms ZnZkGlQU2J7AOQj5SxSRy+DypcTkYeUQiizYJUexe5x3DkzapoMzHc53JMFR1lL/HZqHUbzDkZl6 VJHEuSh1mMq0jJMgGTjsJGQevkuoSYR6RQW9Mk9j01usdP4wdVYhdBbcRh8aCsPKrIrPYd1aRwsD sMy7RkyPv0X5lEUecpnCHb4NECKDE3oOVWcwsZMRURpEH6XlAY4ChPFyCJmZzoa84ZnDornvFCJX rR5ovI2mDA6SK0hkZbEEDhyKHYH69xRWN0t6525bVi1XoUZotmByyofZYTBDjPSOsmwF2VhFPx1X zWhO1j5crk9CLkeg1Msl4MwtoeOcE0RQ52k56GGIZzw9s/C5gqjWzoKAao2281N45mrJwe+/n8Ig /GY4MbLdmMwHkQXWxErSPQq+4w70McqhdpyHrkQ7zlocyI8iDh5mQxGq7juLPEuFlIoaneJtguv6 Iv6kMBqx4EuD15m0ceUPa9/ZqdJC3MnDaGnN49JgxZWJESGBsJ0rrHiB00qFttV4lOTSMkdehGA+ I7TMSmC9pw1QBETHVLWgIhHW0MaGaKHF0BrQYnjE4Ou8dJm1ZRK8E5WmrjXJgm3Qhd7irokPQDmm rGb5ZlQpcjnTmAYnrLbT8S0Jd4GCeAxsuSzcYFi8rTh7I6li/qfZzGYvN/lzwoHO02NLgc/xXbBM ZlW5bwHyGfluc9A8etNEdOTMaGgmQ7Nily9Ib0MHHIe9YcF+mjgPd0kMJVGX1G3pmYBhICM4BERo aiXDovh9fgakE9hkMMyNiEOQ5kMixxa8PNeLxEfMyyEEpqlZE7vVf/qQbypGlVymOA0Iaj0svDdo sujZ7GezhI9rbO1MGS0mFEVBKGk2Z0g3c77YnK3G0UFXGoqjRLaZK4jExskxsDOMSPIJMXpnYlTK j4UpKbJRAYYSckMtAltYjBEofN4KYQScrsiiOl5Yl+CJoVaohFCnqDfjSz0X4grmAmnLz13+IMuK 8B3HJF65gnIXY/s0RXbCICofnJy7XJyPagGWJia5pmqQUqiZ5rzrGf0hHGhVkKZJYbBcDg8m0A+7 Tr47TqyR2p0TLvZDJMdcBYLB6etnsGHp/hNFUKjeb4mUVpw5GuQl2c8YlgFNdWWjPZsDjaCDlJqC SaEXJoXyJyY+0u1aoNgZsdKFC2UBciptgNoYGW1a3acgZE7FiObEtFHcqmVFtLR2ohaBKV6kETe4 smnSF1HNWan75G2SmpXslJyUciknuDtUwySm/Te3sHlVk3aR2rPZBKKWUIIyp0EHxPBwFcEa4paZ GB9S9zU1Ac4cZK5qb3BgQGRiA/1Dx+KWymTKblt6FqivJxCjoVLnGcXFldj+gqHA99l11aKu4Q0A qtZN5snMsKBIkUKnYNC5zWtq1ywqSMQhyhe9P0tJSaTfdbhPJnPgN5xZKpTZ8BPEYg9qyVM8U7A6 qZOkVu0rtCoFm3kFolD9HmB96+uIgbU8tYMZohkD1D61RaKJSdi/768lZxhM2ApDdatLkc2RZxwA f67gf6Tv5iI/qlQEXuuO4TkbxLcRrtyjExMGiDLvnN8g2zr2CWc7VNIrsdU4x81srEWNvgPSdZBz 7krMRc4EdCljMouYbVGhar+Ev+LuSKcKEh4I96OA --===============2254448847006676657==--