pg.ddx.io  pgsql-docs@postgresql.org mailing list archive  
help / color / mirror / Atom feed
Re: Re: [COMMITTERS] pgsql: Make standby server 	continuously retry restoring the next WAL
32+ messages / 6 participants
[nested] [flat]

* Re: Re: [COMMITTERS] pgsql: Make standby server 	continuously retry restoring the next WAL
@ 2010-03-17 10:35  Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  0 siblings, 2 replies; 32+ messages in thread

From: Heikki Linnakangas @ 2010-03-17 10:35 UTC (permalink / raw)
  To: Fujii Masao <masao.fujii@gmail.com>; +Cc: Aidan Van Dyk <aidan@highrise.ca>; Simon Riggs <simon@2ndquadrant.com>; PostgreSQL-development <pgsql-hackers@postgresql.org>

Fujii Masao wrote:
> I found another missing feature in new file-based log shipping (i.e.,
> standby_mode is enabled and 'cp' is used as restore_command).
> 
> After the trigger file is found, the startup process with pg_standby
> tries to replay all of the WAL files in both pg_xlog and the archive.
> So, when the primary fails, if the latest WAL file in pg_xlog of the
> primary can be read, we can prevent the data loss by copying it to
> pg_xlog of the standby before creating the trigger file.
> 
> On the other hand, the startup process with standby mode doesn't
> replay the WAL files in pg_xlog after the trigger file is found. So
> failover always causes the data loss even if the latest WAL file can
> be read from the primary. And if the latest WAL file is copied to the
> archive instead, it can be replayed but a PANIC error would happen
> because it's not filled.
> 
> We should remove this restriction?

Looking into this, I realized that we have a bigger problem related to
this. Although streaming replication stores the streamed WAL files in
pg_xlog, so that they can be re-replayed after a standby restart without
connecting to the master, we don't try to replay those either. So if you
restart standby, it will fail to start up if the WAL it needs can't be
found in archive or by connecting to the master. That must be fixed.

I'd imagine that the ability to restore WAL files manually copied to
pg_xlog will fall out of that fix too.

-- 
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com



^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL
@ 2010-03-17 10:49  Simon Riggs <simon@2ndQuadrant.com>
  parent: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  1 sibling, 0 replies; 32+ messages in thread

From: Simon Riggs @ 2010-03-17 10:49 UTC (permalink / raw)
  To: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; +Cc: Fujii Masao <masao.fujii@gmail.com>; Aidan Van Dyk <aidan@highrise.ca>; PostgreSQL-development <pgsql-hackers@postgresql.org>

On Wed, 2010-03-17 at 12:35 +0200, Heikki Linnakangas wrote:

> Looking into this, I realized that we have a bigger problem...

A lot of this would be easier if you do the docs first, then work
through the problems. The new system is more complex, since it has two
modes rather than one and also multiple processes and a live connection.
The number of failure cases must be higher than previously.

Documenting how it is supposed to work in the event of failure will help
everyone check those and comment on them.

-- 
 Simon Riggs           www.2ndQuadrant.com




^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL
@ 2010-03-18 14:27  Fujii Masao <masao.fujii@gmail.com>
  parent: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  1 sibling, 1 reply; 32+ messages in thread

From: Fujii Masao @ 2010-03-18 14:27 UTC (permalink / raw)
  To: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; +Cc: Aidan Van Dyk <aidan@highrise.ca>; Simon Riggs <simon@2ndquadrant.com>; PostgreSQL-development <pgsql-hackers@postgresql.org>

On Wed, Mar 17, 2010 at 7:35 PM, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:
> Fujii Masao wrote:
>> I found another missing feature in new file-based log shipping (i.e.,
>> standby_mode is enabled and 'cp' is used as restore_command).
>>
>> After the trigger file is found, the startup process with pg_standby
>> tries to replay all of the WAL files in both pg_xlog and the archive.
>> So, when the primary fails, if the latest WAL file in pg_xlog of the
>> primary can be read, we can prevent the data loss by copying it to
>> pg_xlog of the standby before creating the trigger file.
>>
>> On the other hand, the startup process with standby mode doesn't
>> replay the WAL files in pg_xlog after the trigger file is found. So
>> failover always causes the data loss even if the latest WAL file can
>> be read from the primary. And if the latest WAL file is copied to the
>> archive instead, it can be replayed but a PANIC error would happen
>> because it's not filled.
>>
>> We should remove this restriction?
>
> Looking into this, I realized that we have a bigger problem related to
> this. Although streaming replication stores the streamed WAL files in
> pg_xlog, so that they can be re-replayed after a standby restart without
> connecting to the master, we don't try to replay those either. So if you
> restart standby, it will fail to start up if the WAL it needs can't be
> found in archive or by connecting to the master. That must be fixed.

I agree that this is a bigger problem. Since the standby always starts
walreceiver before replaying any WAL files in pg_xlog, walreceiver tries
to receive the WAL files following the REDO starting point even if they
have already been in pg_xlog. IOW, the same WAL files might be shipped
from the primary to the standby many times. This behavior is unsmart,
and should be addressed.

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center



^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL
@ 2010-03-19 08:52  Simon Riggs <simon@2ndQuadrant.com>
  parent: Fujii Masao <masao.fujii@gmail.com>
  0 siblings, 1 reply; 32+ messages in thread

From: Simon Riggs @ 2010-03-19 08:52 UTC (permalink / raw)
  To: Fujii Masao <masao.fujii@gmail.com>; +Cc: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; Aidan Van Dyk <aidan@highrise.ca>; PostgreSQL-development <pgsql-hackers@postgresql.org>

On Thu, 2010-03-18 at 23:27 +0900, Fujii Masao wrote:

> I agree that this is a bigger problem. Since the standby always starts
> walreceiver before replaying any WAL files in pg_xlog, walreceiver tries
> to receive the WAL files following the REDO starting point even if they
> have already been in pg_xlog. IOW, the same WAL files might be shipped
> from the primary to the standby many times. This behavior is unsmart,
> and should be addressed.

We might also have written half a file many times. The files in pg_xlog
are suspect whereas the files in the archive are not. If we have both we
should prefer the archive.

-- 
 Simon Riggs           www.2ndQuadrant.com




^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: Re: [COMMITTERS] pgsql: Make standby server  continuously retry restoring the next WAL
@ 2010-03-19 11:37  Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  parent: Simon Riggs <simon@2ndQuadrant.com>
  0 siblings, 3 replies; 32+ messages in thread

From: Heikki Linnakangas @ 2010-03-19 11:37 UTC (permalink / raw)
  To: Simon Riggs <simon@2ndQuadrant.com>; +Cc: Fujii Masao <masao.fujii@gmail.com>; Aidan Van Dyk <aidan@highrise.ca>; PostgreSQL-development <pgsql-hackers@postgresql.org>

Simon Riggs wrote:
> On Thu, 2010-03-18 at 23:27 +0900, Fujii Masao wrote:
> 
>> I agree that this is a bigger problem. Since the standby always starts
>> walreceiver before replaying any WAL files in pg_xlog, walreceiver tries
>> to receive the WAL files following the REDO starting point even if they
>> have already been in pg_xlog. IOW, the same WAL files might be shipped
>> from the primary to the standby many times. This behavior is unsmart,
>> and should be addressed.
> 
> We might also have written half a file many times. The files in pg_xlog
> are suspect whereas the files in the archive are not. If we have both we
> should prefer the archive.

Yep.

Here's a patch I've been playing with. The idea is that in standby mode,
the server keeps trying to make progress in the recovery by:

a) restoring files from archive
b) replaying files from pg_xlog
c) streaming from master

When recovery reaches an invalid WAL record, typically caused by a
half-written WAL file, it closes the file and moves to the next source.
If an error is found in a file restored from archive or in a portion
just streamed from master, however, a PANIC is thrown, because it's not
expected to have errors in the archive or in the master.

When a file is streamed from master, it's left in pg_xlog, so it's found
there after a standby restart, and recovery can progress to the same
point as before restart. It also means that you can copy partial WAL
files to pg_xlog at any time and have them replayed in a few seconds.

The code structure is a bit spaghetti-like, I'm afraid. Any suggestions
on how to improve that are welcome..

-- 
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

Attachments:

retry-wal-from-pg_xlog-1.patch (text/x-diff, 19.0K) download *** a/src/backend/access/transam/xlog.c --- b/src/backend/access/transam/xlog.c *************** *** 445,464 **** static uint32 openLogSeg = 0; static uint32 openLogOff = 0; /* * These variables are used similarly to the ones above, but for reading * the XLOG. Note, however, that readOff generally represents the offset * of the page just read, not the seek position of the FD itself, which * will be just past that page. readLen indicates how much of the current ! * page has been read into readBuf. */ static int readFile = -1; static uint32 readId = 0; static uint32 readSeg = 0; static uint32 readOff = 0; static uint32 readLen = 0; ! /* Is the currently open segment being streamed from primary? */ ! static bool readStreamed = false; /* Buffer for currently read page (XLOG_BLCKSZ bytes) */ static char *readBuf = NULL; --- 445,477 ---- static uint32 openLogOff = 0; /* + * Codes indicating where we got a WAL file from during recovery, or where + * to attempt to get one. + */ + #define XLOG_FROM_ARCHIVE (1<<0) /* Restored using restore_command */ + #define XLOG_FROM_PG_XLOG (1<<1) /* Existing file in pg_xlog */ + #define XLOG_FROM_STREAM (1<<2) /* Streamed from master */ + + /* * These variables are used similarly to the ones above, but for reading * the XLOG. Note, however, that readOff generally represents the offset * of the page just read, not the seek position of the FD itself, which * will be just past that page. readLen indicates how much of the current ! * page has been read into readBuf, and readSource indicates where we got ! * the currently open file from. */ static int readFile = -1; static uint32 readId = 0; static uint32 readSeg = 0; static uint32 readOff = 0; static uint32 readLen = 0; + static int readSource = 0; /* XLOG_FROM_* code */ ! /* ! * Keeps track of which sources we've tried to read the current WAL ! * record from and failed. ! */ ! static int failedSources = 0; /* Buffer for currently read page (XLOG_BLCKSZ bytes) */ static char *readBuf = NULL; *************** *** 512,520 **** static bool InstallXLogFileSegment(uint32 *log, uint32 *seg, char *tmppath, bool find_free, int *max_advance, bool use_lock); static int XLogFileRead(uint32 log, uint32 seg, int emode, TimeLineID tli, ! bool fromArchive, bool notexistOk); static int XLogFileReadAnyTLI(uint32 log, uint32 seg, int emode, ! bool fromArchive); static bool XLogPageRead(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt, bool randAccess); static void XLogFileClose(void); --- 525,533 ---- bool find_free, int *max_advance, bool use_lock); static int XLogFileRead(uint32 log, uint32 seg, int emode, TimeLineID tli, ! int source, bool notexistOk); static int XLogFileReadAnyTLI(uint32 log, uint32 seg, int emode, ! int sources); static bool XLogPageRead(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt, bool randAccess); static void XLogFileClose(void); *************** *** 2567,2573 **** XLogFileOpen(uint32 log, uint32 seg) */ static int XLogFileRead(uint32 log, uint32 seg, int emode, TimeLineID tli, ! bool fromArchive, bool notfoundOk) { char xlogfname[MAXFNAMELEN]; char activitymsg[MAXFNAMELEN + 16]; --- 2580,2586 ---- */ static int XLogFileRead(uint32 log, uint32 seg, int emode, TimeLineID tli, ! int source, bool notfoundOk) { char xlogfname[MAXFNAMELEN]; char activitymsg[MAXFNAMELEN + 16]; *************** *** 2576,2598 **** XLogFileRead(uint32 log, uint32 seg, int emode, TimeLineID tli, XLogFileName(xlogfname, tli, log, seg); ! if (fromArchive) { ! /* Report recovery progress in PS display */ ! snprintf(activitymsg, sizeof(activitymsg), "waiting for %s", ! xlogfname); ! set_ps_display(activitymsg, false); ! restoredFromArchive = RestoreArchivedFile(path, xlogfname, ! "RECOVERYXLOG", ! XLogSegSize); ! if (!restoredFromArchive) ! return -1; ! } ! else ! { ! XLogFilePath(path, tli, log, seg); ! restoredFromArchive = false; } fd = BasicOpenFile(path, O_RDONLY | PG_BINARY, 0); --- 2589,2616 ---- XLogFileName(xlogfname, tli, log, seg); ! switch (source) { ! case XLOG_FROM_ARCHIVE: ! /* Report recovery progress in PS display */ ! snprintf(activitymsg, sizeof(activitymsg), "waiting for %s", ! xlogfname); ! set_ps_display(activitymsg, false); ! restoredFromArchive = RestoreArchivedFile(path, xlogfname, ! "RECOVERYXLOG", ! XLogSegSize); ! if (!restoredFromArchive) ! return -1; ! break; ! ! case XLOG_FROM_PG_XLOG: ! XLogFilePath(path, tli, log, seg); ! restoredFromArchive = false; ! break; ! ! default: ! elog(ERROR, "invalid XLogFileRead source %d", source); } fd = BasicOpenFile(path, O_RDONLY | PG_BINARY, 0); *************** *** 2606,2611 **** XLogFileRead(uint32 log, uint32 seg, int emode, TimeLineID tli, --- 2624,2631 ---- xlogfname); set_ps_display(activitymsg, false); + readSource = source; + return fd; } if (errno != ENOENT || !notfoundOk) /* unexpected failure? */ *************** *** 2624,2630 **** XLogFileRead(uint32 log, uint32 seg, int emode, TimeLineID tli, * searched in pg_xlog if not found in archive. */ static int ! XLogFileReadAnyTLI(uint32 log, uint32 seg, int emode, bool fromArchive) { char path[MAXPGPATH]; ListCell *cell; --- 2644,2650 ---- * searched in pg_xlog if not found in archive. */ static int ! XLogFileReadAnyTLI(uint32 log, uint32 seg, int emode, int sources) { char path[MAXPGPATH]; ListCell *cell; *************** *** 2647,2666 **** XLogFileReadAnyTLI(uint32 log, uint32 seg, int emode, bool fromArchive) if (tli < curFileTLI) break; /* don't bother looking at too-old TLIs */ ! fd = XLogFileRead(log, seg, emode, tli, fromArchive, true); ! if (fd != -1) ! return fd; ! /* ! * If not in StandbyMode, fall back to searching pg_xlog. In ! * StandbyMode we're streaming segments from the primary to pg_xlog, ! * and we mustn't confuse the (possibly partial) segments in pg_xlog ! * with complete segments ready to be applied. We rather wait for the ! * records to arrive through streaming. ! */ ! if (!StandbyMode && fromArchive) { ! fd = XLogFileRead(log, seg, emode, tli, false, true); if (fd != -1) return fd; } --- 2667,2685 ---- if (tli < curFileTLI) break; /* don't bother looking at too-old TLIs */ ! if (sources & XLOG_FROM_ARCHIVE) ! { ! fd = XLogFileRead(log, seg, emode, tli, XLOG_FROM_ARCHIVE, true); ! if (fd != -1) ! { ! elog(DEBUG1, "got WAL segment from archive"); ! return fd; ! } ! } ! if (sources & XLOG_FROM_PG_XLOG) { ! fd = XLogFileRead(log, seg, emode, tli, XLOG_FROM_PG_XLOG, true); if (fd != -1) return fd; } *************** *** 3530,3545 **** ReadRecord(XLogRecPtr *RecPtr, int emode_arg, bool fetching_ckpt) uint32 pageHeaderSize; int emode; - /* - * We don't expect any invalid records during streaming recovery: we - * should never hit the end of WAL because we wait for it to be streamed. - * Therefore treat any broken WAL as PANIC, instead of failing over. - */ - if (StandbyMode) - emode = PANIC; - else - emode = emode_arg; - if (readBuf == NULL) { /* --- 3549,3554 ---- *************** *** 3591,3600 **** ReadRecord(XLogRecPtr *RecPtr, int emode_arg, bool fetching_ckpt) randAccess = true; /* allow curFileTLI to go backwards too */ } /* Read the page containing the record */ ! if (!XLogPageRead(RecPtr, emode, fetching_ckpt, randAccess)) return NULL; pageHeaderSize = XLogPageHeaderSize((XLogPageHeader) readBuf); targetRecOff = RecPtr->xrecoff % XLOG_BLCKSZ; if (targetRecOff == 0) --- 3600,3623 ---- randAccess = true; /* allow curFileTLI to go backwards too */ } + /* This is the first try read this page. */ + failedSources = 0; + retry: /* Read the page containing the record */ ! if (!XLogPageRead(RecPtr, emode_arg, fetching_ckpt, randAccess)) return NULL; + /* + * We don't expect any invalid records in archive or in records streamed + * from master: we should never hit the end of WAL because we wait for it + * to be streamed. Therefore treat any broken WAL as PANIC, instead of + * failing over. + */ + if (readSource == XLOG_FROM_STREAM || readSource == XLOG_FROM_ARCHIVE) + emode = PANIC; + else + emode = emode_arg; + pageHeaderSize = XLogPageHeaderSize((XLogPageHeader) readBuf); targetRecOff = RecPtr->xrecoff % XLOG_BLCKSZ; if (targetRecOff == 0) *************** *** 3828,3833 **** next_record_is_invalid:; --- 3851,3864 ---- close(readFile); readFile = -1; } + + /* In standby-mode, retry from another source */ + if (StandbyMode) + { + failedSources |= readSource; + goto retry; + } + return NULL; } *************** *** 8698,8704 **** StartupProcessMain(void) * as for waiting for the requested WAL record to arrive in standby mode. */ static bool ! XLogPageRead(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt, bool randAccess) { static XLogRecPtr receivedUpto = {0, 0}; --- 8729,8735 ---- * as for waiting for the requested WAL record to arrive in standby mode. */ static bool ! XLogPageRead(XLogRecPtr *RecPtr, int emode_arg, bool fetching_ckpt, bool randAccess) { static XLogRecPtr receivedUpto = {0, 0}; *************** *** 8707,8719 **** XLogPageRead(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt, uint32 targetRecOff; uint32 targetId; uint32 targetSeg; XLByteToSeg(*RecPtr, targetId, targetSeg); targetPageOff = ((RecPtr->xrecoff % XLogSegSize) / XLOG_BLCKSZ) * XLOG_BLCKSZ; targetRecOff = RecPtr->xrecoff % XLOG_BLCKSZ; /* Fast exit if we have read the record in the current buffer already */ ! if (targetId == readId && targetSeg == readSeg && targetPageOff == readOff && targetRecOff < readLen) return true; --- 8738,8752 ---- uint32 targetRecOff; uint32 targetId; uint32 targetSeg; + int emode; + static pg_time_t last_fail_time = 0; XLByteToSeg(*RecPtr, targetId, targetSeg); targetPageOff = ((RecPtr->xrecoff % XLogSegSize) / XLOG_BLCKSZ) * XLOG_BLCKSZ; targetRecOff = RecPtr->xrecoff % XLOG_BLCKSZ; /* Fast exit if we have read the record in the current buffer already */ ! if (failedSources == 0 && targetId == readId && targetSeg == readSeg && targetPageOff == readOff && targetRecOff < readLen) return true; *************** *** 8725,8742 **** XLogPageRead(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt, { close(readFile); readFile = -1; } XLByteToSeg(*RecPtr, readId, readSeg); /* See if we need to retrieve more data */ if (readFile < 0 || ! (readStreamed && !XLByteLT(*RecPtr, receivedUpto))) { if (StandbyMode) { - bool last_restore_failed = false; - /* * In standby mode, wait for the requested record to become * available, either via restore_command succeeding to restore the --- 8758,8775 ---- { close(readFile); readFile = -1; + readSource = 0; } XLByteToSeg(*RecPtr, readId, readSeg); + retry: /* See if we need to retrieve more data */ if (readFile < 0 || ! (readSource == XLOG_FROM_STREAM && !XLByteLT(*RecPtr, receivedUpto))) { if (StandbyMode) { /* * In standby mode, wait for the requested record to become * available, either via restore_command succeeding to restore the *************** *** 8746,8751 **** XLogPageRead(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt, --- 8779,8786 ---- { if (WalRcvInProgress()) { + failedSources = 0; + /* * While walreceiver is active, wait for new WAL to arrive * from primary. *************** *** 8761,8775 **** XLogPageRead(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt, { readFile = XLogFileRead(readId, readSeg, PANIC, ! recoveryTargetTLI, false, false); switched_segment = true; ! readStreamed = true; } break; } if (CheckForStandbyTrigger()) ! goto next_record_is_invalid; /* * When streaming is active, we want to react quickly when --- 8796,8811 ---- { readFile = XLogFileRead(readId, readSeg, PANIC, ! recoveryTargetTLI, ! XLOG_FROM_PG_XLOG, false); switched_segment = true; ! readSource = XLOG_FROM_STREAM; } break; } if (CheckForStandbyTrigger()) ! goto triggered; /* * When streaming is active, we want to react quickly when *************** *** 8779,8784 **** XLogPageRead(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt, --- 8815,8823 ---- } else { + int sources; + pg_time_t now; + /* * Until walreceiver manages to reconnect, poll the * archive. *************** *** 8791,8828 **** XLogPageRead(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt, /* Reset curFileTLI if random fetch. */ if (randAccess) curFileTLI = 0; ! readFile = XLogFileReadAnyTLI(readId, readSeg, DEBUG2, true); switched_segment = true; - readStreamed = false; if (readFile != -1) { - elog(DEBUG1, "got WAL segment from archive"); break; } /* ! * If we succeeded restoring some segments from archive ! * since the last connection attempt (or we haven't tried ! * streaming yet, retry immediately. But if we haven't, ! * assume the problem is persistent, so be less ! * aggressive. */ ! if (last_restore_failed) { ! /* ! * Check to see if the trigger file exists. Note that ! * we do this only after failure, so when you create ! * the trigger file, we still finish replaying as much ! * as we can before failover. ! */ ! if (CheckForStandbyTrigger()) ! goto next_record_is_invalid; ! pg_usleep(5000000L); /* 5 seconds */ } ! last_restore_failed = true; /* ! * Nope, not found in archive. Try to stream it. * * If fetching_ckpt is TRUE, RecPtr points to the initial * checkpoint location. In that case, we use RedoStartLSN --- 8830,8877 ---- /* Reset curFileTLI if random fetch. */ if (randAccess) curFileTLI = 0; ! ! /* ! * Try to restore the file from archive, or read an ! * existing file from pg_xlog. ! */ ! sources = XLOG_FROM_ARCHIVE | XLOG_FROM_PG_XLOG; ! sources &= ~failedSources; ! readFile = XLogFileReadAnyTLI(readId, readSeg, DEBUG2, ! sources); switched_segment = true; if (readFile != -1) { break; } /* ! * Nope, not found in archive. ! */ ! ! /* ! * Check to see if the trigger file exists. Note that ! * we do this only after failure, so when you create ! * the trigger file, we still finish replaying as much ! * as we can from archive and pg_xlog before failover. */ ! if (CheckForStandbyTrigger()) ! goto triggered; ! ! /* ! * Sleep if it hasn't been long since last attempt. ! */ ! now = (pg_time_t) time(NULL); ! if ((now - last_fail_time) < 5) { ! pg_usleep(1000000L * (5 - (now - last_fail_time))); ! now = (pg_time_t) time(NULL); } ! last_fail_time = now; /* ! * If primary_conninfo is set, launch walreceiver to ! * try to stream the missing WAL. * * If fetching_ckpt is TRUE, RecPtr points to the initial * checkpoint location. In that case, we use RedoStartLSN *************** *** 8847,8859 **** XLogPageRead(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt, /* In archive or crash recovery. */ if (readFile < 0) { /* Reset curFileTLI if random fetch. */ if (randAccess) curFileTLI = 0; ! readFile = XLogFileReadAnyTLI(readId, readSeg, emode, ! InArchiveRecovery); switched_segment = true; - readStreamed = false; if (readFile < 0) return false; } --- 8896,8914 ---- /* In archive or crash recovery. */ if (readFile < 0) { + int sources; /* Reset curFileTLI if random fetch. */ if (randAccess) curFileTLI = 0; ! ! sources = XLOG_FROM_PG_XLOG; ! if (InArchiveRecovery) ! sources |= XLOG_FROM_ARCHIVE; ! sources &= ~failedSources; ! ! readFile = XLogFileReadAnyTLI(readId, readSeg, emode_arg, ! sources); switched_segment = true; if (readFile < 0) return false; } *************** *** 8861,8878 **** XLogPageRead(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt, } /* ! * At this point, we have the right segment open and we know the requested ! * record is in it. */ Assert(readFile != -1); /* * If the current segment is being streamed from master, calculate how * much of the current page we have received already. We know the * requested record has been received, but this is for the benefit of * future calls, to allow quick exit at the top of this function. */ ! if (readStreamed) { if (RecPtr->xlogid != receivedUpto.xlogid || (RecPtr->xrecoff / XLOG_BLCKSZ) != (receivedUpto.xrecoff / XLOG_BLCKSZ)) --- 8916,8944 ---- } /* ! * At this point, we have the right segment open and if we're streaming ! * we know the requested record is in it. */ Assert(readFile != -1); /* + * We don't expect any invalid records in archive or in records streamed + * from master: we should never hit the end of WAL because we wait for it + * to be streamed. Therefore treat any broken WAL as PANIC, instead of + * failing over. + */ + if (readSource == XLOG_FROM_STREAM || readSource == XLOG_FROM_ARCHIVE) + emode = PANIC; + else + emode = emode_arg; + + /* * If the current segment is being streamed from master, calculate how * much of the current page we have received already. We know the * requested record has been received, but this is for the benefit of * future calls, to allow quick exit at the top of this function. */ ! if (readSource == XLOG_FROM_STREAM) { if (RecPtr->xlogid != receivedUpto.xlogid || (RecPtr->xrecoff / XLOG_BLCKSZ) != (receivedUpto.xrecoff / XLOG_BLCKSZ)) *************** *** 8936,8946 **** XLogPageRead(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt, return true; next_record_is_invalid: if (readFile >= 0) close(readFile); readFile = -1; - readStreamed = false; readLen = 0; return false; } --- 9002,9026 ---- return true; next_record_is_invalid: + failedSources |= readSource; + + if (readFile >= 0) + close(readFile); + readFile = -1; + readLen = 0; + readSource = 0; + + if (StandbyMode) + goto retry; + else + return false; + + triggered: if (readFile >= 0) close(readFile); readFile = -1; readLen = 0; + readSource = 0; return false; }
^ permalink raw reply [nested|flat] 32+ messages in thread

* Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL
@ 2010-03-19 12:43  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  2 siblings, 1 reply; 32+ messages in thread

From: Tom Lane @ 2010-03-19 12:43 UTC (permalink / raw)
  To: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; +Cc: Simon Riggs <simon@2ndQuadrant.com>; Fujii Masao <masao.fujii@gmail.com>; Aidan Van Dyk <aidan@highrise.ca>; PostgreSQL-development <pgsql-hackers@postgresql.org>

Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:
> Simon Riggs wrote:
>> We might also have written half a file many times. The files in pg_xlog
>> are suspect whereas the files in the archive are not. If we have both we
>> should prefer the archive.

> Yep.

Really?  That will result in a change in the longstanding behavior of
ordinary recovery.  I'm unconvinced that this is wise.

			regards, tom lane



^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL
@ 2010-03-19 12:50  Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 0 replies; 32+ messages in thread

From: Heikki Linnakangas @ 2010-03-19 12:50 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Simon Riggs <simon@2ndQuadrant.com>; Fujii Masao <masao.fujii@gmail.com>; Aidan Van Dyk <aidan@highrise.ca>; PostgreSQL-development <pgsql-hackers@postgresql.org>

Tom Lane wrote:
> Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> writes:
>> Simon Riggs wrote:
>>> We might also have written half a file many times. The files in pg_xlog
>>> are suspect whereas the files in the archive are not. If we have both we
>>> should prefer the archive.
> 
>> Yep.
> 
> Really?  That will result in a change in the longstanding behavior of
> ordinary recovery.

Really? Not as far as I can see. Recovery has always tried to restore
WAL files from archive first, falling back to the copy in pg_xlog only
if restore_command returns failure.

-- 
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com



^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL
@ 2010-03-19 13:28  Alvaro Herrera <alvherre@commandprompt.com>
  parent: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  2 siblings, 1 reply; 32+ messages in thread

From: Alvaro Herrera @ 2010-03-19 13:28 UTC (permalink / raw)
  To: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; +Cc: Simon Riggs <simon@2ndQuadrant.com>; Fujii Masao <masao.fujii@gmail.com>; Aidan Van Dyk <aidan@highrise.ca>; PostgreSQL-development <pgsql-hackers@postgresql.org>

Heikki Linnakangas escribió:

> When recovery reaches an invalid WAL record, typically caused by a
> half-written WAL file, it closes the file and moves to the next source.
> If an error is found in a file restored from archive or in a portion
> just streamed from master, however, a PANIC is thrown, because it's not
> expected to have errors in the archive or in the master.

Hmm, I think I've heard that tools like walmgr do incremental copies of
the current WAL segment to the archive.  Doesn't this change break that?
(Maybe I'm confused and it doesn't work that way)

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support



^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL
@ 2010-03-19 13:44  Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  parent: Alvaro Herrera <alvherre@commandprompt.com>
  0 siblings, 0 replies; 32+ messages in thread

From: Heikki Linnakangas @ 2010-03-19 13:44 UTC (permalink / raw)
  To: Alvaro Herrera <alvherre@commandprompt.com>; +Cc: Simon Riggs <simon@2ndQuadrant.com>; Fujii Masao <masao.fujii@gmail.com>; Aidan Van Dyk <aidan@highrise.ca>; PostgreSQL-development <pgsql-hackers@postgresql.org>

Alvaro Herrera wrote:
> Heikki Linnakangas escribió:
> 
>> When recovery reaches an invalid WAL record, typically caused by a
>> half-written WAL file, it closes the file and moves to the next source.
>> If an error is found in a file restored from archive or in a portion
>> just streamed from master, however, a PANIC is thrown, because it's not
>> expected to have errors in the archive or in the master.
> 
> Hmm, I think I've heard that tools like walmgr do incremental copies of
> the current WAL segment to the archive.  Doesn't this change break that?

Hmm, you could have a restore_command that checks the size before
restoring to make it still work. I note that pg_standby does that, but
of course you can't use pg_standby with the built-in standby mode. Or
maybe we should modify the built-in standby mode to handle partial files
coming from restore_command by not throwing an error but recovering to
the end of the partial file, and then retrying restore_command again
with the same filename until the whole file is recovered (or the missing
WAL is received through other means, ie. streaming replication).

-- 
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com



^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL
@ 2010-03-23 07:17  Fujii Masao <masao.fujii@gmail.com>
  parent: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  2 siblings, 1 reply; 32+ messages in thread

From: Fujii Masao @ 2010-03-23 07:17 UTC (permalink / raw)
  To: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; +Cc: Simon Riggs <simon@2ndquadrant.com>; Aidan Van Dyk <aidan@highrise.ca>; PostgreSQL-development <pgsql-hackers@postgresql.org>

Sorry for the delay.

On Fri, Mar 19, 2010 at 8:37 PM, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:
> Here's a patch I've been playing with.

Thanks! I'm reading the patch.

> The idea is that in standby mode,
> the server keeps trying to make progress in the recovery by:
>
> a) restoring files from archive
> b) replaying files from pg_xlog
> c) streaming from master
>
> When recovery reaches an invalid WAL record, typically caused by a
> half-written WAL file, it closes the file and moves to the next source.
> If an error is found in a file restored from archive or in a portion
> just streamed from master, however, a PANIC is thrown, because it's not
> expected to have errors in the archive or in the master.

But in the current (v8.4 or before) behavior, recovery ends normally
when an invalid record is found in an archived WAL file. Otherwise,
the server would never be able to start normal processing when there
is a corrupted archived file for some reasons. So, that invalid record
should not be treated as a PANIC if the server is not in standby mode
or the trigger file has been created. Thought?

When I tested the patch, the following PANIC error was thrown in the
normal archive recovery. This seems to derive from the above change.
The detail error sequence:
1. In ReadRecord(), emode was set to PANIC after 00000001000000000000000B
   was read.
2. 00000001000000000000000C including the contrecord tried to be read
   by using the emode (= PANIC). But since 00000001000000000000000C did
   not exist, PANIC error was thrown.

-----------------
LOG:  restored log file "00000001000000000000000B" from archive
cp: cannot stat `../data.arh/00000001000000000000000C': No such file
or directory
PANIC:  could not open file "pg_xlog/00000001000000000000000C" (log
file 0, segment 12): No such file or directory
LOG:  startup process (PID 17204) was terminated by signal 6: Aborted
LOG:  terminating any other active server processes
-----------------

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center



^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: Re: [COMMITTERS] pgsql: Make standby server 	continuously retry restoring the next WAL
@ 2010-03-24 12:31  Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  parent: Fujii Masao <masao.fujii@gmail.com>
  0 siblings, 2 replies; 32+ messages in thread

From: Heikki Linnakangas @ 2010-03-24 12:31 UTC (permalink / raw)
  To: Fujii Masao <masao.fujii@gmail.com>; +Cc: Simon Riggs <simon@2ndquadrant.com>; Aidan Van Dyk <aidan@highrise.ca>; PostgreSQL-development <pgsql-hackers@postgresql.org>

Fujii Masao wrote:
> But in the current (v8.4 or before) behavior, recovery ends normally
> when an invalid record is found in an archived WAL file. Otherwise,
> the server would never be able to start normal processing when there
> is a corrupted archived file for some reasons. So, that invalid record
> should not be treated as a PANIC if the server is not in standby mode
> or the trigger file has been created. Thought?

Hmm, true, this changes behavior over previous releases. I tend to think
that it's always an error if there's a corrupt file in the archive,
though, and PANIC is appropriate. If the administrator wants to start up
the database anyway, he can remove the corrupt file from the archive and
place it directly in pg_xlog instead.

> When I tested the patch, the following PANIC error was thrown in the
> normal archive recovery. This seems to derive from the above change.
> The detail error sequence:
> 1. In ReadRecord(), emode was set to PANIC after 00000001000000000000000B
>    was read.
> 2. 00000001000000000000000C including the contrecord tried to be read
>    by using the emode (= PANIC). But since 00000001000000000000000C did
>    not exist, PANIC error was thrown.
> 
> -----------------
> LOG:  restored log file "00000001000000000000000B" from archive
> cp: cannot stat `../data.arh/00000001000000000000000C': No such file
> or directory
> PANIC:  could not open file "pg_xlog/00000001000000000000000C" (log
> file 0, segment 12): No such file or directory
> LOG:  startup process (PID 17204) was terminated by signal 6: Aborted
> LOG:  terminating any other active server processes
> -----------------

Thanks. That's easily fixable (applies over the previous patch):

inline patch 1: src/backend/access/transam/xlog.c (+1/-1 lines) download inline-1.patch--- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -3773,7 +3773,7 @@ retry: pagelsn.xrecoff = 0; } /* Wait for the next page to become available */ - if (!XLogPageRead(&pagelsn, emode, false, false)) + if (!XLogPageRead(&pagelsn, emode_arg, false, false)) return NULL; /* Check that the continuation record looks valid */
Perhaps the emode/emode_arg convention is a bit hard to read. I'll go through the patch myself once more, and commit later today or tomorrow if now new issues crop up. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com ^ permalink raw reply [nested|flat] 32+ messages in thread

* Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL
@ 2010-03-24 13:20  Fujii Masao <masao.fujii@gmail.com>
  parent: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  1 sibling, 2 replies; 32+ messages in thread

From: Fujii Masao @ 2010-03-24 13:20 UTC (permalink / raw)
  To: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; +Cc: Simon Riggs <simon@2ndquadrant.com>; Aidan Van Dyk <aidan@highrise.ca>; PostgreSQL-development <pgsql-hackers@postgresql.org>

On Wed, Mar 24, 2010 at 9:31 PM, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:
> Hmm, true, this changes behavior over previous releases. I tend to think
> that it's always an error if there's a corrupt file in the archive,
> though, and PANIC is appropriate. If the administrator wants to start up
> the database anyway, he can remove the corrupt file from the archive and
> place it directly in pg_xlog instead.

Okay.

> Thanks. That's easily fixable (applies over the previous patch):
>
> --- a/src/backend/access/transam/xlog.c
> +++ b/src/backend/access/transam/xlog.c
> @@ -3773,7 +3773,7 @@ retry:
>                pagelsn.xrecoff = 0;
>            }
>            /* Wait for the next page to become available */
> -           if (!XLogPageRead(&pagelsn, emode, false, false))
> +           if (!XLogPageRead(&pagelsn, emode_arg, false, false))
>                return NULL;
>
>            /* Check that the continuation record looks valid */

Seems correct.

> sources &= ~failedSources;
> failedSources |= readSource;

The above lines in XLogPageRead() seem not to be required in normal
recovery case (i.e., standby_mode = off). So how about the attached
patch?

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

Attachments:

failedSource_v1.patch (application/octet-stream, 777B) download *** a/src/backend/access/transam/xlog.c --- b/src/backend/access/transam/xlog.c *************** *** 8943,8949 **** retry: sources = XLOG_FROM_PG_XLOG; if (InArchiveRecovery) sources |= XLOG_FROM_ARCHIVE; - sources &= ~failedSources; readFile = XLogFileReadAnyTLI(readId, readSeg, emode_arg, sources); --- 8943,8948 ---- *************** *** 9041,9048 **** retry: return true; next_record_is_invalid: - failedSources |= readSource; - if (readFile >= 0) close(readFile); readFile = -1; --- 9040,9045 ---- *************** *** 9050,9056 **** next_record_is_invalid: --- 9047,9056 ---- readSource = 0; if (StandbyMode) + { + failedSources |= readSource; goto retry; + } else return false;
^ permalink raw reply [nested|flat] 32+ messages in thread

* Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL
@ 2010-03-24 13:36  Fujii Masao <masao.fujii@gmail.com>
  parent: Fujii Masao <masao.fujii@gmail.com>
  1 sibling, 1 reply; 32+ messages in thread

From: Fujii Masao @ 2010-03-24 13:36 UTC (permalink / raw)
  To: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; +Cc: Simon Riggs <simon@2ndquadrant.com>; Aidan Van Dyk <aidan@highrise.ca>; PostgreSQL-development <pgsql-hackers@postgresql.org>

On Wed, Mar 24, 2010 at 10:20 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
>> Thanks. That's easily fixable (applies over the previous patch):
>>
>> --- a/src/backend/access/transam/xlog.c
>> +++ b/src/backend/access/transam/xlog.c
>> @@ -3773,7 +3773,7 @@ retry:
>>                pagelsn.xrecoff = 0;
>>            }
>>            /* Wait for the next page to become available */
>> -           if (!XLogPageRead(&pagelsn, emode, false, false))
>> +           if (!XLogPageRead(&pagelsn, emode_arg, false, false))
>>                return NULL;
>>
>>            /* Check that the continuation record looks valid */
>
> Seems correct.

On second thought, the following lines seem to be necessary just after
calling XLogPageRead() since it reads new WAL file from another source.

>	if (readSource == XLOG_FROM_STREAM || readSource == XLOG_FROM_ARCHIVE)
>		emode = PANIC;
>	else
>		emode = emode_arg;

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center



^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL
@ 2010-03-24 23:23  Simon Riggs <simon@2ndQuadrant.com>
  parent: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  1 sibling, 1 reply; 32+ messages in thread

From: Simon Riggs @ 2010-03-24 23:23 UTC (permalink / raw)
  To: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; +Cc: Fujii Masao <masao.fujii@gmail.com>; Aidan Van Dyk <aidan@highrise.ca>; PostgreSQL-development <pgsql-hackers@postgresql.org>

On Wed, 2010-03-24 at 14:31 +0200, Heikki Linnakangas wrote:
> Fujii Masao wrote:
> > But in the current (v8.4 or before) behavior, recovery ends normally
> > when an invalid record is found in an archived WAL file. Otherwise,
> > the server would never be able to start normal processing when there
> > is a corrupted archived file for some reasons. So, that invalid record
> > should not be treated as a PANIC if the server is not in standby mode
> > or the trigger file has been created. Thought?
> 
> Hmm, true, this changes behavior over previous releases. I tend to think
> that it's always an error if there's a corrupt file in the archive,
> though, and PANIC is appropriate. If the administrator wants to start up
> the database anyway, he can remove the corrupt file from the archive and
> place it directly in pg_xlog instead.

I don't agree with changing the behaviour from previous releases.

PANICing won't change the situation, so it just destroys server
availability. If we had 1 master and 42 slaves then this behaviour would
take down almost the whole server farm at once. Very uncool.

You might have reason to prevent the server starting up at that point,
when in standby mode, but that is not a reason to PANIC. We don't really
want all of the standbys thinking they can be the master all at once
either. Better to throw a serious ERROR and have the server still up and
available for reads.

-- 
 Simon Riggs           www.2ndQuadrant.com




^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL
@ 2010-03-25 02:08  Fujii Masao <masao.fujii@gmail.com>
  parent: Simon Riggs <simon@2ndQuadrant.com>
  0 siblings, 3 replies; 32+ messages in thread

From: Fujii Masao @ 2010-03-25 02:08 UTC (permalink / raw)
  To: Simon Riggs <simon@2ndquadrant.com>; +Cc: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; Aidan Van Dyk <aidan@highrise.ca>; PostgreSQL-development <pgsql-hackers@postgresql.org>

On Thu, Mar 25, 2010 at 8:23 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
> PANICing won't change the situation, so it just destroys server
> availability. If we had 1 master and 42 slaves then this behaviour would
> take down almost the whole server farm at once. Very uncool.
>
> You might have reason to prevent the server starting up at that point,
> when in standby mode, but that is not a reason to PANIC. We don't really
> want all of the standbys thinking they can be the master all at once
> either. Better to throw a serious ERROR and have the server still up and
> available for reads.

OK. How about making the startup process emit WARNING, stop WAL replay and
wait for the presence of trigger file, when an invalid record is found?
Which keeps the server up for readonly queries. And if the trigger file is
found, I think that the startup process should emit a FATAL, i.e., the
server should exit immediately, to prevent the server from becoming the
primary in a half-finished state. Also to allow such a halfway failover,
we should provide fast failover mode as pg_standby does?

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center



^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL
@ 2010-03-25 02:14  Tom Lane <tgl@sss.pgh.pa.us>
  parent: Fujii Masao <masao.fujii@gmail.com>
  2 siblings, 1 reply; 32+ messages in thread

From: Tom Lane @ 2010-03-25 02:14 UTC (permalink / raw)
  To: Fujii Masao <masao.fujii@gmail.com>; +Cc: Simon Riggs <simon@2ndquadrant.com>; Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; Aidan Van Dyk <aidan@highrise.ca>; PostgreSQL-development <pgsql-hackers@postgresql.org>

Fujii Masao <masao.fujii@gmail.com> writes:
> OK. How about making the startup process emit WARNING, stop WAL replay and
> wait for the presence of trigger file, when an invalid record is found?
> Which keeps the server up for readonly queries. And if the trigger file is
> found, I think that the startup process should emit a FATAL, i.e., the
> server should exit immediately, to prevent the server from becoming the
> primary in a half-finished state. Also to allow such a halfway failover,
> we should provide fast failover mode as pg_standby does?

I find it extremely scary to read this sort of blue-sky design
discussion going on now, two months after we were supposedly
feature-frozen for 9.0.  We need to be looking for the *rock bottom
minimum* amount of work to do to get 9.0 out the door in a usable
state; not what would be nice to have later on.

			regards, tom lane



^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL
@ 2010-03-25 08:08  Simon Riggs <simon@2ndQuadrant.com>
  parent: Fujii Masao <masao.fujii@gmail.com>
  2 siblings, 1 reply; 32+ messages in thread

From: Simon Riggs @ 2010-03-25 08:08 UTC (permalink / raw)
  To: Fujii Masao <masao.fujii@gmail.com>; +Cc: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; Aidan Van Dyk <aidan@highrise.ca>; PostgreSQL-development <pgsql-hackers@postgresql.org>

On Thu, 2010-03-25 at 11:08 +0900, Fujii Masao wrote:
> On Thu, Mar 25, 2010 at 8:23 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
> > PANICing won't change the situation, so it just destroys server
> > availability. If we had 1 master and 42 slaves then this behaviour would
> > take down almost the whole server farm at once. Very uncool.
> >
> > You might have reason to prevent the server starting up at that point,
> > when in standby mode, but that is not a reason to PANIC. We don't really
> > want all of the standbys thinking they can be the master all at once
> > either. Better to throw a serious ERROR and have the server still up and
> > available for reads.
> 
> OK. How about making the startup process emit WARNING, stop WAL replay and
> wait for the presence of trigger file, when an invalid record is found?
> Which keeps the server up for readonly queries. And if the trigger file is
> found, I think that the startup process should emit a FATAL, i.e., the
> server should exit immediately, to prevent the server from becoming the
> primary in a half-finished state. Also to allow such a halfway failover,
> we should provide fast failover mode as pg_standby does?

The lack of docs begins to show a lack of coherent high-level design
here. By now, I've forgotten what this thread was even about. The major
design decision in this that keeps showing up is "remove pg_standby, at
all costs" but no reason has ever been given for that. I do believe
there is a "better way", but we won't find it by trial and error, even
if we had time to do so.

Please work on some clear docs for the failure modes in this system.
That way we can all read them and understand them, or point out further
issues. Moving straight to code is not a solution to this, since what we
need now is to all agree on the way forwards. If we ignore this, then
there is considerable risk that streaming rep will have a fatal
operational flaw.

Please just document/diagram how it works now, highlighting the problems
that still remain to be solved. We're all behind you and I'm helping
wherever I can.

-- 
 Simon Riggs           www.2ndQuadrant.com




^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL
@ 2010-03-25 08:11  Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  parent: Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 32+ messages in thread

From: Heikki Linnakangas @ 2010-03-25 08:11 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Fujii Masao <masao.fujii@gmail.com>; Simon Riggs <simon@2ndquadrant.com>; Aidan Van Dyk <aidan@highrise.ca>; PostgreSQL-development <pgsql-hackers@postgresql.org>

Tom Lane wrote:
> Fujii Masao <masao.fujii@gmail.com> writes:
>> OK. How about making the startup process emit WARNING, stop WAL replay and
>> wait for the presence of trigger file, when an invalid record is found?
>> Which keeps the server up for readonly queries. And if the trigger file is
>> found, I think that the startup process should emit a FATAL, i.e., the
>> server should exit immediately, to prevent the server from becoming the
>> primary in a half-finished state. Also to allow such a halfway failover,
>> we should provide fast failover mode as pg_standby does?
> 
> I find it extremely scary to read this sort of blue-sky design
> discussion going on now, two months after we were supposedly
> feature-frozen for 9.0.  We need to be looking for the *rock bottom
> minimum* amount of work to do to get 9.0 out the door in a usable
> state; not what would be nice to have later on.

Agreed, this is getting complicated. I'm already worried about the
amount of changes needed to make it work, I don't want to add any new
modes. PANIC seems like the appropriate solution for now.

-- 
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com



^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL
@ 2010-03-25 08:22  Simon Riggs <simon@2ndQuadrant.com>
  parent: Fujii Masao <masao.fujii@gmail.com>
  2 siblings, 1 reply; 32+ messages in thread

From: Simon Riggs @ 2010-03-25 08:22 UTC (permalink / raw)
  To: Fujii Masao <masao.fujii@gmail.com>; +Cc: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; Aidan Van Dyk <aidan@highrise.ca>; PostgreSQL-development <pgsql-hackers@postgresql.org>

On Thu, 2010-03-25 at 11:08 +0900, Fujii Masao wrote:
> On Thu, Mar 25, 2010 at 8:23 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
> > PANICing won't change the situation, so it just destroys server
> > availability. If we had 1 master and 42 slaves then this behaviour would
> > take down almost the whole server farm at once. Very uncool.
> >
> > You might have reason to prevent the server starting up at that point,
> > when in standby mode, but that is not a reason to PANIC. We don't really
> > want all of the standbys thinking they can be the master all at once
> > either. Better to throw a serious ERROR and have the server still up and
> > available for reads.
> 
> OK. How about making the startup process emit WARNING, stop WAL replay and
> wait for the presence of trigger file, when an invalid record is found?
> Which keeps the server up for readonly queries. 

Yes. Receiving new WAL records is a completely separate activity from
running the rest of the server (in this release...).

> And if the trigger file is
> found, I think that the startup process should emit a FATAL, i.e., the
> server should exit immediately, to prevent the server from becoming the
> primary in a half-finished state. 

Please remember that "half-finished" is your judgment on what has
happened in the particular scenario you are considering. In many cases,
an invalid WAL record clearly and simply indicates the end of WAL and we
should start up normally.

"State" is a good word here. I'd like to see the server have a clear
state model with well documented transitions between them. The state
should also be externally queriable, so we can work out what its doing
and how long we can expect it to keep doing it for.

I don't want to be in a position where we are waiting for the server to
sort itself out from a complex set of retries. 

> Also to allow such a halfway failover,
> we should provide fast failover mode as pg_standby does?

Yes, we definitely need a JFDI solution for immediate failover.

-- 
 Simon Riggs           www.2ndQuadrant.com




^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL
@ 2010-03-25 08:23  Simon Riggs <simon@2ndQuadrant.com>
  parent: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  0 siblings, 1 reply; 32+ messages in thread

From: Simon Riggs @ 2010-03-25 08:23 UTC (permalink / raw)
  To: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Fujii Masao <masao.fujii@gmail.com>; Aidan Van Dyk <aidan@highrise.ca>; PostgreSQL-development <pgsql-hackers@postgresql.org>

On Thu, 2010-03-25 at 10:11 +0200, Heikki Linnakangas wrote:

> PANIC seems like the appropriate solution for now.

It definitely is not. Think some more.

-- 
 Simon Riggs           www.2ndQuadrant.com




^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: Re: [COMMITTERS] pgsql: Make standby server  continuously retry restoring the next WAL
@ 2010-03-25 10:05  Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  parent: Simon Riggs <simon@2ndQuadrant.com>
  0 siblings, 0 replies; 32+ messages in thread

From: Heikki Linnakangas @ 2010-03-25 10:05 UTC (permalink / raw)
  To: Simon Riggs <simon@2ndQuadrant.com>; +Cc: Fujii Masao <masao.fujii@gmail.com>; Aidan Van Dyk <aidan@highrise.ca>; PostgreSQL-development <pgsql-hackers@postgresql.org>

Simon Riggs wrote:
> On Thu, 2010-03-25 at 11:08 +0900, Fujii Masao wrote:
>> And if the trigger file is
>> found, I think that the startup process should emit a FATAL, i.e., the
>> server should exit immediately, to prevent the server from becoming the
>> primary in a half-finished state. 
> 
> Please remember that "half-finished" is your judgment on what has
> happened in the particular scenario you are considering. In many cases,
> an invalid WAL record clearly and simply indicates the end of WAL and we
> should start up normally.

Not in the archive. An invalid WAL record in a file in pg_xlog is
usually an indication of end-of-WAL, but there should be no invalid
records in the archived WAL files, or streamed from the master.

> "State" is a good word here. I'd like to see the server have a clear
> state model with well documented transitions between them. The state
> should also be externally queriable, so we can work out what its doing
> and how long we can expect it to keep doing it for.

Agreed.

-- 
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com



^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: [HACKERS] Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL
@ 2010-03-25 10:15  Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  parent: Simon Riggs <simon@2ndQuadrant.com>
  0 siblings, 1 reply; 32+ messages in thread

From: Heikki Linnakangas @ 2010-03-25 10:15 UTC (permalink / raw)
  To: Simon Riggs <simon@2ndQuadrant.com>; +Cc: Fujii Masao <masao.fujii@gmail.com>; Aidan Van Dyk <aidan@highrise.ca>; PostgreSQL-development <pgsql-hackers@postgresql.org>; pgsql-docs

(cc'ing docs list)

Simon Riggs wrote:
> The lack of docs begins to show a lack of coherent high-level design
> here.

Yeah, I think you're right. It's becoming hard to keep track of how it's
supposed to behave.

> By now, I've forgotten what this thread was even about. The major
> design decision in this that keeps showing up is "remove pg_standby, at
> all costs" but no reason has ever been given for that. I do believe
> there is a "better way", but we won't find it by trial and error, even
> if we had time to do so.

This has nothing to do with pg_standby.

> Please work on some clear docs for the failure modes in this system.
> That way we can all read them and understand them, or point out further
> issues. Moving straight to code is not a solution to this, since what we
> need now is to all agree on the way forwards. If we ignore this, then
> there is considerable risk that streaming rep will have a fatal
> operational flaw.
> 
> Please just document/diagram how it works now, highlighting the problems
> that still remain to be solved. We're all behind you and I'm helping
> wherever I can.

Ok, here's my attempt at the docs. Read it as a replacement for the
"High Availability, Load Balancing, and Replication" chapter, but of
course many of the sections will be unchanged, as indicated below.

-------------
Chapter 25. High Availability, Load Balancing, and Replication

25.1 Comparison of different solutions

<no changes>


25.2 Log-Shipping Standby servers

<overview from current "File-based Log Shipping" section. With small
changes so that it applies to the built-in standby mode as well as
pg_standby like solutions>

A standby server can also be used for read-only queries. This is called
Hot Standby mode, see chapter XXX

25.2.1 Planning

Set up two servers with identical hardware ...

<two first paragraphs of current File-based log-shipping / Planning section>

25.2.3 Standby mode

In standby mode, the server continously applies WAL received from the
master server. The standby server can receive WAL from a WAL archive
(see restore_command) or directly from the master over a TCP connection
(streaming replication). The standby server will also attempt to restore
any WAL found in the standby's pg_xlog. That typically happens after a
server restart, to replay again any WAL streamed from the master before
the restart, but you can also manually copy files to pg_xlog at any time
to have them replayed.

At startup, the standby begins by restoring all WAL available in the
archive location, calling restore_command. Once it reaches the end of
WAL available there and restore_command fails, it tries to restore any
WAL available in the pg_xlog directory (possibly stored there by
streaming replication before restart). If that fails, and streaming
replication has been configured, the standby tries to connect to the
master server and stream WAL from it. If that fails or streaming
replication is not configured, or if the connection is disconnected
later on, the standby goes back to step 1 and tries to restoring the
file from the archive again. This loop of retries from the archive,
pg_xlog, and via streaming replication goes on until the server is
stopped or failover is triggered by a trigger file.

A corrupt or half-finished WAL file in the archive, or streamed from the
master, causes a PANIC and immediate shutdown of the standby server. A
corrupt WAL file is always a serious event which requires administrator
action. If you want to recover a WAL file known to be corrupt as far as
it can be, you can copy the file manually into pg_xlog.

Standby mode is exited and the server switches to normal operation, when
a trigger file is found (trigger_file). Before failover, it will restore
any WAL available in the archive or in pg_xlog, but won't try to connect
to the master or wait for files to become available in the archive.


25.2.4 Preparing Master for Standby servers

Set up continous archiving to a WAL archive on the master, as described
in the chapter "Continous Archiving and Point-In-Time_recovery". The
archive location should be accessible from the standby even when the
master is down, ie. it should reside on the standby server itself or
another trusted server, not on the master server.

If you want to use streaming replication, set up authentication to allow
streaming replication connections. Set max_wal_senders.

Take a base backup as described in chapter Continous Archiving and
Point-In-Time_recovery / Making a Base Backup.

25.2.4.1 Authentication for streaming replication

Ensure that listen_addresses allows connections from the standby server.

<current Streaming Replication / Authentication section, describing
pg_hba.conf>


25.2.5 Setting up the standby server

1. Take a base backup, and copy it to the standby

2. Create a restore_command to restore files from the WAL archive.

3. Set standby_mode=on

4. If you want to use streaming replicaton, set primary_conninfo


You can use restartpoint_command to prune the archive of files no longer
needed by the standby.

You can have any number of standby servers as long as you set
max_wal_senders high enough in the master to allow them to be connected
simultaneously.

25.2.6 Archive recovery based log shipping

An alternative to the built-in standby mode desribed in the previous
sections is to use a restore_command that polls the archive location.
This was the only option available in versions 8.4 and below. In this
setup, set standby_mode=off, because you are implementing the polling
required for a standby server yourself. See contrib/pg_standby for a
reference implementation of this.

Note that the in this mode, the server will apply WAL one file at a
time, so if you use the standby server for queries (see Hot Standby),
there is a bigger delay between an action in the master and when the
action becomes visible in the standby, corresponding the time it takes
to fill up the WAL file. archive_timeout can be used to make that delay
shorter. Also note that you can't combine streaming replication with
this method.

<all the sections from current File-based log shipping that don't apply
to built-in standby mode go here>

25.3 Hot Standby

<no changes>

25.4 Incrementally Updated backups

<no changes>

-------------


-- 
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com



^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL
@ 2010-03-25 10:26  Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  parent: Simon Riggs <simon@2ndQuadrant.com>
  0 siblings, 2 replies; 32+ messages in thread

From: Heikki Linnakangas @ 2010-03-25 10:26 UTC (permalink / raw)
  To: Simon Riggs <simon@2ndQuadrant.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Fujii Masao <masao.fujii@gmail.com>; Aidan Van Dyk <aidan@highrise.ca>; PostgreSQL-development <pgsql-hackers@postgresql.org>

Simon Riggs wrote:
> On Thu, 2010-03-25 at 10:11 +0200, Heikki Linnakangas wrote:
> 
>> PANIC seems like the appropriate solution for now.
> 
> It definitely is not. Think some more.

Well, what happens now in previous versions with pg_standby et al is
that the standby starts up. That doesn't seem appropriate either.

Hmm, it would be trivial to just stay in the standby mode at a corrupt
file, continuously retrying to restore it and continue replay. If it's
genuinely corrupt, it will never succeed and the standby gets stuck at
that point. Maybe that's better; it's close to what Fujii suggested
except that you don't need a new mode for it.

I'm worried that the administrator won't notice the error promptly
because at a quick glance the server is up and running, while it's
actually stuck at the error and falling indefinitely behind the master.
Maybe if we make it a WARNING, that's enough to alleviate that. It's
true that if the standby is actively being used for read-only queries,
shutting it down to just get the administrators attention isn't good either.

-- 
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com



^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL
@ 2010-03-25 10:47  Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  parent: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  1 sibling, 0 replies; 32+ messages in thread

From: Heikki Linnakangas @ 2010-03-25 10:47 UTC (permalink / raw)
  To: Simon Riggs <simon@2ndQuadrant.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Fujii Masao <masao.fujii@gmail.com>; Aidan Van Dyk <aidan@highrise.ca>; PostgreSQL-development <pgsql-hackers@postgresql.org>

Heikki Linnakangas wrote:
> Simon Riggs wrote:
>> On Thu, 2010-03-25 at 10:11 +0200, Heikki Linnakangas wrote:
>>
>>> PANIC seems like the appropriate solution for now.
>> It definitely is not. Think some more.
> 
> Well, what happens now in previous versions with pg_standby et al is
> that the standby starts up. That doesn't seem appropriate either.
> 
> Hmm, it would be trivial to just stay in the standby mode at a corrupt
> file, continuously retrying to restore it and continue replay. If it's
> genuinely corrupt, it will never succeed and the standby gets stuck at
> that point. Maybe that's better; it's close to what Fujii suggested
> except that you don't need a new mode for it.

On second thought, that's easy for the built-in standby mode, but not
for archive recovery where we're not currently retrying anything. In
archive recovery, we could throw a WARNING and start up, which would be
like the current behavior in older versions except you get a WARNING
instead of LOG, or we could PANIC. I'm leaning towards PANIC, which
makes most sense for genuine point-in-time or archive recovery (ie. not
a standby server), but I can see the rationale for WARNING too, for
pg_standby and for the sake of preserving old behavior.

-- 
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com



^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: Re: [COMMITTERS] pgsql: Make standby server 	continuously retry restoring the next WAL
@ 2010-03-25 12:48  Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  parent: Fujii Masao <masao.fujii@gmail.com>
  1 sibling, 0 replies; 32+ messages in thread

From: Heikki Linnakangas @ 2010-03-25 12:48 UTC (permalink / raw)
  To: Fujii Masao <masao.fujii@gmail.com>; +Cc: Simon Riggs <simon@2ndquadrant.com>; Aidan Van Dyk <aidan@highrise.ca>; PostgreSQL-development <pgsql-hackers@postgresql.org>

Fujii Masao wrote:
>> sources &= ~failedSources;
>> failedSources |= readSource;
> 
> The above lines in XLogPageRead() seem not to be required in normal
> recovery case (i.e., standby_mode = off). So how about the attached
> patch?
>
> *** 9050,9056 **** next_record_is_invalid:
> --- 9047,9056 ----
>   	readSource = 0;
>   
>   	if (StandbyMode)
> + 	{
> + 		failedSources |= readSource;
>   		goto retry;
> + 	}
>   	else
>   		return false;

That doesn't work because readSource is cleared above. But yeah,
failedSources is not needed in archive recovery, so that line can be
removed.

-- 
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com



^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: Re: [COMMITTERS] pgsql: Make standby server 	continuously retry restoring the next WAL
@ 2010-03-25 12:55  Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  parent: Fujii Masao <masao.fujii@gmail.com>
  0 siblings, 2 replies; 32+ messages in thread

From: Heikki Linnakangas @ 2010-03-25 12:55 UTC (permalink / raw)
  To: Fujii Masao <masao.fujii@gmail.com>; +Cc: Simon Riggs <simon@2ndquadrant.com>; Aidan Van Dyk <aidan@highrise.ca>; PostgreSQL-development <pgsql-hackers@postgresql.org>

Fujii Masao wrote:
> On second thought, the following lines seem to be necessary just after
> calling XLogPageRead() since it reads new WAL file from another source.
> 
>> 	if (readSource == XLOG_FROM_STREAM || readSource == XLOG_FROM_ARCHIVE)
>> 		emode = PANIC;
>> 	else
>> 		emode = emode_arg;

Yep.

Here's an updated patch, with these changes since the last patch:

* Fix the bug of a spurious PANIC in archive recovery, if the WAL ends
in the middle of a WAL record that continues over a WAL segment boundary.

* If a corrupt WAL record is found in archive or streamed from master in
standby mode, throw WARNING instead of PANIC, and keep trying. In
archive recovery (ie. standby_mode=off) it's still a PANIC. We can make
it a WARNING too, which gives the pre-9.0 behavior of starting up the
server on corruption. I prefer PANIC but the discussion is still going on.

* Small code changes to handling of failedSources, inspired by your
comment. No change in functionality.

This is also available in my git repository at
git://git.postgresql.org/git/users/heikki/postgres.git, branch "xlogchanges"

-- 
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com

Attachments:

retry-wal-from-pg_xlog-2.patch (text/x-diff, 22.6K) download diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index e57f22e..4aa1870 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -450,20 +450,33 @@ static uint32 openLogSeg = 0; static uint32 openLogOff = 0; /* + * Codes indicating where we got a WAL file from during recovery, or where + * to attempt to get one. + */ +#define XLOG_FROM_ARCHIVE (1<<0) /* Restored using restore_command */ +#define XLOG_FROM_PG_XLOG (1<<1) /* Existing file in pg_xlog */ +#define XLOG_FROM_STREAM (1<<2) /* Streamed from master */ + +/* * These variables are used similarly to the ones above, but for reading * the XLOG. Note, however, that readOff generally represents the offset * of the page just read, not the seek position of the FD itself, which * will be just past that page. readLen indicates how much of the current - * page has been read into readBuf. + * page has been read into readBuf, and readSource indicates where we got + * the currently open file from. */ static int readFile = -1; static uint32 readId = 0; static uint32 readSeg = 0; static uint32 readOff = 0; static uint32 readLen = 0; +static int readSource = 0; /* XLOG_FROM_* code */ -/* Is the currently open segment being streamed from primary? */ -static bool readStreamed = false; +/* + * Keeps track of which sources we've tried to read the current WAL + * record from and failed. + */ +static int failedSources = 0; /* Buffer for currently read page (XLOG_BLCKSZ bytes) */ static char *readBuf = NULL; @@ -517,11 +530,12 @@ static bool InstallXLogFileSegment(uint32 *log, uint32 *seg, char *tmppath, bool find_free, int *max_advance, bool use_lock); static int XLogFileRead(uint32 log, uint32 seg, int emode, TimeLineID tli, - bool fromArchive, bool notexistOk); + int source, bool notexistOk); static int XLogFileReadAnyTLI(uint32 log, uint32 seg, int emode, - bool fromArchive); + int sources); static bool XLogPageRead(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt, bool randAccess); +static int emode_for_corrupt_record(int endofwalmode); static void XLogFileClose(void); static bool RestoreArchivedFile(char *path, const char *xlogfname, const char *recovername, off_t expectedSize); @@ -2573,7 +2587,7 @@ XLogFileOpen(uint32 log, uint32 seg) */ static int XLogFileRead(uint32 log, uint32 seg, int emode, TimeLineID tli, - bool fromArchive, bool notfoundOk) + int source, bool notfoundOk) { char xlogfname[MAXFNAMELEN]; char activitymsg[MAXFNAMELEN + 16]; @@ -2582,23 +2596,28 @@ XLogFileRead(uint32 log, uint32 seg, int emode, TimeLineID tli, XLogFileName(xlogfname, tli, log, seg); - if (fromArchive) + switch (source) { - /* Report recovery progress in PS display */ - snprintf(activitymsg, sizeof(activitymsg), "waiting for %s", - xlogfname); - set_ps_display(activitymsg, false); + case XLOG_FROM_ARCHIVE: + /* Report recovery progress in PS display */ + snprintf(activitymsg, sizeof(activitymsg), "waiting for %s", + xlogfname); + set_ps_display(activitymsg, false); - restoredFromArchive = RestoreArchivedFile(path, xlogfname, - "RECOVERYXLOG", - XLogSegSize); - if (!restoredFromArchive) - return -1; - } - else - { - XLogFilePath(path, tli, log, seg); - restoredFromArchive = false; + restoredFromArchive = RestoreArchivedFile(path, xlogfname, + "RECOVERYXLOG", + XLogSegSize); + if (!restoredFromArchive) + return -1; + break; + + case XLOG_FROM_PG_XLOG: + XLogFilePath(path, tli, log, seg); + restoredFromArchive = false; + break; + + default: + elog(ERROR, "invalid XLogFileRead source %d", source); } fd = BasicOpenFile(path, O_RDONLY | PG_BINARY, 0); @@ -2612,6 +2631,8 @@ XLogFileRead(uint32 log, uint32 seg, int emode, TimeLineID tli, xlogfname); set_ps_display(activitymsg, false); + readSource = source; + return fd; } if (errno != ENOENT || !notfoundOk) /* unexpected failure? */ @@ -2630,7 +2651,7 @@ XLogFileRead(uint32 log, uint32 seg, int emode, TimeLineID tli, * searched in pg_xlog if not found in archive. */ static int -XLogFileReadAnyTLI(uint32 log, uint32 seg, int emode, bool fromArchive) +XLogFileReadAnyTLI(uint32 log, uint32 seg, int emode, int sources) { char path[MAXPGPATH]; ListCell *cell; @@ -2653,20 +2674,19 @@ XLogFileReadAnyTLI(uint32 log, uint32 seg, int emode, bool fromArchive) if (tli < curFileTLI) break; /* don't bother looking at too-old TLIs */ - fd = XLogFileRead(log, seg, emode, tli, fromArchive, true); - if (fd != -1) - return fd; + if (sources & XLOG_FROM_ARCHIVE) + { + fd = XLogFileRead(log, seg, emode, tli, XLOG_FROM_ARCHIVE, true); + if (fd != -1) + { + elog(DEBUG1, "got WAL segment from archive"); + return fd; + } + } - /* - * If not in StandbyMode, fall back to searching pg_xlog. In - * StandbyMode we're streaming segments from the primary to pg_xlog, - * and we mustn't confuse the (possibly partial) segments in pg_xlog - * with complete segments ready to be applied. We rather wait for the - * records to arrive through streaming. - */ - if (!StandbyMode && fromArchive) + if (sources & XLOG_FROM_PG_XLOG) { - fd = XLogFileRead(log, seg, emode, tli, false, true); + fd = XLogFileRead(log, seg, emode, tli, XLOG_FROM_PG_XLOG, true); if (fd != -1) return fd; } @@ -3520,7 +3540,7 @@ RecordIsValid(XLogRecord *record, XLogRecPtr recptr, int emode) * the returned record pointer always points there. */ static XLogRecord * -ReadRecord(XLogRecPtr *RecPtr, int emode_arg, bool fetching_ckpt) +ReadRecord(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt) { XLogRecord *record; char *buffer; @@ -3530,17 +3550,6 @@ ReadRecord(XLogRecPtr *RecPtr, int emode_arg, bool fetching_ckpt) total_len; uint32 targetRecOff; uint32 pageHeaderSize; - int emode; - - /* - * We don't expect any invalid records during streaming recovery: we - * should never hit the end of WAL because we wait for it to be streamed. - * Therefore treat any broken WAL as PANIC, instead of failing over. - */ - if (StandbyMode) - emode = PANIC; - else - emode = emode_arg; if (readBuf == NULL) { @@ -3593,6 +3602,9 @@ ReadRecord(XLogRecPtr *RecPtr, int emode_arg, bool fetching_ckpt) randAccess = true; /* allow curFileTLI to go backwards too */ } + /* This is the first try to read this page. */ + failedSources = 0; +retry: /* Read the page containing the record */ if (!XLogPageRead(RecPtr, emode, fetching_ckpt, randAccess)) return NULL; @@ -3611,7 +3623,7 @@ ReadRecord(XLogRecPtr *RecPtr, int emode_arg, bool fetching_ckpt) } else if (targetRecOff < pageHeaderSize) { - ereport(emode, + ereport(emode_for_corrupt_record(emode), (errmsg("invalid record offset at %X/%X", RecPtr->xlogid, RecPtr->xrecoff))); goto next_record_is_invalid; @@ -3619,7 +3631,7 @@ ReadRecord(XLogRecPtr *RecPtr, int emode_arg, bool fetching_ckpt) if ((((XLogPageHeader) readBuf)->xlp_info & XLP_FIRST_IS_CONTRECORD) && targetRecOff == pageHeaderSize) { - ereport(emode, + ereport(emode_for_corrupt_record(emode), (errmsg("contrecord is requested by %X/%X", RecPtr->xlogid, RecPtr->xrecoff))); goto next_record_is_invalid; @@ -3634,7 +3646,7 @@ ReadRecord(XLogRecPtr *RecPtr, int emode_arg, bool fetching_ckpt) { if (record->xl_len != 0) { - ereport(emode, + ereport(emode_for_corrupt_record(emode), (errmsg("invalid xlog switch record at %X/%X", RecPtr->xlogid, RecPtr->xrecoff))); goto next_record_is_invalid; @@ -3642,7 +3654,7 @@ ReadRecord(XLogRecPtr *RecPtr, int emode_arg, bool fetching_ckpt) } else if (record->xl_len == 0) { - ereport(emode, + ereport(emode_for_corrupt_record(emode), (errmsg("record with zero length at %X/%X", RecPtr->xlogid, RecPtr->xrecoff))); goto next_record_is_invalid; @@ -3651,14 +3663,14 @@ ReadRecord(XLogRecPtr *RecPtr, int emode_arg, bool fetching_ckpt) record->xl_tot_len > SizeOfXLogRecord + record->xl_len + XLR_MAX_BKP_BLOCKS * (sizeof(BkpBlock) + BLCKSZ)) { - ereport(emode, + ereport(emode_for_corrupt_record(emode), (errmsg("invalid record length at %X/%X", RecPtr->xlogid, RecPtr->xrecoff))); goto next_record_is_invalid; } if (record->xl_rmid > RM_MAX_ID) { - ereport(emode, + ereport(emode_for_corrupt_record(emode), (errmsg("invalid resource manager ID %u at %X/%X", record->xl_rmid, RecPtr->xlogid, RecPtr->xrecoff))); goto next_record_is_invalid; @@ -3671,7 +3683,7 @@ ReadRecord(XLogRecPtr *RecPtr, int emode_arg, bool fetching_ckpt) */ if (!XLByteLT(record->xl_prev, *RecPtr)) { - ereport(emode, + ereport(emode_for_corrupt_record(emode), (errmsg("record with incorrect prev-link %X/%X at %X/%X", record->xl_prev.xlogid, record->xl_prev.xrecoff, RecPtr->xlogid, RecPtr->xrecoff))); @@ -3687,7 +3699,7 @@ ReadRecord(XLogRecPtr *RecPtr, int emode_arg, bool fetching_ckpt) */ if (!XLByteEQ(record->xl_prev, ReadRecPtr)) { - ereport(emode, + ereport(emode_for_corrupt_record(emode), (errmsg("record with incorrect prev-link %X/%X at %X/%X", record->xl_prev.xlogid, record->xl_prev.xrecoff, RecPtr->xlogid, RecPtr->xrecoff))); @@ -3716,7 +3728,7 @@ ReadRecord(XLogRecPtr *RecPtr, int emode_arg, bool fetching_ckpt) { readRecordBufSize = 0; /* We treat this as a "bogus data" condition */ - ereport(emode, + ereport(emode_for_corrupt_record(emode), (errmsg("record length %u at %X/%X too long", total_len, RecPtr->xlogid, RecPtr->xrecoff))); goto next_record_is_invalid; @@ -3756,7 +3768,7 @@ ReadRecord(XLogRecPtr *RecPtr, int emode_arg, bool fetching_ckpt) /* Check that the continuation record looks valid */ if (!(((XLogPageHeader) readBuf)->xlp_info & XLP_FIRST_IS_CONTRECORD)) { - ereport(emode, + ereport(emode_for_corrupt_record(emode), (errmsg("there is no contrecord flag in log file %u, segment %u, offset %u", readId, readSeg, readOff))); goto next_record_is_invalid; @@ -3766,7 +3778,7 @@ ReadRecord(XLogRecPtr *RecPtr, int emode_arg, bool fetching_ckpt) if (contrecord->xl_rem_len == 0 || total_len != (contrecord->xl_rem_len + gotlen)) { - ereport(emode, + ereport(emode_for_corrupt_record(emode), (errmsg("invalid contrecord length %u in log file %u, segment %u, offset %u", contrecord->xl_rem_len, readId, readSeg, readOff))); @@ -3784,7 +3796,7 @@ ReadRecord(XLogRecPtr *RecPtr, int emode_arg, bool fetching_ckpt) contrecord->xl_rem_len); break; } - if (!RecordIsValid(record, *RecPtr, emode)) + if (!RecordIsValid(record, *RecPtr, emode_for_corrupt_record(emode))) goto next_record_is_invalid; pageHeaderSize = XLogPageHeaderSize((XLogPageHeader) readBuf); EndRecPtr.xlogid = readId; @@ -3798,7 +3810,7 @@ ReadRecord(XLogRecPtr *RecPtr, int emode_arg, bool fetching_ckpt) } /* Record does not cross a page boundary */ - if (!RecordIsValid(record, *RecPtr, emode)) + if (!RecordIsValid(record, *RecPtr, emode_for_corrupt_record(emode))) goto next_record_is_invalid; EndRecPtr.xlogid = RecPtr->xlogid; EndRecPtr.xrecoff = RecPtr->xrecoff + MAXALIGN(total_len); @@ -3824,13 +3836,20 @@ ReadRecord(XLogRecPtr *RecPtr, int emode_arg, bool fetching_ckpt) } return (XLogRecord *) buffer; -next_record_is_invalid:; +next_record_is_invalid: + failedSources |= readSource; + if (readFile >= 0) { close(readFile); readFile = -1; } - return NULL; + + /* In standby-mode, keep trying */ + if (StandbyMode) + goto retry; + else + return NULL; } /* @@ -8731,10 +8750,15 @@ StartupProcessMain(void) /* * Read the XLOG page containing RecPtr into readBuf (if not read already). - * Returns true if successful, false otherwise or fails if emode is PANIC. + * Returns true if the page is read successfully. * * This is responsible for restoring files from archive as needed, as well * as for waiting for the requested WAL record to arrive in standby mode. + * + * 'emode' specifies the log level used for reporting "file not found" or + * "end of WAL" situations in archive recovery, or in standby mode if a trigger + * file is found. If set to WARNING or below, XLogPageRead() returns false + * in those situations, otherwise the ereport() will cause an error exit. */ static bool XLogPageRead(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt, @@ -8746,13 +8770,14 @@ XLogPageRead(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt, uint32 targetRecOff; uint32 targetId; uint32 targetSeg; + static pg_time_t last_fail_time = 0; XLByteToSeg(*RecPtr, targetId, targetSeg); targetPageOff = ((RecPtr->xrecoff % XLogSegSize) / XLOG_BLCKSZ) * XLOG_BLCKSZ; targetRecOff = RecPtr->xrecoff % XLOG_BLCKSZ; /* Fast exit if we have read the record in the current buffer already */ - if (targetId == readId && targetSeg == readSeg && + if (failedSources == 0 && targetId == readId && targetSeg == readSeg && targetPageOff == readOff && targetRecOff < readLen) return true; @@ -8764,18 +8789,18 @@ XLogPageRead(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt, { close(readFile); readFile = -1; + readSource = 0; } XLByteToSeg(*RecPtr, readId, readSeg); +retry: /* See if we need to retrieve more data */ if (readFile < 0 || - (readStreamed && !XLByteLT(*RecPtr, receivedUpto))) + (readSource == XLOG_FROM_STREAM && !XLByteLT(*RecPtr, receivedUpto))) { if (StandbyMode) { - bool last_restore_failed = false; - /* * In standby mode, wait for the requested record to become * available, either via restore_command succeeding to restore the @@ -8800,15 +8825,16 @@ XLogPageRead(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt, { readFile = XLogFileRead(readId, readSeg, PANIC, - recoveryTargetTLI, false, false); + recoveryTargetTLI, + XLOG_FROM_PG_XLOG, false); switched_segment = true; - readStreamed = true; + readSource = XLOG_FROM_STREAM; } break; } if (CheckForStandbyTrigger()) - goto next_record_is_invalid; + goto triggered; /* * When streaming is active, we want to react quickly when @@ -8818,6 +8844,9 @@ XLogPageRead(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt, } else { + int sources; + pg_time_t now; + /* * Until walreceiver manages to reconnect, poll the * archive. @@ -8830,48 +8859,73 @@ XLogPageRead(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt, /* Reset curFileTLI if random fetch. */ if (randAccess) curFileTLI = 0; - readFile = XLogFileReadAnyTLI(readId, readSeg, DEBUG2, true); - switched_segment = true; - readStreamed = false; - if (readFile != -1) - { - elog(DEBUG1, "got WAL segment from archive"); - break; - } /* - * If we succeeded restoring some segments from archive - * since the last connection attempt (or we haven't tried - * streaming yet, retry immediately. But if we haven't, - * assume the problem is persistent, so be less - * aggressive. + * Try to restore the file from archive, or read an + * existing file from pg_xlog. */ - if (last_restore_failed) + sources = XLOG_FROM_ARCHIVE | XLOG_FROM_PG_XLOG; + if (!(sources & ~failedSources)) { /* - * Check to see if the trigger file exists. Note that - * we do this only after failure, so when you create - * the trigger file, we still finish replaying as much - * as we can before failover. + * We've exhausted all options for retrieving the + * file. Retry ... */ - if (CheckForStandbyTrigger()) - goto next_record_is_invalid; - pg_usleep(5000000L); /* 5 seconds */ + failedSources = 0; + + /* + * ... but sleep first if it hasn't been long since + * last attempt. + */ + now = (pg_time_t) time(NULL); + if ((now - last_fail_time) < 5) + { + pg_usleep(1000000L * (5 - (now - last_fail_time))); + now = (pg_time_t) time(NULL); + } + last_fail_time = now; + + /* + * If primary_conninfo is set, launch walreceiver to + * try to stream the missing WAL, before retrying + * to restore from archive/pg_xlog. + * + * If fetching_ckpt is TRUE, RecPtr points to the + * initial checkpoint location. In that case, we use + * RedoStartLSN as the streaming start position instead + * of RecPtr, so that when we later jump backwards to + * start redo at RedoStartLSN, we will have the logs + * streamed already. + */ + if (PrimaryConnInfo) + { + RequestXLogStreaming( + fetching_ckpt ? RedoStartLSN : *RecPtr, + PrimaryConnInfo); + continue; + } } - last_restore_failed = true; + /* Don't try to read from a source that just failed */ + sources &= ~failedSources; + readFile = XLogFileReadAnyTLI(readId, readSeg, DEBUG2, + sources); + switched_segment = true; + if (readFile != -1) + break; /* - * Nope, not found in archive. Try to stream it. - * - * If fetching_ckpt is TRUE, RecPtr points to the initial - * checkpoint location. In that case, we use RedoStartLSN - * as the streaming start position instead of RecPtr, so - * that when we later jump backwards to start redo at - * RedoStartLSN, we will have the logs streamed already. + * Nope, not found in archive and/or pg_xlog. */ - if (PrimaryConnInfo) - RequestXLogStreaming(fetching_ckpt ? RedoStartLSN : *RecPtr, - PrimaryConnInfo); + failedSources |= sources; + + /* + * Check to see if the trigger file exists. Note that + * we do this only after failure, so when you create + * the trigger file, we still finish replaying as much + * as we can from archive and pg_xlog before failover. + */ + if (CheckForStandbyTrigger()) + goto triggered; } /* @@ -8886,13 +8940,18 @@ XLogPageRead(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt, /* In archive or crash recovery. */ if (readFile < 0) { + int sources; /* Reset curFileTLI if random fetch. */ if (randAccess) curFileTLI = 0; + + sources = XLOG_FROM_PG_XLOG; + if (InArchiveRecovery) + sources |= XLOG_FROM_ARCHIVE; + readFile = XLogFileReadAnyTLI(readId, readSeg, emode, - InArchiveRecovery); + sources); switched_segment = true; - readStreamed = false; if (readFile < 0) return false; } @@ -8900,8 +8959,8 @@ XLogPageRead(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt, } /* - * At this point, we have the right segment open and we know the requested - * record is in it. + * At this point, we have the right segment open and if we're streaming + * we know the requested record is in it. */ Assert(readFile != -1); @@ -8911,7 +8970,7 @@ XLogPageRead(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt, * requested record has been received, but this is for the benefit of * future calls, to allow quick exit at the top of this function. */ - if (readStreamed) + if (readSource == XLOG_FROM_STREAM) { if (RecPtr->xlogid != receivedUpto.xlogid || (RecPtr->xrecoff / XLOG_BLCKSZ) != (receivedUpto.xrecoff / XLOG_BLCKSZ)) @@ -8936,13 +8995,14 @@ XLogPageRead(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt, readOff = 0; if (read(readFile, readBuf, XLOG_BLCKSZ) != XLOG_BLCKSZ) { - ereport(emode, + ereport(emode_for_corrupt_record(emode), (errcode_for_file_access(), errmsg("could not read from log file %u, segment %u, offset %u: %m", readId, readSeg, readOff))); goto next_record_is_invalid; } - if (!ValidXLOGHeader((XLogPageHeader) readBuf, emode)) + if (!ValidXLOGHeader((XLogPageHeader) readBuf, + emode_for_corrupt_record(emode))) goto next_record_is_invalid; } @@ -8950,7 +9010,7 @@ XLogPageRead(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt, readOff = targetPageOff; if (lseek(readFile, (off_t) readOff, SEEK_SET) < 0) { - ereport(emode, + ereport(emode_for_corrupt_record(emode), (errcode_for_file_access(), errmsg("could not seek in log file %u, segment %u to offset %u: %m", readId, readSeg, readOff))); @@ -8958,13 +9018,13 @@ XLogPageRead(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt, } if (read(readFile, readBuf, XLOG_BLCKSZ) != XLOG_BLCKSZ) { - ereport(emode, + ereport(emode_for_corrupt_record(emode), (errcode_for_file_access(), errmsg("could not read from log file %u, segment %u, offset %u: %m", readId, readSeg, readOff))); goto next_record_is_invalid; } - if (!ValidXLOGHeader((XLogPageHeader) readBuf, emode)) + if (!ValidXLOGHeader((XLogPageHeader) readBuf, emode_for_corrupt_record(emode))) goto next_record_is_invalid; Assert(targetId == readId); @@ -8975,16 +9035,67 @@ XLogPageRead(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt, return true; next_record_is_invalid: + failedSources |= readSource; + if (readFile >= 0) close(readFile); readFile = -1; - readStreamed = false; readLen = 0; + readSource = 0; + + /* In standby-mode, keep trying */ + if (StandbyMode) + goto retry; + else + return false; + +triggered: + if (readFile >= 0) + close(readFile); + readFile = -1; + readLen = 0; + readSource = 0; return false; } /* + * Determine what log level should be used to report a corrupt WAL record + * in the current WAL page, previously read by XLogPageRead(). + * + * 'emode' is the error mode that would be used to report a file-not-found + * or legitimate end-of-WAL situation. It is upgraded to WARNING or PANIC + * if the an corrupt record is not expected at this point. + */ +static int +emode_for_corrupt_record(int emode) +{ + /* + * We don't expect any invalid records in archive or in records streamed + * from master. Files in the archive should be complete, and we should + * never hit the end of WAL because we stop and wait for more WAL to + * arrive before replaying it. + * + * In standby mode, throw a WARNING and keep retrying. If we're lucky + * it's a transient error and will go away by itself, and in any case + * it's better to keep the standby open for any possible read-only + * queries. In PITR, however, stop recovery immediately, rather than + * fail over. + */ + if (readSource == XLOG_FROM_STREAM || readSource == XLOG_FROM_ARCHIVE) + { + if (StandbyMode) + { + if (emode < WARNING) + emode = WARNING; + } + else + emode = PANIC; + } + return emode; +} + +/* * Check to see if the trigger file exists. If it does, request postmaster * to shut down walreceiver, wait for it to exit, remove the trigger * file, and return true.
^ permalink raw reply [nested|flat] 32+ messages in thread

* Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL
@ 2010-03-25 13:43  Robert Haas <robertmhaas@gmail.com>
  parent: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  1 sibling, 0 replies; 32+ messages in thread

From: Robert Haas @ 2010-03-25 13:43 UTC (permalink / raw)
  To: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; +Cc: Fujii Masao <masao.fujii@gmail.com>; Simon Riggs <simon@2ndquadrant.com>; Aidan Van Dyk <aidan@highrise.ca>; PostgreSQL-development <pgsql-hackers@postgresql.org>

On Thu, Mar 25, 2010 at 8:55 AM, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:
> * If a corrupt WAL record is found in archive or streamed from master in
> standby mode, throw WARNING instead of PANIC, and keep trying. In
> archive recovery (ie. standby_mode=off) it's still a PANIC. We can make
> it a WARNING too, which gives the pre-9.0 behavior of starting up the
> server on corruption. I prefer PANIC but the discussion is still going on.

I don't think we should be changing pre-9.0 behavior more than necessary.

...Robert



^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: [HACKERS] Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL
@ 2010-03-25 19:45  Simon Riggs <simon@2ndQuadrant.com>
  parent: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  0 siblings, 0 replies; 32+ messages in thread

From: Simon Riggs @ 2010-03-25 19:45 UTC (permalink / raw)
  To: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; +Cc: Fujii Masao <masao.fujii@gmail.com>; Aidan Van Dyk <aidan@highrise.ca>; PostgreSQL-development <pgsql-hackers@postgresql.org>; pgsql-docs

On Thu, 2010-03-25 at 12:15 +0200, Heikki Linnakangas wrote:
> (cc'ing docs list)
> 
> Simon Riggs wrote:
> > The lack of docs begins to show a lack of coherent high-level design
> > here.
> 
> Yeah, I think you're right. It's becoming hard to keep track of how it's
> supposed to behave.

Thank you for responding to that comment positively, I am relieved.

-- 
 Simon Riggs           www.2ndQuadrant.com




^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL
@ 2010-03-25 19:48  Simon Riggs <simon@2ndQuadrant.com>
  parent: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  1 sibling, 0 replies; 32+ messages in thread

From: Simon Riggs @ 2010-03-25 19:48 UTC (permalink / raw)
  To: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; +Cc: Tom Lane <tgl@sss.pgh.pa.us>; Fujii Masao <masao.fujii@gmail.com>; Aidan Van Dyk <aidan@highrise.ca>; PostgreSQL-development <pgsql-hackers@postgresql.org>

On Thu, 2010-03-25 at 12:26 +0200, Heikki Linnakangas wrote:
> Simon Riggs wrote:
> > On Thu, 2010-03-25 at 10:11 +0200, Heikki Linnakangas wrote:
> > 
> >> PANIC seems like the appropriate solution for now.
> > 
> > It definitely is not. Think some more.
> 
> Well, what happens now in previous versions with pg_standby et al is
> that the standby starts up. That doesn't seem appropriate either.

Agreed. I said that also, immediately upthread.

Bottom line is I am against anyone being allowed to PANIC the server
just because their piece of it ain't working. The whole purpose of all
of this is High Availability and we don't get that if everybody keeps
stopping for a tea break every time things get tricky. Staying up when
problems occur is the only way to avoid a falling domino taking out the
whole farm.

> I'm worried that the administrator won't notice the error promptly
> because at a quick glance the server is up and running, while it's
> actually stuck at the error and falling indefinitely behind the master.
> Maybe if we make it a WARNING, that's enough to alleviate that. It's
> true that if the standby is actively being used for read-only queries,
> shutting it down to just get the administrators attention isn't good either.

That's what monitoring is for. Let's just make sure this state is
accessible, so people will notice.

-- 
 Simon Riggs           www.2ndQuadrant.com




^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL
@ 2010-03-26 05:24  Fujii Masao <masao.fujii@gmail.com>
  parent: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  1 sibling, 1 reply; 32+ messages in thread

From: Fujii Masao @ 2010-03-26 05:24 UTC (permalink / raw)
  To: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; +Cc: Simon Riggs <simon@2ndquadrant.com>; Aidan Van Dyk <aidan@highrise.ca>; PostgreSQL-development <pgsql-hackers@postgresql.org>

On Thu, Mar 25, 2010 at 9:55 PM, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:
> * Fix the bug of a spurious PANIC in archive recovery, if the WAL ends
> in the middle of a WAL record that continues over a WAL segment boundary.
>
> * If a corrupt WAL record is found in archive or streamed from master in
> standby mode, throw WARNING instead of PANIC, and keep trying. In
> archive recovery (ie. standby_mode=off) it's still a PANIC. We can make
> it a WARNING too, which gives the pre-9.0 behavior of starting up the
> server on corruption. I prefer PANIC but the discussion is still going on.

Seems reasonable for me.

> * Small code changes to handling of failedSources, inspired by your
> comment. No change in functionality.
>
> This is also available in my git repository at
> git://git.postgresql.org/git/users/heikki/postgres.git, branch "xlogchanges"

I looked the patch and was not able to find any big problems until now.
The attached small patch fixes the typo.

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center

Attachments:

typo.patch (application/octet-stream, 1.4K) download *** a/src/backend/access/transam/xlog.c --- b/src/backend/access/transam/xlog.c *************** *** 535,541 **** static int XLogFileReadAnyTLI(uint32 log, uint32 seg, int emode, int sources); static bool XLogPageRead(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt, bool randAccess); ! static int emode_for_corrupt_record(int endofwalmode); static void XLogFileClose(void); static bool RestoreArchivedFile(char *path, const char *xlogfname, const char *recovername, off_t expectedSize); --- 535,541 ---- int sources); static bool XLogPageRead(XLogRecPtr *RecPtr, int emode, bool fetching_ckpt, bool randAccess); ! static int emode_for_corrupt_record(int emode); static void XLogFileClose(void); static bool RestoreArchivedFile(char *path, const char *xlogfname, const char *recovername, off_t expectedSize); *************** *** 9065,9071 **** triggered: * * 'emode' is the error mode that would be used to report a file-not-found * or legitimate end-of-WAL situation. It is upgraded to WARNING or PANIC ! * if the an corrupt record is not expected at this point. */ static int emode_for_corrupt_record(int emode) --- 9065,9071 ---- * * 'emode' is the error mode that would be used to report a file-not-found * or legitimate end-of-WAL situation. It is upgraded to WARNING or PANIC ! * if a corrupt record is not expected at this point. */ static int emode_for_corrupt_record(int emode)
^ permalink raw reply [nested|flat] 32+ messages in thread

* Re: Re: [COMMITTERS] pgsql: Make standby server 	continuously retry restoring the next WAL
@ 2010-03-30 16:28  Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  parent: Fujii Masao <masao.fujii@gmail.com>
  0 siblings, 1 reply; 32+ messages in thread

From: Heikki Linnakangas @ 2010-03-30 16:28 UTC (permalink / raw)
  To: Fujii Masao <masao.fujii@gmail.com>; +Cc: Simon Riggs <simon@2ndquadrant.com>; Aidan Van Dyk <aidan@highrise.ca>; PostgreSQL-development <pgsql-hackers@postgresql.org>

Fujii Masao wrote:
>> * Small code changes to handling of failedSources, inspired by your
>> comment. No change in functionality.
>>
>> This is also available in my git repository at
>> git://git.postgresql.org/git/users/heikki/postgres.git, branch "xlogchanges"
> 
> I looked the patch and was not able to find any big problems until now.
> The attached small patch fixes the typo.

Thanks. Committed with that typo-fix, and I also added a comment
explaining how failedSources and retrying XLogPageRead() works.

I'm now happy with the standby mode logic. It was a bigger struggle than
I anticipated back in January/February, I hope others now find it
intuitive as well. I'm going to work on the documentation of this, along
the lines of the draft I posted last week.

-- 
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com



^ permalink  raw  reply  [nested|flat] 32+ messages in thread

* Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL
@ 2010-03-31 02:01  Fujii Masao <masao.fujii@gmail.com>
  parent: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
  0 siblings, 0 replies; 32+ messages in thread

From: Fujii Masao @ 2010-03-31 02:01 UTC (permalink / raw)
  To: Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>; +Cc: Simon Riggs <simon@2ndquadrant.com>; Aidan Van Dyk <aidan@highrise.ca>; PostgreSQL-development <pgsql-hackers@postgresql.org>

On Wed, Mar 31, 2010 at 1:28 AM, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:
> Fujii Masao wrote:
>>> * Small code changes to handling of failedSources, inspired by your
>>> comment. No change in functionality.
>>>
>>> This is also available in my git repository at
>>> git://git.postgresql.org/git/users/heikki/postgres.git, branch "xlogchanges"
>>
>> I looked the patch and was not able to find any big problems until now.
>> The attached small patch fixes the typo.
>
> Thanks. Committed with that typo-fix, and I also added a comment
> explaining how failedSources and retrying XLogPageRead() works.

Great! Thanks a lot!

This change affects various recovery patterns that we support now.
For example, normal crash recovery, archive recovery, recovery using
pg_standby, recovery in standby mode, and so on. So we would need to
test whether all of those patterns work fine with the change.
I'll do it as much as possible.

Regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center



^ permalink  raw  reply  [nested|flat] 32+ messages in thread


end of thread, other threads:[~2010-03-31 02:01 UTC | newest]

Thread overview: 32+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2010-03-17 10:35 Re: Re: [COMMITTERS] pgsql: Make standby server 	continuously retry restoring the next WAL Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
2010-03-17 10:49 ` Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL Simon Riggs <simon@2ndQuadrant.com>
2010-03-18 14:27 ` Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL Fujii Masao <masao.fujii@gmail.com>
2010-03-19 08:52   ` Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL Simon Riggs <simon@2ndQuadrant.com>
2010-03-19 11:37     ` Re: Re: [COMMITTERS] pgsql: Make standby server  continuously retry restoring the next WAL Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
2010-03-19 12:43       ` Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL Tom Lane <tgl@sss.pgh.pa.us>
2010-03-19 12:50         ` Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
2010-03-19 13:28       ` Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL Alvaro Herrera <alvherre@commandprompt.com>
2010-03-19 13:44         ` Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
2010-03-23 07:17       ` Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL Fujii Masao <masao.fujii@gmail.com>
2010-03-24 12:31         ` Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
2010-03-24 13:20           ` Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL Fujii Masao <masao.fujii@gmail.com>
2010-03-24 13:36             ` Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL Fujii Masao <masao.fujii@gmail.com>
2010-03-25 12:55               ` Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
2010-03-25 13:43                 ` Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL Robert Haas <robertmhaas@gmail.com>
2010-03-26 05:24                 ` Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL Fujii Masao <masao.fujii@gmail.com>
2010-03-30 16:28                   ` Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
2010-03-31 02:01                     ` Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL Fujii Masao <masao.fujii@gmail.com>
2010-03-25 12:48             ` Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
2010-03-24 23:23           ` Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL Simon Riggs <simon@2ndQuadrant.com>
2010-03-25 02:08             ` Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL Fujii Masao <masao.fujii@gmail.com>
2010-03-25 02:14               ` Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL Tom Lane <tgl@sss.pgh.pa.us>
2010-03-25 08:11                 ` Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
2010-03-25 08:23                   ` Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL Simon Riggs <simon@2ndQuadrant.com>
2010-03-25 10:26                     ` Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
2010-03-25 10:47                       ` Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
2010-03-25 19:48                       ` Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL Simon Riggs <simon@2ndQuadrant.com>
2010-03-25 08:08               ` Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL Simon Riggs <simon@2ndQuadrant.com>
2010-03-25 10:15                 ` Re: [HACKERS] Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>
2010-03-25 19:45                   ` Re: [HACKERS] Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL Simon Riggs <simon@2ndQuadrant.com>
2010-03-25 08:22               ` Re: Re: [COMMITTERS] pgsql: Make standby server continuously retry restoring the next WAL Simon Riggs <simon@2ndQuadrant.com>
2010-03-25 10:05                 ` Re: Re: [COMMITTERS] pgsql: Make standby server  continuously retry restoring the next WAL Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox