Merge lp:~sergei.glushchenko/percona-xtrabackup/2.1-xb-bug1347698 into lp:percona-xtrabackup/2.1

Proposed by Sergei Glushchenko on 2014-09-07
Status: Merged
Approved by: Alexey Kopytov on 2014-09-09
Approved revision: 760
Merged at revision: 761
Proposed branch: lp:~sergei.glushchenko/percona-xtrabackup/2.1-xb-bug1347698
Merge into: lp:percona-xtrabackup/2.1
Diff against target: 66 lines (+28/-1)
3 files modified
src/xtrabackup.cc (+8/-0)
test/t/bug1347698.sh (+19/-0)
test/t/bug884737.sh (+1/-1)
To merge this branch: bzr merge lp:~sergei.glushchenko/percona-xtrabackup/2.1-xb-bug1347698
Reviewer Review Type Date Requested Status
Alexey Kopytov (community) 2014-09-07 Approve on 2014-09-09
Review via email: mp+233641@code.launchpad.net

Description of the change

Bug 1347698: log parameter on mysqld section shouldn't be read by xtrabackup

Fixed by adding dummy option --log.

http://jenkins.percona.com/view/PXB%202.1/job/percona-xtrabackup-2.1-param-new/11/

To post a comment you must log in.
Alexey Kopytov (akopytov) wrote :

Sergei,

Looks good except "Ingnoed" and irrelevant comment about --parallel in the test case.

Also, thanks for fixing bug884737.sh, but I don't understand how it could pass Jenkins before the fix? I have verified that the syntax is correct in the 2.2 tree, and that the test fails for me locally in the 2.1 tree. Which looks serious. Any ideas why 2.1 succeeded in Jenkins?

review: Needs Fixing

Hi Alexey,

I fixed the typo and comment in test case. Test bug884737.sh
actually has no reason not to pass in Jenkins. xtrabackup by default
will create backup in the current directory in subdirectory
xtrabackup_backupfiles (i.e. test/xtrabackup_backupfiles). While this
test is only one doing it, it will always pass. Jenkins wipes current
directory for every new run. This test will start failing if there is
another test making backup in test/xtrabackup_backupfiles, or if you
run test suite twice with the same working directory.

http://jenkins.percona.com/view/PXB%202.1/job/percona-xtrabackup-2.1-param-new/12/

Alexey Kopytov (akopytov) wrote :

OK, so the problem is that xtrabackup does not reject arguments which are not prefixed with double dash. Reported as bug #1367377.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/xtrabackup.cc'
2--- src/xtrabackup.cc 2014-06-24 09:58:44 +0000
3+++ src/xtrabackup.cc 2014-09-09 15:36:06 +0000
4@@ -184,6 +184,9 @@
5 in milliseconds (default is 1 second) */
6 ulint xtrabackup_log_copy_interval = 1000;
7
8+/* Ignored option (--log) for MySQL option compatibility */
9+char* log_ignored_opt = NULL;
10+
11 /* === metadata of backup === */
12 #define XTRABACKUP_METADATA_FILENAME "xtrabackup_checkpoints"
13 char metadata_type[30] = ""; /*[full-backuped|full-prepared|incremental]*/
14@@ -419,6 +422,7 @@
15 OPT_XTRA_ENCRYPT_KEY_FILE,
16 OPT_XTRA_ENCRYPT_THREADS,
17 OPT_XTRA_ENCRYPT_CHUNK_SIZE,
18+ OPT_LOG,
19 OPT_INNODB,
20 OPT_INNODB_CHECKSUMS,
21 OPT_INNODB_DATA_FILE_PATH,
22@@ -650,6 +654,10 @@
23 (G_PTR*) &xtrabackup_encrypt_chunk_size, (G_PTR*) &xtrabackup_encrypt_chunk_size,
24 0, GET_ULL, REQUIRED_ARG, (1 << 16), 1024, ULONGLONG_MAX, 0, 0, 0},
25
26+ {"log", OPT_LOG, "Ignored option for MySQL option compatibility",
27+ (G_PTR*) &log_ignored_opt, (G_PTR*) &log_ignored_opt, 0,
28+ GET_STR, OPT_ARG, 0, 0, 0, 0, 0, 0},
29+
30 {"innodb", OPT_INNODB, "Ignored option for MySQL option compatibility",
31 (G_PTR*) &innobase_ignored_opt, (G_PTR*) &innobase_ignored_opt, 0,
32 GET_STR, OPT_ARG, 0, 0, 0, 0, 0, 0},
33
34=== added file 'test/t/bug1347698.sh'
35--- test/t/bug1347698.sh 1970-01-01 00:00:00 +0000
36+++ test/t/bug1347698.sh 2014-09-09 15:36:06 +0000
37@@ -0,0 +1,19 @@
38+########################################################################
39+# Bug #1347698: log parameter on mysqld section shouldn't be read by
40+# xtrabackup
41+########################################################################
42+
43+. inc/common.sh
44+
45+require_server_version_lower_than 5.6.0
46+
47+# add --log parameter to [mysqld] section of my.cnf
48+MYSQLD_EXTRA_MY_CNF_OPTS="
49+log=$MYSQLD_VARDIR/general.log
50+"
51+
52+start_server
53+
54+vlog "Creating the backup directory: $topdir/backup"
55+mkdir -p $topdir/backup
56+xtrabackup --backup --target-dir=$topdir/backup
57
58=== modified file 'test/t/bug884737.sh'
59--- test/t/bug884737.sh 2013-07-30 09:31:12 +0000
60+++ test/t/bug884737.sh 2014-09-09 15:36:06 +0000
61@@ -10,4 +10,4 @@
62 # Check that --parallel=<negative value> doesn't blow up
63 vlog "Creating the backup directory: $topdir/backup"
64 mkdir -p $topdir/backup
65-xtrabackup --backup --parallel=-1 $topdir/backup
66+xtrabackup --backup --parallel=-1 --target-dir=$topdir/backup

Subscribers

People subscribed via source and target branches