Re: [PATCHES] Infrastructure changes for recovery (v8)
От | Heikki Linnakangas |
---|---|
Тема | Re: [PATCHES] Infrastructure changes for recovery (v8) |
Дата | |
Msg-id | 48EC9CC5.6060803@enterprisedb.com обсуждение исходный текст |
Ответ на | Infrastructure changes for recovery (v8) (Simon Riggs <simon@2ndQuadrant.com>) |
Ответы |
Re: [PATCHES] Infrastructure changes for recovery (v8)
(Simon Riggs <simon@2ndQuadrant.com>)
|
Список | pgsql-hackers |
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
В списке pgsql-hackers по дате отправления: