bug#8151 - truncate leaves a transaction open

deadlock in MYSQL_LOG::new_file()
style fixes


mysql-test/r/innodb.result:
  one more test for truncate
mysql-test/t/innodb.test:
  one more test for truncate
sql/ha_innodb.cc:
  bug#8151 - truncate leaves a transaction open
sql/handler.cc:
  bug#8151 - truncate leaves a transaction open
sql/item.h:
  style fix
sql/item_cmpfunc.cc:
  style fix
sql/item_cmpfunc.h:
  style fix
sql/item_func.cc:
  style fix
sql/item_func.h:
  style fix
sql/log.cc:
  let new_file() to lock LOCK_index,
  don't check for prepared_xids in rotate_and_purge()
  increase thread_safe_increment when LOCK_log is taken
sql/log_event.cc:
  mysqlbinlog now prints a warning if binlog was not closed properly
sql/sql_class.h:
  comments
sql/sql_repl.cc:
  DBUG_ENTER tag corrected

11 files changed, 59 insertions, 58 deletions

diff --git a/sql/ha_innodb.cc b/sql/ha_innodb.cc
index f68ad99ac44..cc37410e5f6 100644
--- a/sql/ha_innodb.cc
+++ b/sql/ha_innodb.cc
@@ -4256,8 +4256,6 @@ ha_innobase::delete_all_rows(void)
 		goto fallback;
 	}
 
-	innobase_commit(thd, 1);
-
 	error = convert_error_code_to_mysql(error, NULL);
 
 	DBUG_RETURN(error);
diff --git a/sql/handler.cc b/sql/handler.cc
index cd3656c6f0d..cc1f1635556 100644
--- a/sql/handler.cc
+++ b/sql/handler.cc
@@ -1606,7 +1606,12 @@ int handler::rename_table(const char * from, const char * to)
 }
 
 /*
-  Tell the handler to turn on or off transaction in the handler
+  Tell the storage engine that it is allowed to "disable transaction" in the
+  handler. It is a hint that ACID is not required - it is used in NDB for
+  ALTER TABLE, for example, when data are copied to temporary table.
+  A storage engine may treat this hint any way it likes. NDB for example
+  starts to commit every now and then automatically.
+  This hint can be safely ignored.
 */
 
 int ha_enable_transaction(THD *thd, bool on)
@@ -1616,7 +1621,15 @@ int ha_enable_transaction(THD *thd, bool on)
   DBUG_ENTER("ha_enable_transaction");
   thd->transaction.on= on;
   if (on)
-    ha_commit(thd);
+  {
+    /*
+      Now all storage engines should have transaction handling enabled.
+      But some may have it enabled all the time - "disabling" transactions
+      is an optimization hint that storage engine is free to ignore.
+      So, let's commit an open transaction (if any) now.
+    */
+    error= end_trans(thd, COMMIT);
+  }
   DBUG_RETURN(error);
 }
 
diff --git a/sql/item.h b/sql/item.h
index 4082e77efad..ca69af39f62 100644
--- a/sql/item.h
+++ b/sql/item.h
@@ -113,7 +113,7 @@ public:
 typedef bool (Item::*Item_processor)(byte *arg);
 typedef Item* (Item::*Item_transformer) (byte *arg);
 
-typedef void (*Item_cond_traverser) (const Item *item, void *arg);
+typedef void (*Cond_traverser) (const Item *item, void *arg);
 
 class Item {
   Item(const Item &);			/* Prevent use of these */
@@ -393,18 +393,17 @@ public:
     return (this->*processor)(arg);
   }
 
-   virtual Item* transform(Item_transformer transformer, byte *arg)
+  virtual Item* transform(Item_transformer transformer, byte *arg)
   {
     return (this->*transformer)(arg);
   }
 
-   virtual void traverse_cond(Item_cond_traverser traverser, 
-			      void *arg,
-			      traverse_order order = POSTFIX)
+   virtual void traverse_cond(Cond_traverser traverser,
+                              void *arg, traverse_order order)
    {
      (*traverser)(this, arg);
    }
- 
+
   virtual bool remove_dependence_processor(byte * arg) { return 0; }
   virtual bool remove_fixed(byte * arg) { fixed= 0; return 0; }
   virtual bool cleanup_processor(byte *arg);
diff --git a/sql/item_cmpfunc.cc b/sql/item_cmpfunc.cc
index 5d764a734bc..3204e5ac711 100644
--- a/sql/item_cmpfunc.cc
+++ b/sql/item_cmpfunc.cc
@@ -2363,9 +2363,8 @@ Item *Item_cond::transform(Item_transformer transformer, byte *arg)
   return Item_func::transform(transformer, arg);
 }
 
-void Item_cond::traverse_cond(Item_cond_traverser traverser, 
-			      void *arg,
-			      traverse_order order)
+void Item_cond::traverse_cond(Cond_traverser traverser,
+                              void *arg, traverse_order order)
 {
   List_iterator<Item> li(list);
   Item *item;
diff --git a/sql/item_cmpfunc.h b/sql/item_cmpfunc.h
index 3ec7131aac1..212a3dd4501 100644
--- a/sql/item_cmpfunc.h
+++ b/sql/item_cmpfunc.h
@@ -1028,9 +1028,7 @@ public:
   void copy_andor_arguments(THD *thd, Item_cond *item);
   bool walk(Item_processor processor, byte *arg);
   Item *transform(Item_transformer transformer, byte *arg);
-  void traverse_cond(Item_cond_traverser, 
-		     void *arg,
-		     traverse_order order = POSTFIX);
+  void traverse_cond(Cond_traverser, void *arg, traverse_order order);
   void neg_arguments(THD *thd);
 };
 
@@ -1039,8 +1037,8 @@ public:
   The class Item_equal is used to represent conjunctions of equality
   predicates of the form field1 = field2, and field=const in where
   conditions and on expressions.
- 
-  All equality predicates of the form field1=field2 contained in a 
+
+  All equality predicates of the form field1=field2 contained in a
   conjunction are substituted for a sequence of items of this class.
   An item of this class Item_equal(f1,f2,...fk) represents a
   multiple equality f1=f2=...=fk.
diff --git a/sql/item_func.cc b/sql/item_func.cc
index 19228108bfc..5ca2ee2b11f 100644
--- a/sql/item_func.cc
+++ b/sql/item_func.cc
@@ -360,9 +360,8 @@ bool Item_func::walk (Item_processor processor, byte *argument)
   return (this->*processor)(argument);
 }
 
-void Item_func::traverse_cond(Item_cond_traverser traverser, 
-			      void *argument,
-			      traverse_order order)
+void Item_func::traverse_cond(Cond_traverser traverser,
+                              void *argument, traverse_order order)
 {
   if (arg_count)
   {
diff --git a/sql/item_func.h b/sql/item_func.h
index b3fa73bb15b..b1ccbfeea53 100644
--- a/sql/item_func.h
+++ b/sql/item_func.h
@@ -163,9 +163,8 @@ public:
                         uint flags= 0);
   bool walk(Item_processor processor, byte *arg);
   Item *transform(Item_transformer transformer, byte *arg);
-  void traverse_cond(Item_cond_traverser traverser, 
-		     void * arg,
-		     traverse_order order = POSTFIX);
+  void traverse_cond(Cond_traverser traverser,
+                     void * arg, traverse_order order);
 };
 
 
diff --git a/sql/log.cc b/sql/log.cc
index 56d16c9c7cf..5d77197a9b9 100644
--- a/sql/log.cc
+++ b/sql/log.cc
@@ -1296,10 +1296,9 @@ void MYSQL_LOG::new_file(bool need_lock)
   }
 
   if (need_lock)
-  {
     pthread_mutex_lock(&LOCK_log);
-    pthread_mutex_lock(&LOCK_index);
-  }
+  pthread_mutex_lock(&LOCK_index);
+
   safe_mutex_assert_owner(&LOCK_log);
   safe_mutex_assert_owner(&LOCK_index);
 
@@ -1377,10 +1376,9 @@ void MYSQL_LOG::new_file(bool need_lock)
 
 end:
   if (need_lock)
-  {
-    pthread_mutex_unlock(&LOCK_index);
     pthread_mutex_unlock(&LOCK_log);
-  }
+  pthread_mutex_unlock(&LOCK_index);
+
   DBUG_VOID_RETURN;
 }
 
@@ -1404,11 +1402,7 @@ bool MYSQL_LOG::append(Log_event* ev)
   bytes_written+= ev->data_written;
   DBUG_PRINT("info",("max_size: %lu",max_size));
   if ((uint) my_b_append_tell(&log_file) > max_size)
-  {
-    pthread_mutex_lock(&LOCK_index);
     new_file(0);
-    pthread_mutex_unlock(&LOCK_index);
-  }
 
 err:
   pthread_mutex_unlock(&LOCK_log);
@@ -1423,9 +1417,9 @@ bool MYSQL_LOG::appendv(const char* buf, uint len,...)
   DBUG_ENTER("MYSQL_LOG::appendv");
   va_list(args);
   va_start(args,len);
-  
+
   DBUG_ASSERT(log_file.type == SEQ_READ_APPEND);
-  
+
   pthread_mutex_lock(&LOCK_log);
   do
   {
@@ -1438,11 +1432,7 @@ bool MYSQL_LOG::appendv(const char* buf, uint len,...)
   } while ((buf=va_arg(args,const char*)) && (len=va_arg(args,uint)));
   DBUG_PRINT("info",("max_size: %lu",max_size));
   if ((uint) my_b_append_tell(&log_file) > max_size)
-  {
-    pthread_mutex_lock(&LOCK_index);
     new_file(0);
-    pthread_mutex_unlock(&LOCK_index);
-  }
 
 err:
   pthread_mutex_unlock(&LOCK_log);
@@ -1774,15 +1764,10 @@ err:
 
 void MYSQL_LOG::rotate_and_purge(uint flags)
 {
-  if (!prepared_xids &&             // see new_file() for the explanation
-      ((flags & RP_FORCE_ROTATE) ||
-       (my_b_tell(&log_file) >= (my_off_t) max_size)))
+  if ((flags & RP_FORCE_ROTATE) ||
+      (my_b_tell(&log_file) >= (my_off_t) max_size))
   {
-    if (flags & RP_LOCK_LOG_IS_ALREADY_LOCKED)
-      pthread_mutex_lock(&LOCK_index);
     new_file(!(flags & RP_LOCK_LOG_IS_ALREADY_LOCKED));
-    if (flags & RP_LOCK_LOG_IS_ALREADY_LOCKED)
-      pthread_mutex_unlock(&LOCK_index);
 #ifdef HAVE_REPLICATION
     // QQ why do we need #ifdef here ???
     if (expire_logs_days)
@@ -1828,9 +1813,8 @@ uint MYSQL_LOG::next_file_id()
 
 bool MYSQL_LOG::write(THD *thd, IO_CACHE *cache, Log_event *commit_event)
 {
-  bool error= 0;
-  VOID(pthread_mutex_lock(&LOCK_log));
   DBUG_ENTER("MYSQL_LOG::write(THD *, IO_CACHE *, Log_event *)");
+  VOID(pthread_mutex_lock(&LOCK_log));
 
   if (likely(is_open()))                       // Should always be true
   {
@@ -1888,12 +1872,22 @@ DBUG_skip_commit:
       goto err;
     }
     signal_update();
-    DBUG_PRINT("info",("max_size: %lu",max_size));
-    rotate_and_purge(RP_LOCK_LOG_IS_ALREADY_LOCKED);
+    /*
+      if commit_event is Xid_log_event, increase the number of
+      prepared_xids (it's decreasd in ::unlog()). Binlog cannot be rotated
+      if there're prepared xids in it - see the comment in new_file() for
+      an explanation.
+      If the commit_event is not Xid_log_event (then it's a Query_log_event)
+      rotate binlog, if necessary.
+    */
+    if (commit_event->get_type_code() == XID_EVENT)
+      thread_safe_increment(prepared_xids, &LOCK_prep_xids);
+    else
+      rotate_and_purge(RP_LOCK_LOG_IS_ALREADY_LOCKED);
   }
   VOID(pthread_mutex_unlock(&LOCK_log));
 
-  DBUG_RETURN(error);
+  DBUG_RETURN(0);
 
 err:
   if (!write_error)
@@ -2992,7 +2986,6 @@ int TC_LOG_BINLOG::log(THD *thd, my_xid xid)
 {
   Xid_log_event xle(thd, xid);
   IO_CACHE *trans_log= (IO_CACHE*)thd->ha_data[binlog_hton.slot];
-  thread_safe_increment(prepared_xids, &LOCK_prep_xids);
   return !binlog_end_trans(thd, trans_log, &xle);  // invert return value
 }
 
@@ -3000,7 +2993,7 @@ void TC_LOG_BINLOG::unlog(ulong cookie, my_xid xid)
 {
   if (thread_safe_dec_and_test(prepared_xids, &LOCK_prep_xids))
     pthread_cond_signal(&COND_prep_xids);
-  rotate_and_purge(0);     // in case ::write() was not able to rotate
+  rotate_and_purge(0);     // as ::write() did not rotate
 }
 
 int TC_LOG_BINLOG::recover(IO_CACHE *log, Format_description_log_event *fdle)
diff --git a/sql/log_event.cc b/sql/log_event.cc
index 7f92159208b..813636a9ba5 100644
--- a/sql/log_event.cc
+++ b/sql/log_event.cc
@@ -1684,6 +1684,9 @@ void Start_log_event_v3::print(FILE* file, bool short_form, LAST_EVENT_INFO* las
     if (created)
       fprintf(file," at startup");
     fputc('\n', file);
+    if (flags & LOG_EVENT_BINLOG_IN_USE_F)
+      fprintf(file, "# Warning: this binlog was not closed properly. "
+              "Most probably mysqld crashed writing it.\n");
   }
   if (!artificial_event && created)
   {
diff --git a/sql/sql_class.h b/sql/sql_class.h
index 2cd525f54bc..ff30faf1150 100644
--- a/sql/sql_class.h
+++ b/sql/sql_class.h
@@ -1061,9 +1061,9 @@ public:
     SAVEPOINT *savepoints;
     THD_TRANS all;			// Trans since BEGIN WORK
     THD_TRANS stmt;			// Trans for current statement
-    bool on;
-    XID  xid;
-    enum xa_states xa_state;
+    bool on;                            // see ha_enable_transaction()
+    XID  xid;                           // transaction identifier
+    enum xa_states xa_state;            // used by external XA only
     /*
        Tables changed in transaction (that must be invalidated in query cache).
        List contain only transactional tables, that not invalidated in query
diff --git a/sql/sql_repl.cc b/sql/sql_repl.cc
index 3b4e822a3df..b3c963f5f03 100644
--- a/sql/sql_repl.cc
+++ b/sql/sql_repl.cc
@@ -1271,7 +1271,7 @@ int cmp_master_pos(const char* log_file_name1, ulonglong log_pos1,
 bool mysql_show_binlog_events(THD* thd)
 {
   Protocol *protocol= thd->protocol;
-  DBUG_ENTER("show_binlog_events");
+  DBUG_ENTER("mysql_show_binlog_events");
   List<Item> field_list;
   const char *errmsg = 0;
   IO_CACHE log;