Merge lp:~igor-tverdovskiy/percona-xtrabackup/move-back_opt into lp:percona-xtrabackup/2.0

Proposed by Igor Tverdovskiy on 2012-02-18
Status: Rejected
Rejected by: Alexey Kopytov on 2012-09-04
Proposed branch: lp:~igor-tverdovskiy/percona-xtrabackup/move-back_opt
Merge into: lp:percona-xtrabackup/2.0
Diff against target: 246 lines (+92/-13) (has conflicts)
2 files modified
innobackupex (+59/-13)
test/t/bug803636.sh (+33/-0)
Text conflict in innobackupex
To merge this branch: bzr merge lp:~igor-tverdovskiy/percona-xtrabackup/move-back_opt
Reviewer Review Type Date Requested Status
Alexey Kopytov (community) 2012-02-18 Resubmit on 2012-09-04
Laurynas Biveinis (community) Needs Fixing on 2012-09-03
Stewart Smith (community) Needs Fixing on 2012-03-18
Review via email: mp+93711@code.launchpad.net

Description of the change

move-back opt was added. It does the same as copy-back, but moves files. - useful if not much of free disk space is available.

To post a comment you must log in.
Stewart Smith (stewart) wrote :

The only thing missing here is a test case. It will be fairly easy to write one though. Check out some of the ib_ tests in test/t - they're shell scripts that use a bunch of helper functions provided by the test suite.

review: Needs Fixing
Vojtech Kurka (vojtech-kurka) wrote :

Please how is the status of this patch? When recovering a compressed backup of 1TB of data, it's not acceptable to wait for copying the data, it can take hours - moving is the only option.
Thank you, Vojtech

Stewart Smith (stewart) wrote :

> Please how is the status of this patch? When recovering a compressed backup of
> 1TB of data, it's not acceptable to wait for copying the data, it can take
> hours - moving is the only option.
> Thank you, Vojtech

If you're able to supply a test case, we can merge it a lot quicker.

394. By Igor Tverdovskiy on 2012-08-29

changes applied to the latest innobackupex, test case added

395. By Igor Tverdovskiy on 2012-08-29

test case added

Igor Tverdovskiy (igor-tverdovskiy) wrote :
Download full text (4.3 KiB)

Hi All,

> > Please how is the status of this patch? When recovering a compressed backup
> of
> > 1TB of data, it's not acceptable to wait for copying the data, it can take
> > hours - moving is the only option.
> > Thank you, Vojtech
>
> If you're able to supply a test case, we can merge it a lot quicker.

Test case has been added (bug803636.sh)

All cases are passed or skipped:

> sudo ./run.sh -d /usr
2012-08-29 16:56:50: run.sh: Stopping server with id=1...
2012-08-29 16:56:51: run.sh: Server with id=1 has been stopped
Running against MySQL 5.1.52-log (InnoDB )
Using 'xtrabackup_51' as xtrabackup binary

========================================================================

t/bug1002688.sh [passed]
t/bug1022562.sh [passed]
t/bug1028949.sh [skipped] Requires InnoDB plugin or XtraDB
t/bug483827.sh [passed]
t/bug489290.sh [passed]
t/bug514068.sh [passed]
t/bug606981.sh [passed]
t/bug722638.sh [passed]
t/bug723097.sh [passed]
t/bug723318.sh [passed]
t/bug729843.sh [passed]
t/bug733651.sh [passed]
t/bug759225.sh [skipped] Requires XtraDB
t/bug759701.sh [passed]
t/bug766033.sh [passed]
t/bug766607.sh [passed]
t/bug803636.sh [passed]
t/bug810269.sh [skipped] Requires InnoDB plugin or XtraDB
t/bug817132.sh [skipped] Requires xtradb51
t/bug884737.sh [passed]
t/bug891496.sh [passed]
t/bug932623.sh [passed]
t/bug972169.sh [passed]
t/bug976945.sh [skipped] Requires XtraDB
t/bug977101.sh [passed]
t/bug977652.sh [skipped] Requires qpress to be installed
t/bug983685.sh [passed]
t/bug983695.sh [skipped] Requires qpress to be installed
t/bug983720_galerainfo.sh [skipped] Requires WSREP enabled
t/bug983720_lrudump.sh [skipped] Requires XtraDB
t/bug989397.sh [passed]
t/bug996493.sh [passed]
t/bug999750.sh [skipped] Requires InnoDB plugin or XtraDB
t/ib_binlog_info.sh [passed]
t/ib_csm_csv.sh [passed]
t/ib_empty_dir.sh [passed]
t/ib_incremental.sh [passed]
t/ib_lru_dump_basic.sh [skipped] Requires XtraDB
t/ib_lru_dump_rsync.sh [skipped] Requires XtraDB
t/ib_lru_dump_stream.sh [skipped] Requires XtraDB
t/ib_rsync.sh [passed]
t/ib_slave_info.sh [passed]
t/ib_specialchar.sh [passed]
t/ib_stream_compress.sh [skipped] ...

Read more...

Igor Tverdovskiy (igor-tverdovskiy) wrote :

PS: I have committed the latest innobackupex file with my changes.

Unfortunately there are merge conflicts against the current 2.0 trunk, see the diff below.

review: Needs Fixing
Igor Tverdovskiy (igor-tverdovskiy) wrote :

> Unfortunately there are merge conflicts against the current 2.0 trunk, see the
> diff below.

Okay, I'm going to add modifications to innobackupex from lp:percona-xtrabackup/2.0 branch. I'll be back soon.

396. By Igor Tverdovskiy on 2012-09-02

option added to 2.0 trunk

Igor Tverdovskiy (igor-tverdovskiy) wrote :

done please check

Alexey Kopytov (akopytov) wrote :

There's still one conflict in the pod section of innobackupex:

176 -innobackupex --copy-back [--defaults-file=MY.CNF] [--defaults-group=GROUP-NAME] BACKUP-DIR
177 +<<<<<<< TREE
178 +innobackupex --copy-back [--defaults-file=MY.CNF] [--defaults-group=GROUP-NAME] BACKUP-DIR
179 +=======
180 +innobackupex --copy-back [--defaults-file=MY.CNF] [--defaults-group=GROUP-NAME] BACKUP-DIR
181 + --move-back [--defaults-file=MY.CNF] [--defaults-group=GROUP-NAME] BACKUP-DIR
182 +>>>>>>> MERGE-SOURCE

I would create a separate line in the help text, rather than add --move-back to the existing one describing the --copy-back synopsis, i.e.:

innobackupex --copy-back [--defaults-file=MY.CNF] [--defaults-group=GROUP-NAME] BACKUP-DIR

innobackupex --move-back [--defaults-file=MY.CNF] [--defaults-group=GROUP-NAME] BACKUP-DIR

review: Needs Fixing
397. By Igor Tverdovskiy on 2012-09-02

move-back hepl text has been separated

Igor Tverdovskiy (igor-tverdovskiy) wrote :

committed please check

> committed please check

There are still merge conflicts shown in the diff: around lines 58 and 176.

review: Needs Fixing
Igor Tverdovskiy (igor-tverdovskiy) wrote :

> > committed please check
>
> There are still merge conflicts shown in the diff: around lines 58 and 176.

Hi,

I have looked through, however do not see any conflicts there.. Just a single new line of code was added in both cases.

Alexey Kopytov (akopytov) wrote :

Igor,

You can find the remaining conflicts if you look for "<<<" in the Preview Diff. But don't worry, I'll fix it myself in a separate MP along with some minor edits to the patch. Closing this MP.

review: Resubmit
Igor Tverdovskiy (igor-tverdovskiy) wrote :

I saw these conflicts <<<, however I do not understand what is wrong there... One line was replaced by two lines of code...

Probably the reason for all these conflicts is that the submitted branch has been branched off XtraBackup trunk a long time ago, so it is very likely they have diverged.

Igor Tverdovskiy (igor-tverdovskiy) wrote :

It has been branched on 2nd of September from lp:percona-xtrabackup/2.0 branch. Is this branch wrong? I should have used another one?

thanks

It does not seem to be branched off lp:percona-xtrabackup/2.0: the last parent branch commit on your branch is rev 392, dated in February. lp:percona-xtrabackup/2.0 currently is at rev 464, made on August 22.

Igor Tverdovskiy (igor-tverdovskiy) wrote :

Well my branch is pretty old because I have added this option in February, however on 2nd September I downloaded innobackupex from 2.0 branch, added my code, replaced old innobackupex in my branch with new one and committed changes.

I see, that probably confused bzr into thinking that there are conflicts. One of the correct procedures would be to branch freshly from recent 2.0 and make your changes - this might come handy in the future, as Alexey said he will take care of this MP.

Thanks for working on this.

Igor Tverdovskiy (igor-tverdovskiy) wrote :

ok, thanks for clarification

Unmerged revisions

397. By Igor Tverdovskiy on 2012-09-02

move-back hepl text has been separated

396. By Igor Tverdovskiy on 2012-09-02

option added to 2.0 trunk

395. By Igor Tverdovskiy on 2012-08-29

test case added

394. By Igor Tverdovskiy on 2012-08-29

changes applied to the latest innobackupex, test case added

393. By Igor Tverdovskiy on 2012-02-18

added new option --move-back, just the same as --copy-back, but moves files instead.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'innobackupex'
2--- innobackupex 2012-08-02 12:38:15 +0000
3+++ innobackupex 2012-09-02 15:43:18 +0000
4@@ -72,6 +72,7 @@
5 my $option_apply_log = '';
6 my $option_redo_only = '';
7 my $option_copy_back = '';
8+my $option_move_back = '';
9 my $option_include = '';
10 my $option_databases = '';
11 my $option_tables_file = '';
12@@ -217,6 +218,8 @@
13 my $dst_name;
14 my $win = ($^O eq 'MSWin32' ? 1 : 0);
15 my $CP_CMD = ($win eq 1 ? "copy /Y" : "cp -p");
16+my $MV_CMD = ($win eq 1 ? "move /Y" : "mv -f");
17+my $MV_CP_CMD = '';
18 my $xtrabackup_binary_file = 'xtrabackup_binary';
19 my $xtrabackup_pid_file = 'xtrabackup_pid';
20 my %rsync_files_hash;
21@@ -237,7 +240,7 @@
22 print_version();
23
24 # initialize global variables and perform some checks
25-if ($option_copy_back) {
26+if ($option_copy_back || $option_move_back) {
27 $option_ibbackup_binary = 'xtrabackup' if ($option_ibbackup_binary eq 'autodetect');
28 } elsif ($option_apply_log) {
29 # Read XtraBackup version from backup dir
30@@ -280,6 +283,9 @@
31 if ($option_copy_back) {
32 # copy files from backup directory back to their original locations
33 copy_back();
34+} elsif ($option_move_back) {
35+ # move files from backup directory back to their original locations
36+ move_back();
37 } elsif ($option_apply_log) {
38 # expand data files in backup directory by applying log files to them
39 apply_log();
40@@ -627,11 +633,25 @@
41 }
42
43 #
44+# move_back subroutine uses copy_back subroutine to move data and index files
45+# from backup directory back to their original locations.
46+#
47+sub move_back {
48+ my $move = 'yes';
49+ copy_back($move);
50+}
51+
52+#
53 # copy_back subroutine copies data and index files from backup directory
54 # back to their original locations.
55 #
56 sub copy_back {
57- my $orig_datadir = get_option(\%config, $option_defaults_group, 'datadir');
58+<<<<<<< TREE
59+ my $orig_datadir = get_option(\%config, $option_defaults_group, 'datadir');
60+=======
61+ my($move) = @_;
62+ my $orig_datadir = get_option(\%config, $option_defaults_group, 'datadir');
63+>>>>>>> MERGE-SOURCE
64 my $orig_ibdata_dir =
65 get_option(\%config, $option_defaults_group, 'innodb_data_home_dir');
66 my $orig_innodb_data_file_path =
67@@ -647,6 +667,14 @@
68 my $compressed_data_file = '.*\.ibz$';
69 my $file;
70 my $backup_innodb_data_file_path;
71+
72+ # check whether files should be copied or moved to dest directory
73+ if ($move eq 'yes') {
74+ $MV_CP_CMD = $MV_CMD;
75+ } else {
76+ $MV_CP_CMD = $CP_CMD;
77+ }
78+
79 # check that original data directories exist and they are empty
80 if_directory_exists_and_empty($orig_datadir, "Original data");
81 if_directory_exists_and_empty($orig_ibdata_dir, "Original InnoDB data");
82@@ -678,7 +706,8 @@
83 Die "Backup data file '$backup_dir/$filename' does not exist, but "
84 . "its compressed copy '${path}.ibz' exists. Check "
85 . "that you have run '$innobackup_script --apply-log --uncompress "
86- . "...' before attempting '$innobackup_script --copy-back ...' !";
87+ . "...' before attempting '$innobackup_script --copy-back ...' "
88+ . "...' or '$innobackup_script --move-back ...' !";
89 } else {
90 Die "Backup data file '$backup_dir/$filename' does not exist.";
91 }
92@@ -706,8 +735,8 @@
93 print STDERR "$prefix Copying file '$backup_dir/$filename'\n";
94 $src_name = escape_path("$backup_dir/$filename");
95 $dst_name = escape_path("$orig_ibdata_dir/$path");
96- system("$CP_CMD \"$src_name\" \"$dst_name\"")
97- and Die "Failed to copy file '$filename': $!";
98+ system("$MV_CP_CMD \"$src_name\" \"$dst_name\"")
99+ and Die "Failed to copy/move file '$filename': $!";
100 }
101
102 # copy InnoDB log files to original InnoDB log directory
103@@ -721,8 +750,8 @@
104 print STDERR "$prefix Copying file '$backup_dir/$file'\n";
105 $src_name = escape_path("$backup_dir/$file");
106 $dst_name = escape_path("$orig_iblog_dir");
107- system("$CP_CMD \"$src_name\" \"$dst_name\"")
108- and Die "Failed to copy file '$file': $!";
109+ system("$MV_CP_CMD \"$src_name\" \"$dst_name\"")
110+ and Die "Failed to copy/move file '$file': $!";
111 }
112 }
113 closedir(DIR);
114@@ -1528,10 +1557,12 @@
115 my $run = '';
116
117 # print some instructions to the user
118- if (!$option_apply_log && !$option_copy_back) {
119+ if (!$option_apply_log && !$option_copy_back && !$option_move_back) {
120 $run = 'backup';
121 } elsif ($option_copy_back) {
122 $run = 'copy-back';
123+ } elsif ($option_move_back) {
124+ $run = 'move-back';
125 } else {
126 $run = 'apply-log';
127 }
128@@ -1541,7 +1572,7 @@
129
130 # check that MySQL client program and InnoDB Hot Backup program
131 # are runnable via shell
132- if (!$option_copy_back) {
133+ if (!$option_copy_back && !$option_move_back) {
134 # we are making a backup or applying log to backup
135 if (!$option_apply_log) {
136 # we are making a backup, we need mysql server
137@@ -1589,7 +1620,7 @@
138 #$innodb_log_group_home_dir =
139 # get_option(\%config, 'mysqld', 'innodb_log_group_home_dir');
140
141- if (!$option_apply_log && !$option_copy_back) {
142+ if (!$option_apply_log && !$option_copy_back && !$option_move_back) {
143 # we are making a backup, create a new backup directory
144 if (!$option_remote_host) {
145 $backup_dir = File::Spec->rel2abs(make_backup_dir());
146@@ -1619,7 +1650,7 @@
147 }
148 }
149 write_backup_config_file($backup_config_file);
150- } elsif ($option_copy_back) {
151+ } elsif ($option_copy_back || $option_move_back) {
152 #$backup_config_file = $backup_dir . '/backup-my.cnf';
153 #read_config_file($backup_config_file, \%backup_config);
154 }
155@@ -1715,6 +1746,7 @@
156 'apply-log' => \$option_apply_log,
157 'redo-only' => \$option_redo_only,
158 'copy-back' => \$option_copy_back,
159+ 'move-back' => \$option_move_back,
160 'include=s' => \$option_include,
161 'databases=s' => \$option_databases,
162 'tables-file=s', => \$option_tables_file,
163@@ -1792,7 +1824,7 @@
164 # get options file name
165 #$config_file = $ARGV[0];
166
167- if (!$option_apply_log && !$option_copy_back) {
168+ if (!$option_apply_log && !$option_copy_back && !$option_move_back) {
169 # we are making a backup, get backup root directory
170 $backup_root = $ARGV[0];
171 if ($option_incremental && !$option_incremental_lsn) {
172@@ -2633,7 +2665,13 @@
173 [--export] [--redo-only] [--ibbackup=IBBACKUP-BINARY]
174 BACKUP-DIR
175
176-innobackupex --copy-back [--defaults-file=MY.CNF] [--defaults-group=GROUP-NAME] BACKUP-DIR
177+<<<<<<< TREE
178+innobackupex --copy-back [--defaults-file=MY.CNF] [--defaults-group=GROUP-NAME] BACKUP-DIR
179+=======
180+innobackupex --copy-back [--defaults-file=MY.CNF] [--defaults-group=GROUP-NAME] BACKUP-DIR
181+
182+innobackupex --move-back [--defaults-file=MY.CNF] [--defaults-group=GROUP-NAME] BACKUP-DIR
183+>>>>>>> MERGE-SOURCE
184
185 =head1 DESCRIPTION
186
187@@ -2663,6 +2701,10 @@
188 The MY.CNF options file defines the original location of the database.
189 The BACKUP-DIR is the path to a backup directory created by xtrabackup.
190
191+The --move-back command does just the same as --copy-back. The only difference is
192+that files are moved to data directory instead of be copied. Useful in case you
193+have not much free space at your destination server.
194+
195 On success the exit code innobackupex is 0. A non-zero exit code
196 indicates an error.
197
198@@ -2691,6 +2733,10 @@
199
200 Copy all the files in a previously made backup from the backup directory to their original locations.
201
202+=item --move-back
203+
204+Move all the files in a previously made backup from the backup directory to the actual datadir location.
205+
206 =item --databases=LIST
207
208 This option specifies the list of databases that innobackupex should back up. The option accepts a string argument. The list is of the form "databasename1[.table_name1] databasename2[.table_name2] . . .". If this option is not specified, all databases containing MyISAM and InnoDB tables will be backed up. Please make sure that --databases contains all of the InnoDB databases and tables, so that all of the innodb.frm files are also backed up. In case the list is very long, this can be specified in a file, and the full path of the file can be specified instead of the list. (See option --tables-file.)
209
210=== added file 'test/t/bug803636.sh'
211--- test/t/bug803636.sh 1970-01-01 00:00:00 +0000
212+++ test/t/bug803636.sh 2012-09-02 15:43:18 +0000
213@@ -0,0 +1,33 @@
214+. inc/common.sh
215+
216+start_server
217+
218+load_dbase_schema sakila
219+load_dbase_data sakila
220+
221+mkdir -p $topdir/backup
222+innobackupex $topdir/backup
223+backup_dir=`grep "innobackupex: Backup created in directory" $OUTFILE | awk -F\' '{ print $2}'`
224+vlog "Backup created in directory $backup_dir"
225+
226+stop_server
227+# Remove datadir
228+rm -r $mysql_datadir
229+#init_mysql_dir
230+# Restore sakila
231+vlog "Applying log"
232+vlog "###########"
233+vlog "# PREPARE #"
234+vlog "###########"
235+innobackupex --apply-log $backup_dir
236+vlog "Restoring MySQL datadir"
237+mkdir -p $mysql_datadir
238+vlog "###########"
239+vlog "# RESTORE #"
240+vlog "###########"
241+innobackupex --move-back $backup_dir
242+
243+start_server
244+# Check sakila
245+echo ${MYSQL}
246+run_cmd ${MYSQL} ${MYSQL_ARGS} -e "SELECT count(*) from actor" sakila

Subscribers

People subscribed via source and target branches