bzr commit into mysql-6.0-backup branch (Rafal.Somla:2688) Bug#38261

[Rafal Somla]

#At file:///ext/mysql/bzr/backup/bug38261/

 2688 Rafal Somla	2008-09-12
      BUG#38261 (Online backup: BACKUP command does implicit commit)
      
      It was clarified that it is OK for BACKUP to perform implicit commit, so it 
      is not a problem. Also, the server was modified so that SQL commands marked with 
      CF_AUTO_COMMIT_TRANS flag automatically perform implicit commits at the 
      beginning and at the end of execution. The BACKUP and RESTORE commands are 
      already marked with the flag.
      
      Hence, there is no need for doing any commits inside backup kernel code - calls 
      to ha_autocommit_or_rollback() and end_active_trans() are removed from kernel.cc 
      by this patch.
      
      The patch adds new test backup_commit_backup which tests that BACKUP command 
      performs implicit commit for the current transaction and does not interfere with 
      transactions in other connections.
added:
  mysql-test/r/backup_commit_backup.result
  mysql-test/t/backup_commit_backup.test
modified:
  sql/backup/kernel.cc

per-file messages:
  mysql-test/t/backup_commit_backup.test
    New test for checking interraction of BACKUP with transactions in the same 
    and different connections.
  sql/backup/kernel.cc
    Remove code performing commit and add comment explaining how implicit commits
    are done by the server.
=== added file 'mysql-test/r/backup_commit_backup.result'
--- a/mysql-test/r/backup_commit_backup.result	1970-01-01 00:00:00 +0000
+++ b/mysql-test/r/backup_commit_backup.result	2008-09-12 17:19:02 +0000
@@ -0,0 +1,324 @@
+DROP DATABASE IF EXISTS db1;
+DROP DATABASE IF EXISTS db2;
+CREATE DATABASE db1;
+CREATE DATABASE db2;
+CREATE TABLE db1.t1 (s1 CHAR(3)) ENGINE=innodb;
+CREATE TABLE db1.t2 (s1 CHAR(3)) ENGINE=falcon;
+CREATE TABLE db1.t3 (s1 CHAR(3)) ENGINE=memory;
+CREATE TABLE db1.t4 (s1 CHAR(3)) ENGINE=myisam;
+CREATE TABLE db2.t1 (s1 CHAR(3)) ENGINE=innodb;
+CREATE TABLE db2.t2 (s1 CHAR(3)) ENGINE=falcon;
+CREATE TABLE db2.t3 (s1 CHAR(3)) ENGINE=myisam;
+connection B - starting transaction B
+SET autocommit=0;
+BEGIN;
+SELECT get_lock("sync",10);
+get_lock("sync",10)
+1
+INSERT INTO db1.t1 VALUES ('b1');
+INSERT INTO db1.t2 VALUES ('b1');
+INSERT INTO db1.t3 VALUES ('b1');
+INSERT INTO db1.t4 VALUES ('b1');
+INSERT INTO db2.t1 VALUES ('b1');
+INSERT INTO db2.t2 VALUES ('b1');
+INSERT INTO db2.t3 VALUES ('b1');
+SELECT release_lock("sync");
+release_lock("sync")
+1
+SELECT get_lock("sync",10);
+get_lock("sync",10)
+1
+SELECT * FROM db1.t1;
+s1
+SELECT * FROM db1.t2;
+s1
+SELECT * FROM db1.t3;
+s1
+b1
+SELECT * FROM db1.t4;
+s1
+b1
+SELECT * FROM db2.t1;
+s1
+SELECT * FROM db2.t2;
+s1
+SELECT * FROM db2.t3;
+s1
+b1
+SELECT release_lock("sync");
+release_lock("sync")
+1
+connection A - starting transaction A
+SET autocommit=0;
+BEGIN;
+SELECT get_lock("sync",10);
+get_lock("sync",10)
+1
+INSERT INTO db1.t1 VALUES ('a1');
+INSERT INTO db1.t2 VALUES ('a1');
+INSERT INTO db1.t3 VALUES ('a1');
+INSERT INTO db1.t4 VALUES ('a1');
+INSERT INTO db2.t1 VALUES ('a1');
+INSERT INTO db2.t2 VALUES ('a1');
+INSERT INTO db2.t3 VALUES ('a1');
+SELECT release_lock("sync");
+release_lock("sync")
+1
+SELECT get_lock("sync",10);
+get_lock("sync",10)
+1
+SELECT * FROM db1.t1;
+s1
+SELECT * FROM db1.t2;
+s1
+SELECT * FROM db1.t3;
+s1
+b1
+a1
+SELECT * FROM db1.t4;
+s1
+b1
+a1
+SELECT * FROM db2.t1;
+s1
+SELECT * FROM db2.t2;
+s1
+SELECT * FROM db2.t3;
+s1
+b1
+a1
+SELECT release_lock("sync");
+release_lock("sync")
+1
+connection B - performing BACKUP which commits transaction B
+SELECT get_lock("sync",10);
+get_lock("sync",10)
+1
+INSERT INTO db1.t1 VALUES ('b2');
+INSERT INTO db1.t2 VALUES ('b2');
+INSERT INTO db1.t3 VALUES ('b2');
+INSERT INTO db1.t4 VALUES ('b2');
+INSERT INTO db2.t1 VALUES ('b2');
+INSERT INTO db2.t2 VALUES ('b2');
+INSERT INTO db2.t3 VALUES ('b2');
+BACKUP DATABASE db1 TO 'db1.bak';
+backup_id
+#
+SELECT release_lock("sync");
+release_lock("sync")
+1
+SELECT get_lock("sync",10);
+get_lock("sync",10)
+1
+SELECT * FROM db1.t1;
+s1
+b1
+b2
+SELECT * FROM db1.t2;
+s1
+b1
+b2
+SELECT * FROM db1.t3;
+s1
+b1
+a1
+b2
+SELECT * FROM db1.t4;
+s1
+b1
+a1
+b2
+SELECT * FROM db2.t1;
+s1
+b1
+b2
+SELECT * FROM db2.t2;
+s1
+b1
+b2
+SELECT * FROM db2.t3;
+s1
+b1
+a1
+b2
+SELECT release_lock("sync");
+release_lock("sync")
+1
+connection B - doing ROLLBACK which should be a no-op after the implicit commit
+SELECT get_lock("sync",10);
+get_lock("sync",10)
+1
+ROLLBACK;
+SELECT release_lock("sync");
+release_lock("sync")
+1
+SELECT get_lock("sync",10);
+get_lock("sync",10)
+1
+SELECT * FROM db1.t1;
+s1
+b1
+b2
+SELECT * FROM db1.t2;
+s1
+b1
+b2
+SELECT * FROM db1.t3;
+s1
+b1
+a1
+b2
+SELECT * FROM db1.t4;
+s1
+b1
+a1
+b2
+SELECT * FROM db2.t1;
+s1
+b1
+b2
+SELECT * FROM db2.t2;
+s1
+b1
+b2
+SELECT * FROM db2.t3;
+s1
+b1
+a1
+b2
+SELECT release_lock("sync");
+release_lock("sync")
+1
+connection A - finishing transaction A
+SELECT get_lock("sync",10);
+get_lock("sync",10)
+1
+INSERT INTO db1.t1 VALUES ('a2');
+INSERT INTO db1.t2 VALUES ('a2');
+INSERT INTO db1.t3 VALUES ('a2');
+INSERT INTO db1.t4 VALUES ('a2');
+INSERT INTO db2.t1 VALUES ('a2');
+INSERT INTO db2.t2 VALUES ('a2');
+INSERT INTO db2.t3 VALUES ('a2');
+SELECT release_lock("sync");
+release_lock("sync")
+1
+SELECT get_lock("sync",10);
+get_lock("sync",10)
+1
+SELECT * FROM db1.t1;
+s1
+b1
+b2
+SELECT * FROM db1.t2;
+s1
+b1
+b2
+SELECT * FROM db1.t3;
+s1
+b1
+a1
+b2
+a2
+SELECT * FROM db1.t4;
+s1
+b1
+a1
+b2
+a2
+SELECT * FROM db2.t1;
+s1
+b1
+b2
+SELECT * FROM db2.t2;
+s1
+b1
+b2
+SELECT * FROM db2.t3;
+s1
+b1
+a1
+b2
+a2
+SELECT release_lock("sync");
+release_lock("sync")
+1
+SELECT get_lock("sync",10);
+get_lock("sync",10)
+1
+COMMIT;
+SELECT release_lock("sync");
+release_lock("sync")
+1
+SELECT get_lock("sync",10);
+get_lock("sync",10)
+1
+SELECT * FROM db1.t1;
+s1
+b1
+a1
+b2
+a2
+SELECT * FROM db1.t2;
+s1
+b1
+a1
+b2
+a2
+SELECT * FROM db1.t3;
+s1
+b1
+a1
+b2
+a2
+SELECT * FROM db1.t4;
+s1
+b1
+a1
+b2
+a2
+SELECT * FROM db2.t1;
+s1
+b1
+a1
+b2
+a2
+SELECT * FROM db2.t2;
+s1
+b1
+a1
+b2
+a2
+SELECT * FROM db2.t3;
+s1
+b1
+a1
+b2
+a2
+SELECT release_lock("sync");
+release_lock("sync")
+1
+Checking contents of the backup image
+RESTORE FROM 'db1.bak';
+backup_id
+#
+SELECT * FROM db1.t1;
+s1
+b1
+b2
+SELECT * FROM db1.t2;
+s1
+b1
+b2
+SELECT * FROM db1.t3;
+s1
+b1
+a1
+b2
+SELECT * FROM db1.t4;
+s1
+b1
+a1
+b2
+DROP DATABASE db1;
+DROP DATABASE db2;

=== added file 'mysql-test/t/backup_commit_backup.test'
--- a/mysql-test/t/backup_commit_backup.test	1970-01-01 00:00:00 +0000
+++ b/mysql-test/t/backup_commit_backup.test	2008-09-12 17:19:02 +0000
@@ -0,0 +1,230 @@
+#
+# This test tests interraction of BACKUP command with transactions in
+# the same and different connection (BUG#38261)
+#
+# BACKUP should commit ongoing transaction in the same connection but
+# should not interferre with transactions executing in other connections.
+#
+
+--source include/not_embedded.inc
+--source include/have_innodb.inc
+--source include/have_falcon.inc
+
+
+LET $BDIR=`select @@backupdir`;
+
+disable_warnings;
+DROP DATABASE IF EXISTS db1;
+DROP DATABASE IF EXISTS db2;
+error 0,1;
+remove_file $BDIR/db1.bak;
+enable_warnings;
+
+CREATE DATABASE db1;
+CREATE DATABASE db2;
+
+CREATE TABLE db1.t1 (s1 CHAR(3)) ENGINE=innodb; # CS driver
+CREATE TABLE db1.t2 (s1 CHAR(3)) ENGINE=falcon; # CS driver
+CREATE TABLE db1.t3 (s1 CHAR(3)) ENGINE=memory; # default driver
+CREATE TABLE db1.t4 (s1 CHAR(3)) ENGINE=myisam; # native driver
+
+CREATE TABLE db2.t1 (s1 CHAR(3)) ENGINE=innodb; # trx1
+CREATE TABLE db2.t2 (s1 CHAR(3)) ENGINE=falcon; # trx2
+CREATE TABLE db2.t3 (s1 CHAR(3)) ENGINE=myisam; # non-trx
+
+
+# Create test connections. The setup is as follows
+#
+# connA		connB
+# -----		-----
+#		BEGIN
+#		insert data
+# BEGIN
+# insert data
+#		insert data
+#		BACKUP
+#		ROLLBACK
+# insert data
+# COMMIT	
+#
+# The default connection is used to check contents of the tables after each
+# operation.
+#
+# Note: we use "sync" lock to synchronize connections.  Without it, data 
+# inserted in one thread could be not always seen in another one, which would
+# made test non-deterministic.
+
+  connect(connA, localhost, root,,);
+    connect(connB, localhost, root,,);
+
+    connection connB;
+    echo connection B - starting transaction B; 
+    #------------------
+    SET autocommit=0;
+    BEGIN;
+
+    SELECT get_lock("sync",10);
+    INSERT INTO db1.t1 VALUES ('b1');
+    INSERT INTO db1.t2 VALUES ('b1');
+    INSERT INTO db1.t3 VALUES ('b1');
+    INSERT INTO db1.t4 VALUES ('b1');
+
+    INSERT INTO db2.t1 VALUES ('b1');
+    INSERT INTO db2.t2 VALUES ('b1');
+    INSERT INTO db2.t3 VALUES ('b1');
+    SELECT release_lock("sync");
+
+connection default;
+#------------------
+SELECT get_lock("sync",10);
+SELECT * FROM db1.t1;
+SELECT * FROM db1.t2;
+SELECT * FROM db1.t3;
+SELECT * FROM db1.t4;
+
+SELECT * FROM db2.t1;
+SELECT * FROM db2.t2;
+SELECT * FROM db2.t3;
+SELECT release_lock("sync");
+
+  connection connA;
+  echo connection A - starting transaction A;
+  #------------------
+  SET autocommit=0;
+  BEGIN;
+
+  SELECT get_lock("sync",10);
+  INSERT INTO db1.t1 VALUES ('a1');
+  INSERT INTO db1.t2 VALUES ('a1');
+  INSERT INTO db1.t3 VALUES ('a1');
+  INSERT INTO db1.t4 VALUES ('a1');
+
+  INSERT INTO db2.t1 VALUES ('a1');
+  INSERT INTO db2.t2 VALUES ('a1');
+  INSERT INTO db2.t3 VALUES ('a1');
+  SELECT release_lock("sync");
+
+connection default;
+#------------------
+SELECT get_lock("sync",10);
+SELECT * FROM db1.t1;
+SELECT * FROM db1.t2;
+SELECT * FROM db1.t3;
+SELECT * FROM db1.t4;
+
+SELECT * FROM db2.t1;
+SELECT * FROM db2.t2;
+SELECT * FROM db2.t3;
+SELECT release_lock("sync");
+
+    connection connB;
+    echo connection B - performing BACKUP which commits transaction B;
+    #------------------
+    SELECT get_lock("sync",10);
+    INSERT INTO db1.t1 VALUES ('b2');
+    INSERT INTO db1.t2 VALUES ('b2');
+    INSERT INTO db1.t3 VALUES ('b2');
+    INSERT INTO db1.t4 VALUES ('b2');
+
+    INSERT INTO db2.t1 VALUES ('b2');
+    INSERT INTO db2.t2 VALUES ('b2');
+    INSERT INTO db2.t3 VALUES ('b2');
+
+    replace_column 1 #;
+    BACKUP DATABASE db1 TO 'db1.bak';
+    SELECT release_lock("sync");
+
+connection default;
+#------------------
+SELECT get_lock("sync",10);
+SELECT * FROM db1.t1;
+SELECT * FROM db1.t2;
+SELECT * FROM db1.t3;
+SELECT * FROM db1.t4;
+
+SELECT * FROM db2.t1;
+SELECT * FROM db2.t2;
+SELECT * FROM db2.t3;
+SELECT release_lock("sync");
+
+    connection connB;
+    echo connection B - doing ROLLBACK which should be a no-op after the implicit commit;
+    #------------------
+    SELECT get_lock("sync",10);
+    ROLLBACK;
+    SELECT release_lock("sync");
+
+connection default;
+#------------------
+SELECT get_lock("sync",10);
+SELECT * FROM db1.t1;
+SELECT * FROM db1.t2;
+SELECT * FROM db1.t3;
+SELECT * FROM db1.t4;
+
+SELECT * FROM db2.t1;
+SELECT * FROM db2.t2;
+SELECT * FROM db2.t3;
+SELECT release_lock("sync");
+
+  connection connA;
+  echo connection A - finishing transaction A;
+  #------------------
+  SELECT get_lock("sync",10);
+  INSERT INTO db1.t1 VALUES ('a2');
+  INSERT INTO db1.t2 VALUES ('a2');
+  INSERT INTO db1.t3 VALUES ('a2');
+  INSERT INTO db1.t4 VALUES ('a2');
+
+  INSERT INTO db2.t1 VALUES ('a2');
+  INSERT INTO db2.t2 VALUES ('a2');
+  INSERT INTO db2.t3 VALUES ('a2');
+  SELECT release_lock("sync");
+
+connection default;
+#------------------
+SELECT get_lock("sync",10);
+SELECT * FROM db1.t1;
+SELECT * FROM db1.t2;
+SELECT * FROM db1.t3;
+SELECT * FROM db1.t4;
+
+SELECT * FROM db2.t1;
+SELECT * FROM db2.t2;
+SELECT * FROM db2.t3;
+SELECT release_lock("sync");
+
+  connection connA;
+  #------------------
+  SELECT get_lock("sync",10);
+  COMMIT;
+  SELECT release_lock("sync");
+
+connection default;
+#------------------
+SELECT get_lock("sync",10);
+SELECT * FROM db1.t1;
+SELECT * FROM db1.t2;
+SELECT * FROM db1.t3;
+SELECT * FROM db1.t4;
+
+SELECT * FROM db2.t1;
+SELECT * FROM db2.t2;
+SELECT * FROM db2.t3;
+SELECT release_lock("sync");
+
+echo Checking contents of the backup image;
+
+replace_column 1 #;
+RESTORE FROM 'db1.bak';
+
+SELECT * FROM db1.t1;
+SELECT * FROM db1.t2;
+SELECT * FROM db1.t3;
+SELECT * FROM db1.t4;
+
+# Cleanup
+
+DROP DATABASE db1;
+DROP DATABASE db2;
+remove_file $BDIR/db1.bak;

=== modified file 'sql/backup/kernel.cc'
--- a/sql/backup/kernel.cc	2008-08-27 17:30:49 +0000
+++ b/sql/backup/kernel.cc	2008-09-12 17:19:02 +0000
@@ -125,6 +125,10 @@ static int send_reply(Backup_restore_ctx
 
   @note This function sends response to the client (ok, result set or error).
 
+  @note Both BACKUP and RESTORE should perform implicit commit at the beginning
+  and at the end of execution. This is done by the parser after marking these
+  commands with appropriate flags in @c sql_command_flags[] in sql_parse.cc.
+
   @returns 0 on success, error code otherwise.
  */
 
@@ -798,17 +802,6 @@ int Backup_restore_ctx::close()
 
   time_t when= my_time(0);
 
-  // If auto commit is turned off, be sure to commit the transaction
-  /* 
-    Note: this code needs to be refactored (see BUG#38261). When refactoring
-    make sure that errors are detected and reported.
-  */
-  if (m_thd->options & (OPTION_NOT_AUTOCOMMIT | OPTION_BEGIN))
-  {
-    ha_autocommit_or_rollback(m_thd, 0);
-    end_active_trans(m_thd);
-  }
-
   // unlock tables if they are still locked
 
   // FIXME: detect errors if reported.