Merge lp:~gl-az/percona-xtrabackup/bug1211263 into lp:percona-xtrabackup/2.1

[George Ormond Lorch III]

Fix for bug 1211263 : innobackupex unnecessarily calls 'cp' for metadata files

Fix is fairly simple an inelegant, check for streaming in innobckupex write_to_backup_file.
If streaming, continue to use the original method of writing a temp file, then calling backup_file.
Otherwise write directly to the result file.

Slightly modified the ib_rsync.sh test case to use a 'fake' cp command that will always fail and thus cause the test to fail if cp is ever called from innobackupex during --rsync.

http://jenkins.percona.com/view/XtraBackup/job/percona-xtrabackup-2.1-param/430/

[Alexey Kopytov (akopytov)]

Here's an idea for a test case:

cat >$topdir/cp <<EOF
#!/bin/sh
false
EOF

chmod +x $topdir/cp

PATH=$topdir:$PATH innobackupex --rsync ...

I.e. supply a fake 'cp' to innobackupex which always fails to make sure it is never used for --rsync.

review: Needs Fixing

[George Ormond Lorch III (gl-az)]

Aye! Very cool idea!

On 8/14/2013 12:55 AM, Alexey Kopytov wrote:
> Review: Needs Fixing
>
> Here's an idea for a test case:
>
> cat >$topdir/cp <<EOF
> #!/bin/sh
> false
> EOF
>
> chmod +x $topdir/cp
>
> PATH=$topdir:$PATH innobackupex --rsync ...
>
> I.e. supply a fake 'cp' to innobackupex which always fails to make sure it is never used for --rsync.

--
George O. Lorch III
Software Engineer, Percona
[phone] x542 US/Arizona (GMT -7)
skype: george.ormond.lorch.iii

[Alexey Kopytov (akopytov)]

review: Approve

=== modified file 'innobackupex'
--- innobackupex	2013-07-31 11:28:11 +0000
+++ innobackupex	2013-08-14 19:21:01 +0000
@@ -3414,16 +3414,24 @@
 sub write_to_backup_file {
   my $file_name = shift;
   my $write_data = shift;
-  my $filepos = rindex($file_name, '/');
-  my $dst_file = substr($file_name, $filepos + 1);
-  my $dst_path = substr($file_name, 0, $filepos + 1);
 
-  open XTRABACKUP_FH, "> $option_tmpdir/$dst_file"
-    || die "Cannot open file $option_tmpdir/$dst_file: $!\n";
-  print XTRABACKUP_FH $write_data;
-  close XTRABACKUP_FH;
-  backup_file($option_tmpdir, $dst_file, $file_name);
-  unlink "$option_tmpdir/$dst_file";
+  if ($option_stream) {
+    my $filepos = rindex($file_name, '/');
+    my $dst_file = substr($file_name, $filepos + 1);
+    my $dst_path = substr($file_name, 0, $filepos + 1);
+ 
+    open XTRABACKUP_FH, "> $option_tmpdir/$dst_file"
+      || die "Cannot open file $option_tmpdir/$dst_file: $!\n";
+    print XTRABACKUP_FH $write_data;
+    close XTRABACKUP_FH;
+    backup_file($option_tmpdir, $dst_file, $file_name);
+    unlink "$option_tmpdir/$dst_file";
+  } else {
+    open XTRABACKUP_FH, "> $file_name"
+      || die "Cannot open file $file_name: $!\n";
+    print XTRABACKUP_FH $write_data;
+    close XTRABACKUP_FH;
+  }
 }
 
 #

=== modified file 'test/t/ib_rsync.sh'
--- test/t/ib_rsync.sh	2013-07-25 15:04:49 +0000
+++ test/t/ib_rsync.sh	2013-08-14 19:21:01 +0000
@@ -8,7 +8,19 @@
 start_server --innodb_file_per_table
 load_sakila
 
-innobackupex --rsync --no-timestamp $topdir/backup
+# Bug #1211263: innobackupex unnecessarily calls 'cp' for metadata files
+# Create a 'fake' cp command that will be called instead and will always 'fail'
+# If any portion of innobackupex script attempts to use cp instead of rsync,
+# the backup and subsequently the test will fail.
+cat >$topdir/cp <<EOF
+#!/bin/sh
+false
+EOF
+
+chmod +x $topdir/cp
+
+# Calling with $topdir in the path first so the 'fake' cp is used
+PATH=$topdir:$PATH innobackupex --rsync --no-timestamp $topdir/backup
 
 stop_server