On 11/21/2014 05:58 AM, Amit Kapila wrote:
On Thu, Nov 20, 2014 at 10:36 PM, Heikki Linnakangas <
hlinnakan...@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



--
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