On Tue, Mar 22, 2016 at 5:08 PM, David Steele <da...@pgmasters.net> wrote:

>
> On 3/19/16 8:15 AM, Magnus Hagander wrote:
>
> > I've attached an updated patch, which is rebased on current master and
> > includes the oid fix.
>
> Before doing a thorough review of this patch there are a few points I
> would like to consider:
>
> * I think it's really important to provide the stop time in some fashion
> when using this new technique.  I would prefer a new column to be
> returned from pg_stop_backup() but I could live with STOP TIME being
> recorded in the label file.  STOP TIME should probably be included in
> the label file anyway.
>

Adding the stop time column should be a simple addition and I don't see a
problem with that. I think I misunderstood your original request on that.
Because you are just talking about returning a timestamptz with the "right
now" value for when you called pg_stop_backup()? Or to be specific, just
before pg_Stop_backup *finished*. Or do you mean when pg_stop_backup()
started?

Doing it in the backup label file is obviously a different target, where we
might need to consider backwards compatibility, Should we?



> * It seems like STOP WAL LOCATION should now also be recorded in the
> label file.  Preferably this would used by recovery to determine when
> the database has reach consistency but that could be a future patch.
> I'm not very happy with the current method of using pg_control to get
> this information as it assumes that pg_control is copied last (at least
> based on the code comments).
>

That seems entirely out of scope for this patch, though. Doesn't mean it
shouldn't be done, but that's a separate thing.

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/

Reply via email to