Re: WAL dirty-buffer management bug

I wrote:
> I'm thinking we should change the code and the README to specify that
> you must mark the buffer dirty before you can END_CRIT_SECTION().

While looking at this I realized that in fact we need to, and do,
mark the buffer dirty even earlier than that: look at bufmgr.c
LockBuffer and SyncOneBuffer comments. LockBuffer is marking the
buffer dirty before we even start the critical section. Because of
that, there is no actual bug here at present. But what we've got is
confusing, klugy, inefficient code (it's inefficient because it
sometimes marks buffers dirty without changing them).

I think it's time to clean this up. The correct sequence of operations
is really

pin and lock buffer(s)

START_CRIT_SECTION()

apply change to buffer(s), and mark them dirty

emit XLOG record

END_CRIT_SECTION()

unlock and unpin buffer(s)

I think we ought to rename WriteNoReleaseBuffer to MarkBufferDirty
to convey its true function, and use it in the third step of this
sequence. We could get rid of the klugy force-dirty in LockBuffer,
and get rid of the poorly named WriteBuffer altogether --- the unpin
operation would now always just be ReleaseBuffer.

Any objections?

regards, tom lane