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
=== modified file 'innobackupex'
--- innobackupex 2012-01-31 00:20:48 +0000
+++ innobackupex 2012-01-31 11:23:23 +0000
@@ -1572,10 +1572,6 @@
1572 my $filename = shift;1572 my $filename = shift;
1573 my $innodb_data_file_path = 1573 my $innodb_data_file_path =
1574 get_option(\%config, 'mysqld', 'innodb_data_file_path');1574 get_option(\%config, 'mysqld', 'innodb_data_file_path');
1575 my $innodb_log_files_in_group =
1576 get_option(\%config, 'mysqld', 'innodb_log_files_in_group');
1577 my $innodb_log_file_size =
1578 get_option(\%config, 'mysqld', 'innodb_log_file_size');
1579 my $root;1575 my $root;
15801576
1581 my @array = split(/;/, $innodb_data_file_path);1577 my @array = split(/;/, $innodb_data_file_path);
@@ -1594,15 +1590,29 @@
1594 || Die "Failed to open file '$option_remote_host:$filename': $!";1590 || Die "Failed to open file '$option_remote_host:$filename': $!";
1595 }1591 }
15961592
1597 print FILE "# This MySQL options file was generated by $innobackup_script.\n\n" .1593 my @option_names = (
1594 "innodb_log_files_in_group",
1595 "innodb_log_file_size",
1596 "innodb_fast_checksum",
1597 "innodb_page_size",
1598 "innodb_log_block_size",
1599 );
1600
1601 my $options_dump = "# This MySQL options file was generated by $innobackup_script.\n\n" .
1598 "# The MySQL server\n" .1602 "# The MySQL server\n" .
1599 "[mysqld]\n" .1603 "[mysqld]\n" .
1600 "datadir=$root\n" .1604 "datadir=$root\n" .
1601 "innodb_data_home_dir=$root\n" .1605 "innodb_data_home_dir=$root\n";
1602 "innodb_data_file_path=$innodb_data_file_path\n" .1606
1603 "innodb_log_group_home_dir=$root\n" .1607 my $option_name;
1604 "innodb_log_files_in_group=$innodb_log_files_in_group\n" .1608 foreach $option_name (@option_names) {
1605 "innodb_log_file_size=$innodb_log_file_size\n";1609 if (has_option(\%config, 'mysqld', $option_name)) {
1610 my $option_value = get_option(\%config, 'mysqld', $option_name);
1611 $options_dump .= "$option_name=$option_value\n";
1612 }
1613 }
1614
1615 print FILE $options_dump;
1606 close(FILE);1616 close(FILE);
16071617
1608 if ($option_stream) {1618 if ($option_stream) {
@@ -2215,7 +2225,33 @@
2215}2225}
2216 2226
22172227
2228# has_option return whether the config has an option with the given name
2229# Parameters:
2230# config_ref a reference to a config data
2231# group option group name
2232# option_name name of the option
2233# Return value:
2234# true if option exists, otherwise false
2218#2235#
2236sub has_option {
2237 my $config_ref = shift;
2238 my $group = shift;
2239 my $option_name = shift;
2240 my $group_hash_ref;
2241
2242 if (!exists ${$config_ref}{$group}) {
2243 # no group
2244 print STDERR "$prefix fatal error: no '$group' group in MySQL options\n";
2245 print STDERR "$prefix fatal error: OR no 'datadir' option in group '$group' in MySQL options\n";
2246 exit(1);
2247 }
2248
2249 $group_hash_ref = ${$config_ref}{$group};
2250
2251 return exists ${$group_hash_ref}{$option_name};
2252}
2253
2254
2219# get_option subroutine returns the value of given option in the config2255# get_option subroutine returns the value of given option in the config
2220# structure. If option is missing, this subroutine calls exit.2256# structure. If option is missing, this subroutine calls exit.
2221# Parameters:2257# Parameters:
22222258
=== added file 'test/t/bug733651.sh'
--- test/t/bug733651.sh 1970-01-01 00:00:00 +0000
+++ test/t/bug733651.sh 2012-01-31 11:23:23 +0000
@@ -0,0 +1,32 @@
1########################################################################
2# Bug #733651: innobackupex not stores some critical
3# innodb options in backup-my.cnf
4########################################################################
5
6. inc/common.sh
7
8init
9
10options="innodb_log_files_in_group innodb_log_file_size"
11if [ ! -z "$XTRADB_VERSION" ]; then
12 options="$options innodb_page_size innodb_fast_checksum innodb_log_block_size"
13fi
14
15run_mysqld
16
17mkdir -p $topdir/backup
18innobackupex $topdir/backup
19backup_dir=`grep "innobackupex: Backup created in directory" $OUTFILE | awk -F\' '{ print $2}'`
20vlog "Backup created in directory $backup_dir"
21
22# test presence of options
23for option in $options ; do
24
25 if [ "`cat $backup_dir/backup-my.cnf | grep $option | wc -l`" == "0" ] ; then
26 vlog "Option $option is absent"
27 exit -1
28 else
29 vlog "Option $option is present"
30 fi
31
32done
033
=== modified file 'xtrabackup.c'
--- xtrabackup.c 2012-01-31 00:19:20 +0000
+++ xtrabackup.c 2012-01-31 11:23:23 +0000
@@ -6269,6 +6269,11 @@
6269 printf("innodb_flush_method = \"%s\"\n",6269 printf("innodb_flush_method = \"%s\"\n",
6270 (innobase_unix_file_flush_method != NULL) ?6270 (innobase_unix_file_flush_method != NULL) ?
6271 innobase_unix_file_flush_method : "");6271 innobase_unix_file_flush_method : "");
6272#ifdef XTRADB_BASED
6273 printf("innodb_fast_checksum = %d\n", innobase_fast_checksum);
6274 printf("innodb_page_size = %ld\n", innobase_page_size);
6275 printf("innodb_log_block_size = %lu\n", innobase_log_block_size);
6276#endif
6272 exit(EXIT_SUCCESS);6277 exit(EXIT_SUCCESS);
6273 }6278 }
62746279

Subscribers

People subscribed via source and target branches