Thread: Re: Patch-2 (2-move-continuation-record-to-page-header.patch) WAL Format Changes
Re: Patch-2 (2-move-continuation-record-to-page-header.patch) WAL Format Changes
From
Heikki Linnakangas
Date:
On 28.06.2012 17:40, Amit Kapila wrote: > 1. > @@ -693,7 +693,6 @@ XLogInsert(RmgrId rmid, uint8 info, XLogRecData *rdata) > { > XLogCtlInsert *Insert =&XLogCtl->Insert; > XLogRecord *record; > - XLogContRecord *contrecord; > XLogRecPtr RecPtr; > XLogRecPtr WriteRqst; > uint32 freespace; > @@ -1082,9 +1081,7 @@ begin:; > curridx = Insert->curridx; > /* Insert cont-record header */ > Insert->currpage->xlp_info |= XLP_FIRST_IS_CONTRECORD; > - contrecord = (XLogContRecord *) Insert->currpos; > - contrecord->xl_rem_len = write_len; > - Insert->currpos += SizeOfXLogContRecord; > + Insert->currpage->xlp_rem_len = write_len; > > After above code changes the comment "/* Insert cont-record header */" > should be changed. Thanks, fixed. > 2. > Is XLP_FIRST_IS_CONTRECORD required after putting xl_rem_len in page header; > > Can't we do handling based on xl_rem_len? Hmm, yeah, it's redundant now, we could use xl_rem_len > 0 to indicate a continued record. I thought I'd keep the flag to avoid unnecessary changes, to make life a bit easier for 3rd party tools that read the WAL, but I don't know if it really makes any difference. There is no shortage of xlog page header flag bits, so there's no hurry to get rid of it. > Sorry for sending the observations in pieces rather than all-together, as I > am not sure how much I will be able to complete. > > So what ever I am able to read, I am sending you my doubts or observations. Thanks for the review, much appreciated! -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com