bug#8151 - truncate leaves a transaction open

deadlock in MYSQL_LOG::new_file()
style fixes

mysql-test/r/innodb.result

@@ -906,6 +906,7 @@ truncate table t1;
 ERROR HY000: Can't execute the given command because you have active locked tables or an active transaction
 commit;
 truncate table t1;
+truncate table t1;
 select * from t1;
 a
 insert into t1 values(1),(2);
@@ -924,6 +925,7 @@ a
 1
 2
 truncate table t1;
+truncate table t1;
 insert into t1 values(1),(2);
 delete from t1;
 select * from t1;
@@ -1637,14 +1639,14 @@ t2	CREATE TABLE `t2` (
 drop table t2, t1;
 show status like "binlog_cache_use";
 Variable_name	Value
-Binlog_cache_use	152
+Binlog_cache_use	154
 show status like "binlog_cache_disk_use";
 Variable_name	Value
 Binlog_cache_disk_use	0
 create table t1 (a int) engine=innodb;
 show status like "binlog_cache_use";
 Variable_name	Value
-Binlog_cache_use	153
+Binlog_cache_use	155
 show status like "binlog_cache_disk_use";
 Variable_name	Value
 Binlog_cache_disk_use	1
@@ -1653,7 +1655,7 @@ delete from t1;
 commit;
 show status like "binlog_cache_use";
 Variable_name	Value
-Binlog_cache_use	154
+Binlog_cache_use	156
 show status like "binlog_cache_disk_use";
 Variable_name	Value
 Binlog_cache_disk_use	1

mysql-test/t/innodb.test

@@ -591,6 +591,7 @@ insert into t1 values(1),(2);
 truncate table t1;
 commit;
 truncate table t1;
+truncate table t1;
 select * from t1;
 insert into t1 values(1),(2);
 delete from t1;
@@ -605,6 +606,7 @@ truncate table t1;
 insert into t1 values(1),(2);
 select * from t1;
 truncate table t1;
+truncate table t1;
 insert into t1 values(1),(2);
 delete from t1;
 select * from t1;

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);

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);
 }
 

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);

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;

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.

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)
   {

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);
 };
 
 

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)

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)
   {

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

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;