On Mon, Oct 24, 2016 at 1:26 PM, Amit Kapila <amit.kapil...@gmail.com> wrote:
> On Fri, Oct 21, 2016 at 10:32 PM, Robert Haas <robertmh...@gmail.com> wrote:
>> 2. In perform_base_backup(), if the endptr returned by
>> do_pg_stop_backup() precedes the end of the checkpoint record returned
>> by do_pg_start_backup(), use the latter value instead.  Unfortunately,
>> that's not so easy: we can't just say if (endptr < starptr) startptr =
>> endptr; because startptr is the *start* of the checkpoint record, not
>> the end.  I suspect somebody could figure out a solution to this
>> problem, though.
> With this approach, don't we need something similar for API's
> pg_stop_backup()/pg_stop_backup_v2()?

Yes, I think so. That would sort of map with the idea I mentioned
upthread to have pg_stop_backup() return the contents of the control
file and have the caller write it to the backup by itself.

>> If we decide we don't want to aim for one of these tighter solutions
>> and just adopt the previously-discussed patch, then at the very least
>> it needs better comments.
> +1.

Yeah, here is an attempt at doing that:
-    * We return the current minimum recovery point as the backup end
+    * We return the current last replayed point as the backup end
     * location. Note that it can be greater than the exact backup end
-    * location if the minimum recovery point is updated after the backup of
+    * location if the last replayed point is updated after the backup of
     * pg_control. This is harmless for current uses.
+    * Using the last replayed point as the backup end location ensures that
+    * the end location will never be older than the start position, something
+    * that could happen if for example minRecoveryPoint is used as backup
+    * end location when it never gets updated because no buffer flushes
+    * occurred. By using the last replay location, note that the backup may
+    * include more WAL segments than necessary. If the additional WAL
+    * replayed since minRecoveryPoint does not include a checkpoint, there
+    * is actually no need for it. Even if it does include a checkpoint,
+    * only the portion up to the checkpoint itself is necessary and not
+    * the WAL generated beyond that. Still, in the case of a backup taken
+    * from a standby, with its master disconnected, this ensures that the
+    * backup is valid.
+    *

Thoughts welcome.

Attachment: backup-standby-v3.patch
Description: invalid/octet-stream

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to