Re: WAL dirty-buffer management bug - Mailing list pgsql-hackers

From Tom Lane
Subject Re: WAL dirty-buffer management bug
Date
Msg-id 16245.1143827313@sss.pgh.pa.us
Whole thread Raw
In response to WAL dirty-buffer management bug  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
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


pgsql-hackers by date:

Previous
From: "Jonah H. Harris"
Date:
Subject: Re: pg_class catalog question...
Next
From: David Wheeler
Date:
Subject: Suggestion: Which Binary?