Merge lp:~sergei.glushchenko/percona-xtrabackup/bug733651_mycnf_opt-1.6 into lp:percona-xtrabackup/1.6

Proposed by Sergei Glushchenko on 2012-01-27
Status: Merged
Approved by: Alexey Kopytov on 2012-03-26
Approved revision: 317
Merged at revision: 332
Proposed branch: lp:~sergei.glushchenko/percona-xtrabackup/bug733651_mycnf_opt-1.6
Merge into: lp:percona-xtrabackup/1.6
Diff against target: 136 lines (+83/-10)
3 files modified
innobackupex (+46/-10)
test/t/bug733651.sh (+32/-0)
xtrabackup.c (+5/-0)
To merge this branch: bzr merge lp:~sergei.glushchenko/percona-xtrabackup/bug733651_mycnf_opt-1.6
Reviewer Review Type Date Requested Status
Alexey Kopytov (community) 2012-01-27 Approve on 2012-01-31
Review via email: mp+90375@code.launchpad.net

Description of the change

Bug #733651
backup-my.cnf file generated and placed into the backup doesn't include
fast-checksums, and therefore the preparation stage fails when run on a
different server that doesn't have this parameter included in the
my.cnf file.
Following parameters were added to backup-my.cnf:
 - innodb_page_size
 - innodb_fast_checksum
 - innodb_log_block_size

Jenkins buld:
http://jenkins.percona.com/view/Percona%20Xtrabackup/job/percona-xtrabackup-1.6-param/104/

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

Sergei,

On 27.01.12 10:14, Sergei Glushchenko wrote:
> @@ -2192,7 +2202,35 @@
> }
>
>
> +# has_option return whether the config has an option with the given name
> +# Parameters:
> +# config_ref a reference to a config data
> +# group option group name
> +# option_name name of the option
> +# Return value:
> +# true if option exists, otherwise false
> #
> +sub has_option {
> + my $config_ref = shift;
> + my $group = shift;
> + my $option_name = shift;
> + my $default_value = shift;
> + my $group_hash_ref;
> + my $option_value;

You don't seem to be using $default_value and $option_value. Otherwise
looks good to me.

Alexey Kopytov (akopytov) :
review: Needs Fixing

Alexey,

> You don't seem to be using $default_value and $option_value. Otherwise
> looks good to me.

Yep. Thanks for noticed it. I removed unused variables. I hope no new jenkins build is needed.

Alexey Kopytov (akopytov) wrote :

Sure, no need for another Jenkins build. OK to merge, thanks!

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'innobackupex'
2--- innobackupex 2012-01-31 00:20:48 +0000
3+++ innobackupex 2012-01-31 11:23:23 +0000
4@@ -1572,10 +1572,6 @@
5 my $filename = shift;
6 my $innodb_data_file_path =
7 get_option(\%config, 'mysqld', 'innodb_data_file_path');
8- my $innodb_log_files_in_group =
9- get_option(\%config, 'mysqld', 'innodb_log_files_in_group');
10- my $innodb_log_file_size =
11- get_option(\%config, 'mysqld', 'innodb_log_file_size');
12 my $root;
13
14 my @array = split(/;/, $innodb_data_file_path);
15@@ -1594,15 +1590,29 @@
16 || Die "Failed to open file '$option_remote_host:$filename': $!";
17 }
18
19- print FILE "# This MySQL options file was generated by $innobackup_script.\n\n" .
20+ my @option_names = (
21+ "innodb_log_files_in_group",
22+ "innodb_log_file_size",
23+ "innodb_fast_checksum",
24+ "innodb_page_size",
25+ "innodb_log_block_size",
26+ );
27+
28+ my $options_dump = "# This MySQL options file was generated by $innobackup_script.\n\n" .
29 "# The MySQL server\n" .
30 "[mysqld]\n" .
31 "datadir=$root\n" .
32- "innodb_data_home_dir=$root\n" .
33- "innodb_data_file_path=$innodb_data_file_path\n" .
34- "innodb_log_group_home_dir=$root\n" .
35- "innodb_log_files_in_group=$innodb_log_files_in_group\n" .
36- "innodb_log_file_size=$innodb_log_file_size\n";
37+ "innodb_data_home_dir=$root\n";
38+
39+ my $option_name;
40+ foreach $option_name (@option_names) {
41+ if (has_option(\%config, 'mysqld', $option_name)) {
42+ my $option_value = get_option(\%config, 'mysqld', $option_name);
43+ $options_dump .= "$option_name=$option_value\n";
44+ }
45+ }
46+
47+ print FILE $options_dump;
48 close(FILE);
49
50 if ($option_stream) {
51@@ -2215,7 +2225,33 @@
52 }
53
54
55+# has_option return whether the config has an option with the given name
56+# Parameters:
57+# config_ref a reference to a config data
58+# group option group name
59+# option_name name of the option
60+# Return value:
61+# true if option exists, otherwise false
62 #
63+sub has_option {
64+ my $config_ref = shift;
65+ my $group = shift;
66+ my $option_name = shift;
67+ my $group_hash_ref;
68+
69+ if (!exists ${$config_ref}{$group}) {
70+ # no group
71+ print STDERR "$prefix fatal error: no '$group' group in MySQL options\n";
72+ print STDERR "$prefix fatal error: OR no 'datadir' option in group '$group' in MySQL options\n";
73+ exit(1);
74+ }
75+
76+ $group_hash_ref = ${$config_ref}{$group};
77+
78+ return exists ${$group_hash_ref}{$option_name};
79+}
80+
81+
82 # get_option subroutine returns the value of given option in the config
83 # structure. If option is missing, this subroutine calls exit.
84 # Parameters:
85
86=== added file 'test/t/bug733651.sh'
87--- test/t/bug733651.sh 1970-01-01 00:00:00 +0000
88+++ test/t/bug733651.sh 2012-01-31 11:23:23 +0000
89@@ -0,0 +1,32 @@
90+########################################################################
91+# Bug #733651: innobackupex not stores some critical
92+# innodb options in backup-my.cnf
93+########################################################################
94+
95+. inc/common.sh
96+
97+init
98+
99+options="innodb_log_files_in_group innodb_log_file_size"
100+if [ ! -z "$XTRADB_VERSION" ]; then
101+ options="$options innodb_page_size innodb_fast_checksum innodb_log_block_size"
102+fi
103+
104+run_mysqld
105+
106+mkdir -p $topdir/backup
107+innobackupex $topdir/backup
108+backup_dir=`grep "innobackupex: Backup created in directory" $OUTFILE | awk -F\' '{ print $2}'`
109+vlog "Backup created in directory $backup_dir"
110+
111+# test presence of options
112+for option in $options ; do
113+
114+ if [ "`cat $backup_dir/backup-my.cnf | grep $option | wc -l`" == "0" ] ; then
115+ vlog "Option $option is absent"
116+ exit -1
117+ else
118+ vlog "Option $option is present"
119+ fi
120+
121+done
122
123=== modified file 'xtrabackup.c'
124--- xtrabackup.c 2012-01-31 00:19:20 +0000
125+++ xtrabackup.c 2012-01-31 11:23:23 +0000
126@@ -6269,6 +6269,11 @@
127 printf("innodb_flush_method = \"%s\"\n",
128 (innobase_unix_file_flush_method != NULL) ?
129 innobase_unix_file_flush_method : "");
130+#ifdef XTRADB_BASED
131+ printf("innodb_fast_checksum = %d\n", innobase_fast_checksum);
132+ printf("innodb_page_size = %ld\n", innobase_page_size);
133+ printf("innodb_log_block_size = %lu\n", innobase_log_block_size);
134+#endif
135 exit(EXIT_SUCCESS);
136 }
137

Subscribers

People subscribed via source and target branches