Re: testing cvs HEAD - HS/SR - missing file - Mailing list pgsql-hackers

From Heikki Linnakangas
Subject Re: testing cvs HEAD - HS/SR - missing file
Date
Msg-id 4B606BE8.2070901@enterprisedb.com
Whole thread Raw
In response to Re: testing cvs HEAD - HS/SR - missing file  (Fujii Masao <masao.fujii@gmail.com>)
Responses Re: testing cvs HEAD - HS/SR - missing file  ("Erik Rijkers" <er@xs4all.nl>)
List pgsql-hackers
Fujii Masao wrote:
> On Wed, Jan 27, 2010 at 8:37 PM, Heikki Linnakangas
> <heikki.linnakangas@enterprisedb.com> wrote:
>> But SendRqstPtr comes from LogwrtResult.Write, surely that's correct, no?
> 
> Right. But the point is that LogwrtResult.Write might indicate 0/FF000000
> because it's the last byte + 1 written out. XLogRead() treats this as the
> location where WAL begins to be read, so converts it to the WAL file name
> 0000000100000000000000FF by using XLByteToSeg.

Ah, I see it now, thanks. How confusing.

Your patch clearly works, but I wonder how we should logically treat
that boundary value. You're setting sentPtr to the beginning of the
logid, when LogwrtResult.Write is 0/FF000000. So you're logically
thinking that the non-existent FF log segment has been sent to the
standby as soon as the previous segment has been fully sent. Another
option would be to set SendRqstPtr to the beginning of next logid, when
LogwrtResult.Write is 0/FF000000.. Yet another option would be to set
startptr to the beginning of next logid, when sentPtr is 0/FF000000:

***************
*** 633,638 ****
--- 633,648 ----          * WAL record.          */         startptr = sentPtr;
+         if (startptr.xrecoff >= XLogFileSize)
+         {
+             /*
+              * crossing a logid boundary, skip the non-existent last log
+              * segment in previous logical log file.
+              */
+             startptr.xlogid += 1;
+             startptr.xrecoff = 0;
+         }
+         endptr = startptr;         XLByteAdvance(endptr, MAX_SEND_SIZE);         /* round down to page boundary. */

I think I prefer that, it feels logically more correct. Setting sentPtr
beyond LogwrtResult.Write feels wrong, even though 0/FF000000 and
1/00000000 are really the same physical location. I'll commit it that way.

I also note that I broke that same thing in ReadRecord() in the
retry-logic patch I just comitted. I'll fix that too.

Thanks for the testing, Erik!

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


pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Add on_perl_init and proper destruction to plperl [PATCH]
Next
From: Tom Lane
Date:
Subject: Re: Patch: psql \whoami option