Re: Minor changes to Recovery related code

On Fri, 2007-03-30 at 16:34 -0400, Tom Lane wrote:
> "Simon Riggs" <simon(at)2ndquadrant(dot)com> writes:
> > I'd like to make the following changes to recovery related code over the
> > next few days/weeks. If anybody insists I do this by freeze or not at
> > all, then I'll submit patches for 1,3,4,5,10 before Saturday night. I'd
> > rather take a bit more time and do this in one drop and there are some
> > code dependencies between these changes and other patches from
> > Koichi-san and myself.
>
> Well, I've got a proposal for a pg_proc change that I haven't even started
> coding yet, so personally I won't hold you to having the patch submitted
> as long as the design is agreed to before feature freeze. However:

Cool

> > 2. pg_stop_backup() should wait until all archive files are safely
> > archived before returning
>
> Not sure I agree with that one. If it fails, you can't tell whether the
> action is done and it failed while waiting for the archiver, or if you
> need to redo it.

There's a slight delay between pg_stop_backup() completing and the
archiver doing its stuff. Currently if somebody does a -m fast straight
after the pg_stop_backup() the backup may be unusable.

We need a way to plug that small hole.

I suggest that pg_stop_backup() polls once per second until
pg_xlog/archive_status/LOG.ready disappears, in which case it ends
successfully. If it does this for more than 60 seconds it ends
successfully but produces a WARNING.

> > 6. refactor recovery.conf so that it uses a GUC-like parser
>
> I would suggest that after feature freeze is not the time for code
> beautification efforts like this. (I rather doubt that it's worth doing
> at all actually, but definitely not now.)

OK

> > 7. refactor all xlog _desc routines into one module, so these can be
> > more easily used by xlogviewer utility
>
> Even more so.

OK

> > 8. get xlogviewer utility a freshen-up so it can be part of main
> > release, possibly including further refactoring of xlog.c
>
> This is not happening for 8.3, either.

OK

> > 10. Changes to ensure WAL-avoiding operations and hot backups cannot be
> > executed simultaneously. One of these two options, ISTM:
> > b) Introduce a new parameter, archive_mode = on | off that can only be
> > set at server start. If archive_mode = on then XLogArchivingActive();
> > archiving only takes place when archive_command is not ''. This allows
> > archive_command to be changed while server running, yet without any
> > danger from WAL-avoiding operations.
>
> I think I'd go with (b) since a lot of people felt it should've been
> like that from the beginning, and found the magic "empty string"
> behavior confusing.

OK

> We are hoping to go beta in less time than that. While I'm willing to
> cut a little slack, anything you can't submit before about mid-April
> is not going to make it into 8.3.

No probs, you struck a line through the more time consuming items. :-)

--
Simon Riggs
EnterpriseDB http://www.enterprisedb.com