Xtrabackup for Drizzle (and issues surrounding it)
Stewart Smith
Last weekend I had a go at porting xtrabackup over to Drizzle. I was
successful - there's a tree up at lp:~stewart/drizzle/xtrabackup that
produces an xtrabackup binary that's built for Drizzle (it's not ready
for merging yet, there are some obivous bugs... but a backup and restore
did work).
I wanted the following:
- build to be integrated with Drizzle, using the same innobase build
that we use to build the server
- build with strict compiler warnings and -Werror (which we do for
Drizzle)
- build with a C++ compiler (as we do with innobase in Drizzle)
- not re-add parts of mysys into the Drizzle build just for xtrabackup
I have already submitted some merge requests for cleaning up a lot of
the compiler warnings in xtrabackup itself. Some of these were solved
slightly differently in what I've done for Drizzle, but aim to merge
xtrabackup into Drizzle as needed.
One issue is that the command line parsing part is long since gone in
Drizzle - we don't use my_getopt. We currently use Boost
program_options. Thanks to the heroic efforts of Andrew Hutchings,
xtrabackp in Drizzle is also using boost::program_options.
An unresolved issue is how to deal with this going forward. I don't know
if you want to require Boost (my guess is not).
One solution could be just to factor out command line options into a sep
file that we can ignore for Drizzle and replace with our own.
the other option could be to use a differnt command line option parsing
library (perhaps from CCAN, as it's then maintained by somebody else and
doesn't require heaps and heaps of other stuff).
Another issue is the patch to innobase that's required to build
xtrabackup.
I took a very minimal approach for the Drizzle patch. We are currently
based on innobase 1.1.4 from MySQL 5.5 - so I mostly looked at the
xtradb55 patch. I think it would be great if these were instead of one
giant patch a series of patches to apply (a-la quilt) to a) make it
easier maintain and b) easier for myself to work out the exact reasoning
of each bit (also, generating the patches with -p would help a fair bit too)
Step 0 was removing support for old innobase - we totally don't need it
for Drizzle.
Step 1 was creating a srv_read_only option for Drizzle's innobase. This
was fairly easy. The one thing I did have to change was adding a check
in os_file_lock() so that we don't attempt to write lock the ibdata
files when in read only (otherwise backups can't be taken while drizzled
is running). I'm a little surprised that this wasn't hit in 5.5 at all.
Step 2 was implementing srv_fake_write. I'm pretty sure I've gotten this
right in the Drizzle implementation, but the patch wasn't as easy to
read as I'd really like. I probably need to do a bit more of a code
audit that this is actually correct (I may try and come up with an
LD_PRELOAD library that will scream loudly if writes are made to files
matching a pattern)
Step 3 was implemnting srv_apply_log_only. Pretty sure I have this
right, again, more testing will be required.
Step 4 was to go through all the functions that xtrabackup
needed to not be static. Instead of having prototypes for them in
xtrabackup.cc, I instead added a xtrabackup_api.h header to Innobase
and included it where needed (including in xtrabackup). I'd recommend
this way going forward as it could be a lot less problematic to maintain
(and makes xtrabackup source a bit easier to read).
Step 5 was fixing up a few skeleton functions that were needed possibly
just due to our innobase handler. It may not be a bad idea to split out
the skeleton functions into a sep source file so it's a bit easier to
track (and some #ifdefs around those not needed for certain releases).
I'd love feedback on suggestions above.
Another thought of mine is to port xtrabackup into HailDB where we can
use much more neat API functions to create good tests for xtrabackup.
thanks to all who've hacked on xtrabackup. It honestly wasn't too hard
getting it ported across to Drizzle - and with a bit of collaboration I
think we can make it easy to keep up to date.
cheers,
--
Stewart Smith
Alexey Kopytov
On 21.03.11 16:34, Stewart Smith wrote:
> Hi all.
>
> Last weekend I had a go at porting xtrabackup over to Drizzle. I was
> successful - there's a tree up at lp:~stewart/drizzle/xtrabackup that
> produces an xtrabackup binary that's built for Drizzle (it's not ready
> for merging yet, there are some obivous bugs... but a backup and restore
> did work).
>
> I wanted the following:
> - build to be integrated with Drizzle, using the same innobase build
> that we use to build the server
> - build with strict compiler warnings and -Werror (which we do for
> Drizzle)
> - build with a C++ compiler (as we do with innobase in Drizzle)
> - not re-add parts of mysys into the Drizzle build just for xtrabackup
>
> I have already submitted some merge requests for cleaning up a lot of
> the compiler warnings in xtrabackup itself. Some of these were solved
> slightly differently in what I've done for Drizzle, but aim to merge
> xtrabackup into Drizzle as needed.
>
Thank you once again for working on the compiler warnings. All merge
proposals you have submitted are now in trunk. And we will certainly
look into fixing the remaining warnings.
> One issue is that the command line parsing part is long since gone in
> Drizzle - we don't use my_getopt. We currently use Boost
> program_options. Thanks to the heroic efforts of Andrew Hutchings,
> xtrabackp in Drizzle is also using boost::program_options.
>
> An unresolved issue is how to deal with this going forward. I don't know
> if you want to require Boost (my guess is not).
>
Right, moving to Boost does not look feasible to us for various reasons,
see below.
> One solution could be just to factor out command line options into a sep
> file that we can ignore for Drizzle and replace with our own.
>
> the other option could be to use a differnt command line option parsing
> library (perhaps from CCAN, as it's then maintained by somebody else and
> doesn't require heaps and heaps of other stuff).
>
I could only find one options library at CCAN ('opt'). It's GPLv3 (I'm
not sure if we can mix it with GPLv2-licensed code). What's more
important is that it doesn't seem to support parsing the options from
configuration files, and XtraBackup depends on that.
An important feature of XtraBackup is that you can pass server's my.cnf
to innobackupex or xtrabackup and they will use it to figure out the
server and InnoDB configuration (datadir, tmpdir, innodb_flush_method
and so on) from that file.
So Boost is apparently out of the question for us. And it looks like
configuration file parsing is currently not supported by the Drizzle
port of XtraBackup.
But I think moving the options parsing into a separate file should work.
Please note that we are going to introduce more code depending on
my_getopt in the future (see src/xbstream.c in
lp:~percona-dev/percona-xtrabackup/xtrabackup-parallel-compression). So
the only solution to this issue that I see is to introduce some
indirection between the xtrabackup code and the specific option parsing
library. Please let me know if you have better ideas.
>
> Another issue is the patch to innobase that's required to build
> xtrabackup.
>
> I took a very minimal approach for the Drizzle patch. We are currently
> based on innobase 1.1.4 from MySQL 5.5 - so I mostly looked at the
> xtradb55 patch. I think it would be great if these were instead of one
> giant patch a series of patches to apply (a-la quilt) to a) make it
> easier maintain and b) easier for myself to work out the exact reasoning
> of each bit (also, generating the patches with -p would help a fair bit too)
>
Yes, we can do that. Speaking of which, I have recently restored the
patch against the upstream MySQL 5.5 (see innodb55.patch), which should
probably be easier for you to base your work on.
> Step 0 was removing support for old innobase - we totally don't need it
> for Drizzle.
>
> Step 1 was creating a srv_read_only option for Drizzle's innobase. This
> was fairly easy. The one thing I did have to change was adding a check
> in os_file_lock() so that we don't attempt to write lock the ibdata
> files when in read only (otherwise backups can't be taken while drizzled
> is running). I'm a little surprised that this wasn't hit in 5.5 at all.
>
Hm, both innodb55.patch and xtradb55.patch disable os_file_lock(), so
that should be an issue:
--- a/storage/innobase/os/os0file.c 2011-01-19 14:37:08.000000000 -0800
+++ b/storage/innobase/os/os0file.c 2011-03-11 07:20:18.000000000 -0800
@@ -656,7 +656,7 @@
}
#undef USE_FILE_LOCK
-#define USE_FILE_LOCK
+//#define USE_FILE_LOCK
#if defined(UNIV_HOTBACKUP) || defined(__WIN__)
/* InnoDB Hot Backup does not lock the data files.
* On Windows, mandatory locking is used.
> Step 2 was implementing srv_fake_write. I'm pretty sure I've gotten this
> right in the Drizzle implementation, but the patch wasn't as easy to
> read as I'd really like. I probably need to do a bit more of a code
> audit that this is actually correct (I may try and come up with an
> LD_PRELOAD library that will scream loudly if writes are made to files
> matching a pattern)
>
Were there any problems with srv_fake_write implementation in
innodb55.patch/xtradb55.patch, or Drizzle's innobase is too different to
apply that implementation directly?
> Step 3 was implemnting srv_apply_log_only. Pretty sure I have this
> right, again, more testing will be required.
>
> Step 4 was to go through all the functions that xtrabackup
> needed to not be static. Instead of having prototypes for them in
> xtrabackup.cc, I instead added a xtrabackup_api.h header to Innobase
> and included it where needed (including in xtrabackup). I'd recommend
> this way going forward as it could be a lot less problematic to maintain
> (and makes xtrabackup source a bit easier to read).
>
Agree, that would improve xtrabackup.c readability.
> Step 5 was fixing up a few skeleton functions that were needed possibly
> just due to our innobase handler. It may not be a bad idea to split out
> the skeleton functions into a sep source file so it's a bit easier to
> track (and some #ifdefs around those not needed for certain releases).
>
Yes, again, I fully agree with that.
> I'd love feedback on suggestions above.
>
> Another thought of mine is to port xtrabackup into HailDB where we can
> use much more neat API functions to create good tests for xtrabackup.
>
Yes, that was my first thought when I heard about HailDB for the first
time. It looks like an interesting project to me.
Cheers,
Alexey.
Stewart Smith
> > One solution could be just to factor out command line options into a sep
> > file that we can ignore for Drizzle and replace with our own.
> >
> > the other option could be to use a differnt command line option parsing
> > library (perhaps from CCAN, as it's then maintained by somebody else and
> > doesn't require heaps and heaps of other stuff).
> >
>
> I could only find one options library at CCAN ('opt'). It's GPLv3 (I'm
> not sure if we can mix it with GPLv2-licensed code). What's more
> important is that it doesn't seem to support parsing the options from
> configuration files, and XtraBackup depends on that.
>
> An important feature of XtraBackup is that you can pass server's my.cnf
> to innobackupex or xtrabackup and they will use it to figure out the
> server and InnoDB configuration (datadir, tmpdir, innodb_flush_method
> and so on) from that file.
I thought as much... no doubt useful.
> So Boost is apparently out of the question for us. And it looks like
> configuration file parsing is currently not supported by the Drizzle
> port of XtraBackup.
correct.
> But I think moving the options parsing into a separate file should
> work.
Great - I'll see if I can get something going in an xtrabackup tree for this.
> Please note that we are going to introduce more code depending on
> my_getopt in the future (see src/xbstream.c in
> lp:~percona-dev/percona-xtrabackup/xtrabackup-parallel-compression). So
> the only solution to this issue that I see is to introduce some
> indirection between the xtrabackup code and the specific option parsing
> library. Please let me know if you have better ideas.
From a quick look, it looks like this would be fairly easy to maintain
ported to Boost options for Drizzle. The main reason I was thinking of
having the xtrabackup options in a sep file was that there were so many
of them... when it's only a couple the merge for any changes is a lot
easier to spot.
> > Another issue is the patch to innobase that's required to build
> > xtrabackup.
> >
> > I took a very minimal approach for the Drizzle patch. We are currently
> > based on innobase 1.1.4 from MySQL 5.5 - so I mostly looked at the
> > xtradb55 patch. I think it would be great if these were instead of one
> > giant patch a series of patches to apply (a-la quilt) to a) make it
> > easier maintain and b) easier for myself to work out the exact reasoning
> > of each bit (also, generating the patches with -p would help a fair bit too)
> >
>
> Yes, we can do that. Speaking of which, I have recently restored the
> patch against the upstream MySQL 5.5 (see innodb55.patch), which should
> probably be easier for you to base your work on.
The big difference I have is that we're using the same innobase source
tree (and binaries) for linking into the server and producing the
xtrabackup binary - so instead of some places just changing a define,
commenting out something or #ifdefing it out, I want to have it done in
a runtime compatible way.
The one bit that I haven't applied is the DB_TABLESPACE_DELETED
part... mainly because I'd love to get Patrick Crews to create a
repeatable test case for us first (although I don't doubt that it's
needed - I'm surprised there aren't more recovery bugs with InnoDB in
general actually).
> > Step 0 was removing support for old innobase - we totally don't need it
> > for Drizzle.
> >
> > Step 1 was creating a srv_read_only option for Drizzle's innobase. This
> > was fairly easy. The one thing I did have to change was adding a check
> > in os_file_lock() so that we don't attempt to write lock the ibdata
> > files when in read only (otherwise backups can't be taken while drizzled
> > is running). I'm a little surprised that this wasn't hit in 5.5 at all.
> >
>
> Hm, both innodb55.patch and xtradb55.patch disable os_file_lock(), so
> that should be an issue:
>
> --- a/storage/innobase/os/os0file.c 2011-01-19 14:37:08.000000000 -0800
> +++ b/storage/innobase/os/os0file.c 2011-03-11 07:20:18.000000000 -0800
> @@ -656,7 +656,7 @@
> }
>
> #undef USE_FILE_LOCK
> -#define USE_FILE_LOCK
> +//#define USE_FILE_LOCK
> #if defined(UNIV_HOTBACKUP) || defined(__WIN__)
> /* InnoDB Hot Backup does not lock the data files.
> * On Windows, mandatory locking is used.
Ahhh.. must have missed that.
I've instead gone down the path of having a srv_read_only check in
os_file_lock() - this means that we still get file lock for running the
server, just not for xtrabackup (which is what we want.)
> > Step 2 was implementing srv_fake_write. I'm pretty sure I've gotten this
> > right in the Drizzle implementation, but the patch wasn't as easy to
> > read as I'd really like. I probably need to do a bit more of a code
> > audit that this is actually correct (I may try and come up with an
> > LD_PRELOAD library that will scream loudly if writes are made to files
> > matching a pattern)
> >
>
> Were there any problems with srv_fake_write implementation in
> innodb55.patch/xtradb55.patch, or Drizzle's innobase is too different to
> apply that implementation directly?
I pretty much applied it directly... I just manually went through the
patch to better enhance my understanding of it.
> > I'd love feedback on suggestions above.
> >
> > Another thought of mine is to port xtrabackup into HailDB where we can
> > use much more neat API functions to create good tests for xtrabackup.
> >
>
> Yes, that was my first thought when I heard about HailDB for the first
> time. It looks like an interesting project to me.
Currently it's about striking a balance between moving to HailDB while
retaining features and stability and also keeping our in-tree innobase
plugin going...
--
Stewart Smith
Baron Schwartz
Just a thought. What about building xtrabackup's functionality into
the server itself and letting it be controlled from within the server?
Instead of command-line options, create a set of tables that defines
the backup parameters such as destination, periodicity, incrementality,
retention policy and so on. The DBA would just insert rows into
those tables, then let the database back itself up.
I have no clue whether this is a good idea. I'm just throwing it out
there.
Baron
Henrik Ingo
don't just use xtrabackup.
1) Also in Drizzle they might need to use some solution like
innobackupex, where you backup innodb tables + other things in a
concerted fashion. Such a script needs a way to manage xtrabackup. 2)
When I take a backup usually tar, gzip and ssh might be involved. If
backups are done from within the server suddenly the server is
depending on these utilities and needs to know how to call them from a
command line shell.
For 1), if xtrabackup was inside Drizzle and Drizzle implemented a
whole new set of SQL commands to do backups, it might do it. Like
mysql -e "START BACKUP *.* PIPE INTO tar|gzip|ssh..."
...then sure, it is a way to do it.
For 2 it seems this is just too messy to push into the server (such as
the PIPE INTO above).
henrik
> --
> You received this message because you are subscribed to the Google Groups "Percona Dev" group.
> To post to this group, send email to perco...@googlegroups.com.
> To unsubscribe from this group, send email to percona-dev...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/percona-dev?hl=en.
>
>
--
henri...@avoinelama.fi
+358-40-5697354 skype: henrik.ingo irc: hingo
www.openlife.cc
My LinkedIn profile: http://www.linkedin.com/profile/view?id=9522559