Merge lp:~gl-az/percona-xtrabackup/BT-32889-history-on-server-2.2 into lp:percona-xtrabackup/2.2
- BT-32889-history-on-server-2.2
- Merge into 2.2
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Alexey Kopytov | ||||
Approved revision: | no longer in the source branch. | ||||
Merged at revision: | 4879 | ||||
Proposed branch: | lp:~gl-az/percona-xtrabackup/BT-32889-history-on-server-2.2 | ||||
Merge into: | lp:percona-xtrabackup/2.2 | ||||
Diff against target: |
667 lines (+542/-1) (has conflicts) 2 files modified
xtrabackup/innobackupex (+332/-1) xtrabackup/test/t/history_on_server.sh (+210/-0) Text conflict in xtrabackup/innobackupex |
||||
To merge this branch: | bzr merge lp:~gl-az/percona-xtrabackup/BT-32889-history-on-server-2.2 | ||||
Related bugs: |
|
||||
Related blueprints: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Alexey Kopytov (community) | Approve | ||
Review via email: mp+184860@code.launchpad.net |
Commit message
Description of the change
BT-32889
Implementation of xtrabackup http://
http://
matrix reloaded for two failures jenkis had when branching...
http://
George Ormond Lorch III (gl-az) wrote : | # |
On 9/23/2013 9:58 AM, Alexey Kopytov wrote:
> Review: Needs Fixing
>
> George,
>
> - I propose we rename ‘mysql_version’ to ‘server_version’, since we
> support not only MySQL.
Makes sense, can do.
> - let’s also rename xtrabackup_history to xtrabackup_info. It
> contains meta information about the backup where it’s stored, not
> the history. I think we can even deprecate xtrabackup_
> with xtrabackup_info in future.
Do you want to rename the entire feature and all options (--no-info,
--info=<name>, --incremental-
or just the table name?
> - the table does not define the character set as we discussed
> previously. This means the character set will depend on
> server/connection settings.
Very true, forgot about that. Will add utf8 specifier.
> - update_history() is vulnerable to SQL injection attacks :) what if
> something like this is passed to --history, for example:
>
> --history="foo'; DROP DATABASE customers".
>
> Please use DBI prepared statements. This way you don’t have to roll
> your own quoting in other places.
Yes, I actually had a discussion w/ Vadim on whether or not we are
concerned w/ SQL injection at that level and the answer was not really
so I just stuck with the original prototype code I had that used all
literals. I suppose it wouldn't hurt and would be more proper to use DBD
prepare and bind values instead of all the literal bits.
>
> - it also appears that xtrabackup_history will not be created if
> --history is not passed to innobackupex. I think it should always
> be created. I.e. update_history() should collect all info
> unconditionally, but just avoid executing the actual INSERT if
> $option_history if false.
Correct. There were some XB test case failures that would need to be
fixed up and permissions manipulated if we always created the
schema/filled the record. At that point I thought too that it may not be
a great idea to change the default assumption that XB was read only on a
server database by now having it start to write stuff back into it,
possibly requiring a user to change their backup user privileges. So, I
changed it to be entirely optional and any upgrades would be compatible
with any existing backup user privileges without having to explicitly
disable it or alter backup user privileges. I should have checked that
out w/ you first. That being said, if you do still want it on/creating
schema and filling the table and generating the file by default, I can
change it back and go fix up whatever failing tests occur as a result
(just adding --no-history/
enough info doc wise to try to avoid confusion.
> - for start_time / end_time it’s better to use time() and then
> convert values it returns to TIMESTAMP with FROM_UNIXTIME():
>
> $history_start_time = time();
> ...
>
> # start_time
> $insert_vals .= ", FROM_UNIXTIME(
>
> # end_time
> $tmp = time();
> $insert_vals .= ", FROM_UNIXTIME(
OK, will do.
> - /proc is only available on Li...
Alexey Kopytov (akopytov) wrote : | # |
Hi George,
On Mon, 23 Sep 2013 15:49:15 -0700, George Lorch wrote:
> On 9/23/2013 9:58 AM, Alexey Kopytov wrote:
>> Review: Needs Fixing
>>
>> George,
>>
>> - I propose we rename ‘mysql_version’ to ‘server_version’, since we
>> support not only MySQL.
> Makes sense, can do.
>> - let’s also rename xtrabackup_history to xtrabackup_info. It
>> contains meta information about the backup where it’s stored, not
>> the history. I think we can even deprecate xtrabackup_
>> with xtrabackup_info in future.
> Do you want to rename the entire feature and all options (--no-info,
> --info=<name>, --incremental-
> or just the table name?
No, sorry I should have been more specific. I was referring to the file
created in the backup directory. 'xtrabackup_info' reflects its content
better than 'xtrabackup_
>> - the table does not define the character set as we discussed
>> previously. This means the character set will depend on
>> server/connection settings.
> Very true, forgot about that. Will add utf8 specifier.
>> - update_history() is vulnerable to SQL injection attacks :) what if
>> something like this is passed to --history, for example:
>>
>> --history="foo'; DROP DATABASE customers".
>>
>> Please use DBI prepared statements. This way you don’t have to roll
>> your own quoting in other places.
> Yes, I actually had a discussion w/ Vadim on whether or not we are
> concerned w/ SQL injection at that level and the answer was not really
> so I just stuck with the original prototype code I had that used all
> literals. I suppose it wouldn't hurt and would be more proper to use DBD
> prepare and bind values instead of all the literal bits.
Yes, it's certainly not a critical issue in the XtraBackup context, but
still opens the way for users shooting themselves in the foot with some
bad argument to --history.
>>
>> - it also appears that xtrabackup_history will not be created if
>> --history is not passed to innobackupex. I think it should always
>> be created. I.e. update_history() should collect all info
>> unconditionally, but just avoid executing the actual INSERT if
>> $option_history if false.
> Correct. There were some XB test case failures that would need to be
> fixed up and permissions manipulated if we always created the
> schema/filled the record. At that point I thought too that it may not be
> a great idea to change the default assumption that XB was read only on a
> server database by now having it start to write stuff back into it,
> possibly requiring a user to change their backup user privileges. So, I
> changed it to be entirely optional and any upgrades would be compatible
> with any existing backup user privileges without having to explicitly
> disable it or alter backup user privileges. I should have checked that
> out w/ you first. That being said, if you do still want it on/creating
> schema and filling the table and generating the file by default, I can
> change it back and go fix up whatever failing tests occur as a result
> (just adding --no-history/
George Ormond Lorch III (gl-az) wrote : | # |
Hey there Alexey...
On 9/24/2013 4:18 AM, Alexey Kopytov wrote:
> - let’s also rename xtrabackup_history to xtrabackup_info. It
> contains meta information about the backup where it’s stored, not
> the history. I think we can even deprecate xtrabackup_
> with xtrabackup_info in future.
>> Do you want to rename the entire feature and all options (--no-info,
>> --info=<name>, --incremental-
>> or just the table name?
> No, sorry I should have been more specific. I was referring to the file
> created in the backup directory. 'xtrabackup_info' reflects its content
> better than 'xtrabackup_
>
>>> - it also appears that xtrabackup_history will not be created if
>>> --history is not passed to innobackupex. I think it should always
>>> be created. I.e. update_history() should collect all info
>>> unconditionally, but just avoid executing the actual INSERT if
>>> $option_history if false.
>> Correct. There were some XB test case failures that would need to be
>> fixed up and permissions manipulated if we always created the
>> schema/filled the record. At that point I thought too that it may not be
>> a great idea to change the default assumption that XB was read only on a
>> server database by now having it start to write stuff back into it,
>> possibly requiring a user to change their backup user privileges. So, I
>> changed it to be entirely optional and any upgrades would be compatible
>> with any existing backup user privileges without having to explicitly
>> disable it or alter backup user privileges. I should have checked that
>> out w/ you first. That being said, if you do still want it on/creating
>> schema and filling the table and generating the file by default, I can
>> change it back and go fix up whatever failing tests occur as a result
>> (just adding --no-history/
>> enough info doc wise to try to avoid confusion.
> Sorry, I should have been more specific here to. I just meant to say
> that we have to create the xtrabackup_history (or xtrabackup_info) file
> regardless of whether --history is enabled.
>
> The reason we are implementing that file is
> https:/
Ahh, OK, Now that makes a lot more sense. Yes, I can easily do this,
just have to shuffle around the means of data collection and writing in
update_history with the new DBD prepare and bind_param stuff so there
are not 20 if (!defined(
>>> - here’s a more elegant and correct scrub_option() implementation:
>>>
>>> sub scrub_option {
>>> my $scrub = shift;
>>> my $command = shift;
>>>
>>> return $command =~ s/--$scrub=[^ ]+ /--$scrub=... /g
>>>
>>> }
>> Nice.
> Actually that won't work (I didn't verify it before posting). The
> following will work:
>
> sub scrub_option {
> my $scrub = shift;
> my $command = shift;
>
> $command =~ s/--$scrub=[^ ]+/--$scrub=.../g;
> return $command;
> }
>
>
>
Yup, I figured that out yesterday and fixed it myself. Actually, I just
removed the whole scrub_option call and just us...
George Ormond Lorch III (gl-az) wrote : | # |
New jenkins for changes based on comments above:
http://
Don't know what that debian6/innodb51 failure is about in compact.sh. That has nothing to do with this feature.
Alexey Kopytov (akopytov) wrote : | # |
The following lines:
$tmp =~ s/--password=[^ ]+ /--password=... /g;
$tmp =~ s/--encrypt-key=[^ ]+ /--encrypt-key=... /g;
Should not contain spaces, i.e.:
$tmp =~ s/--password=[^ ]+/--password=
$tmp =~ s/--encrypt-key=[^ ]+/--encrypt-
That's my fault, it was in the first incorrect version of scrub_option() I proposed, but then fixed in a later version.
The difference is that when one of the options to be scrubbed is at the end of the command line, there will be no space after it, so no substitution would occur with the current code then.
George Ormond Lorch III (gl-az) wrote : | # |
> The following lines:
>
> $tmp =~ s/--password=[^ ]+ /--password=... /g;
> $tmp =~ s/--encrypt-key=[^ ]+ /--encrypt-key=... /g;
>
> Should not contain spaces, i.e.:
>
> $tmp =~ s/--password=[^ ]+/--password=
> $tmp =~ s/--encrypt-key=[^ ]+/--encrypt-
>
> That's my fault, it was in the first incorrect version of scrub_option() I
> proposed, but then fixed in a later version.
>
> The difference is that when one of the options to be scrubbed is at the end of
> the command line, there will be no space after it, so no substitution would
> occur with the current code then.
Yeah, I see that now, good catch. Fixed and went ahead and added a test for the command_tool with encrypt-key at the end of the line to validate that it gets scrubbed correctly.
George Ormond Lorch III (gl-az) wrote : | # |
Alexey Kopytov (akopytov) : | # |
Preview Diff
1 | === modified file 'xtrabackup/innobackupex' |
2 | --- xtrabackup/innobackupex 2013-09-23 10:58:53 +0000 |
3 | +++ xtrabackup/innobackupex 2013-09-25 17:11:22 +0000 |
4 | @@ -147,6 +147,10 @@ |
5 | my %mysql; |
6 | my $option_backup = ''; |
7 | |
8 | +my $option_history; |
9 | +my $option_incremental_history_name = ''; |
10 | +my $option_incremental_history_uuid = ''; |
11 | + |
12 | # name of the my.cnf configuration file |
13 | #my $config_file = ''; |
14 | |
15 | @@ -195,6 +199,9 @@ |
16 | # name of the file where slave info is written |
17 | my $slave_info; |
18 | |
19 | +# name of the file where version and backup history is written |
20 | +my $backup_history; |
21 | + |
22 | # mysql binlog position as given by "SHOW MASTER STATUS" command |
23 | my $mysql_binlog_position = ''; |
24 | |
25 | @@ -270,6 +277,7 @@ |
26 | my $copy_dir_overwrite; |
27 | my $copy_dir_resolve_isl; |
28 | |
29 | +<<<<<<< TREE |
30 | # ########################################################################### |
31 | # HTTPMicro package |
32 | # This package is a copy without comments from the original. The original |
33 | @@ -1511,6 +1519,14 @@ |
34 | # End VersionCheck package |
35 | # ########################################################################### |
36 | |
37 | +======= |
38 | +# for history on server record |
39 | +my $history_tool_command = "@ARGV"; |
40 | +my $history_start_time = time(); |
41 | +my $history_lock_time = 0; |
42 | +my $history_delete_checkpoints = 0; |
43 | + |
44 | +>>>>>>> MERGE-SOURCE |
45 | ###################################################################### |
46 | # program execution begins here |
47 | ###################################################################### |
48 | @@ -1919,6 +1935,43 @@ |
49 | detect_mysql_capabilities_for_backup(\%mysql); |
50 | } |
51 | |
52 | + # |
53 | + # if one of the history incrementals is being used, try to grab the |
54 | + # innodb_to_lsn from the history table and set the option_incremental_lsn |
55 | + # |
56 | + if ($option_incremental && !$option_incremental_lsn) { |
57 | + if ($option_incremental_history_name) { |
58 | + my $query = "SELECT innodb_to_lsn ". |
59 | + "FROM PERCONA_SCHEMA.xtrabackup_history ". |
60 | + "WHERE name = '$option_incremental_history_name' ". |
61 | + "AND innodb_to_lsn IS NOT NULL ". |
62 | + "ORDER BY innodb_to_lsn DESC LIMIT 1"; |
63 | + |
64 | + eval { |
65 | + $option_incremental_lsn = |
66 | + $mysql{dbh}->selectrow_hashref($query)->{innodb_to_lsn}; |
67 | + } || die("Error while attempting to find history record for name ". |
68 | + "'$option_incremental_history_name'\n"); |
69 | + print STDERR "$prefix --incremental-history-name=". |
70 | + "$option_incremental_history_name specified.". |
71 | + "Found and using lsn: $option_incremental_lsn"; |
72 | + } elsif ($option_incremental_history_uuid) { |
73 | + my $query = "SELECT innodb_to_lsn ". |
74 | + "FROM PERCONA_SCHEMA.xtrabackup_history ". |
75 | + "WHERE uuid = '$option_incremental_history_uuid' ". |
76 | + "AND innodb_to_lsn IS NOT NULL"; |
77 | + |
78 | + eval { |
79 | + $option_incremental_lsn = |
80 | + $mysql{dbh}->selectrow_hashref($query)->{innodb_to_lsn}; |
81 | + } || die("Error while attempting to find history record for uuid ". |
82 | + "'$option_incremental_history_uuid'\n"); |
83 | + print STDERR "$prefix --incremental-history-uuid=". |
84 | + "$option_incremental_history_uuid specified.". |
85 | + "Found and using lsn: $option_incremental_lsn"; |
86 | + } |
87 | + } |
88 | + |
89 | # start ibbackup as a child process |
90 | start_ibbackup(); |
91 | |
92 | @@ -1943,6 +1996,9 @@ |
93 | # make a prep copy before locking tables, if using rsync |
94 | backup_files(1); |
95 | |
96 | + # start counting how long lock is held for history |
97 | + $history_lock_time = time(); |
98 | + |
99 | # flush tables with read lock |
100 | mysql_lockall(\%mysql); |
101 | |
102 | @@ -1976,6 +2032,13 @@ |
103 | # release read locks on all tables |
104 | mysql_unlockall(\%mysql) if !$option_no_lock; |
105 | |
106 | + # calculate how long lock was held for history |
107 | + if ($option_no_lock) { |
108 | + $history_lock_time = 0; |
109 | + } else { |
110 | + $history_lock_time = time() - $history_lock_time; |
111 | + } |
112 | + |
113 | my $ibbackup_exit_code = wait_for_ibbackup_finish(); |
114 | |
115 | if ( $option_safe_slave_backup && $sql_thread_started) { |
116 | @@ -1999,6 +2062,8 @@ |
117 | print STDERR "$prefix MySQL slave binlog position: $mysql_slave_position\n"; |
118 | } |
119 | |
120 | + write_xtrabackup_info(\%mysql); |
121 | + |
122 | return $ibbackup_exit_code; |
123 | } |
124 | |
125 | @@ -3510,8 +3575,14 @@ |
126 | $binlog_info = $work_dir . '/xtrabackup_binlog_info'; |
127 | $galera_info = $work_dir . '/xtrabackup_galera_info'; |
128 | $slave_info = $work_dir . '/xtrabackup_slave_info'; |
129 | + $backup_history = $work_dir . '/xtrabackup_info'; |
130 | write_backup_config_file($backup_config_file); |
131 | |
132 | + if (!$option_extra_lsndir) { |
133 | + $option_extra_lsndir = $option_tmpdir; |
134 | + $history_delete_checkpoints = 1; |
135 | + } |
136 | + |
137 | foreach (@xb_suspend_files) { |
138 | my $suspend_file = $work_dir . "$_"; |
139 | if ( -e "$suspend_file" ) { |
140 | @@ -3615,6 +3686,7 @@ |
141 | 'encrypt-threads=i' => \$option_encrypt_threads, |
142 | 'encrypt-chunk-size=s' => \$option_encrypt_chunk_size, |
143 | 'help' => \$option_help, |
144 | + 'history:s' => \$option_history, |
145 | 'version' => \$option_version, |
146 | 'throttle=i' => \$option_throttle, |
147 | 'log-copy-interval=i', \$option_log_copy_interval, |
148 | @@ -3642,6 +3714,8 @@ |
149 | 'incremental' => \$option_incremental, |
150 | 'incremental-basedir=s' => \$option_incremental_basedir, |
151 | 'incremental-force-scan' => \$option_incremental_force_scan, |
152 | + 'incremental-history-name=s' => \$option_incremental_history_name, |
153 | + 'incremental-history-uuid=s' => \$option_incremental_history_uuid, |
154 | 'incremental-lsn=s' => \$option_incremental_lsn, |
155 | 'incremental-dir=s' => \$option_incremental_dir, |
156 | 'extra-lsndir=s' => \$option_extra_lsndir, |
157 | @@ -3750,7 +3824,9 @@ |
158 | # we are making a backup, get backup root directory |
159 | $option_backup = "1"; |
160 | $backup_root = $ARGV[0]; |
161 | - if ($option_incremental && !$option_incremental_lsn) { |
162 | + if ($option_incremental && !$option_incremental_lsn && |
163 | + !$option_incremental_history_name && |
164 | + !$option_incremental_history_uuid) { |
165 | if ($option_incremental_basedir) { |
166 | $incremental_basedir = $option_incremental_basedir; |
167 | } else { |
168 | @@ -4717,6 +4793,247 @@ |
169 | 'WHERE PLUGIN_NAME LIKE "INNODB_CHANGED_PAGES"') |
170 | } |
171 | |
172 | +# |
173 | +# Writes xtrabackup_info file and if backup_history is enable creates |
174 | +# PERCONA_SCHEMA.xtrabackup_history and writes a new history record to the |
175 | +# table containing all the history info particular to the just completed |
176 | +# backup. |
177 | +# |
178 | +sub write_xtrabackup_info() |
179 | +{ |
180 | + my $con = shift; |
181 | + my $sth; |
182 | + my $uuid; |
183 | + my $tmp; |
184 | + my $file_content; |
185 | + |
186 | + eval { |
187 | + $sth = $con->{dbh}->prepare("insert into PERCONA_SCHEMA.xtrabackup_history(". |
188 | + "uuid, name, tool_name, tool_command, tool_version, ". |
189 | + "ibbackup_version, server_version, start_time, end_time, ". |
190 | + "lock_time, binlog_pos, innodb_from_lsn, innodb_to_lsn, ". |
191 | + "partial, incremental, format, compact, compressed, ". |
192 | + "encrypted) ". |
193 | + "values(?,?,?,?,?,?,?,from_unixtime(?),from_unixtime(?),?,?,". |
194 | + "?,?,?,?,?,?,?,?)"); |
195 | + } || die("Eror while preparing history record\n"); |
196 | + |
197 | + |
198 | + # uuid |
199 | + # we use the mysql UUID() function to avoid platform dependencies with uuidgen |
200 | + # and Data::UUID. We select uuid here individually here so that it can be |
201 | + # reported to stderr after successful history record insertion |
202 | + eval { |
203 | + $uuid = $con->{dbh}->selectrow_hashref("SELECT UUID() AS uuid")->{uuid}; |
204 | + } || die("Error while attempting to create UUID for history record.\n"); |
205 | + $sth->bind_param(1, $uuid); |
206 | + $file_content = "uuid = $uuid"; |
207 | + |
208 | + # name |
209 | + if ($option_history) { |
210 | + $sth->bind_param(2, $option_history); |
211 | + $file_content .= "\nname = $option_history"; |
212 | + } else { |
213 | + $sth->bind_param(2, undef); |
214 | + $file_content .= "\nname = "; |
215 | + } |
216 | + |
217 | + # tool_name |
218 | + $tmp = basename($0); |
219 | + $sth->bind_param(3, $tmp); |
220 | + $file_content .= "\ntool_name = $tmp"; |
221 | + |
222 | + # tool_command |
223 | + # scrub --password and --encrypt-key from tool_command |
224 | + $tmp = $history_tool_command; |
225 | + $tmp =~ s/--password=[^ ]+/--password=.../g; |
226 | + $tmp =~ s/--encrypt-key=[^ ]+/--encrypt-key=.../g; |
227 | + $sth->bind_param(4, $tmp); |
228 | + $file_content .= "\ntool_command = $tmp"; |
229 | + |
230 | + # tool_version |
231 | + $sth->bind_param(5, $innobackup_version); |
232 | + $file_content .= "\ntool_version = $innobackup_version"; |
233 | + |
234 | + # ibbackup_version |
235 | + $tmp = "$option_ibbackup_binary -v 2>&1"; |
236 | + $tmp = qx($tmp); |
237 | + chomp($tmp); |
238 | + if (length $tmp) { |
239 | + $sth->bind_param(6, $tmp); |
240 | + $file_content .= "\nibbackup_version = $tmp"; |
241 | + } else { |
242 | + $sth->bind_param(6, undef); |
243 | + $file_content .= "\nibbackup_version = "; |
244 | + } |
245 | + |
246 | + # server_version |
247 | + $tmp = |
248 | + $con->{dbh}->selectrow_hashref("SELECT VERSION() as version")->{version}; |
249 | + $sth->bind_param(7, $tmp); |
250 | + $file_content .= "\nserver_version = $tmp"; |
251 | + |
252 | + # start_time |
253 | + $sth->bind_param(8, $history_start_time); |
254 | + $file_content .= "\nstart_time = " . strftime("%Y-%m-%d %H:%M:%S", localtime($history_start_time)); |
255 | + |
256 | + # end_time |
257 | + $tmp = time(); |
258 | + $sth->bind_param(9, $tmp); |
259 | + $file_content .= "\nend_time = " . strftime("%Y-%m-%d %H:%M:%S", localtime($tmp)); |
260 | + |
261 | + # lock_time |
262 | + $sth->bind_param(10, $history_lock_time); |
263 | + $file_content .= "\nlock_time = $history_lock_time"; |
264 | + |
265 | + # binlog_pos |
266 | + if ($mysql_binlog_position) { |
267 | + $sth->bind_param(11, $mysql_binlog_position); |
268 | + $file_content .= "\nbinlog_pos = $mysql_binlog_position"; |
269 | + } else { |
270 | + $sth->bind_param(11, undef); |
271 | + $file_content .= "\nbinlog_pos = "; |
272 | + } |
273 | + |
274 | + # innodb_from_lsn |
275 | + # grab the from_lsn from the extra checkpoints file, parse and delete it |
276 | + # if necessary. This could generate an error if the checkpoints file |
277 | + # isn't 100% correct in the format. |
278 | + $tmp = "cat $option_extra_lsndir/xtrabackup_checkpoints | grep from_lsn"; |
279 | + $tmp = qx($tmp); |
280 | + chomp($tmp); |
281 | + substr($tmp, 0, 11, ""); # strip "from_lsn = " |
282 | + if (length $tmp) { |
283 | + $sth->bind_param(12, $tmp); |
284 | + $file_content .= "\ninnodb_from_lsn = $tmp"; |
285 | + } else { |
286 | + print STDERR "WARNING : Unable to obtain from_lsn from " . |
287 | + "$option_extra_lsndir/xtrabackup_checkpoints, " . |
288 | + "writing NULL to history record.\n"; |
289 | + $sth->bind_param(12, undef); |
290 | + $file_content .= "\ninnodb_from_lsn = "; |
291 | + } |
292 | + |
293 | + # innodb_to_lsn |
294 | + # grab the to_lsn from the extra checkpoints file, parse and delete it |
295 | + # if necessary. This could generate an error if the checkpoints file |
296 | + # isn't 100% correct in the format. |
297 | + $tmp = "cat $option_extra_lsndir/xtrabackup_checkpoints | grep to_lsn"; |
298 | + $tmp = qx($tmp); |
299 | + chomp($tmp); |
300 | + substr($tmp, 0, 9, ""); # strip "to_lsn = " |
301 | + if (length $tmp) { |
302 | + $sth->bind_param(13, $tmp); |
303 | + $file_content .= "\ninnodb_to_lsn = $tmp"; |
304 | + } else { |
305 | + print STDERR "WARNING : Unable to obtain to_lsn from " . |
306 | + "$option_extra_lsndir/xtrabackup_checkpoints, " . |
307 | + "writing NULL to history record.\n"; |
308 | + $sth->bind_param(13, undef); |
309 | + $file_content .= "\ninnodb_to_lsn = "; |
310 | + } |
311 | + |
312 | + # we're finished with the checkpoints file, delete it if it was a temp |
313 | + # only for the history |
314 | + if ($history_delete_checkpoints > 0) { |
315 | + unlink("$option_extra_lsndir/xtrabackup_checkpoints"); |
316 | + } |
317 | + |
318 | + # partial (Y | N) |
319 | + if ($option_include || $option_databases || $option_tables_file || $option_export) { |
320 | + $sth->bind_param(14, "Y"); |
321 | + $file_content .= "\npartial = Y"; |
322 | + } else { |
323 | + $sth->bind_param(14, "N"); |
324 | + $file_content .= "\npartial = N"; |
325 | + } |
326 | + |
327 | + # incremental (Y | N) |
328 | + if ($option_incremental) { |
329 | + $sth->bind_param(15, "Y"); |
330 | + $file_content .= "\nincremental = Y"; |
331 | + } else { |
332 | + $sth->bind_param(15, "N"); |
333 | + $file_content .= "\nincremental = N"; |
334 | + } |
335 | + |
336 | + # format (file | tar | xbsream) |
337 | + if ($option_stream eq 'tar') { |
338 | + $sth->bind_param(16, "tar"); |
339 | + $file_content .= "\nformat = tar"; |
340 | + } elsif ($option_stream eq 'xbstream') { |
341 | + $sth->bind_param(16, "xbstream"); |
342 | + $file_content .= "\nformat = xbstream"; |
343 | + } else { |
344 | + $sth->bind_param(16, "file"); |
345 | + $file_content .= "\nformat = file"; |
346 | + } |
347 | + |
348 | + # compact (Y | N) |
349 | + if ($option_compact) { |
350 | + $sth->bind_param(17, "Y"); |
351 | + $file_content .= "\ncompact = Y"; |
352 | + } else { |
353 | + $sth->bind_param(17, "N"); |
354 | + $file_content .= "\ncompact = N"; |
355 | + } |
356 | + |
357 | + # compressed (Y | N) |
358 | + if ($option_compress) { |
359 | + $sth->bind_param(18, "Y"); |
360 | + $file_content .= "\ncompressed = Y"; |
361 | + } else { |
362 | + $sth->bind_param(18, "N"); |
363 | + $file_content .= "\ncompressed = N"; |
364 | + } |
365 | + |
366 | + # encrypted (Y | N) |
367 | + if ($option_encrypt) { |
368 | + $sth->bind_param(19, "Y"); |
369 | + $file_content .= "\nencrypted = Y"; |
370 | + } else { |
371 | + $sth->bind_param(19, "N"); |
372 | + $file_content .= "\nencrypted = N"; |
373 | + } |
374 | + |
375 | + # create the backup history file |
376 | + write_to_backup_file("$backup_history", "$file_content"); |
377 | + |
378 | + if (!defined($option_history)) { |
379 | + return; |
380 | + } |
381 | + |
382 | + mysql_query($con, "CREATE DATABASE IF NOT EXISTS PERCONA_SCHEMA"); |
383 | + mysql_query($con, "CREATE TABLE IF NOT EXISTS PERCONA_SCHEMA.xtrabackup_history(". |
384 | + "uuid VARCHAR(40) NOT NULL PRIMARY KEY,". |
385 | + "name VARCHAR(255) DEFAULT NULL,". |
386 | + "tool_name VARCHAR(255) DEFAULT NULL,". |
387 | + "tool_command TEXT DEFAULT NULL,". |
388 | + "tool_version VARCHAR(255) DEFAULT NULL,". |
389 | + "ibbackup_version VARCHAR(255) DEFAULT NULL,". |
390 | + "server_version VARCHAR(255) DEFAULT NULL,". |
391 | + "start_time TIMESTAMP NULL DEFAULT NULL,". |
392 | + "end_time TIMESTAMP NULL DEFAULT NULL,". |
393 | + "lock_time BIGINT UNSIGNED DEFAULT NULL,". |
394 | + "binlog_pos VARCHAR(128) DEFAULT NULL,". |
395 | + "innodb_from_lsn BIGINT UNSIGNED DEFAULT NULL,". |
396 | + "innodb_to_lsn BIGINT UNSIGNED DEFAULT NULL,". |
397 | + "partial ENUM('Y', 'N') DEFAULT NULL,". |
398 | + "incremental ENUM('Y', 'N') DEFAULT NULL,". |
399 | + "format ENUM('file', 'tar', 'xbstream') DEFAULT NULL,". |
400 | + "compact ENUM('Y', 'N') DEFAULT NULL,". |
401 | + "compressed ENUM('Y', 'N') DEFAULT NULL,". |
402 | + "encrypted ENUM('Y', 'N') DEFAULT NULL". |
403 | + ") CHARACTER SET utf8 ENGINE=innodb"); |
404 | + |
405 | + eval { |
406 | + $sth->execute; |
407 | + } || die("Error while attempting to insert history record.\n"); |
408 | + |
409 | + print STDERR "$prefix Backup history record uuid $uuid successfully written\n"; |
410 | +} |
411 | + |
412 | + |
413 | =pod |
414 | |
415 | =head1 NAME |
416 | @@ -4735,8 +5052,10 @@ |
417 | [--defaults-file=MY.CNF] [--defaults-group=GROUP-NAME] |
418 | [--databases=LIST] [--no-lock] |
419 | [--tmpdir=DIRECTORY] [--tables-file=FILE] |
420 | + [--history=NAME] |
421 | [--incremental] [--incremental-basedir] |
422 | [--incremental-dir] [--incremental-force-scan] [--incremental-lsn] |
423 | + [--incremental-history-name=NAME] [--incremental-history-uuid=UUID] |
424 | [--compact] |
425 | BACKUP-ROOT-DIR |
426 | |
427 | @@ -4906,6 +5225,10 @@ |
428 | |
429 | This option displays a help screen and exits. |
430 | |
431 | +=item --history=NAME |
432 | + |
433 | +This option enables the tracking of backup history in the PERCONA_SCHEMA.xtrabackup_history table. An optional history series name may be specified that will be placed with the history record for the current backup being taken. |
434 | + |
435 | =item --host=HOST |
436 | |
437 | This option specifies the host to use when connecting to the database server with TCP/IP. The option accepts a string argument. It is passed to the mysql child process without alteration. See mysql --help for details. |
438 | @@ -4930,6 +5253,14 @@ |
439 | |
440 | This option specifies the directory where the incremental backup will be combined with the full backup to make a new full backup. The option accepts a string argument. It is used with the --incremental option. |
441 | |
442 | +=item --incremental-history-name=NAME |
443 | + |
444 | +This option specifies the name of the backup series stored in the PERCONA_SCHEMA.xtrabackup_history history record to base an incremental backup on. Xtrabackup will search the history table looking for the most recent (highest innodb_to_lsn), successful backup in the series and take the to_lsn value to use as the starting lsn for the incremental backup. This will be mutually exclusive with --incremental-history-uuid, --incremental-basedir and --incremental-lsn. If no valid lsn can be found (no series by that name, no successful backups by that name) xtrabackup will return with an error. It is used with the --incremental option. |
445 | + |
446 | +=item --incremental-history-uuid=UUID |
447 | + |
448 | +This option specifies the UUID of the specific history record stored in the PERCONA_SCHEMA.xtrabackup_history to base an incremental backup on. --incremental-history-name, --incremental-basedir and --incremental-lsn. If no valid lsn can be found (no success record with that uuid) xtrabackup will return with an error. It is used with the --incremental option. |
449 | + |
450 | =item --incremental-force-scan |
451 | |
452 | This options tells xtrabackup to perform full scan of data files for taking an incremental backup even if full changed page bitmap data is available to enable the backup without the full scan. |
453 | |
454 | === added file 'xtrabackup/test/t/history_on_server.sh' |
455 | --- xtrabackup/test/t/history_on_server.sh 1970-01-01 00:00:00 +0000 |
456 | +++ xtrabackup/test/t/history_on_server.sh 2013-09-25 17:11:22 +0000 |
457 | @@ -0,0 +1,210 @@ |
458 | +############################################################################### |
459 | +# Test history-on-server feature |
460 | +############################################################################### |
461 | + |
462 | + |
463 | +############################################################################### |
464 | +# Gets a single column value from the last history record added |
465 | +function get_one_value() |
466 | +{ |
467 | + local column=$1 |
468 | + val=`${MYSQL} ${MYSQL_ARGS} -Ns -e "SELECT $column FROM PERCONA_SCHEMA.xtrabackup_history ORDER BY start_time DESC LIMIT 1"` |
469 | +} |
470 | + |
471 | + |
472 | +############################################################################### |
473 | +# Checks a single column from the last history record added for some value and |
474 | +# not NULL. |
475 | +function check_for_not_NULL() |
476 | +{ |
477 | + local column=$1 |
478 | + get_one_value "$column" |
479 | + if [ -z "$val" ] || [ "$val" = "NULL" ]; |
480 | + then |
481 | + vlog "Error: $column in history record invalid, expected NOT NULL, got \"$val\"" |
482 | + exit 1 |
483 | + fi |
484 | +} |
485 | + |
486 | + |
487 | +############################################################################### |
488 | +# Checks a single column from the last history record added to see if is a |
489 | +# specific value. |
490 | +function check_for_value() |
491 | +{ |
492 | + local column=$1 |
493 | + shift |
494 | + get_one_value "$column" |
495 | + if [ -z "$val" ] || [ "$val" != "$@" ]; |
496 | + then |
497 | + vlog "Error: $column in history record invalid, got \"$val\" expected \"$@\"" |
498 | + exit 1 |
499 | + fi |
500 | +} |
501 | + |
502 | + |
503 | +############################################################################### |
504 | +vlog "Prepping server" |
505 | +start_server |
506 | +load_dbase_schema incremental_sample |
507 | +multi_row_insert incremental_sample.test \({1..100},100\) |
508 | +backup_dir=$topdir/backups |
509 | +mkdir $backup_dir |
510 | + |
511 | + |
512 | +############################################################################### |
513 | +# This tests the to make sure that no xtrabackup_history unless --history is |
514 | +# specified |
515 | +#vlog "Testing no --history" |
516 | +#innobackupex --stream=tar $backup_dir > /dev/null |
517 | + |
518 | +#run_cmd_expect_failure get_one_value "uuid" |
519 | + |
520 | + |
521 | +############################################################################### |
522 | +# This tests the basic creation of a history record and that fields are |
523 | +# populated with some data. It also tests specifically that |
524 | +# partial, incremental, compact, compressed, encrypted and format are exactly |
525 | +# the correct values after the backup. |
526 | +# Missing is a test that binlog_pos is NULL, that would require restarting the |
527 | +# server without the log-bin option in the .cnf file but that has been tested |
528 | +# manually and doesn't seem to be something that would be required to be |
529 | +# validated. |
530 | +vlog "Testing basic history record" |
531 | +innobackupex --history=test1 --stream=tar $backup_dir > /dev/null |
532 | + |
533 | +for column in uuid name tool_name tool_command tool_version ibbackup_version \ |
534 | +server_version start_time end_time lock_time binlog_pos innodb_from_lsn \ |
535 | +innodb_to_lsn |
536 | +do |
537 | + check_for_not_NULL "$column" |
538 | +done |
539 | + |
540 | +for column in partial incremental compact compressed encrypted |
541 | +do |
542 | + check_for_value "$column" "N" |
543 | +done |
544 | + |
545 | +check_for_value "format" "tar" |
546 | + |
547 | +# saving for later |
548 | +get_one_value "innodb_to_lsn" |
549 | +first_to_lsn=$val |
550 | + |
551 | + |
552 | +############################################################################### |
553 | +# This tests the taking of an incremental backup based on the last record |
554 | +# of a history series and validates that the lsns in the record are correct. |
555 | +# It also tests that format, incremental and compact are exactly the correct |
556 | +# values after the backup. |
557 | +vlog "Testing incremental based on history name" |
558 | + |
559 | +multi_row_insert incremental_sample.test \({101..200},100\) |
560 | + |
561 | +innobackupex --history=test1 --incremental \ |
562 | +--incremental-history-name=test1 --compact $backup_dir > /dev/null |
563 | + |
564 | +# saving for later |
565 | +get_one_value "uuid" |
566 | +second_uuid=$val |
567 | +get_one_value "innodb_from_lsn" |
568 | +second_from_lsn=$val |
569 | +get_one_value "innodb_to_lsn" |
570 | +second_to_lsn=$val |
571 | + |
572 | +check_for_value "format" "file" |
573 | +check_for_value "incremental" "Y" |
574 | +check_for_value "compact" "Y" |
575 | + |
576 | +if [ -z "$second_from_lsn" ] || [ "$second_from_lsn" != "$first_to_lsn" ] |
577 | +then |
578 | + vlog "Second backup was not properly based on the to_lsn of the first" |
579 | + exit 1 |
580 | +fi |
581 | + |
582 | +multi_row_insert incremental_sample.test \({201..300},100\) |
583 | + |
584 | +# This will be a backup based on the last incremental just done, so, its |
585 | +# innodb_from_lsn (third_from_lsn) should be the same as the value in |
586 | +# second_to_lsn. This tests that we find the right record in the test1 series |
587 | +# out of the two records that should be present before the backup is done. |
588 | +innobackupex --history=test1 --incremental \ |
589 | +--incremental-history-name=test1 $backup_dir > /dev/null |
590 | + |
591 | +# saving for later |
592 | +get_one_value "uuid" |
593 | +third_uuid=$val |
594 | +get_one_value "innodb_from_lsn" |
595 | +third_from_lsn=$val |
596 | +get_one_value "innodb_to_lsn" |
597 | +third_to_lsn=$val |
598 | + |
599 | +if [ -z "$third_from_lsn" ] || [ "$third_from_lsn" != "$second_to_lsn" ] |
600 | +then |
601 | + vlog "Third backup was not properly based on the to_lsn of the second" |
602 | + exit 1 |
603 | +fi |
604 | + |
605 | + |
606 | +############################################################################### |
607 | +# This tests that we can base an incremental on a specific history record |
608 | +# identified by its uuid that we captured earlier from a history record or it |
609 | +# could be scraped from the output of innobackupex at some point in the past. |
610 | +# It also tests specifically that incremental, compressed, encrypted and format |
611 | +# are exactly the correct values after the backup. |
612 | +# It tests that --history can be specified, resulting in a history record with |
613 | +# no name |
614 | +vlog "Testing incremental based on history uuid" |
615 | +multi_row_insert incremental_sample.test \({301..400},100\) |
616 | + |
617 | +innobackupex --history --incremental --incremental-history-uuid=$third_uuid \ |
618 | +--stream=xbstream --compress --encrypt=AES256 \ |
619 | +$backup_dir --encrypt-key=percona_xtrabackup_is_awesome___ > /dev/null |
620 | + |
621 | +get_one_value "innodb_from_lsn" |
622 | +fourth_from_lsn=$val |
623 | + |
624 | +for column in incremental compressed encrypted |
625 | +do |
626 | + check_for_value "$column" "Y" |
627 | +done |
628 | + |
629 | +check_for_value "format" "xbstream" |
630 | +check_for_value "name" "NULL" |
631 | + |
632 | +# validate command tool and encrypt key scrubbibng but need to pop off first |
633 | +# two arguments in the result added by test framework function innobackupex |
634 | +get_one_value "tool_command" |
635 | +val=${val:`expr index "$val" " "`}; val=${val:`expr index "$val" " "`} |
636 | +expected_val="--history --incremental "\ |
637 | +"--incremental-history-uuid=$third_uuid --stream=xbstream --compress "\ |
638 | +"--encrypt=AES256 $backup_dir --encrypt-key=..." |
639 | + |
640 | +if [ -z "$val" ] || [ "$val" != "$expected_val" ] |
641 | +then |
642 | + vlog "Error: $column in history record invalid, got \"$val\" expected \"$expected_val\"" |
643 | + exit 1 |
644 | +fi |
645 | + |
646 | +if [ -z "$fourth_from_lsn" ] || [ "$fourth_from_lsn" != "$third_to_lsn" ] |
647 | +then |
648 | + vlog "Fourth backup was not properly based on the to_lsn of the third" |
649 | + exit 1 |
650 | +fi |
651 | + |
652 | + |
653 | +############################################################################### |
654 | +# This tests that innobackupex fails when an invalid --incremental-history-name |
655 | +# is given. |
656 | +vlog "Testing bad --incremental-history-name" |
657 | +run_cmd_expect_failure $IB_BIN $IB_ARGS --incremental \ |
658 | +--incremental-history-name=foo --stream=tar $backup_dir > /dev/null |
659 | + |
660 | + |
661 | + |
662 | +############################################################################### |
663 | +# This tests that innobackupex fails when an invalid --incremental-history-uuid |
664 | +# is given. |
665 | +vlog "Testing bad --incremental-history-uuid" |
666 | +run_cmd_expect_failure $IB_BIN $IB_ARGS --incremental \ |
667 | +--incremental-history-uuid=foo --stream=tar $backup_dir > /dev/null |
George,
- I propose we rename ‘mysql_version’ to ‘server_version’, since we
support not only MySQL.
- let’s also rename xtrabackup_history to xtrabackup_info. It checkpoints
contains meta information about the backup where it’s stored, not
the history. I think we can even deprecate xtrabackup_
with xtrabackup_info in future.
- the table does not define the character set as we discussed connection settings.
previously. This means the character set will depend on
server/
- update_history() is vulnerable to SQL injection attacks :) what if
something like this is passed to --history, for example:
-- history= "foo'; DROP DATABASE customers".
Please use DBI prepared statements. This way you don’t have to roll
your own quoting in other places.
- it also appears that xtrabackup_history will not be created if nally, but just avoid executing the actual INSERT if history if false.
--history is not passed to innobackupex. I think it should always
be created. I.e. update_history() should collect all info
unconditio
$option_
- for start_time / end_time it’s better to use time() and then
convert values it returns to TIMESTAMP with FROM_UNIXTIME():
$history_ start_time = time();
...
# start_time $history_ start_time) ";
$insert_vals .= ", FROM_UNIXTIME(
# end_time $tmp)";
$tmp = time();
$insert_vals .= ", FROM_UNIXTIME(
- /proc is only available on Linux, so the following will fail on
everything else. And there are really no reasons to use it,
there are many portable ways to get the cmd line from a Perl script
+ $tmp = sprintf("cat /proc/%d/cmdline", $$);
+ $tmp = qx($tmp);
+ system("cp /proc/$$/cmdline /tmp");
+ $tmp =~ s/\0/ /g; # transform embedded nulls between args to spaces
- system("rm -f $file") is equivalent to unlink($file)
- do you really want to scrub --encrypt and --encrypt-key-file?
- here’s a more elegant and correct scrub_option() implementation:
sub scrub_option {
my $scrub = shift;
my $command = shift;
return $command =~ s/--$scrub=[^ ]+ /--$scrub=... /g
}
- in case --incremental- history- name or --incremental- history- uuid is
used, it might be useful to have a diagnostic message about the LSN
value being used and why it is being used.
- I guess built-in innobackupex help needs some updates (still
mentions --history-name, --no-history, etc.)
- in the test case: it is not necessary to include inc/common.sh
anymore. it is also not necessary to use “shift” if you don’t
access the parameters array later in a function