Merge lp:~percona-dev/percona-xtrabackup/bug723097 into lp:percona-xtrabackup/2.0

[Valentine Gostev]

Added fix and test for bug 723097

1. Removed unnecessary lines from SQL dump
2. Added checksum validation

[Alexey Kopytov (akopytov)]

The test case passes even on an unmodifed xtrabackup.

review: Needs Fixing

[Alexey Kopytov (akopytov)]

Valentine,

Some comments after analyzing the test case more closely:

- The test does not really involve any non-latin characters and thus, does not really test the problem. It's easy to verify by looking at the hex dump for the .sql file:

$ hexdump -C experimental/bug723097.sql | grep INSERT
000004c0 2a 21 34 30 30 30 30 20 41 4c 54 45 52 20 54 41 |*!40000 ALTER TA|
000004d0 42 4c 45 20 60 6d 65 73 73 61 67 65 73 60 20 44 |BLE `messages` D|
000004e0 49 53 41 42 4c 45 20 4b 45 59 53 20 2a 2f 3b 0a |ISABLE KEYS */;.|
000004f0 49 4e 53 45 52 54 20 49 4e 54 4f 20 60 6d 65 73 |INSERT INTO `mes|
00000500 73 61 67 65 73 60 20 56 41 4c 55 45 53 20 28 27 |sages` VALUES ('|
00000510 3f 3f 3f 3f 20 3f 3f 3f c2 b1 20 3f 3f 3f 3f 3f |???? ???.. ?????|
00000520 3f 3f 3f 3f c2 b1 27 29 3b 0a 2f 2a 21 34 30 30 |????..');./*!400|

So those '?' characters are literally question marks (3f).

That's why the test cases passes even on an unmodified xtrabackup binary.

- I don't think we need a 49-line .sql file just to create 1-row table.

- I don't think the root problem has anything to do with race conditions and hence requires some parallel background activities. This has to be investigated further. If it turns out we don't really need to run any background activities, the test case has to be added to the main 'suite' rather than 'experimental'. Otherwise we have to think about implementing some kind of synchronization facility for xtrabackup tests, because the number tests which are experimental simply due to the lack of such facility is growing.

- The query following query: "insert into messages select * from messages" doubles the test table size and is run in a tight loop during the test. On my laptop it resulted in ~60 MB table during the first few seconds of the test run. Does this test really have to consume that much resources?

[Alexey Kopytov (akopytov)]

Valentine,

1. I think "SET NAMES utf8" must also be issued before loading the data. You can put it into the same .sql file.

2. I don't think you need to create a separate database for this. The default one ('test') will do and does not have to be created explicitly.

review: Needs Fixing

[Alexey Kopytov (akopytov)]

Please also note the conflict in build.sh.

review: Needs Fixing

[Alexey Kopytov (akopytov)]

Sorry, please disregard the comment about the conflict, I was reviewing the old MP version.

[Alexey Kopytov (akopytov)]

Copying the comments from the older MP:

Valentine,

1. I think "SET NAMES utf8" must also be issued before loading the data. You can put it into the same .sql file.

2. I don't think you need to create a separate database for this. The default one ('test') will do and does not have to be created explicitly.

review: Needs Fixing

=== added file 'test/t/bug723097.sh'
--- test/t/bug723097.sh	1970-01-01 00:00:00 +0000
+++ test/t/bug723097.sh	2011-04-22 02:57:29 +0000
@@ -0,0 +1,38 @@
+. inc/common.sh
+
+init
+run_mysqld --innodb_file_per_table
+
+vlog "Loading data from sql file"
+run_cmd ${MYSQL} ${MYSQL_ARGS} test < t/bug723097.sql
+
+vlog "Saving checksum"
+checksum_a=`${MYSQL} ${MYSQL_ARGS} -Ns -e "CHECKSUM TABLE messages" test | awk {'print $2'}`
+vlog "Checksum before is $checksum_a"
+
+vlog "Creating backup directory"
+mkdir -p $topdir/data/full
+vlog "Starting backup"
+xtrabackup --datadir=$mysql_datadir --backup --target-dir=$topdir/data/full
+vlog "Backup is done"
+xtrabackup --datadir=$mysql_datadir --prepare --target-dir=$topdir/data/full
+vlog "Data prepared fo restore"
+stop_mysqld
+
+cd $topdir/data/full
+cp -r * $mysql_datadir
+cd -
+run_mysqld --innodb_file_per_table
+checksum_b=`${MYSQL} ${MYSQL_ARGS} -Ns -e "CHECKSUM TABLE messages" test | awk {'print $2'}`
+vlog "Checksum after is $checksum_b"
+stop_mysqld
+
+if [ $checksum_a -eq $checksum_b ]
+then
+	vlog "Checksums are Ok"
+else
+	vlog "Checksums are not equal"
+	exit -1
+fi
+
+clean 

=== added file 'test/t/bug723097.sql'
--- test/t/bug723097.sql	1970-01-01 00:00:00 +0000
+++ test/t/bug723097.sql	2011-04-22 02:57:29 +0000
@@ -0,0 +1,5 @@
+SET NAMES utf8;
+CREATE TABLE `messages` (
+  `a` varchar(20) DEFAULT NULL
+) ENGINE=InnoDB DEFAULT CHARSET=greek;
+INSERT INTO `messages` VALUES ('πτήσης');

=== modified file 'utils/build.sh'
--- utils/build.sh	2011-04-21 06:40:52 +0000
+++ utils/build.sh	2011-04-22 02:57:29 +0000
@@ -160,7 +160,7 @@
 	    --with-plugins=innobase \
 	    --with-zlib-dir=bundled \
 	    --enable-shared \
-	    --with-extra-charsets=complex"
+	    --with-extra-charsets=all"
 
 	build_all
 	;;
@@ -176,7 +176,7 @@
 		-DWITH_INNOBASE_STORAGE_ENGINE=ON \
 		-DWITH_PARTITION_STORAGE_ENGINE=ON \
 		-DWITH_ZLIB=bundled \
-		-DWITH_EXTRA_CHARSETS=complex \
+		-DWITH_EXTRA_CHARSETS=all \
 		-DENABLE_DTRACE=OFF"
 
 	build_all
@@ -193,7 +193,7 @@
 	    --with-plugins=innodb_plugin \
 	    --with-zlib-dir=bundled \
 	    --enable-shared \
-	    --with-extra-charsets=complex"
+	    --with-extra-charsets=all"
 	if [ "`uname -s`" = "Linux" ]
 	then
 		configure_cmd="LIBS=-lrt $configure_cmd"
@@ -240,7 +240,7 @@
 		-DWITH_INNOBASE_STORAGE_ENGINE=ON \
 		-DWITH_PARTITION_STORAGE_ENGINE=ON \
 		-DWITH_ZLIB=bundled \
-		-DWITH_EXTRA_CHARSETS=complex \
+		-DWITH_EXTRA_CHARSETS=all \
 		-DENABLE_DTRACE=OFF"
 	if [ "`uname -s`" = "Linux" ]
 	then