On 20/04/16 23:44, Tom Lane wrote:
Over in <17456.1460832...@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:Advertising
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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
