Re: log_newpage header comment

On Fri, Jun 8, 2012 at 9:33 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Thu, Jun 7, 2012 at 8:06 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> AFAICS, not passing the buffer ID to XLogInsert is not an issue, since
>>> we are logging the whole page in any case.  However, failing to perform
>>> MarkBufferDirty within the critical section definitely is an issue.
>
>> However, I'm not failing to do that: there's an enclosing critical section.
>
> Mph.  But is it being done in the right order relative to the other XLOG
> related steps?  See the code sketch in transam/README.

It appears to me that it is being done in the right order.

>> So we have to write the page out immediately, then we have to XLOG it,
>> and then even though we've XLOG'd it, we still have to fsync it
>> immediately.  It might be better to go through shared_buffers, which
>> would allow the write and fsync to happen in the background.
>
> Well, that's a fair point, but on the other hand we've not had any
> complaints traceable to the cost of making init forks.
>
> On the whole, I don't care for the idea that the
> modify-and-xlog-a-buffer sequence is being split between log_newpage and
> its caller.  That sounds like bugs waiting to happen anytime somebody
> refactors XLOG operations.  It would probably be best to refactor this
> as you're suggesting, so that that becomes less nonstandard.

OK. So what I'm thinking is that we should add a new function that
takes a relfilenode and a buffer and steps 4-6 of what's described in
transam/README: mark the buffer dirty, xlog it, and set the LSN and
TLI. We might want to have this function assert that it is in a
critical section, for the avoidance of error. Then anyone who wants
to use it can do steps 1-3, call the function, and then finish up with
steps 6-7. I don't think we can cleanly encapsulate any more than
that.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company