bk commit into 5.1 tree (svoj:1.2559) BUG#29806

[Sergey Vojtovich]

Below is the list of changes that have just been committed into a local
5.1 repository of svoj. When svoj does a push these changes will
be propagated to the main repository and, within 24 hours after the
push, to the public repository.
For information on how to access the public repository
see http://dev.mysql.com/doc/mysql/en/installing-source-tree.html

ChangeSet@stripped, 2007-07-25 19:56:17+05:00, svoj@stripped +5 -0
  BUG#29806 - binlog_innodb.test creates a server log
  
  Stopping mysql server could result in an entry in mysql error
  file: "InnoDB: Error: MySQL is freeing a thd".
  
  This happened because InnoDB assumes that the server will never
  call external_lock(F_UNLCK) in case external_lock(READ/WRITE)
  failed.
  
  Prior to this patch we haven't had strict definition whether
  external_lock(F_UNLCK) must be called in case external_lock(READ/WRITE)
  fails.
  
  This patch states that we never call external_lock(F_UNLCK) in case
  external_lock(READ/WRITE) fails.

  mysql-test/suite/binlog/t/disabled.def@stripped, 2007-07-25 19:56:13+05:00, svoj@stripped +0
-3
    Re-enabled binlog_innodb and binlog_killed tests.

  sql/ha_ndbcluster.cc@stripped, 2007-07-25 19:56:13+05:00, svoj@stripped +6 -0
    Restore handler state in case external_lock() failed.

  sql/ha_partition.cc@stripped, 2007-07-25 19:56:14+05:00, svoj@stripped +14 -1
    Do not call external_lock(F_UNLCK) in case external_lock(READ/WRITE) failed.

  sql/lock.cc@stripped, 2007-07-25 19:56:14+05:00, svoj@stripped +7 -1
    Do not call external_lock(F_UNLCK) in case external_lock(READ/WRITE) failed.

  storage/myisammrg/myrg_locking.c@stripped, 2007-07-25 19:56:14+05:00, svoj@stripped +8 -0
    Restore handler state in case external_lock() failed.

diff -Nrup a/mysql-test/suite/binlog/t/disabled.def
b/mysql-test/suite/binlog/t/disabled.def
--- a/mysql-test/suite/binlog/t/disabled.def	2007-07-17 12:24:58 +05:00
+++ b/mysql-test/suite/binlog/t/disabled.def	2007-07-25 19:56:13 +05:00
@@ -9,6 +9,3 @@
 #  Do not use any TAB characters for whitespace.
 #
 ##############################################################################
-
-binlog_innodb        : Bug#29806 2007-07-15 ingo master.err: InnoDB: Error: MySQL is
freeing a thd
-binlog_killed        : Bug#29806 2007-07-17 ingo master.err: InnoDB: Error: MySQL is
freeing a thd
diff -Nrup a/sql/ha_ndbcluster.cc b/sql/ha_ndbcluster.cc
--- a/sql/ha_ndbcluster.cc	2007-07-09 15:04:52 +05:00
+++ b/sql/ha_ndbcluster.cc	2007-07-25 19:56:13 +05:00
@@ -4383,7 +4383,10 @@ int ha_ndbcluster::external_lock(THD *th
 
         trans= ndb->startTransaction();
         if (trans == NULL)
+        {
+          thd_ndb->lock_count= 0;
           ERR_RETURN(ndb->getNdbError());
+        }
         thd_ndb->init_open_tables();
         thd_ndb->stmt= trans;
 	thd_ndb->query_state&= NDB_QUERY_NORMAL;
@@ -4409,7 +4412,10 @@ int ha_ndbcluster::external_lock(THD *th
           
           trans= ndb->startTransaction();
           if (trans == NULL)
+          {
+            thd_ndb->lock_count= 0;
             ERR_RETURN(ndb->getNdbError());
+          }
           thd_ndb->init_open_tables();
           thd_ndb->all= trans; 
 	  thd_ndb->query_state&= NDB_QUERY_NORMAL;
diff -Nrup a/sql/ha_partition.cc b/sql/ha_partition.cc
--- a/sql/ha_partition.cc	2007-07-05 00:55:21 +05:00
+++ b/sql/ha_partition.cc	2007-07-25 19:56:14 +05:00
@@ -1142,6 +1142,12 @@ int ha_partition::prepare_new_partition(
   create_flag= TRUE;
   if ((error= file->ha_open(table, part_name, m_mode, m_open_test_lock)))
     goto error;
+  /*
+    Note: if you plan to add another call that may return failure,
+    better to do it before external_lock() as cleanup_new_partition()
+    assumes that external_lock() is last call that may fail here.
+    Otherwise see description for cleanup_new_partition().
+  */
   if ((error= file->external_lock(current_thd, m_lock_type)))
     goto error;
 
@@ -1164,6 +1170,14 @@ error:
     NONE
 
   DESCRIPTION
+    This function is called immediately after prepare_new_partition() in
+    case the latter fails.
+
+    In prepare_new_partition() last call that may return failure is
+    external_lock(). That means if prepare_new_partition() fails,
+    partition does not have external lock. Thus no need to call
+    external_lock(F_UNLCK) here.
+
   TODO:
     We must ensure that in the case that we get an error during the process
     that we call external_lock with F_UNLCK, close the table and delete the
@@ -1182,7 +1196,6 @@ void ha_partition::cleanup_new_partition
     m_file= m_added_file;
     m_added_file= NULL;
 
-    external_lock(current_thd, F_UNLCK);
     /* delete_table also needed, a bit more complex */
     close();
 
diff -Nrup a/sql/lock.cc b/sql/lock.cc
--- a/sql/lock.cc	2007-06-19 01:13:18 +05:00
+++ b/sql/lock.cc	2007-07-25 19:56:14 +05:00
@@ -60,6 +60,11 @@
   - When calling UNLOCK TABLES we call mysql_unlock_tables() for all
     tables used in LOCK TABLES
 
+  If table_handler->external_lock(thd, locktype) fails, we call
+  table_handler->external_lock(thd, F_UNLCK) for each table that was locked,
+  excluding one that caused failure. That means handler must cleanup itself
+  in case external_lock() fails.
+
 TODO:
   Change to use my_malloc() ONLY when using LOCK TABLES command or when
   we are forced to use mysql_lock_merge.
@@ -269,8 +274,9 @@ static int lock_external(THD *thd, TABLE
     if ((error=(*tables)->file->ha_external_lock(thd,lock_type)))
     {
       print_lock_error(error, (*tables)->file->table_type());
-      for (; i-- ; tables--)
+      while (--i)
       {
+        tables--;
 	(*tables)->file->ha_external_lock(thd, F_UNLCK);
 	(*tables)->current_lock=F_UNLCK;
       }
diff -Nrup a/storage/myisammrg/myrg_locking.c b/storage/myisammrg/myrg_locking.c
--- a/storage/myisammrg/myrg_locking.c	2006-12-31 04:06:41 +04:00
+++ b/storage/myisammrg/myrg_locking.c	2007-07-25 19:56:14 +05:00
@@ -37,7 +37,15 @@ int myrg_lock_database(MYRG_INFO *info, 
     (file->table)->owned_by_merge = TRUE;
 #endif
     if ((new_error=mi_lock_database(file->table,lock_type)))
+    {
       error=new_error;
+      if (lock_type != F_UNLCK)
+      {
+        while (--file >= info->open_tables)
+          mi_lock_database(file->table, F_UNLCK);
+        break;
+      }
+    }
   }
   return(error);
 }