Re: bk commit into 5.0 tree (davi:1.2498) BUG#25164

[Konstantin Osipov]

* Davi Arnaut <davi@stripped> [07/08/21 06:35]:
> ChangeSet@stripped, 2007-08-20 23:11:08-03:00, davi@stripped +1 -0
>   Bug#25164 create table `a` as select * from `A` hangs
>   
>   If the thr_multi_lock() detects a deadlock (or timeouts), the table handler
> (storage engine) lock must be unlocked. As the function comment indicates: each call to
> external_lock(F_[RD|WR]LOCK) is followed by a call to external_lock(F_UNLCK) and if it is
> not, it is a bug in MySQL.

This changeset comment is a bit too short.

Please describe:

 * what the problem is from user point of view:
   A user creates table A, and then tries to 
   to CREATE TABLE a SELECT from A - and this causes a deadlock
   error, a hang, or fails with a debug assert, but only if the
   storage engine is InnoDB.
  
 * the origin of the problem - InnoDB uses case-insensitive
   collation  when looking up the internal table share and returns
   the same share for 'a' and 'A'. 

 * what causes the user-visible behaviour:
   since the same share is returned to SQL locking subsystem, it
   assumes that the same table is locked in the same session
   first for WRITE, and then for READ, and returns a deadlock
   error. However, the code is wrong in not cleaning up properly
   upon an error, and external locks are left in place, which
   leads to assertion failures and deadlocks.

 * the solution, and the part of it that has been implemented:
    - the SQL layer should properly propagate the deadlock error,
      cleaning up and freeing all resources
    - innodb should not use case-insensitive collation for table
      share hash if table names on disk honour the case.

>   sql/lock.cc@stripped, 2007-08-20 23:11:04-03:00, davi@stripped +2 -0
>     Unlock the table handler if it fails to acquire the thread lock.

Unlock the storage engine "external" table level locks, if MySQL
thr_lock locking subsystem detected a deadlock error.


> diff -Nrup a/sql/lock.cc b/sql/lock.cc
> --- a/sql/lock.cc	2007-08-16 14:51:30 -03:00
> +++ b/sql/lock.cc	2007-08-20 23:11:04 -03:00
> @@ -172,6 +172,8 @@ MYSQL_LOCK *mysql_lock_tables(THD *thd, 
>                                                       thd->lock_id)];
>      if (rc > 1)                                 /* a timeout or a deadlock */
>      {
> +      if (sql_lock->table_count)
> +        VOID(unlock_external(thd, sql_lock->table, sql_lock->table_count));
>        my_error(rc, MYF(0));
>        my_free((gptr) sql_lock,MYF(0));
>        sql_lock= 0;

OK.

Please re-submit the patch with a test case and a bit more
elaborate changeset comments.

After the bug is approved and documented, please open a new bug
about the deadlock error, in Category: InnoDB.

In the meanwhile, I am assigning to you Bug#28870 "check that
table locks are released/reset", since it's in the same area.
28870 should be fixed in 5.1 only.

-- 
-- Konstantin Osipov              Software Developer, Moscow, Russia
-- MySQL AB, www.mysql.com   The best DATABASE COMPANY in the GALAXY