Merge lp:~sergei.glushchenko/percona-xtrabackup/xb20-bug932623 into lp:percona-xtrabackup/2.0

Proposed by Sergei Glushchenko on 2012-07-06
Status: Merged
Approved by: Alexey Kopytov on 2012-07-11
Approved revision: no longer in the source branch.
Merged at revision: 452
Proposed branch: lp:~sergei.glushchenko/percona-xtrabackup/xb20-bug932623
Merge into: lp:percona-xtrabackup/2.0
Diff against target: 904 lines (+582/-113)
3 files modified
patches/innodb51_builtin.patch (+67/-4)
src/xtrabackup.c (+430/-109)
test/t/bug932623.sh (+85/-0)
To merge this branch: bzr merge lp:~sergei.glushchenko/percona-xtrabackup/xb20-bug932623
Reviewer Review Type Date Requested Status
Alexey Kopytov (community) 2012-07-06 Approve on 2012-07-11
Review via email: mp+113690@code.launchpad.net

Description of the change

Bug #932623: RENAME TABLE causes incremental prepare to fail
Fixed by applying .delta changes to tablespace with matching ID.
Matching tablespace also renamed to match .delta name.
When tablespace with matching name but different ID exists in full
backup, it renamed to tmp#ID.ibd. When no matching tablespace found,
new one created.

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

#24669

To post a comment you must log in.
Alexey Kopytov (akopytov) wrote :

Sergei,

Tables may be renamed into another database directory, which this fix doesn't take into account. I think we should do space ID verification in a different way:

let's initialize fil_system, in the same --backup or --prepare does it and then use fil_space_get_by_id() and possibly fil_space_get_by_name().

Now the problem is that fil_system will also be initialized later in --prepare when innodb_init() is called, and initializing fil_system multiple times in the same process is tricky. I had to write some extra code for that in the compact backups branch, see xb_data_files_init() and xb_data_files_close() in lp:~akopytov/percona-xtrabackup/compact-backups.

Other minor things:
- s/matchong/matching
- "cat $topdir/inc/ibdata1.meta" in the test case was probably just for debugging and doesn't make any sense as far as regression testing is concerned.

review: Needs Fixing
Alexey Kopytov (akopytov) wrote :

Forgot to mention that the following code can be simplified:

14 if (fscanf(fp, "page_size = %lu\n", &info->page_size) != 1)
15 r= FALSE;
16 + if (!r || fscanf(fp, "space_id = %lu\n", &info->space_id) != 1)
17 + r= FALSE;

and

31 snprintf(buf, sizeof(buf), "page_size = %lu\n", info->page_size);
32 -
33 len = strlen(buf);
34 +
35 + snprintf(buf + len, sizeof(buf) - len, "space_id = %lu\n", info->space_id);
36 + len += strlen(buf + len);
37 +

It was really tricky to initialize fil_system, especially for built-in InnoDB and for debug builds.

Jenkins build:
http://jenkins.percona.com/view/XtraBackup/job/percona-xtrabackup-2.0-param/225/

Alexey Kopytov (akopytov) wrote :

Sergei,

Can you explain the problems with initializing fil_system (including the ones in built-in InnoDB) and how you solved them?

Alexey,

1. fil_space_get_by_id and fil_space_get_by_name are for internal usage. I have to modify patches to make them non-static and not inline. Built-in InnoDB does not have fil_space_get_by_id and fil_space_get_by_name at all. Besides we must hold fil_system->mutex when calling these functions.

2. fil_space use hash tables, which are not working without calling mem_init on debug builds.

3. When we are applying deltas we initialize some parts of InnoDB, then releasing and closing them, and then initializing them and some other parts again. The problem with built-it InnoDB is that it doesn't have proper functions to uninitialize fil, mem, and sync. Plugin have some tricks to do it properly (for example we should close sync, which frees all muteness, then close fil, which checks that sync is closed and only after that been done, we close men. But when closing men it use men_hash_mutex, which wasn't closed in sync_close by using special hack). In case of xtrabackup I cheated and not freed mem in many *_close functions, because it will be freed when ut_free_all_mem called.

4. io_handler_thread had an infinite loop:

 for (i = 0;; i++) {
  fil_aio_wait(segment);
 }
and there was no way to finish this thread. I inserted check of srv_shutdown_state.
 for (i = 0; srv_shutdown_state != SRV_SHUTDOWN_EXIT_THREADS; i++) {
    fil_aio_wait(segment);
 }

Spellchecker did bad thing by replacing s/mem/men/ :)

Alexey Kopytov (akopytov) wrote :

Sergei,

Thanks for clarifications. My comments on the new version of the fix:

   - I wonder if it would be less invasive to define our own simple
     wrappers around "HASH_SEARCH(hash, fil_system->spaces, id, space,
     space->id == id);" so we don't have to patch InnoDB or provide our
     own implementations for built-in InnoDB, and mess with
     mutexes. I.e. can we have xb_space_get_by_*()?

  - the code in xtrabackup's version of io_handler_thread() doesn't use
    the 'i' variable, why not just copy the code from InnoDB, i.e.:

 while (srv_shutdown_state != SRV_SHUTDOWN_EXIT_THREADS) {
  fil_aio_wait(segment);
 }

  - fil_free_all_spaces() seems to be defined for built-in InnoDB, but
    is not used anywhere?

  - in fact, ut_free_all_mem() will not free anything when the system
    malloc() is used instead of the InnoDB own malloc manager, i.e. when
    srv_use_sys_malloc == TRUE, which is the default. I have some
    thoughts on how to fix that, but I suggest we'll do it later as a
    part of bigger refactoring.

  - POSIX guarantees the result of snprintf() to be always
    zero-terminated, so there's no need for things like:

+ snprintf(dest_dir, FN_REFLEN, "%s/%s",
+ xtrabackup_target_dir, dbname);
+ dest_dir[FN_REFLEN - 1] = 0;

    if you care about Windows, you should be using either my_snprintf()
    or ut_snprintf(), which do the right thing on Windows. But
    currently, xtrabackup.c just uses snprintf() in other places anyway,
    so what's the point to do some windows-specific stuff in this
    patch. We can do s/snprintf/ut_snprintf/ later, if and when we
    have to support Windows.

  - this code just uses sprintf() instead snprintf() for some reasons:

+ sprintf(tmpname, "./%s/tmp#%lu", dbname, fil_space->id);

  - I think the output in xb_delta_open_matching_space() is too
    verbose. Let's only print messages when something unexpected
    happens, i.e. remove these messages:

+ msg("xtrabackup: Looking for tablespace %s with id %lu by name\n",
+ dest_space_name, space_id);

and

+ msg("xtrabackup: Looking for tablespace %s with id %lu by id\n",
+ dest_space_name, space_id);

and

+ msg("xtrabackup: Found matching tablespace %s\n",
+ fil_space->name);

and

+ msg("xtrabackup: Created tablespace %s with id %lu.\n",
+ dest_space_name, space_id);

  - what's the point to rename a tablespace with the same name but a
    different ID to tmp#id? the only case when it happens is when a
    tablespace was recreated between the full backup and an incremental
    one. In which case, if we rename the dropped table, we will end up
    with ghost 'tmp...' table in the full backup.

  - this if() block can be merged:

+ if (xtrabackup_incremental) {
+ err = xb_data_files_init();
+ if (err != DB_SUCCESS) {
+ msg("xtrabackup: error: xb_data_files_init() failed with"
+ "error code %lu\n", err);
+ goto error;
+ }
+
+ if(!xtrabackup_apply_deltas(TRUE)) {
+ xb_data_files_close();
+ goto error;
+ }
+ }
+
+ if (xtrabackup_incremental) {
+ xb_data_files_close();
+ }

  - can we also a add a cross-db rename test to bug932623.sh?

review: Needs Fixing
Download full text (4.2 KiB)

Alexey,

> - I wonder if it would be less invasive to define our own simple
> wrappers around "HASH_SEARCH(hash, fil_system->spaces, id, space,
> space->id == id);" so we don't have to patch InnoDB or provide our
> own implementations for built-in InnoDB, and mess with
> mutexes. I.e. can we have xb_space_get_by_*()?

I've wrote xb_space_by_* wrappers. The only modification of innodb51_builtin.patch left is mem_close function.

>
> - the code in xtrabackup's version of io_handler_thread() doesn't use
> the 'i' variable, why not just copy the code from InnoDB, i.e.:
>
> while (srv_shutdown_state != SRV_SHUTDOWN_EXIT_THREADS) {
> fil_aio_wait(segment);
> }
>

fixed

> - fil_free_all_spaces() seems to be defined for built-in InnoDB, but
> is not used anywhere?
>

Yes. I forgot to put call in proper place, fixed.

> - in fact, ut_free_all_mem() will not free anything when the system
> malloc() is used instead of the InnoDB own malloc manager, i.e. when
> srv_use_sys_malloc == TRUE, which is the default. I have some
> thoughts on how to fix that, but I suggest we'll do it later as a
> part of bigger refactoring.
>

OK. Hopefully memory leaks will not be critical.

> - POSIX guarantees the result of snprintf() to be always
> zero-terminated, so there's no need for things like:
>
> + snprintf(dest_dir, FN_REFLEN, "%s/%s",
> + xtrabackup_target_dir, dbname);
> + dest_dir[FN_REFLEN - 1] = 0;
>
> if you care about Windows, you should be using either my_snprintf()
> or ut_snprintf(), which do the right thing on Windows. But
> currently, xtrabackup.c just uses snprintf() in other places anyway,
> so what's the point to do some windows-specific stuff in this
> patch. We can do s/snprintf/ut_snprintf/ later, if and when we
> have to support Windows.
>

Great! I removed implicit zero-terminating statement.

> - this code just uses sprintf() instead snprintf() for some reasons:
>
> + sprintf(tmpname, "./%s/tmp#%lu", dbname,
> fil_space->id);
>

Hmm... I expected that generated tmpname cannot be longer than FN_REFLEN. I've changed to snprintf for more safety.

> - I think the output in xb_delta_open_matching_space() is too
> verbose. Let's only print messages when something unexpected
> happens, i.e. remove these messages:
>
> + msg("xtrabackup: Looking for tablespace %s with id %lu by name\n",
> + dest_space_name, space_id);
>
> and
>
> + msg("xtrabackup: Looking for tablespace %s with id %lu by id\n",
> + dest_space_name, space_id);
>
> and
>
> + msg("xtrabackup: Found matching tablespace %s\n",
> + fil_space->name);
>
> and
>
> + msg("xtrabackup: Created tablespace %s with id %lu.\n",
> + dest_space_name, space_id);
>

Removed.

>
> - what's the point to rename a tablespace with the same name but a
> different ID to tmp#id? the only case when it happens is when a
> tablespace was recreated between the full backup and an incremental
> one....

Read more...

Alexey Kopytov (akopytov) wrote :

Approved. Please when merging the fix to 2.1 move xb_space_get_by_*() and fil_free_all_spaces() to innodb_int.h and innodb_int.c (because those files were created exactly for things like this).

review: Approve
447. By Sergei Glushchenko on 2012-07-18

Bug #932623: RENAME TABLE causes incremental prepare to fail
Fix is following. We applying .delta changes to tablespace with matching ID.
Matching tablespace will also be renamed to match the .delta name.
When tablespace with matching name but different ID exists in full
backup, it will be renamed to xtrabackup_tmp#ID.ibd. When no matching
tablespace found, the new one will be created.
fil0fil.c APIs were used for matching.
xb_data_files_init and xb_data_files_close were ported from compact
backups branch. io_handler_thread was modified to handle shutdown signal
properly.
innodb51_builtin.patch was modified, mem_close and mem_pool_free functions
were added.
Calls to mem_init and mem_close were added to avoid assertion failures on debug
builds.

This patch regresses on compressed tablespace creation: the "flags" arg for the fil_space_create() is *not* zip_size. Please do not resubmit this MP. I have follow-up MPs depending on the code and "Approved" status of this one and I will fix this in a follow-up right now.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'patches/innodb51_builtin.patch'
2--- patches/innodb51_builtin.patch 2012-02-10 05:04:04 +0000
3+++ patches/innodb51_builtin.patch 2012-07-18 15:36:21 +0000
4@@ -613,7 +613,21 @@
5
6 --- a/storage/innobase/include/mem0mem.h
7 +++ b/storage/innobase/include/mem0mem.h
8-@@ -401,6 +401,7 @@
9+@@ -63,6 +63,13 @@
10+ mem_init(
11+ /*=====*/
12+ ulint size); /* in: common pool size in bytes */
13++
14++/******************************************************************//**
15++Closes the memory system. */
16++void
17++mem_close(void);
18++/*===========*/
19++
20+ /******************************************************************
21+ Use this macro instead of the corresponding function! Macro for memory
22+ heap creation. */
23+@@ -401,6 +409,7 @@
24 allocated buffer frame, which can be appended as a
25 free block to the heap, if we need more space;
26 otherwise, this is NULL */
27@@ -654,6 +668,21 @@
28
29 if (heap->free_block) {
30 size += UNIV_PAGE_SIZE;
31+--- a/storage/innobase/include/mem0pool.h
32++++ b/storage/innobase/include/mem0pool.h
33+@@ -42,6 +42,12 @@
34+ /*============*/
35+ /* out: memory pool */
36+ ulint size); /* in: pool size in bytes */
37++/********************************************************************//**
38++Frees a memory pool. */
39++void
40++mem_pool_free(
41++/*==========*/
42++ mem_pool_t* pool); /*!< in, own: memory pool */
43+ /************************************************************************
44+ Allocates memory from a pool. NOTE: This low-level function should only be
45+ used in mem0mem.*! */
46 --- a/storage/innobase/include/srv0srv.h
47 +++ b/storage/innobase/include/srv0srv.h
48 @@ -60,6 +60,8 @@
49@@ -958,7 +987,7 @@
50 /**********************************************************
51 --- a/storage/innobase/mem/mem0dbg.c
52 +++ b/storage/innobase/mem/mem0dbg.c
53-@@ -133,6 +133,14 @@
54+@@ -133,9 +133,30 @@
55 mem_hash_initialized = TRUE;
56 #endif
57
58@@ -973,6 +1002,22 @@
59 mem_comm_pool = mem_pool_create(size);
60 }
61
62++/******************************************************************//**
63++Closes the memory system. */
64++void
65++mem_close(void)
66++/*===========*/
67++{
68++ mem_pool_free(mem_comm_pool);
69++ mem_comm_pool = NULL;
70++#ifdef UNIV_MEM_DEBUG
71++ mem_hash_initialized = FALSE;
72++#endif /* UNIV_MEM_DEBUG */
73++}
74++
75+ #ifdef UNIV_MEM_DEBUG
76+ /**********************************************************************
77+ Initializes an allocated memory field in the debug version. */
78 --- a/storage/innobase/mem/mem0mem.c
79 +++ b/storage/innobase/mem/mem0mem.c
80 @@ -472,6 +472,7 @@
81@@ -1026,7 +1071,25 @@
82 pool = ut_malloc(sizeof(mem_pool_t));
83
84 /* We do not set the memory to zero (FALSE) in the pool,
85-@@ -333,6 +332,10 @@
86+@@ -244,6 +243,17 @@
87+ return(pool);
88+ }
89+
90++/********************************************************************//**
91++Frees a memory pool. */
92++void
93++mem_pool_free(
94++/*==========*/
95++ mem_pool_t* pool) /*!< in, own: memory pool */
96++{
97++ ut_free(pool->buf);
98++ ut_free(pool);
99++}
100++
101+ /************************************************************************
102+ Fills the specified free list. */
103+ static
104+@@ -333,6 +344,10 @@
105 ulint n;
106 ibool ret;
107
108@@ -1037,7 +1100,7 @@
109 n = ut_2_log(ut_max(size + MEM_AREA_EXTRA_SIZE, MEM_AREA_MIN_SIZE));
110
111 mutex_enter(&(pool->mutex));
112-@@ -465,6 +468,11 @@
113+@@ -465,6 +480,11 @@
114 ulint size;
115 ulint n;
116
117
118=== modified file 'src/xtrabackup.c'
119--- src/xtrabackup.c 2012-06-20 07:36:26 +0000
120+++ src/xtrabackup.c 2012-07-18 15:36:21 +0000
121@@ -655,10 +655,107 @@
122
123 typedef struct {
124 ulint page_size;
125+ ulint space_id;
126 } xb_delta_info_t;
127
128 extern fil_system_t* fil_system;
129
130+/** Value of fil_space_struct::magic_n */
131+#define FIL_SPACE_MAGIC_N 89472
132+
133+/*******************************************************************//**
134+Returns the table space by a given id, NULL if not found. */
135+fil_space_t*
136+xb_space_get_by_id(
137+/*================*/
138+ ulint id); /*!< in: space id */
139+
140+/*******************************************************************//**
141+Returns the table space by a given name, NULL if not found. */
142+fil_space_t*
143+xb_space_get_by_name(
144+/*==================*/
145+ const char* name); /*!< in: space name */
146+
147+/*******************************************************************//**
148+Returns the table space by a given id, NULL if not found. */
149+fil_space_t*
150+xb_space_get_by_id(
151+/*================*/
152+ ulint id) /*!< in: space id */
153+{
154+ fil_space_t* space;
155+
156+ ut_ad(mutex_own(&fil_system->mutex));
157+
158+#ifdef INNODB_VERSION_SHORT
159+ HASH_SEARCH(hash, fil_system->spaces, id,
160+ fil_space_t*, space,
161+ ut_ad(space->magic_n == FIL_SPACE_MAGIC_N),
162+ space->id == id);
163+#else
164+ HASH_SEARCH(hash, fil_system->spaces, id, space, space->id == id);
165+#endif
166+
167+ return(space);
168+}
169+
170+/*******************************************************************//**
171+Returns the table space by a given name, NULL if not found. */
172+fil_space_t*
173+xb_space_get_by_name(
174+/*==================*/
175+ const char* name) /*!< in: space name */
176+{
177+ fil_space_t* space;
178+ ulint fold;
179+
180+ ut_ad(mutex_own(&fil_system->mutex));
181+
182+#ifdef INNODB_VERSION_SHORT
183+ fold = ut_fold_string(name);
184+ HASH_SEARCH(name_hash, fil_system->name_hash, fold,
185+ fil_space_t*, space,
186+ ut_ad(space->magic_n == FIL_SPACE_MAGIC_N),
187+ !strcmp(name, space->name));
188+#else
189+ HASH_SEARCH(name_hash, fil_system->name_hash, ut_fold_string(name),
190+ space, 0 == strcmp(name, space->name));
191+#endif
192+
193+ return(space);
194+}
195+
196+#ifndef INNODB_VERSION_SHORT
197+
198+/*******************************************************************//**
199+Free all spaces in space_list. */
200+void
201+fil_free_all_spaces(void)
202+/*=====================*/
203+{
204+ fil_space_t* space;
205+
206+ mutex_enter(&fil_system->mutex);
207+
208+ space = UT_LIST_GET_FIRST(fil_system->space_list);
209+
210+ while (space != NULL) {
211+ fil_node_t* node;
212+ fil_space_t* prev_space = space;
213+
214+ space = UT_LIST_GET_NEXT(space_list, space);
215+
216+ fil_space_free(prev_space->id, FALSE);
217+ }
218+
219+ mutex_exit(&fil_system->mutex);
220+}
221+
222+#define SRV_SHUTDOWN_NONE 0
223+
224+#endif
225+
226 /* ==end=== definition at fil0fil.c === */
227
228
229@@ -2752,7 +2849,8 @@
230 return(TRUE);
231 }
232
233- if (fscanf(fp, "page_size = %lu\n", &info->page_size) != 1)
234+ if (fscanf(fp, "page_size = %lu\nspace_id = %lu\n",
235+ &info->page_size, &info->space_id) != 2)
236 r= FALSE;
237
238 fclose(fp);
239@@ -2770,14 +2868,16 @@
240 {
241 datasink_t *ds = ds_ctxt->datasink;
242 ds_file_t *f;
243- char buf[32];
244+ char buf[64];
245 my_bool ret;
246 size_t len;
247 MY_STAT mystat;
248
249- snprintf(buf, sizeof(buf), "page_size = %lu\n", info->page_size);
250-
251+ snprintf(buf, sizeof(buf),
252+ "page_size = %lu\nspace_id = %lu\n",
253+ info->page_size, info->space_id);
254 len = strlen(buf);
255+
256 mystat.st_size = len;
257 mystat.st_mtime = my_time(0);
258
259@@ -2981,6 +3081,7 @@
260 ds_file_t *dstfile = NULL;
261
262 info.page_size = 0;
263+ info.space_id = 0;
264
265 #ifdef XTRADB_BASED
266 if (xtrabackup_tables && (!trx_sys_sys_space(node->space->id)))
267@@ -3163,6 +3264,7 @@
268 page_in_buffer++;
269
270 info.page_size = page_size;
271+ info.space_id = node->space->id;
272 } else
273 info.page_size = 0;
274
275@@ -3722,11 +3824,10 @@
276 void* arg)
277 {
278 ulint segment;
279- ulint i;
280
281 segment = *((ulint*)arg);
282
283- for (i = 0;; i++) {
284+ while (srv_shutdown_state != SRV_SHUTDOWN_EXIT_THREADS) {
285 fil_aio_wait(segment);
286 }
287
288@@ -3744,6 +3845,141 @@
289 #endif
290 }
291
292+#define SRV_N_PENDING_IOS_PER_THREAD OS_AIO_N_PENDING_IOS_PER_THREAD
293+#define SRV_MAX_N_PENDING_SYNC_IOS 100
294+
295+/************************************************************************
296+Initialize the tablespace memory cache and populate it by scanning for and
297+opening data files.
298+@returns DB_SUCCESS or error code.*/
299+ulint
300+xb_data_files_init(void)
301+/*====================*/
302+{
303+ ulint i;
304+ ibool create_new_db;
305+#ifdef XTRADB_BASED
306+ ibool create_new_doublewrite_file;
307+#endif
308+ ulint err;
309+ LSN64 min_flushed_lsn;
310+ LSN64 max_flushed_lsn;
311+ ulint sum_of_new_sizes;
312+
313+#ifndef INNODB_VERSION_SHORT
314+ os_aio_init(8 * SRV_N_PENDING_IOS_PER_THREAD
315+ * srv_n_file_io_threads,
316+ srv_n_file_io_threads,
317+ SRV_MAX_N_PENDING_SYNC_IOS);
318+
319+ fil_init(srv_max_n_open_files);
320+#else
321+ srv_n_file_io_threads = 2 + srv_n_read_io_threads +
322+ srv_n_write_io_threads;
323+
324+ os_aio_init(8 * SRV_N_PENDING_IOS_PER_THREAD,
325+ srv_n_read_io_threads,
326+ srv_n_write_io_threads,
327+ SRV_MAX_N_PENDING_SYNC_IOS);
328+
329+ fil_init(srv_file_per_table ? 50000 : 5000,
330+ srv_max_n_open_files);
331+#endif
332+
333+ fsp_init();
334+
335+ for (i = 0; i < srv_n_file_io_threads; i++) {
336+ thread_nr[i] = i;
337+
338+ os_thread_create(io_handler_thread, thread_nr + i,
339+ thread_ids + i);
340+ }
341+
342+ os_thread_sleep(200000); /*0.2 sec*/
343+
344+ err = open_or_create_data_files(&create_new_db,
345+#ifdef XTRADB_BASED
346+ &create_new_doublewrite_file,
347+#endif
348+ &min_flushed_lsn, &max_flushed_lsn,
349+ &sum_of_new_sizes);
350+ if (err != DB_SUCCESS) {
351+ msg("xtrabackup: Could not open or create data files.\n"
352+ "xtrabackup: If you tried to add new data files, and it "
353+ "failed here,\n"
354+ "xtrabackup: you should now edit innodb_data_file_path in "
355+ "my.cnf back\n"
356+ "xtrabackup: to what it was, and remove the new ibdata "
357+ "files InnoDB created\n"
358+ "xtrabackup: in this failed attempt. InnoDB only wrote "
359+ "those files full of\n"
360+ "xtrabackup: zeros, but did not yet use them in any way. "
361+ "But be careful: do not\n"
362+ "xtrabackup: remove old data files which contain your "
363+ "precious data!\n");
364+ return(err);
365+ }
366+
367+ /* create_new_db must not be TRUE.. */
368+ if (create_new_db) {
369+ msg("xtrabackup: could not find data files at the "
370+ "specified datadir\n");
371+ return(DB_ERROR);
372+ }
373+
374+ return(fil_load_single_table_tablespaces());
375+}
376+
377+/************************************************************************
378+Destroy the tablespace memory cache. */
379+void
380+xb_data_files_close(void)
381+/*====================*/
382+{
383+ ulint i;
384+
385+ /* Shutdown the aio threads. This has been copied from
386+ innobase_shutdown_for_mysql(). */
387+
388+ srv_shutdown_state = SRV_SHUTDOWN_EXIT_THREADS;
389+
390+ for (i = 0; i < 1000; i++) {
391+ os_aio_wake_all_threads_at_shutdown();
392+
393+ os_mutex_enter(os_sync_mutex);
394+
395+ if (os_thread_count == 0) {
396+
397+ os_mutex_exit(os_sync_mutex);
398+
399+ os_thread_sleep(10000);
400+
401+ break;
402+ }
403+
404+ os_mutex_exit(os_sync_mutex);
405+
406+ os_thread_sleep(10000);
407+ }
408+
409+ if (i == 1000) {
410+ msg("xtrabackup: Warning: %lu threads created by InnoDB"
411+ " had not exited at shutdown!\n",
412+ (ulong) os_thread_count);
413+ }
414+
415+#ifdef INNODB_VERSION_SHORT
416+ os_aio_free();
417+#endif
418+ fil_close_all_files();
419+#ifndef INNODB_VERSION_SHORT
420+ fil_free_all_spaces();
421+#endif
422+ fil_system = NULL;
423+
424+ srv_shutdown_state = SRV_SHUTDOWN_NONE;
425+}
426+
427 /***************************************************************************
428 Creates an output directory for a given tablespace, if it does not exist */
429 static
430@@ -4050,85 +4286,25 @@
431 srv_general_init();
432
433 {
434- ibool create_new_db;
435-#ifdef XTRADB_BASED
436- ibool create_new_doublewrite_file;
437-#endif
438 ibool log_file_created;
439 ibool log_created = FALSE;
440 ibool log_opened = FALSE;
441- LSN64 min_flushed_lsn;
442- LSN64 max_flushed_lsn;
443- ulint sum_of_new_sizes;
444 ulint err;
445 ulint i;
446
447-
448-
449-
450-#define SRV_N_PENDING_IOS_PER_THREAD OS_AIO_N_PENDING_IOS_PER_THREAD
451-#define SRV_MAX_N_PENDING_SYNC_IOS 100
452-
453-#ifndef INNODB_VERSION_SHORT
454- os_aio_init(8 * SRV_N_PENDING_IOS_PER_THREAD
455- * srv_n_file_io_threads,
456- srv_n_file_io_threads,
457- SRV_MAX_N_PENDING_SYNC_IOS);
458-
459- fil_init(srv_max_n_open_files);
460-#else
461- srv_n_file_io_threads = 2 + srv_n_read_io_threads + srv_n_write_io_threads;
462-
463- os_aio_init(8 * SRV_N_PENDING_IOS_PER_THREAD,
464- srv_n_read_io_threads,
465- srv_n_write_io_threads,
466- SRV_MAX_N_PENDING_SYNC_IOS);
467-
468- fil_init(srv_file_per_table ? 50000 : 5000,
469- srv_max_n_open_files);
470-#endif
471-
472- fsp_init();
473+ err = xb_data_files_init();
474+ if (err != DB_SUCCESS) {
475+ msg("xtrabackup: error: xb_data_files_init() failed with"
476+ "error code %lu\n", err);
477+ exit(EXIT_FAILURE);
478+ }
479+
480 log_init();
481
482 lock_sys_create(srv_lock_table_size);
483
484- for (i = 0; i < srv_n_file_io_threads; i++) {
485- thread_nr[i] = i;
486-
487- os_thread_create(io_handler_thread, thread_nr + i, thread_ids + i);
488- }
489-
490- os_thread_sleep(200000); /*0.2 sec*/
491-
492- err = open_or_create_data_files(&create_new_db,
493-#ifdef XTRADB_BASED
494- &create_new_doublewrite_file,
495-#endif
496- &min_flushed_lsn, &max_flushed_lsn,
497- &sum_of_new_sizes);
498- if (err != DB_SUCCESS) {
499- msg(
500-"xtrabackup: Could not open or create data files.\n"
501-"xtrabackup: If you tried to add new data files, and it failed here,\n"
502-"xtrabackup: you should now edit innodb_data_file_path in my.cnf back\n"
503-"xtrabackup: to what it was, and remove the new ibdata files InnoDB created\n"
504-"xtrabackup: in this failed attempt. InnoDB only wrote those files full of\n"
505-"xtrabackup: zeros, but did not yet use them in any way. But be careful: do not\n"
506-"xtrabackup: remove old data files which contain your precious data!\n");
507-
508- //return((int) err);
509- exit(EXIT_FAILURE);
510- }
511-
512- /* create_new_db must not be TRUE.. */
513- if (create_new_db) {
514- msg("xtrabackup: Something wrong with source files...\n");
515- exit(EXIT_FAILURE);
516- }
517-
518 for (i = 0; i < srv_n_log_files; i++) {
519- err = open_or_create_log_file(create_new_db, &log_file_created,
520+ err = open_or_create_log_file(FALSE, &log_file_created,
521 log_opened, 0, i);
522 if (err != DB_SUCCESS) {
523
524@@ -4141,8 +4317,7 @@
525 } else {
526 log_opened = TRUE;
527 }
528- if ((log_opened && create_new_db)
529- || (log_opened && log_created)) {
530+ if ((log_opened && log_created)) {
531 msg(
532 "xtrabackup: Error: all log files must be created at the same time.\n"
533 "xtrabackup: All log files must be created also in database creation.\n"
534@@ -4162,8 +4337,6 @@
535 exit(EXIT_FAILURE);
536 }
537
538- fil_load_single_table_tablespaces();
539-
540 }
541
542 /* create extra LSN dir if it does not exist. */
543@@ -4545,6 +4718,8 @@
544 msg("xtrabackup: Transaction log of lsn (%llu) to (%llu) was copied.\n",
545 checkpoint_lsn_start, log_copy_scanned_lsn);
546 #endif
547+
548+ xb_data_files_close();
549 }
550
551 /* ================= stats ================= */
552@@ -5434,6 +5609,154 @@
553 return TRUE;
554 }
555
556+/***********************************************************************
557+Searches for matching tablespace file for given .delta file and space_id
558+in given directory. When matching tablespace found, renames it to match the
559+name of .delta file. If there was a tablespace with matching name and
560+mismatching ID, renames it to xtrabackup_tmp_#ID.ibd. If there was no
561+matching file, creates the new one.
562+@return file handle of matched or created file */
563+static
564+os_file_t
565+xb_delta_open_matching_space(
566+ const char* dbname, /* in: path to destination database dir */
567+ const char* name, /* in: name of delta file (without .delta) */
568+ ulint space_id, /* in: space id of delta file */
569+ ulint zip_size, /* in: zip_size of tablespace */
570+ char* real_name, /* out: full path of destination file */
571+ size_t real_name_len, /* out: buffer size for real_name */
572+ ibool* success) /* out: indicates error. TRUE = success */
573+{
574+ char dest_dir[FN_REFLEN];
575+ char dest_space_name[FN_REFLEN];
576+ ibool ok;
577+ fil_space_t* fil_space;
578+ os_file_t file = 0;
579+
580+ ut_a(dbname || space_id == 0);
581+
582+ *success = FALSE;
583+
584+ if (dbname) {
585+ snprintf(dest_dir, FN_REFLEN, "%s/%s",
586+ xtrabackup_target_dir, dbname);
587+ srv_normalize_path_for_win(dest_dir);
588+
589+ snprintf(dest_space_name, FN_REFLEN, "./%s/%s",
590+ dbname, name);
591+ } else {
592+ snprintf(dest_dir, FN_REFLEN, "%s", xtrabackup_target_dir);
593+ srv_normalize_path_for_win(dest_dir);
594+
595+ snprintf(dest_space_name, FN_REFLEN, "./%s", name);
596+ }
597+
598+ snprintf(real_name, real_name_len,
599+ "%s/%s",
600+ xtrabackup_target_dir, dest_space_name);
601+ srv_normalize_path_for_win(real_name);
602+
603+ /* Create the database directory if it doesn't exist yet */
604+ if (!os_file_create_directory(dest_dir, FALSE)) {
605+ msg("xtrabackup: error: cannot create dir %s\n", dest_dir);
606+ return file;
607+ }
608+
609+ mutex_enter(&fil_system->mutex);
610+ fil_space = xb_space_get_by_name(dest_space_name);
611+ mutex_exit(&fil_system->mutex);
612+
613+ if (fil_space != NULL) {
614+ if (fil_space->id == space_id) {
615+ /* we found matching space */
616+ goto found;
617+ } else {
618+
619+ char tmpname[FN_REFLEN];
620+
621+ snprintf(tmpname, FN_REFLEN, "./%s/xtrabackup_tmp_#%lu",
622+ dbname, fil_space->id);
623+
624+ msg("xtrabackup: Renaming %s to %s.ibd\n",
625+ fil_space->name, tmpname);
626+
627+ if (!fil_rename_tablespace(NULL,
628+ fil_space->id, tmpname))
629+ {
630+ msg("xtrabackup: Cannot rename %s to %s\n",
631+ fil_space->name, tmpname);
632+ goto exit;
633+ }
634+ }
635+ }
636+
637+ mutex_enter(&fil_system->mutex);
638+ fil_space = xb_space_get_by_id(space_id);
639+ mutex_exit(&fil_system->mutex);
640+ if (fil_space != NULL) {
641+ char tmpname[FN_REFLEN];
642+
643+ strncpy(tmpname, dest_space_name, FN_REFLEN);
644+ tmpname[strlen(tmpname) - 4] = 0;
645+
646+ msg("xtrabackup: Renaming %s to %s\n",
647+ fil_space->name, dest_space_name);
648+
649+ if (!fil_rename_tablespace(NULL,
650+ fil_space->id, tmpname))
651+ {
652+ msg("xtrabackup: Cannot rename %s to %s\n",
653+ fil_space->name, dest_space_name);
654+ goto exit;
655+ }
656+
657+ goto found;
658+ }
659+
660+ /* no matching space found. create the new one */
661+
662+#ifdef INNODB_VERSION_SHORT
663+ if (!fil_space_create(dest_space_name, space_id,
664+ zip_size, FIL_TABLESPACE)) {
665+#else
666+ if (!fil_space_create(dest_space_name, space_id,
667+ FIL_TABLESPACE)) {
668+#endif
669+ msg("xtrabackup: Cannot create tablespace %s\n",
670+ dest_space_name);
671+ goto exit;
672+ }
673+
674+ file = xb_file_create_no_error_handling(real_name, OS_FILE_CREATE,
675+ OS_FILE_READ_WRITE,
676+ &ok);
677+
678+ if (ok) {
679+ *success = TRUE;
680+ } else {
681+ msg("xtrabackup: Cannot open file %s\n", real_name);
682+ }
683+
684+ goto exit;
685+
686+found:
687+ /* open the file and return it's handle */
688+
689+ file = xb_file_create_no_error_handling(real_name, OS_FILE_OPEN,
690+ OS_FILE_READ_WRITE,
691+ &ok);
692+
693+ if (ok) {
694+ *success = TRUE;
695+ } else {
696+ msg("xtrabackup: Cannot open file %s\n", real_name);
697+ }
698+
699+exit:
700+
701+ return file;
702+}
703+
704 /************************************************************************
705 Applies a given .delta file to the corresponding data file.
706 @return TRUE on success */
707@@ -5451,6 +5774,7 @@
708 char src_path[FN_REFLEN];
709 char dst_path[FN_REFLEN];
710 char meta_path[FN_REFLEN];
711+ char space_name[FN_REFLEN];
712 ibool success;
713
714 ibool last_buffer = FALSE;
715@@ -5478,6 +5802,9 @@
716 }
717 dst_path[strlen(dst_path) - 6] = '\0';
718
719+ strncpy(space_name, filename, FN_REFLEN);
720+ space_name[strlen(space_name) - 6] = 0;
721+
722 if (!get_meta_path(src_path, meta_path)) {
723 goto error;
724 }
725@@ -5517,36 +5844,11 @@
726
727 xb_file_set_nocache(src_file, src_path, "OPEN");
728
729- dst_file = xb_file_create_no_error_handling(dst_path, OS_FILE_OPEN,
730- OS_FILE_READ_WRITE,
731- &success);
732-again:
733+ dst_file = xb_delta_open_matching_space(
734+ dbname, space_name, info.space_id,
735+ info.page_size == UNIV_PAGE_SIZE ? 0 : info.page_size,
736+ dst_path, sizeof(dst_path), &success);
737 if (!success) {
738- ulint errcode = os_file_get_last_error(TRUE);
739-
740- if (errcode == OS_FILE_NOT_FOUND) {
741- msg("xtrabackup: target data file %s "
742- "is not found, creating a new one\n", dst_path);
743- /* Create the database directory if it doesn't exist yet
744- */
745- if (dbname) {
746- char dst_dir[FN_REFLEN];
747-
748- snprintf(dst_dir, sizeof(dst_dir), "%s/%s",
749- xtrabackup_real_target_dir, dbname);
750- srv_normalize_path_for_win(dst_dir);
751-
752- if (!os_file_create_directory(dst_dir, FALSE))
753- goto error;
754- }
755- dst_file =
756- xb_file_create_no_error_handling(dst_path,
757- OS_FILE_CREATE,
758- OS_FILE_READ_WRITE,
759- &success);
760- goto again;
761- }
762-
763 msg("xtrabackup: error: cannot open %s\n", dst_path);
764 goto error;
765 }
766@@ -5563,7 +5865,7 @@
767 incremental_buffer = ut_align(incremental_buffer_base,
768 UNIV_PAGE_SIZE_MAX);
769
770- msg("Applying %s ...\n", src_path);
771+ msg("Applying %s to %s...\n", src_path, dst_path);
772
773 while (!last_buffer) {
774 ulint cluster_header;
775@@ -5873,6 +6175,8 @@
776 static void
777 xtrabackup_prepare_func(void)
778 {
779+ ulint err;
780+
781 /* cd to target-dir */
782
783 if (my_setwd(xtrabackup_real_target_dir,MYF(MY_WME)))
784@@ -5943,12 +6247,29 @@
785 if(xtrabackup_init_temp_log())
786 goto error;
787
788- if(xtrabackup_incremental && !xtrabackup_apply_deltas(TRUE))
789+ if(innodb_init_param())
790 goto error;
791
792+ mem_init(srv_mem_pool_size);
793+ if (xtrabackup_incremental) {
794+ err = xb_data_files_init();
795+ if (err != DB_SUCCESS) {
796+ msg("xtrabackup: error: xb_data_files_init() failed with"
797+ "error code %lu\n", err);
798+ goto error;
799+ }
800+
801+ if(!xtrabackup_apply_deltas(TRUE)) {
802+ xb_data_files_close();
803+ goto error;
804+ }
805+
806+ xb_data_files_close();
807+ }
808 sync_close();
809 sync_initialized = FALSE;
810 os_sync_free();
811+ mem_close();
812 os_sync_mutex = NULL;
813 ut_free_all_mem();
814
815
816=== added file 'test/t/bug932623.sh'
817--- test/t/bug932623.sh 1970-01-01 00:00:00 +0000
818+++ test/t/bug932623.sh 2012-07-18 15:36:21 +0000
819@@ -0,0 +1,85 @@
820+########################################################################
821+# Bug #932623: RENAME TABLE causes incremental prepare to fail
822+########################################################################
823+
824+. inc/common.sh
825+
826+start_server --innodb_file_per_table
827+
828+run_cmd $MYSQL $MYSQL_ARGS test <<EOF
829+CREATE TABLE t1_old(a INT) ENGINE=InnoDB;
830+INSERT INTO t1_old VALUES (1), (2), (3);
831+
832+CREATE TABLE t1(a INT) ENGINE=InnoDB;
833+INSERT INTO t1 VALUES (4), (5), (6);
834+EOF
835+
836+# Full backup
837+# backup root directory
838+vlog "Starting backup"
839+innobackupex --no-timestamp $topdir/full
840+
841+vlog "Rotating the table"
842+
843+run_cmd $MYSQL $MYSQL_ARGS test <<EOF
844+CREATE TABLE t1_new(a int) ENGINE=InnoDB;
845+INSERT INTO t1_new VALUES (7), (8), (9);
846+
847+CREATE DATABASE db2;
848+RENAME TABLE t1_old TO db2.t1;
849+
850+RENAME TABLE t1 TO t1_old;
851+RENAME TABLE t1_new TO t1;
852+
853+INSERT INTO t1_old VALUES (10), (11), (12);
854+INSERT INTO t1 VALUES (13), (14), (15);
855+
856+EOF
857+
858+vlog "Creating incremental backup"
859+
860+innobackupex --incremental --no-timestamp \
861+ --incremental-basedir=$topdir/full $topdir/inc
862+
863+vlog "Preparing backup"
864+
865+innobackupex --apply-log --redo-only $topdir/full
866+vlog "Log applied to full backup"
867+
868+innobackupex --apply-log --redo-only --incremental-dir=$topdir/inc \
869+ $topdir/full
870+vlog "Delta applied to full backup"
871+
872+innobackupex --apply-log $topdir/full
873+vlog "Data prepared for restore"
874+
875+checksum_t1_a=`checksum_table test t1`
876+checksum_t1_old_a=`checksum_table test t1_old`
877+checksum_t1_db2_a=`checksum_table db2 t1`
878+
879+# Destroying mysql data
880+stop_server
881+rm -rf $mysql_datadir/*
882+vlog "Data destroyed"
883+
884+# Restore backup
885+vlog "Copying files"
886+
887+innobackupex --copy-back $topdir/full
888+vlog "Data restored"
889+
890+start_server --innodb_file_per_table
891+
892+vlog "Checking checksums"
893+checksum_t1_b=`checksum_table test t1`
894+checksum_t1_old_b=`checksum_table test t1_old`
895+checksum_t1_db2_b=`checksum_table db2 t1`
896+
897+if [ "$checksum_t1_a" != "$checksum_t1_b" -o "$checksum_t1_old_a" != "$checksum_t1_old_b" \
898+ -o "$checksum_t1_db2_a" != "$checksum_t1_db2_b" ]
899+then
900+ vlog "Checksums do not match"
901+ exit -1
902+fi
903+
904+vlog "Checksums are OK"

Subscribers

People subscribed via source and target branches