Bug #28838 duplicate external_lock in mysql_alter_table
Submitted: 1 Jun 2007 14:20 Modified: 31 Jul 2007 19:42
Reporter: Sergei Golubchik Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server Severity:S3 (Non-critical)
Version:5.0 OS:Any
Assigned to: Sergey Vojtovich CPU Architecture:Any

[1 Jun 2007 14:20] Sergei Golubchik
Description:
in mysql_alter_table we have

  else
  {
    VOID(pthread_mutex_lock(&LOCK_open));
    wait_while_table_is_used(thd, table, HA_EXTRA_FORCE_REOPEN);
    table->file->ha_external_lock(thd, F_WRLCK);
    alter_table_manage_keys(table, table->file->indexes_are_disabled(),
                            alter_info->keys_onoff);
    table->file->ha_external_lock(thd, F_UNLCK);
    VOID(pthread_mutex_unlock(&LOCK_open));
  }

this is wrong, because the table is already opened and locked. We should not call external_lock() twice on the same table. LOCK_open is probably unnecessary here too.

How to repeat:
.
[10 Jul 2007 8:38] Konstantin Osipov
Dmitri, this is a confirmation that this external_lock needs to be removed.
[17 Jul 2007 13:50] Sergey Vojtovich
Stealing this one as it is likely required to fix BUG#29806.
[18 Jul 2007 11:36] 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/31079

ChangeSet@1.2529, 2007-07-18 15:39:13+05:00, svoj@mysql.com +1 -0
  BUG#28838 - duplicate external_lock in mysql_alter_table
  
  Removed duplicate call to handler::external_lock() when
  ALTER TABLE that doesn't need to copy a table (quick
  ALTER TABLE) was executed.
  
  Also quick ALTER TABLE doesn't hold LOCK_open anymore when
  it enables/disables indexes.
[20 Jul 2007 9:24] 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/31215

ChangeSet@1.2558, 2007-07-20 13:27:12+05:00, svoj@mysql.com +1 -0
  BUG#28838 - duplicate external_lock in mysql_alter_table
  Fixed wrong test case. Added lost row that appeared after fix for
  this bug.
[20 Jul 2007 9:31] Sergey Vojtovich
Docs note: in 5.1 this fix has user visible change. If a row is inserted and is followed by online alter table within LOCK TABLES the row gets lost.

E.g. the following test case will loose the row (table is empty after unlock tables).
create table t1 (c char(10) default "Two");
lock table t1 write;
insert into t1 values ();
alter table t1 modify c char(10) default "Three";
unlock tables;
select * from t1;
(empty result set)
[20 Jul 2007 10:34] Sergey Vojtovich
Sorry for bad note, I meant that the row was lost prior to this fix.
[24 Jul 2007 19:10] Antony Curtis
Row is still lost on Windows (see pushbuild results)
[27 Jul 2007 16:49] Bugs System
Pushed into 5.1.21-beta
[27 Jul 2007 16:51] Bugs System
Pushed into 5.0.48
[31 Jul 2007 19:01] Paul DuBois
Noted in 5.0.48, 5.1.21 changelogs.

ALTER TABLE acquired duplicate locks under some circumstances. This
could result in loss of a data row.
[31 Jul 2007 19:32] Sergei Golubchik
Paul, I think this should be more precise. For example

Fast ALTER TABLE (that works without rebuilding the table) acquired duplicate locks in the storage engine. In MyISAM, if ALTER TABLE was issued under LOCK TABLE, it caused all data inserted after LOCK TABLE to disappear.

which is still not exact (only data inserted at the end of the table, using "concurrent insert" technique, were disappearing), but better.
[31 Jul 2007 19:42] Paul DuBois
Serg, thanks. I'll use your version.