Re: Change pg_last_xlog_receive_location not to move backwards

Thanks for the review!

On Wed, Jan 26, 2011 at 3:40 AM, Jeff Janes <jeff(dot)janes(at)gmail(dot)com> wrote:
> I believe the new walrcv->receiveStart was introduced to divide up those
> behaviors that should go backwards from those that should not.

Yes.

> The non-retreating value is used in 3 places (via GetWalRcvWriteRecPtr)
> in xlog.c.  (And some other places I haven't examined yet.)

Yes.

> The third seems more problematic.  In the XLogPageRead,
> it checks to see if more records have been received beyond what
> has been applied.  By using the non-retreating value here, it seems
> like the xlog replay could start replaying records that the wal
> receiver is in the process of overwriting.  Now, I've argued to myself
> that this is not a problem, because the receiver is overwriting them
> with identical data to that which is already there.

Yes. I don't think that it's a problem, too.

> But by that logic, why does any part of it (walrcv->receiveStart in
> the patch, walrcv->receivedUpto unpatched) need to retreat?  From
> the previous discussion, I understand that the concern is that we don't
> want to retrieve and write out partial xlog files.  What I don't understand
> is how we could get our selves into the position in which we are doing
> that, other than by someone going in and doing impermissible things to
> the PGDATA directory behind our backs.

That logic exists because we'd like to check that newly-received WAL
data is consistent with previous one by validating the header of new
WAL file. So since we need the header of new WAL file, we retreat the
replication starting location to the beginning of the WAL file when
reconnecting to the primary.

The following code (in XLogPageRead) validates the header of new
WAL file.

----------------------
if (switched_segment && targetPageOff != 0)
{
/*
* Whenever switching to a new WAL segment, we read the first page of
* the file and validate its header, even if that's not where the
* target record is. This is so that we can check the additional
* identification info that is present in the first page's "long"
* header.
*/
readOff = 0;
if (read(readFile, readBuf, XLOG_BLCKSZ) != XLOG_BLCKSZ)
{
ereport(emode_for_corrupt_record(emode, *RecPtr),
(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))
goto next_record_is_invalid;
}
----------------------

Regards,

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