Subject: Re: [HACKERS] Point in Time Recovery
From: Simon Riggs <simon@2ndquadrant.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Bruce Momjian <pgman@candle.pha.pa.us>,
	pgsql-patches@postgresql.org
In-Reply-To: <25995.1090127083@sss.pgh.pa.us>
References: <200407172015.i6HKFLK17349@candle.pha.pa.us>
	<25995.1090127083@sss.pgh.pa.us>
Content-Type: text/plain
Organization: 2nd Quadrant
Message-Id: <1090182052.17493.18904.camel@stromboli>
Mime-Version: 1.0
Date: Sun, 18 Jul 2004 21:20:52 +0100
Content-Transfer-Encoding: 7bit

On Sun, 2004-07-18 at 06:04, Tom Lane wrote:
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > So you want to merge them all into a single command string.  That does
> > seem less error-prone.  I see a few variables that turn off
> > when set to '' like unix_socket_*.  How would this command string work? 
> > How do you specify the WAL file name to transfer?
> 

GUC-wise, I implemented what we agreed in discussions...

There are many things in need of refactoring, so my focus was on
delivering what we agreed, even knowing it would probably change...

A few notes on the patch (as I submitted it - so as not to confuse with
other versions being worked upon) 
- archive_dest is definitely used in both archive and recovery. There
wasn't much need for this GUC apart from that and I think we are better
off without it. Removing it improves recovery flexibility (we cannot
assume the recovery is taking place in anything like the original
configuration).

- archive_mode I would prefer to keep - it is explicit then which mode
you are in, rather than implicit from the command string. In all other
ways I agree with everything Tom has said. It allows us to talk about
"being in archive_mode" without people saying "but I can't work out how
to turn archive mode on".

When archiver starts the FIRST thing it does is run a test to confirm
that the command string works, so setting archive_command to '' would
simply generate an error.

Also, I would suggest this:
- changing archive mode requires a postmaster restart
- changing archive command should just be a SIGHUP...we don't want to
force a restart just to switch to a new kind of archiving

If you can only change archive_program at postmaster start that is
restrictive, but making that SIGHUP would allow people to set it to ''
and turn off archiving while postmaster is up == lurking fault.

> No different from before, necessarily.  However I did not like the
> restriction to a single %s in the submitted implementation.  What I
> have in my local copy is
> 	%p -> full path of XLOG file to be archived
> 	%f -> base name of XLOG file to be archived
> and the suggested example becomes
> 	archive_command = 'cp %p /mnt/server/pgarchive/%f'
> 

I'm happy with those changes and would have done them myself given
time... the 2 or 3 %s parameters wasn't the most user friendly way of
doing it.

> Note that this example immediately eliminates one of the failure modes
> Simon enumerates in his README, which is to try 'cp %s /foo' where /foo
> isn't a directory.  More generally, though, *only* a cp-to-directory
> solution is likely to be very happy with not being able to get at the
> base file name.  Yes you can make a shellscript and use basename,
> but I don't think you should have to do that if it could otherwise
> be a one-liner.
> 

Good.

> (In case it's not obvious from the above, I am hacking with intent to
> commit soon.  Maybe tomorrow, if my wife doesn't make me paint the
> bathroom instead...)
> 
...just returned from there... :)


Best Regards, Simon Riggs