Merge lp://staging/~akopytov/percona-xtrabackup/bug950334-2.1 into lp://staging/percona-xtrabackup/2.1
Status: | Merged |
---|---|
Approved by: | Alexey Kopytov |
Approved revision: | no longer in the source branch. |
Merged at revision: | 517 |
Proposed branch: | lp://staging/~akopytov/percona-xtrabackup/bug950334-2.1 |
Merge into: | lp://staging/percona-xtrabackup/2.1 |
Diff against target: |
106 lines (+34/-8) 5 files modified
patches/innodb51.patch (+2/-2) patches/innodb55.patch (+2/-2) patches/xtradb51.patch (+2/-2) patches/xtradb55.patch (+2/-2) test/t/bug950334.sh (+26/-0) |
To merge this branch: | bzr merge lp://staging/~akopytov/percona-xtrabackup/bug950334-2.1 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Sergei Glushchenko (community) | g2 | Approve | |
Registry Administrators | Pending | ||
Review via email: mp+151745@code.staging.launchpad.net |
Description of the change
Bug #950334: disk space grows by >500% while preparing a backup
The fix for bug #461099 introduced the following logic in xtrabackup
--prepare: after applying the log, we walk through tablespaces and
adjust their sizes based on the FSP_SIZE value stored in the FSP
header. This was required to prevent cases when a tablespace is extended
while it is being copied by xtrabackup. In such cases InnoDB updates
FSP_SIZE and extends the tablespace to the required size. However,
unlike FSP_SIZE modifications, tablespace extensions are not
redo-logged. Which means we get inconsistency between FSP_SIZE and the
actual tablespace size after applying the log and that's why adjustments
introduced by the fix for bug #461099 are necessary.
The problem with that code was that it reads the first page of each
tablespace unconditionally, i.e. even for tablespace that have not been
touched (and thus loaded) by recovery. The code is also mostly a
copy-paste from a UNIV_HOTBACKUP-only function
fil_
following logic to _fil_io(): if a request to read a page is beyond the
tablespace size (which is 0 in the case being considered), extend the
tablespace by aligning it to the nearest 1M boundary.
One way to fix it would be to throw away that extra _fil_io() logic from
XtraBackup patches and reuse fil_extend_
the fix for bug #461099. That, however, would require a rather massive
patch update to revert _fil_io() changes and make the
UNIV_
available.
Another approach is to only look for tablespaces touched by recovery
when extending tablespaces, i.e. extend only those for which space->size
> 0. This doesn't involve updating patches, and has an extra benefit
that we will do less work by not processing unmodified spaces. That,
however, doesn't work reliably. In some cases recovery wants to read in
pages before it opens the tablespace, and that's where file extension
logic in _fil_io() is triggered again and the file is unnecessarily
extended.
The third approach is to fix fil_io() so that tablespaces that have not
yet been opened (i.e. with space->size = 0) are not extended. This,
however, is not always sufficient. Even if we know the tablespace size
already, and recovery wants to access a page beyond the current size, we
would still align it to the file extent boundary (1M). But InnoDB itself
doesn't do that for small tablespaces (i.e. less than 1M). Aligning file
extensions to 1M was implemented in rev. 29 to make sure larger
tablespaces are aligned as InnoDB expects them. However, that change was
essentially obsoleted by the fix for bug #461099 (rev. 35), because with
that change tablespaces are always aligned to what is actually stored in
the FSP header and thus, matches what InnoDB expects them to be. So in
addition to checking space->size the 1M alignment rule from rev. 29
should be reverted.
This revision implements the latter approach.
It also looks like under some circumstances xtrabackup can extend the
source file during backup with that extra code in fil_io() added by the
XB patches. But this is a separate issue reported as #1144874 for
further investigation. For now the test case in this patch has been
adjusted so it doesn't hit that bug.
http://
Approve. Thanks for very descriptive comments which made the review much more simple! Btw, innodb56.patch contains this fix already, which is not mentioned neither in this MP, nor in revision commit.