Merge lp:~sergei.glushchenko/percona-xtrabackup/xb2.0-bug483827 into lp:percona-xtrabackup/2.0

Proposed by Sergei Glushchenko
Status: Merged
Approved by: Alexey Kopytov
Approved revision: no longer in the source branch.
Merged at revision: 431
Proposed branch: lp:~sergei.glushchenko/percona-xtrabackup/xb2.0-bug483827
Merge into: lp:percona-xtrabackup/2.0
Prerequisite: lp:~sergei.glushchenko/percona-xtrabackup/xb2.0-bug996493
Diff against target: 325 lines (+108/-19)
6 files modified
doc/source/how-tos.rst (+1/-0)
doc/source/innobackupex/innobackupex_option_reference.rst (+4/-0)
doc/source/xtrabackup_bin/xbk_option_reference.rst (+4/-0)
innobackupex (+34/-16)
src/xtrabackup.c (+20/-3)
test/t/bug483827.sh (+45/-0)
To merge this branch: bzr merge lp:~sergei.glushchenko/percona-xtrabackup/xb2.0-bug483827
Reviewer Review Type Date Requested Status
Alexey Kopytov (community) Approve
Review via email: mp+106348@code.launchpad.net

This proposal supersedes a proposal from 2012-03-29.

Description of the change

Bug #483827. Support for mysqld_multi.
Fix based on work of Daniël van Eeden (lp:~dveeden/percona-xtrabackup/lp483827)
--section option added to innobackupex, specifying which section of my.cnf to handle.
--mysqld-section option added to xtrabackup for the same purpose.
Tescase does not run mysqld_multi, it just tests correctness of --section option.

http://jenkins.percona.com/view/XtraBackup/job/percona-xtrabackup-2.0-param/197/

To post a comment you must log in.
Revision history for this message
Alexey Kopytov (akopytov) wrote : Posted in a previous version of this proposal

Sergei,

   - sections in my.cnf are called groups in MySQL documentation. So
     let's be consistent and call them groups as well, and name the
     options (both the innobackupex and the xtrabackup one)
     --defaults-group, rather than --section and --mysqld-section

   - it looks like get_option is always passed the same value as its
     second argument, so I wonder if it really needs that argument, or
     rather just use $option_mysqld_section (or whatever it will be
     renamed to, i.e. $option_defaults_group) internally.

   - s/laod/load/
   - s/accetps/accepts/
   - s/rstore_args/restore_args/. Though you don't really need that
     function, as tests are executed in a separate shell process, so
     modifications to variables have no effect on other tests anyway.

   - I know that code in main() that scan options for "--mysqld-section"
     was copy-pasted, but please format it according to InnoDB
     style. Because currently it's a terrible mix of all possible
     formatting styles.

   - in the same code, I don't think strstr() is necessary, because you
     already have the pointer to '=' (or terminating zero) in optend.

review: Needs Fixing
Revision history for this message
Sergei Glushchenko (sergei.glushchenko) wrote :

--defaults-group option instead of --section and --mysql-section

kept group name argument for has_option.
fixed typos.

reformatted code to be more InnoDB-style (if there are other requirements, I'm ready for the next iteration).

removed unnecessary strstr.

Revision history for this message
Alexey Kopytov (akopytov) wrote :

Sergei,

As discussed on IRC, --defaults-group does not necessarily have to be the first option, so docs should be corrected.

And, while you are at it would be good to correct built-in help for --defaults-group in innobackupex.

Help text for other options either have units or some description of possible values after the '=' sign, e.g.:

--ibbackup=IBBACKUP-BINARY
--remote-host=HOSTNAME

So the text for --defaults-group should be something like "--defaults-group=GROUP-NAME" rather thatn "--defaults-group=mysqld".

review: Needs Fixing
Revision history for this message
Sergei Glushchenko (sergei.glushchenko) wrote :

Docs fixed

Revision history for this message
Alexey Kopytov (akopytov) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'doc/source/how-tos.rst'
2--- doc/source/how-tos.rst 2011-07-07 05:32:50 +0000
3+++ doc/source/how-tos.rst 2012-05-18 11:25:23 +0000
4@@ -13,6 +13,7 @@
5 howtos/recipes_ibkx_local
6 howtos/recipes_ibkx_stream
7 howtos/recipes_ibkx_inc
8+ howtos/recipes_ibkx_multi
9
10
11 .. _recipes-xbk:
12
13=== modified file 'doc/source/innobackupex/innobackupex_option_reference.rst'
14--- doc/source/innobackupex/innobackupex_option_reference.rst 2012-05-15 09:30:50 +0000
15+++ doc/source/innobackupex/innobackupex_option_reference.rst 2012-05-18 11:25:23 +0000
16@@ -70,6 +70,10 @@
17
18 This option accepts a string argument that specifies the port to use when connecting to the database server with TCP/IP. It is passed to the :command:`mysql` child process. It is passed to the :command:`mysql` child process without alteration. See :command:`mysql --help` for details.
19
20+.. option:: --defaults-group
21+
22+ This option accepts a string argument that specifies the group which should be read from the configuration file. This is needed if you use mysqld_multi.
23+
24 .. option:: --socket
25
26 This option accepts a string argument that specifies the socket to use when connecting to the local database server with a UNIX domain socket. It is passed to the mysql child process without alteration. See :command:`mysql --help` for details.
27
28=== modified file 'doc/source/xtrabackup_bin/xbk_option_reference.rst'
29--- doc/source/xtrabackup_bin/xbk_option_reference.rst 2012-04-21 07:12:28 +0000
30+++ doc/source/xtrabackup_bin/xbk_option_reference.rst 2012-05-18 11:25:23 +0000
31@@ -101,6 +101,10 @@
32 --innodb-read-io-threads
33 --innodb-write-io-threads
34
35+.. option:: --defaults-group
36+
37+ This option is to set the group which should be read from the configuration file. This is used by innobackupex if you use the `--defaults-group` option. It is needed for mysqld_multi deployments.
38+
39 .. option:: --log-stream
40
41 Makes xtrabackup not copy data files, and output the contents of the InnoDB log files to STDOUT until the :option:`--suspend-at-end` file is deleted. This option enables :option:`--suspend-at-end` automatically.
42
43=== modified file 'innobackupex'
44--- innobackupex 2012-05-18 11:25:23 +0000
45+++ innobackupex 2012-05-18 11:25:23 +0000
46@@ -84,6 +84,7 @@
47 my $option_mysql_port = '';
48 my $option_mysql_socket = '';
49 my $option_mysql_host = '';
50+my $option_defaults_group = 'mysqld';
51 my $option_no_timestamp = '';
52 my $option_slave_info = '';
53 my $option_galera_info = '';
54@@ -377,7 +378,7 @@
55 # process.
56 #
57 sub backup {
58- my $orig_datadir = get_option(\%config, 'mysqld', 'datadir');
59+ my $orig_datadir = get_option(\%config, $option_defaults_group, 'datadir');
60
61 # check that we can connect to the database. This done by
62 # connecting, issuing a query, and closing the connection.
63@@ -626,13 +627,13 @@
64 # back to their original locations.
65 #
66 sub copy_back {
67- my $orig_datadir = get_option(\%config, 'mysqld', 'datadir');
68+ my $orig_datadir = get_option(\%config, $option_defaults_group, 'datadir');
69 my $orig_ibdata_dir =
70- get_option(\%config, 'mysqld', 'innodb_data_home_dir');
71+ get_option(\%config, $option_defaults_group, 'innodb_data_home_dir');
72 my $orig_innodb_data_file_path =
73- get_option(\%config, 'mysqld', 'innodb_data_file_path');
74+ get_option(\%config, $option_defaults_group, 'innodb_data_file_path');
75 my $orig_iblog_dir =
76- get_option(\%config, 'mysqld', 'innodb_log_group_home_dir');
77+ get_option(\%config, $option_defaults_group, 'innodb_log_group_home_dir');
78 my $iblog_files = 'ib_logfile.*';
79 my $excluded_files =
80 '\.\.?|backup-my\.cnf|xtrabackup_logfile|' .
81@@ -740,6 +741,10 @@
82 $options = $options . " --defaults-file=\"${backup_dir}/backup-my.cnf\" ";
83 }
84
85+ if ($option_defaults_group) {
86+ $options = $options . " --defaults-group=\"$option_defaults_group\" ";
87+ }
88+
89 $options = $options . "--prepare --target-dir=$backup_dir";
90
91 if ($option_uncompress) {
92@@ -863,6 +868,10 @@
93 $options = $options . " --defaults-file=\"$option_defaults_file\" ";
94 }
95
96+ if ($option_defaults_group) {
97+ $options = $options . " --defaults-group=\"$option_defaults_group\" ";
98+ }
99+
100 $options = $options . "--backup --suspend-at-end";
101
102 if (!$option_remote_host && !$option_stream) {
103@@ -925,13 +934,13 @@
104
105 if($option_remote_host) {
106 #direct copy to remote
107- my $orig_datadir = get_option(\%config, 'mysqld', 'datadir');
108+ my $orig_datadir = get_option(\%config, $option_defaults_group, 'datadir');
109 my $orig_ibdata_dir =
110- get_option(\%config, 'mysqld', 'innodb_data_home_dir');
111+ get_option(\%config, $option_defaults_group, 'innodb_data_home_dir');
112 my $orig_innodb_data_file_path =
113- get_option(\%config, 'mysqld', 'innodb_data_file_path');
114+ get_option(\%config, $option_defaults_group, 'innodb_data_file_path');
115 my $innodb_flush_method =
116- get_option(\%config, 'mysqld', 'innodb_flush_method');
117+ get_option(\%config, $option_defaults_group, 'innodb_flush_method');
118 my $innodb_use_odirect;
119 $innodb_use_odirect = 1 if $innodb_flush_method =~ m/^(ALL_)?O_DIRECT$/i;
120
121@@ -1582,7 +1591,7 @@
122 read_config_file(\%config);
123
124 if(!$option_tmpdir) {
125- $option_tmpdir = get_option(\%config, 'mysqld', 'tmpdir');
126+ $option_tmpdir = get_option(\%config, $option_defaults_group, 'tmpdir');
127 }
128
129 # get innodb log home directory from options file
130@@ -1658,8 +1667,8 @@
131
132 my $option_name;
133 foreach $option_name (@option_names) {
134- if (has_option(\%config, 'mysqld', $option_name)) {
135- my $option_value = get_option(\%config, 'mysqld', $option_name);
136+ if (has_option(\%config, $option_defaults_group, $option_name)) {
137+ my $option_value = get_option(\%config, $option_defaults_group, $option_name);
138 $options_dump .= "$option_name=$option_value\n";
139 }
140 }
141@@ -1725,6 +1734,7 @@
142 'user=s' => \$option_mysql_user,
143 'host=s' => \$option_mysql_host,
144 'port=s' => \$option_mysql_port,
145+ 'defaults-group=s' => \$option_defaults_group,
146 'slave-info' => \$option_slave_info,
147 'galera-info' => \$option_galera_info,
148 'socket=s' => \$option_mysql_socket,
149@@ -1840,7 +1850,7 @@
150 sub make_backup_dir {
151 my $dir;
152 my $innodb_data_file_path =
153- get_option(\%config, 'mysqld', 'innodb_data_file_path');
154+ get_option(\%config, $option_defaults_group, 'innodb_data_file_path');
155
156 # create backup directory
157 $dir = $backup_root;
158@@ -1932,7 +1942,7 @@
159 #
160 sub backup_files {
161 my $prep_mode = shift;
162- my $source_dir = get_option(\%config, 'mysqld', 'datadir');
163+ my $source_dir = get_option(\%config, $option_defaults_group, 'datadir');
164 my @list;
165 my $file;
166 my $database;
167@@ -2211,6 +2221,10 @@
168 $options = $options . " --defaults-file=\"${backup_dir}/backup-my.cnf\" ";
169 }
170
171+ if ($option_defaults_group) {
172+ $options = $options . " --defaults-group=\"$option_defaults_group\" ";
173+ }
174+
175 $options = $options . "--print-param";
176
177
178@@ -2616,7 +2630,7 @@
179 [--no-timestamp] [--ibbackup=IBBACKUP-BINARY]
180 [--slave-info] [--stream=tar|xbstream]
181 [--scpopt=OPTIONS-FOR-SCP] [--sshopt=OPTIONS-FOR-SSH]
182- [--defaults-file=MY.CNF]
183+ [--defaults-file=MY.CNF] [--defaults-group=GROUP-NAME]
184 [--databases=LIST] [--remote-host=HOSTNAME] [--no-lock]
185 [--tmpdir=DIRECTORY] [--tables-file=FILE]
186 [--incremental] [--incremental-basedir]
187@@ -2628,7 +2642,7 @@
188 [--export] [--redo-only] [--ibbackup=IBBACKUP-BINARY]
189 BACKUP-DIR
190
191-innobackupex --copy-back [--defaults-file=MY.CNF] BACKUP-DIR
192+innobackupex --copy-back [--defaults-file=MY.CNF] [--defaults-group=GROUP-NAME] BACKUP-DIR
193
194 =head1 DESCRIPTION
195
196@@ -2811,6 +2825,10 @@
197
198 This option specifies the MySQL username used when connecting to the server, if that's not the current user. The option accepts a string argument. It is passed to the mysql child process without alteration. See mysql --help for details.
199
200+=item --defaults-group=GROUP-NAME
201+
202+This option specifies the group name in my.cnf which should be used. This is needed for mysqld_multi deployments.
203+
204 =item --version
205
206 This option displays the xtrabackup version and copyright notice and then exits.
207
208=== modified file 'src/xtrabackup.c'
209--- src/xtrabackup.c 2012-05-18 11:25:23 +0000
210+++ src/xtrabackup.c 2012-05-18 11:25:23 +0000
211@@ -769,6 +769,8 @@
212 char *opt_mysql_tmpdir = NULL;
213 MY_TMPDIR mysql_tmpdir_list;
214
215+const char *defaults_group = "mysqld";
216+
217 /* === static parameters in ha_innodb.cc */
218
219 #define HA_INNOBASE_ROWS_IN_TABLE 10000 /* to get optimization right */
220@@ -994,7 +996,8 @@
221 OPT_INNODB_SYNC_SPIN_LOOPS,
222 OPT_INNODB_THREAD_CONCURRENCY,
223 OPT_INNODB_THREAD_SLEEP_DELAY,
224- OPT_XTRA_DEBUG_SYNC
225+ OPT_XTRA_DEBUG_SYNC,
226+ OPT_DEFAULTS_GROUP
227 };
228
229 static struct my_option my_long_options[] =
230@@ -1278,6 +1281,9 @@
231 0, GET_STR, REQUIRED_ARG, 0, 0, 0, 0, 0, 0},
232 #endif
233
234+ {"defaults_group", OPT_DEFAULTS_GROUP, "defaults group in config file (default \"mysqld\").",
235+ (G_PTR*) &defaults_group, (G_PTR*) &defaults_group,
236+ 0, GET_STR, REQUIRED_ARG, 0, 0, 0, 0, 0, 0},
237 { 0, 0, 0, 0, 0, 0, GET_NO_ARG, NO_ARG, 0, 0, 0, 0, 0, 0}
238 };
239
240@@ -1337,7 +1343,7 @@
241 #endif
242 }
243
244-static const char *load_default_groups[]= { "mysqld","xtrabackup",0 };
245+static const char *load_default_groups[]= { "mysqld","xtrabackup",0,0 };
246
247 static void print_version(void)
248 {
249@@ -6280,6 +6286,17 @@
250 MY_INIT(argv[0]);
251 xb_regex_init();
252
253+ /* scan options for group to load defaults from */
254+ {
255+ int i;
256+ char* optend;
257+ for (i=1; i < argc; i++) {
258+ optend = strcend(argv[i], '=');
259+ if (strncmp(argv[i], "--defaults-group", optend - argv[i]) == 0) {
260+ load_default_groups[2] = defaults_group = optend + 1;
261+ }
262+ }
263+ }
264 load_defaults("my",load_default_groups,&argc,&argv);
265
266 /* ignore unsupported options */
267@@ -6518,7 +6535,7 @@
268 exit(EXIT_FAILURE);
269
270 printf("# This MySQL options file was generated by XtraBackup.\n");
271- printf("[mysqld]\n");
272+ printf("[%s]\n", defaults_group);
273 printf("datadir = \"%s\"\n", mysql_data_home);
274 printf("tmpdir = \"%s\"\n", mysql_tmpdir_list.list[0]);
275 printf("innodb_data_home_dir = \"%s\"\n",
276
277=== added file 'test/t/bug483827.sh'
278--- test/t/bug483827.sh 1970-01-01 00:00:00 +0000
279+++ test/t/bug483827.sh 2012-05-18 11:25:23 +0000
280@@ -0,0 +1,45 @@
281+########################################################################
282+# Bug #483827: support for mysqld_multi
283+########################################################################
284+
285+function modify_args()
286+{
287+ XB_ARGS=`echo $XB_ARGS | sed -e 's/my.cnf/my_multi.cnf/'`
288+ IB_ARGS=`echo $IB_ARGS | sed -e 's/my.cnf/my_multi.cnf/'`
289+}
290+
291+. inc/common.sh
292+
293+init
294+mv ${mysql_datadir} ${mysql_datadir}1
295+run_mysqld --datadir=${mysql_datadir}1
296+
297+backup_dir=$topdir/backup
298+rm -rf $backup_dir
299+
300+# change defaults file from my.cnf to my_multi.cnf
301+modify_args
302+
303+# make my_multi.cnf
304+echo "
305+[mysqld1]
306+datadir=${mysql_datadir}1
307+tmpdir=$mysql_tmpdir" > $topdir/my_multi.cnf
308+
309+# Backup
310+innobackupex --no-timestamp --defaults-group=mysqld1 $backup_dir
311+innobackupex --apply-log $backup_dir
312+
313+stop_mysqld
314+
315+# clean datadir
316+rm -rf ${mysql_datadir}1/*
317+
318+# restore backup
319+innobackupex --copy-back --defaults-group=mysqld1 $backup_dir
320+
321+# make sure that data are in correct place
322+if [ ! -f ${mysql_datadir}1/ibdata1 ] ; then
323+ vlog "Data not found in ${mysql_datadir}1"
324+ exit -1
325+fi

Subscribers

People subscribed via source and target branches