Re: testing cvs HEAD - HS/SR - missing file

From: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
To: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
Cc: Erik Rijkers <er(at)xs4all(dot)nl>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: testing cvs HEAD - HS/SR - missing file
Date: 2010-01-27 16:38:00
Message-ID: 4B606BE8.2070901@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Fujii Masao wrote:
> On Wed, Jan 27, 2010 at 8:37 PM, Heikki Linnakangas
> <heikki(dot)linnakangas(at)enterprisedb(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2010-01-27 16:39:39 Re: Patch: psql \whoami option
Previous Message Tom Lane 2010-01-27 16:28:02 Re: Add on_perl_init and proper destruction to plperl [PATCH]