From: Date: September 3 2008 3:29pm
Subject: bzr commit into mysql-6.0-backup branch (rsomla:2688) Bug#38261
List-Archive: http://lists.mysql.com/commits/53164
X-Bug: 38261
Message-Id: <E1KasQj-0002wK-Au@localhost.localdomain>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit

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

 2688 Rafal Somla	2008-09-03
      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 backup_commit_restore test is extended to check that also BACKUP does
      an implicit commit. A new test scenario is added to check how BACKUP and
      RESTORE commands interact with transactions running in other connections. 
modified:
  mysql-test/r/backup_commit_restore.result
  mysql-test/t/backup_commit_restore.test
  sql/backup/kernel.cc

per-file messages:
  mysql-test/t/backup_commit_restore.test
    - add selects to check that BACKUP commits active transaction
    - add new test scenario with several connections
    - add more clean-up (deleting backup image files)
  sql/backup/kernel.cc
    Remove code performing commit.
=== modified file 'mysql-test/r/backup_commit_restore.result'
--- a/mysql-test/r/backup_commit_restore.result	2008-06-05 12:26:31 +0000
+++ b/mysql-test/r/backup_commit_restore.result	2008-09-03 13:29:39 +0000
@@ -1,12 +1,16 @@
+DROP DATABASE IF EXISTS commit_test;
 CREATE DATABASE commit_test;
 USE commit_test;
 SET @@autocommit=0;
 CREATE TABLE t1 (s1 CHAR(2)) ENGINE=innodb;
 INSERT INTO t1 VALUES ('a1');
-BACKUP DATABASE commit_test TO '81';
+BACKUP DATABASE commit_test TO 'commit_test.bak';
 backup_id
 #
-RESTORE FROM '81';
+SELECT * FROM t1;
+s1
+a1
+RESTORE FROM 'commit_test.bak';
 backup_id
 #
 SELECT * FROM t1;
@@ -21,10 +25,17 @@ SET @@autocommit=0;
 CREATE TABLE t2 (s1 CHAR(2)) ENGINE=Memory;
 INSERT INTO t1 VALUES ('a2');
 INSERT INTO t2 VALUES ('a2');
-BACKUP DATABASE commit_test TO '82';
+BACKUP DATABASE commit_test TO 'commit_test.bak';
 backup_id
 #
-RESTORE FROM '82';
+SELECT * FROM t1;
+s1
+a1
+a2
+SELECT * FROM t2;
+s1
+a2
+RESTORE FROM 'commit_test.bak';
 backup_id
 #
 SELECT * FROM t1;
@@ -48,10 +59,22 @@ CREATE TABLE t3 (s1 CHAR(2));
 INSERT INTO t1 VALUES ('a3');
 INSERT INTO t2 VALUES ('a3');
 INSERT INTO t3 VALUES ('a3');
-BACKUP DATABASE commit_test TO '83';
+BACKUP DATABASE commit_test TO 'commit_test.bak';
 backup_id
 #
-RESTORE FROM '83';
+SELECT * FROM t1;
+s1
+a1
+a2
+a3
+SELECT * FROM t2;
+s1
+a2
+a3
+SELECT * FROM t3;
+s1
+a3
+RESTORE FROM 'commit_test.bak';
 backup_id
 #
 SELECT * FROM t1;
@@ -86,10 +109,28 @@ INSERT INTO t1 VALUES ('a4');
 INSERT INTO t2 VALUES ('a4');
 INSERT INTO t3 VALUES ('a4');
 INSERT INTO t4 VALUES ('a4');
-BACKUP DATABASE commit_test TO '84';
+BACKUP DATABASE commit_test TO 'commit_test.bak';
 backup_id
 #
-RESTORE FROM '84';
+SELECT * FROM t1;
+s1
+a1
+a2
+a3
+a4
+SELECT * FROM t2;
+s1
+a2
+a3
+a4
+SELECT * FROM t3;
+s1
+a3
+a4
+SELECT * FROM t4;
+s1
+a4
+RESTORE FROM 'commit_test.bak';
 backup_id
 #
 SELECT * FROM t1;
@@ -135,10 +176,10 @@ INSERT INTO t1 VALUES ('a5');
 INSERT INTO t2 VALUES ('a5');
 INSERT INTO t3 VALUES ('a5');
 INSERT INTO t4 VALUES ('a5');
-BACKUP DATABASE commit_test TO '85';
+BACKUP DATABASE commit_test TO 'commit_test.bak';
 backup_id
 #
-RESTORE FROM '85';
+RESTORE FROM 'commit_test.bak';
 backup_id
 #
 SELECT * FROM t1;
@@ -192,11 +233,11 @@ INSERT INTO t1 VALUES ('a6');
 INSERT INTO t2 VALUES ('a6');
 INSERT INTO t3 VALUES ('a6');
 INSERT INTO t4 VALUES ('a6');
-BACKUP DATABASE commit_test TO '86';
+BACKUP DATABASE commit_test TO 'commit_test.bak';
 backup_id
 #
 SET @@autocommit=0;
-RESTORE FROM '86';
+RESTORE FROM 'commit_test.bak';
 backup_id
 #
 SELECT * FROM t1;
@@ -258,11 +299,11 @@ INSERT INTO t1 VALUES ('a7');
 INSERT INTO t2 VALUES ('a7');
 INSERT INTO t3 VALUES ('a7');
 INSERT INTO t4 VALUES ('a7');
-BACKUP DATABASE commit_test TO '87';
+BACKUP DATABASE commit_test TO 'commit_test.bak';
 backup_id
 #
 SET @@autocommit=1;
-RESTORE FROM '87';
+RESTORE FROM 'commit_test.bak';
 backup_id
 #
 SELECT * FROM t1;
@@ -329,3 +370,106 @@ a7
 COMMIT;
 DROP DATABASE commit_test;
 COMMIT;
+DROP TABLE IF EXISTS test.t1;
+DROP DATABASE IF EXISTS db;
+CREATE DATABASE db;
+CREATE TABLE db.t1 (s1 CHAR(2)) ENGINE=innodb;
+CREATE TABLE test.t2 (s1 CHAR(2)) ENGINE=innodb;
+connection A - starting transaction A1
+SET autocommit=0;
+BEGIN;
+INSERT INTO db.t1 VALUES ('a1');
+INSERT INTO test.t2 VALUES ('a2');
+SELECT * FROM db.t1;
+s1
+SELECT * FROM test.t2;
+s1
+connection B - starting transaction B
+SET autocommit=0;
+BEGIN;
+INSERT INTO db.t1 VALUES ('b1');
+INSERT INTO test.t2 VALUES ('b2');
+SELECT * FROM db.t1;
+s1
+SELECT * FROM test.t2;
+s1
+connection B - performing BACKUP which commits transaction B
+BACKUP DATABASE db TO 'db.bak';
+backup_id
+#
+SELECT * FROM db.t1;
+s1
+b1
+SELECT * FROM test.t2;
+s1
+b2
+connection C - starting transaction C
+SET autocommit=0;
+BEGIN;
+INSERT INTO db.t1 VALUES ('c1');
+INSERT INTO test.t2 VALUES ('c2');
+SELECT * FROM db.t1;
+s1
+b1
+SELECT * FROM test.t2;
+s1
+b2
+connection A - finishing transaction A1
+INSERT INTO db.t1 VALUES ('a3');
+INSERT INTO test.t2 VALUES ('a4');
+COMMIT;
+SELECT * FROM db.t1;
+s1
+a1
+b1
+a3
+SELECT * FROM test.t2;
+s1
+a2
+b2
+a4
+connection A - starting transaction A2
+BEGIN;
+INSERT INTO db.t1 VALUES ('a5');
+INSERT INTO test.t2 VALUES ('a6');
+SELECT * FROM db.t1;
+s1
+a1
+b1
+a3
+SELECT * FROM test.t2;
+s1
+a2
+b2
+a4
+connection C - performing RESTORE which commits transaction C
+RESTORE FROM 'db.bak';
+backup_id
+#
+SELECT * FROM db.t1;
+s1
+b1
+SELECT * FROM test.t2;
+s1
+a2
+b2
+c2
+a4
+connection A - finishing transaction A2
+INSERT INTO db.t1 VALUES ('a7');
+INSERT INTO test.t2 VALUES ('a8');
+COMMIT;
+SELECT * FROM db.t1;
+s1
+b1
+a7
+SELECT * FROM test.t2;
+s1
+a2
+b2
+c2
+a4
+a6
+a8
+DROP TABLE test.t2;
+DROP DATABASE db;

=== modified file 'mysql-test/t/backup_commit_restore.test'
--- a/mysql-test/t/backup_commit_restore.test	2008-06-05 12:26:31 +0000
+++ b/mysql-test/t/backup_commit_restore.test	2008-09-03 13:29:39 +0000
@@ -6,6 +6,17 @@
 # tests the drivers individually and in combination.  It also tests
 # different combinations of turning autocommit on and off before
 # backup and restore.  This test the behavior reported in BUG#34210
+# There is also a multi-connection test to test interraction of BACKUP/RESTORE
+# with transactions in other connections.
+
+# Pre-cleanup
+
+LET $BDIR=`select @@backupdir`;
+disable_warnings;
+DROP DATABASE IF EXISTS commit_test;
+error 0,1;
+remove_file $BDIR/commit_test.bak;
+enable_warnings;
 
 CREATE DATABASE commit_test;
 USE commit_test;
@@ -16,10 +27,12 @@ CREATE TABLE t1 (s1 CHAR(2)) ENGINE=inno
 INSERT INTO t1 VALUES ('a1');
 
 replace_column 1 #;
-BACKUP DATABASE commit_test TO '81';
+BACKUP DATABASE commit_test TO 'commit_test.bak';
+
+SELECT * FROM t1;
 
 replace_column 1 #;
-RESTORE FROM '81';
+RESTORE FROM 'commit_test.bak';
 
 SELECT * FROM t1;
 ROLLBACK;
@@ -32,11 +45,15 @@ CREATE TABLE t2 (s1 CHAR(2)) ENGINE=Memo
 INSERT INTO t1 VALUES ('a2');
 INSERT INTO t2 VALUES ('a2');
 
+remove_file $BDIR/commit_test.bak;
 replace_column 1 #;
-BACKUP DATABASE commit_test TO '82';
+BACKUP DATABASE commit_test TO 'commit_test.bak';
+
+SELECT * FROM t1;
+SELECT * FROM t2;
 
 replace_column 1 #;
-RESTORE FROM '82';
+RESTORE FROM 'commit_test.bak';
 
 SELECT * FROM t1;
 SELECT * FROM t2;
@@ -53,11 +70,16 @@ INSERT INTO t1 VALUES ('a3');
 INSERT INTO t2 VALUES ('a3');
 INSERT INTO t3 VALUES ('a3');
 
+remove_file $BDIR/commit_test.bak;
 replace_column 1 #;
-BACKUP DATABASE commit_test TO '83';
+BACKUP DATABASE commit_test TO 'commit_test.bak';
+
+SELECT * FROM t1;
+SELECT * FROM t2;
+SELECT * FROM t3;
 
 replace_column 1 #;
-RESTORE FROM '83';
+RESTORE FROM 'commit_test.bak';
 
 SELECT * FROM t1;
 SELECT * FROM t2;
@@ -76,11 +98,17 @@ INSERT INTO t2 VALUES ('a4');
 INSERT INTO t3 VALUES ('a4');
 INSERT INTO t4 VALUES ('a4');
 
+remove_file $BDIR/commit_test.bak;
 replace_column 1 #;
-BACKUP DATABASE commit_test TO '84';
+BACKUP DATABASE commit_test TO 'commit_test.bak';
+
+SELECT * FROM t1;
+SELECT * FROM t2;
+SELECT * FROM t3;
+SELECT * FROM t4;
 
 replace_column 1 #;
-RESTORE FROM '84';
+RESTORE FROM 'commit_test.bak';
 
 SELECT * FROM t1;
 SELECT * FROM t2;
@@ -101,11 +129,12 @@ INSERT INTO t2 VALUES ('a5');
 INSERT INTO t3 VALUES ('a5');
 INSERT INTO t4 VALUES ('a5');
 
+remove_file $BDIR/commit_test.bak;
 replace_column 1 #;
-BACKUP DATABASE commit_test TO '85';
+BACKUP DATABASE commit_test TO 'commit_test.bak';
 
 replace_column 1 #;
-RESTORE FROM '85';
+RESTORE FROM 'commit_test.bak';
 
 SELECT * FROM t1;
 SELECT * FROM t2;
@@ -125,12 +154,13 @@ INSERT INTO t2 VALUES ('a6');
 INSERT INTO t3 VALUES ('a6');
 INSERT INTO t4 VALUES ('a6');
 
+remove_file $BDIR/commit_test.bak;
 replace_column 1 #;
-BACKUP DATABASE commit_test TO '86';
+BACKUP DATABASE commit_test TO 'commit_test.bak';
 
 SET @@autocommit=0;
 replace_column 1 #;
-RESTORE FROM '86';
+RESTORE FROM 'commit_test.bak';
 
 SELECT * FROM t1;
 SELECT * FROM t2;
@@ -151,12 +181,13 @@ INSERT INTO t2 VALUES ('a7');
 INSERT INTO t3 VALUES ('a7');
 INSERT INTO t4 VALUES ('a7');
 
+remove_file $BDIR/commit_test.bak;
 replace_column 1 #;
-BACKUP DATABASE commit_test TO '87';
+BACKUP DATABASE commit_test TO 'commit_test.bak';
 
 SET @@autocommit=1;
 replace_column 1 #;
-RESTORE FROM '87';
+RESTORE FROM 'commit_test.bak';
 
 SELECT * FROM t1;
 SELECT * FROM t2;
@@ -172,3 +203,142 @@ COMMIT;
 # Clean-up
 DROP DATABASE commit_test;
 COMMIT;
+remove_file $BDIR/commit_test.bak;
+
+#
+# Test with multiple connections
+#
+# This is to test that BACKUP/RESTORE correctly interracts with transactions
+# run in other connections.
+#
+
+disable_warnings;
+DROP TABLE IF EXISTS test.t1;
+DROP DATABASE IF EXISTS db;
+error 0,1;
+remove_file $BDIR/db.bak;
+enable_warnings;
+
+CREATE DATABASE db;
+CREATE TABLE db.t1 (s1 CHAR(2)) ENGINE=innodb;
+CREATE TABLE test.t2 (s1 CHAR(2)) ENGINE=innodb;
+
+# Create test connections. We use the following:
+#
+# connA - runs 2 transactions while BACKUP and RESTORE are executed in the other
+#         two connections.
+# connB - inserts some data in a transaction which is finished with BACKUP 
+#         command.
+# connC - runs transaction which is terminated with RESTORE command.
+#
+# The default connection is used to check contents of the tables after each
+# operation.
+
+connect(connA, localhost, root,,);
+connect(connB, localhost, root,,);
+connect(connC, localhost, root,,);
+
+connection connA;
+echo connection A - starting transaction A1;
+
+SET autocommit=0;
+BEGIN;
+INSERT INTO db.t1 VALUES ('a1');
+INSERT INTO test.t2 VALUES ('a2');
+
+connection default;
+SELECT * FROM db.t1;
+SELECT * FROM test.t2;
+
+connection connB;
+echo connection B - starting transaction B; 
+
+SET autocommit=0;
+BEGIN;
+INSERT INTO db.t1 VALUES ('b1');
+INSERT INTO test.t2 VALUES ('b2');
+
+connection default;
+SELECT * FROM db.t1;
+SELECT * FROM test.t2;
+
+connection connB;
+echo connection B - performing BACKUP which commits transaction B;
+
+# Note: transaction A1 should not be affected.
+
+replace_column 1 #;
+BACKUP DATABASE db TO 'db.bak';
+
+connection default;
+SELECT * FROM db.t1;
+SELECT * FROM test.t2;
+
+connection connC;
+echo connection C - starting transaction C;
+
+SET autocommit=0;
+BEGIN;
+INSERT INTO db.t1 VALUES ('c1');
+INSERT INTO test.t2 VALUES ('c2');
+
+connection default;
+SELECT * FROM db.t1;
+SELECT * FROM test.t2;
+
+connection connA;
+echo connection A - finishing transaction A1;
+
+INSERT INTO db.t1 VALUES ('a3');
+INSERT INTO test.t2 VALUES ('a4');
+
+COMMIT;
+
+connection default;
+SELECT * FROM db.t1;
+SELECT * FROM test.t2;
+
+connection connA;
+echo connection A - starting transaction A2;
+
+BEGIN;
+INSERT INTO db.t1 VALUES ('a5');
+INSERT INTO test.t2 VALUES ('a6');
+
+connection default;
+SELECT * FROM db.t1;
+SELECT * FROM test.t2;
+
+connection connC;
+echo connection C - performing RESTORE which commits transaction C;
+
+# Note that restored contents of t1 overwrites value 'c1' inserted by 
+# transaction C.
+
+replace_column 1 #;
+RESTORE FROM 'db.bak';
+
+connection default;
+SELECT * FROM db.t1;
+SELECT * FROM test.t2;
+
+connection connA;
+echo connection A - finishing transaction A2;
+
+# Note that value 'a3' inserted into t1 at the beginning of the transaction
+# was overwritten by RESTORE command.
+
+INSERT INTO db.t1 VALUES ('a7');
+INSERT INTO test.t2 VALUES ('a8');
+
+COMMIT;
+
+connection default;
+SELECT * FROM db.t1;
+SELECT * FROM test.t2;
+
+# Cleanup
+
+DROP TABLE test.t2;
+DROP DATABASE db;
+remove_file $BDIR/db.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-03 13:29:39 +0000
@@ -798,17 +798,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.