On 3/22/16 12:14 PM, Magnus Hagander wrote: > On Tue, Mar 22, 2016 at 5:08 PM, David Steele <da...@pgmasters.net > <mailto: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?
What would be ideal is the minimum time that could be used for PITR. In an exclusive backup that's the time the end-of-backup record is written to WAL. In a non-exlusive backup I'm not quite sure how that works. > Doing it in the backup label file is obviously a different target, where > we might need to consider backwards compatibility, Should we? Physical backups can only be restored in the same version so I'm not sure why it would be a problem? Do you mean for programs outside of Postgres that are parsing this file? > * 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. Fair enough. -- -David da...@pgmasters.net -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers