Re: Should XLogInsert() be done only inside a critical section?

On 20/04/16 23:44, Tom Lane wrote:
> Over in <17456.1460832307@sss.pgh.pa.us> I speculated about whether
> we should be enforcing that WAL insertion happen only inside critical
> sections.  We don't currently, and a survey of the backend says that
> there are quite a few calls that aren't inside critical sections.
> But there are at least two good reasons why we should, IMO:
>
> 1. It's not very clear that XLogInsert will recover cleanly if it's
> invoked outside a critical section and hits a failure.  Certainly,
> if we allow such usage, then every potential error inside that code
> has to be analyzed under both critical-section and normal rules.

It was certainly designed to recover from errors gracefully. 
XLogInsertRecord(), which does the low-level work of inserting the 
record to the WAL buffer, has a critical section of its own inside it. 
The code in xloginsert.c, for constructing the record to insert, 
operates on backend-private buffers only.

> 2. With no such check, it's quite easy for calling code to forget to
> create a critical section around code stanzas where one is *necessary*
> (because you're changing shared-buffer contents).

Yeah, that is very true.

> Both of these points represent pretty clear hazards for introduction
> of future bugs, whether or not there are any such bugs today.
>
> As against this, it could be argued that adding critical sections where
> they're not absolutely necessary must make crashing failures more probable
> than they need to be.  But first you'd have to prove that they're not
> absolutely necessary, which I'm unsure about because of point #1.

One option would be to put the must-be-in-critical-section check in 
XLogRegisterBlock(). A WAL record that is associated with a shared 
buffer almost certainly needs a critical section, but many of the others 
are safe without it.

- Heikki