On Thu, Oct 27, 2016 at 2:59 AM, Robert Haas <robertmh...@gmail.com> wrote:
> On Wed, Oct 26, 2016 at 2:06 AM, Michael Paquier
> <michael.paqu...@gmail.com> wrote:
>> But yes, thinking *harder*, I agree that updating minRecoveryPoint
>> just after the checkpoint record would be fine and removes the need to
>> have more WAL than necessary in for a backup taken from a standby.
>> That will also prevent cases where minRecoveryPoint is older than the
>> recovery start point. On top of that the cost of an extra call to
>> UpdateControlFile() looks cheap considering that CreateRestartPoint()
>> is called only by the checkpointer or at shutdown.
>>
>> Just coding things this solution gives roughtly the attached? The TAP
>> test passes btw.
>
> I think that still leaves a race condition, right? It's got to be
> part of the SAME control file update that advances the redo pointer.
Right, thanks for double-checking... There is no meaning to do that
out of the ControlFileLock taken previously...
--
Michael
diff --git a/src/backend/access/transam/xlog.c
b/src/backend/access/transam/xlog.c
index d6c057a..ad5bfa3 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8910,6 +8910,28 @@ CreateRestartPoint(int flags)
ControlFile->time = (pg_time_t) time(NULL);
if (flags & CHECKPOINT_IS_SHUTDOWN)
ControlFile->state = DB_SHUTDOWNED_IN_RECOVERY;
+
+ /*
+ * Update minRecoveryPoint just past the last redo checkpoint if
+ * necessary. This ensures that at next startup
minRecoveryPoint will
+ * not be past the next point it would start at, preventing any
+ * weird behaviors with for example backups taken from standbys
that
+ * rely on minRecoveryPoint as end backup location.
+ */
+ if (ControlFile->minRecoveryPoint < lastCheckPointRecPtr)
+ {
+ ControlFile->minRecoveryPoint = lastCheckPointRecPtr;
+ ControlFile->minRecoveryPointTLI = ThisTimeLineID;
+ /* update local copy */
+ minRecoveryPoint = ControlFile->minRecoveryPoint;
+ minRecoveryPointTLI = ControlFile->minRecoveryPointTLI;
+
+ ereport(DEBUG2,
+ (errmsg("updated min recovery point to %X/%X on
timeline %u",
+ (uint32) (minRecoveryPoint >>
32),
+ (uint32) minRecoveryPoint,
+ minRecoveryPointTLI)));
+ }
UpdateControlFile();
}
LWLockRelease(ControlFileLock);
diff --git a/src/test/recovery/t/001_stream_rep.pl
b/src/test/recovery/t/001_stream_rep.pl
index fd71095..981c00b 100644
--- a/src/test/recovery/t/001_stream_rep.pl
+++ b/src/test/recovery/t/001_stream_rep.pl
@@ -24,6 +24,11 @@ $node_standby_1->start;
# pg_basebackup works on a standby).
$node_standby_1->backup($backup_name);
+# Take a second backup of the standby while the master is offline.
+$node_master->stop;
+$node_standby_1->backup('my_backup_2');
+$node_master->start;
+
# Create second standby node linking to standby 1
my $node_standby_2 = get_new_node('standby_2');
$node_standby_2->init_from_backup($node_standby_1, $backup_name,
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
