On 11/21/2014 05:58 AM, Amit Kapila wrote:
> On Thu, Nov 20, 2014 at 10:36 PM, Heikki Linnakangas <
> hlinnakangas@vmware.com> wrote:
>>
>> As you may have noticed, I committed this (after some more cleanup). Of
> course, feel free to still review it, and please point out any issues you
> may find.
>>
>
> Few minor observations:
> 1. Readme
>
> void XLogResetInsertion(void)
>
> Clear any currently registered data and buffers from the WAL record
> construction workspace. This is only needed if you have already
> called XLogBeginInsert(), but decide to not insert the record after all.
>
> I think above sentence could be slightly rephrased as the this function is
> also getting called at end of XLogInsert().
Well, yeah, but that's internal to the xloginsert machinery, not
relevant for someone learning about how to use it.
> 2.
> shiftList()
> {
> ..
> XLogEnsureRecordSpace(data.ndeleted + 1, 0);
> ..
> }
>
> Shouldn't above function call to XLogEnsureRecordSpace() be done
> under if (RelationNeedsWAL(rel))?
True, fixed. (It doesn't hurt to call it, but it's clearly unnecessary)
> 3.
> XLogInsert(RmgrId rmid, uint8 info)
> {
> XLogRecPtr EndPos;
>
> /* XLogBeginInsert() must have been called.
> */
> if (!begininsert_called)
> elog(ERROR, "XLogBeginInsert was not called");
>
> As we are in critical section at this moment, so is it okay to have
> elog(ERROR,). I think this can only happen due to some coding
> mistake, but still not sure if elog(ERROR) is okay.
In a critical section, the ERROR will be promoted automatically to a
PANIC. There isn't much else we can do if that happens; it is a coding
mistake as you say.
BTW, not all XLogInsert() calls are inside a critical section. All the
ones that modify data pages are, but there are others.
- Heikki
