Merge lp:~sergei.glushchenko/percona-xtrabackup/ST-25967-2.0 into lp:percona-xtrabackup/2.0

Proposed by Sergei Glushchenko on 2012-11-16
Status: Merged
Approved by: Alexey Kopytov on 2012-11-16
Approved revision: 473
Merged at revision: 481
Proposed branch: lp:~sergei.glushchenko/percona-xtrabackup/ST-25967-2.0
Merge into: lp:percona-xtrabackup/2.0
Prerequisite: lp:~sergei.glushchenko/percona-xtrabackup/BT-26901-2.0
Diff against target: 214 lines (+141/-37)
2 files modified
src/xtrabackup.c (+44/-37)
test/t/bug1062684.sh (+97/-0)
To merge this branch: bzr merge lp:~sergei.glushchenko/percona-xtrabackup/ST-25967-2.0
Reviewer Review Type Date Requested Status
Alexey Kopytov (community) 2012-11-16 Approve on 2012-11-16
Review via email: mp+134637@code.launchpad.net

This proposal supersedes a proposal from 2012-10-22.

Description of the change

Bug #1062684 Applying incremental backup using xtrabackup 2.0.3 fails
when innodb_data_file_path = ibdata1:2000M;ibdata2:10M:autoextend is set
in [mysqld]

Fixed by initialization of srv_data_file_sizes in xb_delta_open_matching_space.

To post a comment you must log in.
Sergei Glushchenko (sergei.glushchenko) wrote : Posted in a previous version of this proposal

#25967

Alexey Kopytov (akopytov) wrote : Posted in a previous version of this proposal

It looks like in this fix we are doing some changes to some srv_* variables based on their previous values in xb_delta_open_matching_space(). What if this function is called multiple times?

review: Needs Information
Sergei Glushchenko (sergei.glushchenko) wrote : Posted in a previous version of this proposal

This change is in xb_data_files_init, not in xb_delta_open_matching_space. Those srv_* values first inited in innodb_init_param. We should call xb_data_files_init once after calling innodb_init_param, and I can't see why we would need to do it more times. Another possible place to put this code in is innodb_init_param.

Alexey Kopytov (akopytov) wrote : Posted in a previous version of this proposal

OK, I was just reading the commit / MP comment: "Fixed by srv_data_file_sizes in xb_delta_open_matching_space."

Alexey Kopytov (akopytov) wrote : Posted in a previous version of this proposal

Still something doesn't add up. So the code this patch moves from xtrabackup_backup_func() to xb_data_files_init() has been previously copy-pasted from srv_normalize_init_values().

Now srv_normalize_init_values is called from srv_boot() which in turn is called from innobase_start_or_create_for_mysql().

Which means when applying an incremental backup, we first execute that code by calling xb_data_files_init(). And then execute this code once again when starting an InnoDB instance by calling innodb_init().

Am I missing anything here?

Please also provide a proper description of the problem in revision comments. I.e. what exactly on the code level led to the crash. And the current solution ("Fixed by srv_data_file_sizes in xb_delta_open_matching_space.") doesn't make much sense either.

review: Needs Information
Sergei Glushchenko (sergei.glushchenko) wrote : Posted in a previous version of this proposal

Alexey,

> Still something doesn't add up. So the code this patch moves from
> xtrabackup_backup_func() to xb_data_files_init() has been previously copy-
> pasted from srv_normalize_init_values().
>
> Now srv_normalize_init_values is called from srv_boot() which in turn is
> called from innobase_start_or_create_for_mysql().
>
> Which means when applying an incremental backup, we first execute that code by
> calling xb_data_files_init(). And then execute this code once again when
> starting an InnoDB instance by calling innodb_init().
>
> Am I missing anything here?
>

Yes, you do. srv_parse_data_file_paths_and_sizes is called from innodb_init_param which resets srv_data_file_sizes. innodb_init_param called twice during incremental backup.

> Please also provide a proper description of the problem in revision comments.
> I.e. what exactly on the code level led to the crash. And the current solution
> ("Fixed by srv_data_file_sizes in xb_delta_open_matching_space.") doesn't make
> much sense either.

I'd like to update this branch and comments too once prerequisite branch being approved.

Alexey Kopytov (akopytov) wrote : Posted in a previous version of this proposal

Sergei,

Thanks for explanations.

Some things I noticed when branching lp:~sergei.glushchenko/percona-xtrabackup/ST-25967-2.0 locally:

- revision #472 containing the fix for this bug is a _merge_ revision. It appears you committed the fix for #1066843 and #1068470, then merged it into another branch and then implemented a fix for bug #1062684 as a merge revision. Please create separate branches and separate revisions.

- xb_normalize_init_values() contains trailing whitespace for some lines

- both lines with calls to xb_normalize_init_values() use spaces rather than tabs for indentation.

review: Needs Fixing
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 'src/xtrabackup.c'
2--- src/xtrabackup.c 2012-11-16 11:26:22 +0000
3+++ src/xtrabackup.c 2012-11-16 11:26:22 +0000
4@@ -3975,6 +3975,46 @@
5 return(fil_load_single_table_tablespaces());
6 }
7
8+/*********************************************************************//**
9+Normalizes init parameter values to use units we use inside InnoDB.
10+@return DB_SUCCESS or error code */
11+void
12+xb_normalize_init_values(void)
13+/*==========================*/
14+{
15+ ulint i;
16+
17+ for (i = 0; i < srv_n_data_files; i++) {
18+ srv_data_file_sizes[i] = srv_data_file_sizes[i]
19+ * ((1024 * 1024) / UNIV_PAGE_SIZE);
20+ }
21+
22+ srv_last_file_size_max = srv_last_file_size_max
23+ * ((1024 * 1024) / UNIV_PAGE_SIZE);
24+
25+ srv_log_file_size = srv_log_file_size / UNIV_PAGE_SIZE;
26+
27+ srv_log_buffer_size = srv_log_buffer_size / UNIV_PAGE_SIZE;
28+
29+#ifndef INNODB_VERSION_SHORT
30+ srv_pool_size = srv_pool_size / (UNIV_PAGE_SIZE / 1024);
31+
32+ srv_awe_window_size = srv_awe_window_size / UNIV_PAGE_SIZE;
33+
34+ if (srv_use_awe) {
35+ /* If we are using AWE we must save memory in the 32-bit
36+ address space of the process, and cannot bind the lock
37+ table size to the real buffer pool size. */
38+
39+ srv_lock_table_size = 20 * srv_awe_window_size;
40+ } else {
41+ srv_lock_table_size = 5 * srv_pool_size;
42+ }
43+#else
44+ srv_lock_table_size = 5 * (srv_buf_pool_size / UNIV_PAGE_SIZE);
45+#endif
46+}
47+
48 /************************************************************************
49 Destroy the tablespace memory cache. */
50 void
51@@ -4213,6 +4253,8 @@
52 if(innodb_init_param())
53 exit(EXIT_FAILURE);
54
55+ xb_normalize_init_values();
56+
57 #ifndef __WIN__
58 if (srv_file_flush_method_str == NULL) {
59 /* These are the default options */
60@@ -4290,43 +4332,6 @@
61 computers */
62 }
63
64- {
65- ulint nr;
66- ulint i;
67-
68- nr = srv_n_data_files;
69-
70- for (i = 0; i < nr; i++) {
71- srv_data_file_sizes[i] = srv_data_file_sizes[i]
72- * ((1024 * 1024) / UNIV_PAGE_SIZE);
73- }
74-
75- srv_last_file_size_max = srv_last_file_size_max
76- * ((1024 * 1024) / UNIV_PAGE_SIZE);
77-
78- srv_log_file_size = srv_log_file_size / UNIV_PAGE_SIZE;
79-
80- srv_log_buffer_size = srv_log_buffer_size / UNIV_PAGE_SIZE;
81-
82-#ifndef INNODB_VERSION_SHORT
83- srv_pool_size = srv_pool_size / (UNIV_PAGE_SIZE / 1024);
84-
85- srv_awe_window_size = srv_awe_window_size / UNIV_PAGE_SIZE;
86-
87- if (srv_use_awe) {
88- /* If we are using AWE we must save memory in the 32-bit
89- address space of the process, and cannot bind the lock
90- table size to the real buffer pool size. */
91-
92- srv_lock_table_size = 20 * srv_awe_window_size;
93- } else {
94- srv_lock_table_size = 5 * srv_pool_size;
95- }
96-#else
97- srv_lock_table_size = 5 * (srv_buf_pool_size / UNIV_PAGE_SIZE);
98-#endif
99- }
100-
101 os_sync_mutex = NULL;
102 srv_general_init();
103
104@@ -6409,6 +6414,8 @@
105 if(innodb_init_param())
106 goto error;
107
108+ xb_normalize_init_values();
109+
110 mem_init(srv_mem_pool_size);
111 if (xtrabackup_incremental) {
112 err = xb_data_files_init();
113
114=== added file 'test/t/bug1062684.sh'
115--- test/t/bug1062684.sh 1970-01-01 00:00:00 +0000
116+++ test/t/bug1062684.sh 2012-11-16 11:26:22 +0000
117@@ -0,0 +1,97 @@
118+. inc/common.sh
119+
120+start_server --innodb-data-file-path="ibdata1:10M;ibdata2:5M:autoextend"
121+load_dbase_schema incremental_sample
122+
123+echo "innodb-data-file-path=ibdata1:10M;ibdata2:5M:autoextend" >>$topdir/my.cnf
124+
125+# Adding initial rows
126+vlog "Adding initial rows to database..."
127+${MYSQL} ${MYSQL_ARGS} -e "insert into test values (1, 1);" incremental_sample
128+
129+# Full backup
130+# backup root directory
131+mkdir -p $topdir/backup
132+
133+vlog "Starting backup"
134+innobackupex $topdir/backup
135+full_backup_dir=`grep "innobackupex: Backup created in directory" $OUTFILE | awk -F\' '{print $2}'`
136+vlog "Full backup done to directory $full_backup_dir"
137+
138+# Changing data
139+
140+vlog "Making changes to database"
141+${MYSQL} ${MYSQL_ARGS} -e "create table t2 (a int(11) default null, number int(11) default null) engine=innodb" incremental_sample
142+${MYSQL} ${MYSQL_ARGS} -e "insert into test values (10, 1);" incremental_sample
143+${MYSQL} ${MYSQL_ARGS} -e "insert into t2 values (10, 1);" incremental_sample
144+vlog "Changes done"
145+
146+# Saving the checksum of original table
147+checksum_test_a=`checksum_table incremental_sample test`
148+checksum_t2_a=`checksum_table incremental_sample t2`
149+vlog "Table 'test' checksum is $checksum_test_a"
150+vlog "Table 't2' checksum is $checksum_t2_a"
151+
152+vlog "Making incremental backup"
153+
154+vlog "###############"
155+vlog "# INCREMENTAL #"
156+vlog "###############"
157+
158+# Incremental backup
159+innobackupex --incremental --incremental-basedir=$full_backup_dir \
160+ $topdir/backup
161+inc_backup_dir=`grep "innobackupex: Backup created in directory" $OUTFILE | tail -n 1 | awk -F\' '{print $2}'`
162+vlog "Incremental backup done to directory $inc_backup_dir"
163+
164+vlog "Preparing backup"
165+# Prepare backup
166+vlog "##############"
167+vlog "# PREPARE #1 #"
168+vlog "##############"
169+innobackupex --apply-log --redo-only $full_backup_dir
170+vlog "Log applied to full backup"
171+vlog "##############"
172+vlog "# PREPARE #2 #"
173+vlog "##############"
174+innobackupex --apply-log --redo-only --incremental-dir=$inc_backup_dir \
175+ $full_backup_dir
176+vlog "Delta applied to full backup"
177+vlog "##############"
178+vlog "# PREPARE #3 #"
179+vlog "##############"
180+innobackupex --apply-log $full_backup_dir
181+vlog "Data prepared for restore"
182+
183+# Destroying mysql data
184+stop_server
185+rm -rf $mysql_datadir/*
186+vlog "Data destroyed"
187+
188+# Restore backup
189+vlog "Copying files"
190+vlog "###########"
191+vlog "# RESTORE #"
192+vlog "###########"
193+innobackupex --copy-back $full_backup_dir
194+vlog "Data restored"
195+
196+start_server --innodb-data-file-path="ibdata1:10M;ibdata2:5M:autoextend"
197+
198+vlog "Checking checksums"
199+checksum_test_b=`checksum_table incremental_sample test`
200+checksum_t2_b=`checksum_table incremental_sample t2`
201+
202+if [ "$checksum_test_a" != "$checksum_test_b" ]
203+then
204+ vlog "Checksums for table 'test' are not equal"
205+ exit -1
206+fi
207+
208+if [ "$checksum_t2_a" != "$checksum_t2_b" ]
209+then
210+ vlog "Checksums for table 't2' are not equal"
211+ exit -1
212+fi
213+
214+vlog "Checksums are OK"

Subscribers

People subscribed via source and target branches