BUG#26395: if crash during autocommit update to transactional table on master, slave fails

Now, every transaction (including autocommit transactions) start with
a BEGIN and end with a COMMIT/ROLLBACK in the binlog.
Added a test case, and updated lots of test case result files.


mysql-test/t/rpl_transaction-master.opt:
  BitKeeper file /home/sven/bk/b26395-autocommit-xa/5.0-rpl/mysql-test/t/rpl_transaction-master.opt
mysql-test/t/rpl_transaction-slave.opt:
  BitKeeper file /home/sven/bk/b26395-autocommit-xa/5.0-rpl/mysql-test/t/rpl_transaction-slave.opt
mysql-test/r/mix_innodb_myisam_binlog.result:
  Updated result file
mysql-test/r/multi_update.result:
  Updated result file
mysql-test/r/rpl_transaction.result:
  New result file for new test case.
mysql-test/r/sp_trans_log.result:
  Updated result file
mysql-test/r/variables-big.result:
  Updated result file
mysql-test/t/rpl_transaction.test:
  New test case.
sql/log.cc:
   - Always write BEGIN and COMMIT around statements, even in autocommit
     mode.
   - Added comments for binlog_commit and binlog_rollback.
sql/log_event.cc:
  Added debug trigger to avoid writing xid events to the binlog.

2 files changed, 72 insertions, 29 deletions

diff --git a/sql/log.cc b/sql/log.cc
index af03cecd462..eca3cf32228 100644
--- a/sql/log.cc
+++ b/sql/log.cc
@@ -122,6 +122,20 @@ static int binlog_prepare(THD *thd, bool all)
   return 0;
 }
 
+/**
+  This function is called once after each statement.
+
+  It has the responsibility to flush the transaction cache to the
+  binlog file on commits.
+
+  @param thd   The client thread that executes the transaction.
+  @param all   true if this is the last statement before a COMMIT
+               statement; false if either this is a statement in a
+               transaction but not the last, or if this is a statement
+               not inside a BEGIN block and autocommit is on.
+
+  @see handlerton::commit
+*/
 static int binlog_commit(THD *thd, bool all)
 {
   IO_CACHE *trans_log= (IO_CACHE*)thd->ha_data[binlog_hton.slot];
@@ -134,7 +148,15 @@ static int binlog_commit(THD *thd, bool all)
     // we're here because trans_log was flushed in MYSQL_LOG::log_xid()
     DBUG_RETURN(0);
   }
-  if (all)
+  /*
+    Write commit event if at least one of the following holds:
+     - the user sends an explicit COMMIT; or
+     - the autocommit flag is on, and we are not inside a BEGIN.
+    However, if the user has not sent an explicit COMMIT, and we are
+    either inside a BEGIN or run with autocommit off, then this is not
+    the end of a transaction and we should not write a commit event.
+  */
+  if (all || !(thd->options & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN)))
   {
     Query_log_event qev(thd, STRING_WITH_LEN("COMMIT"), TRUE, FALSE);
     qev.error_code= 0; // see comment in MYSQL_LOG::write(THD, IO_CACHE)
@@ -144,6 +166,22 @@ static int binlog_commit(THD *thd, bool all)
     DBUG_RETURN(binlog_end_trans(thd, trans_log, &invisible_commit));
 }
 
+/**
+  This function is called when a transaction involving a transactional
+  table is rolled back.
+
+  It has the responsibility to flush the transaction cache to the
+  binlog file. However, if the transaction does not involve
+  non-transactional tables, nothing needs to be logged.
+
+  @param thd   The client thread that executes the transaction.
+  @param all   true if this is the last statement before a COMMIT
+               statement; false if either this is a statement in a
+               transaction but not the last, or if this is a statement
+               not inside a BEGIN block and autocommit is on.
+
+  @see handlerton::rollback
+*/
 static int binlog_rollback(THD *thd, bool all)
 {
   int error=0;
@@ -1817,9 +1855,11 @@ uint MYSQL_LOG::next_file_id()
   IMPLEMENTATION
     - To support transaction over replication, we wrap the transaction
       with BEGIN/COMMIT or BEGIN/ROLLBACK in the binary log.
-      We want to write a BEGIN/ROLLBACK block when a non-transactional table
-      was updated in a transaction which was rolled back. This is to ensure
-      that the same updates are run on the slave.
+      If a transaction that only involves transactional tables is
+      rolled back, we do not binlog it. However, we write a
+      BEGIN/ROLLBACK block when a non-transactional table was updated
+      in a transaction which was rolled back. This is to ensure that
+      the same updates are run on the slave.
 */
 
 bool MYSQL_LOG::write(THD *thd, IO_CACHE *cache, Log_event *commit_event)
@@ -1837,32 +1877,34 @@ bool MYSQL_LOG::write(THD *thd, IO_CACHE *cache, Log_event *commit_event)
     byte header[LOG_EVENT_HEADER_LEN];
 
     /*
-      Log "BEGIN" at the beginning of the transaction.
-      which may contain more than 1 SQL statement.
+      Log "BEGIN" at the beginning of every transaction.  Here, a
+      transaction is either a BEGIN..COMMIT block or a single
+      statement in autocommit mode.
     */
-    if (thd->options & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN))
-    {
-      Query_log_event qinfo(thd, STRING_WITH_LEN("BEGIN"), TRUE, FALSE);
-      /*
-        Imagine this is rollback due to net timeout, after all statements of
-        the transaction succeeded. Then we want a zero-error code in BEGIN.
-        In other words, if there was a really serious error code it's already
-        in the statement's events, there is no need to put it also in this
-        internally generated event, and as this event is generated late it
-        would lead to false alarms.
-        This is safer than thd->clear_error() against kills at shutdown.
-      */
-      qinfo.error_code= 0;
-      /*
-        Now this Query_log_event has artificial log_pos 0. It must be adjusted
-        to reflect the real position in the log. Not doing it would confuse the
-	slave: it would prevent this one from knowing where he is in the
-	master's binlog, which would result in wrong positions being shown to
-	the user, MASTER_POS_WAIT undue waiting etc.
-      */
-      if (qinfo.write(&log_file))
-	goto err;
-    }
+    Query_log_event qinfo(thd, STRING_WITH_LEN("BEGIN"), TRUE, FALSE);
+    /*
+      Imagine this is rollback due to net timeout, after all
+      statements of the transaction succeeded. Then we want a
+      zero-error code in BEGIN.  In other words, if there was a
+      really serious error code it's already in the statement's
+      events, there is no need to put it also in this internally
+      generated event, and as this event is generated late it would
+      lead to false alarms.
+
+      This is safer than thd->clear_error() against kills at shutdown.
+    */
+    qinfo.error_code= 0;
+    /*
+      Now this Query_log_event has artificial log_pos 0. It must be
+      adjusted to reflect the real position in the log. Not doing it
+      would confuse the slave: it would prevent this one from
+      knowing where he is in the master's binlog, which would result
+      in wrong positions being shown to the user, MASTER_POS_WAIT
+      undue waiting etc.
+    */
+    if (qinfo.write(&log_file))
+      goto err;
+
     /* Read from the file used to cache the queries .*/
     if (reinit_io_cache(cache, READ_CACHE, 0, 0, 0))
       goto err;
diff --git a/sql/log_event.cc b/sql/log_event.cc
index 965dfb5f5cf..a950094a018 100644
--- a/sql/log_event.cc
+++ b/sql/log_event.cc
@@ -3793,6 +3793,7 @@ Xid_log_event(const char* buf,
 #ifndef MYSQL_CLIENT
 bool Xid_log_event::write(IO_CACHE* file)
 {
+  DBUG_EXECUTE_IF("do_not_write_xid", return 0;);
   return write_header(file, sizeof(xid)) ||
          my_b_safe_write(file, (byte*) &xid, sizeof(xid));
 }