> 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.
