Fujii Masao wrote:
> On Wed, Jan 27, 2010 at 8:37 PM, Heikki Linnakangas
> <heikki.linnakan...@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

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to