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

Proposed by Valentine Gostev
Status: Superseded
Proposed branch: lp:~percona-dev/percona-xtrabackup/bug723097
Merge into: lp:percona-xtrabackup/2.0
Diff against target: 99 lines (+58/-3) (has conflicts)
3 files modified
test/t/bug723097.sh (+37/-0)
test/t/bug723097.sql (+4/-0)
utils/build.sh (+17/-3)
Text conflict in utils/build.sh
To merge this branch: bzr merge lp:~percona-dev/percona-xtrabackup/bug723097
Reviewer Review Type Date Requested Status
Alexey Kopytov (community) Needs Fixing
Review via email: mp+55920@code.launchpad.net

This proposal supersedes a proposal from 2011-04-01.

This proposal has been superseded by a proposal from 2011-04-01.

Description of the change

Added fix and test for bug 723097

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

To post a comment you must log in.
Revision history for this message
Alexey Kopytov (akopytov) wrote : Posted in a previous version of this proposal

The test case passes even on an unmodifed xtrabackup.

review: Needs Fixing
Revision history for this message
Alexey Kopytov (akopytov) wrote : Posted in a previous version of this proposal

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?

Revision history for this message
Alexey Kopytov (akopytov) wrote : Posted in a previous version of this proposal

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
Revision history for this message
Alexey Kopytov (akopytov) wrote :

Please also note the conflict in build.sh.

review: Needs Fixing

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== added file 'test/t/bug723097.sh'
2--- test/t/bug723097.sh 1970-01-01 00:00:00 +0000
3+++ test/t/bug723097.sh 2011-04-01 12:37:25 +0000
4@@ -0,0 +1,37 @@
5+. inc/common.sh
6+
7+init
8+run_mysqld --innodb_file_per_table
9+
10+run_cmd ${MYSQL} ${MYSQL_ARGS} -e "create database greektest"
11+run_cmd ${MYSQL} ${MYSQL_ARGS} greektest < t/bug723097.sql
12+
13+checksum_a=`${MYSQL} ${MYSQL_ARGS} -Ns -e "CHECKSUM TABLE messages" greektest | awk {'print $2'}`
14+vlog "Checksum before is $checksum_a"
15+
16+# Full backup
17+mkdir -p $topdir/data/full
18+vlog "Starting backup"
19+xtrabackup --datadir=$mysql_datadir --backup --target-dir=$topdir/data/full
20+vlog "Full backup done"
21+# Preparing backup
22+xtrabackup --datadir=$mysql_datadir --prepare --target-dir=$topdir/data/full
23+stop_mysqld
24+
25+cd $topdir/data/full
26+cp -r * $mysql_datadir
27+cd -
28+run_mysqld --innodb_file_per_table
29+checksum_b=`${MYSQL} ${MYSQL_ARGS} -Ns -e "CHECKSUM TABLE messages" greektest | awk {'print $2'}`
30+vlog "Checksum after is $checksum_b"
31+stop_mysqld
32+
33+if [ $checksum_a -eq $checksum_b ]
34+then
35+ vlog "Checksums are Ok"
36+else
37+ vlog "Checksums are not equal"
38+ exit -1
39+fi
40+
41+clean
42
43=== added file 'test/t/bug723097.sql'
44--- test/t/bug723097.sql 1970-01-01 00:00:00 +0000
45+++ test/t/bug723097.sql 2011-04-01 12:37:25 +0000
46@@ -0,0 +1,4 @@
47+CREATE TABLE `messages` (
48+ `a` varchar(20) DEFAULT NULL
49+) ENGINE=InnoDB DEFAULT CHARSET=greek;
50+INSERT INTO `messages` VALUES ('πτήσης');
51
52=== modified file 'utils/build.sh'
53--- utils/build.sh 2011-03-21 15:22:20 +0000
54+++ utils/build.sh 2011-04-01 12:37:25 +0000
55@@ -160,7 +160,7 @@
56 --with-plugins=innobase \
57 --with-zlib-dir=bundled \
58 --enable-shared \
59- --with-extra-charsets=complex"
60+ --with-extra-charsets=all"
61
62 build_all
63 ;;
64@@ -176,11 +176,25 @@
65 -DWITH_INNOBASE_STORAGE_ENGINE=ON \
66 -DWITH_PARTITION_STORAGE_ENGINE=ON \
67 -DWITH_ZLIB=bundled \
68- -DWITH_EXTRA_CHARSETS=complex \
69+ -DWITH_EXTRA_CHARSETS=all \
70 -DENABLE_DTRACE=OFF"
71
72 build_all
73 ;;
74+<<<<<<< TREE
75+=======
76+"plugin")
77+ mysql_version=$MYSQL_51_VERSION
78+ server_patch=fix_innodb_for_backup_5.1_plugin.patch
79+ innodb_name=innodb_plugin
80+ xtrabackup_target=plugin
81+ configure_cmd="./configure --enable-local-infile \
82+ --enable-thread-safe-client \
83+ --with-plugins=innodb_plugin \
84+ --with-zlib-dir=bundled \
85+ --enable-shared \
86+ --with-extra-charsets=all"
87+>>>>>>> MERGE-SOURCE
88
89 "xtradb51" | "xtradb")
90 server_dir=$top_dir/Percona-Server
91@@ -193,7 +207,7 @@
92 --with-plugins=innodb_plugin \
93 --with-zlib-dir=bundled \
94 --enable-shared \
95- --with-extra-charsets=complex"
96+ --with-extra-charsets=all"
97 if [ "`uname -s`" = "Linux" ]
98 then
99 configure_cmd="LIBS=-lrt $configure_cmd"

Subscribers

People subscribed via source and target branches