Simon Riggs wrote:
* optional recovery_safe_start_location parameter now provided in
recovery.conf, to allow a consistency point to be manually defined if a
base backup was not taken using standard pg_start/stop backup functions
Do we want to cater for that? It only seems safe if you have
full_page_writes turned on, and you perform a checkpoint first. But if
you do that, why don't you just use pg_start_backup()?
Other Changes
* log_restartpoints removed, use log_checkpoints in postgresql.conf
Is this something that would make sense regardless of the rest of the
patch? If so, we could apply that separately, which would make this
patch a little less overwhelming to review.
* additional function signature for pg_start_backup('label', true |
false) to allow definition of immediate checkpoint/not
Wouldn't this need a new entry in pg_proc.h? Again, perhaps we should do
this as a separate patch.
* fixes bug discovered while other testing: if pg_stop_backup() is run
when xlogswitch has just occurred then we do not switch log files, yet
we return current filename even though nothing of value in it. If
archive_timeout not enabled we would wait forever for pg_stop_backup()
to return.
* Substantial comments throughout
These comments on CheckPointLock seem contradictory:
--- 247,256 ----
* ControlFileLock: must be held to read/update control file or create
* new log file.
*
! * CheckpointLock: must be held to do a checkpoint or restartpoint, ensuring
! * we get just one of those at any time. In 8.4+ recovery, both startup and
! * bgwriter processes may take restartpoints, so this locking must be strict
! * to ensure there are no mistakes.
*
*----------
*/
and
--- 5901,5916 ----
XLogRecPtr recptr;
XLogCtlInsert *Insert = &XLogCtl->Insert;
XLogRecData rdata;
uint32 _logId;
uint32 _logSeg;
TransactionId *inCommitXids;
int nInCommit;
+ bool leavingArchiveRecovery = false;
/*
* Acquire CheckpointLock to ensure only one checkpoint happens at a
time.
! * That shouldn't be happening, but checkpoints are an important aspect
! * of our resilience, so we take no chances.
*/
LWLockAcquire(CheckpointLock, LW_EXCLUSIVE);
If I've understood the patch correctly, only bgwriter does checkpoints
and restart points now?
There's a trivial merge conflict in bgwriter.c, due to the FSM patch.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
