Re: buffer README is out of date

On 8/29/15 2:21 PM, Jeff Janes wrote:

    The buffer/README section on buffer clean up locks never got updated
    for the creation of Heap Only Tuples and their associated compaction
    logic.

    I've attached a patch to change the explanation.  I'm sure someone
    can word it better than I have.

! Obtaining the necessary lock is done by the bufmgr routines
! LockBufferForCleanup() or ConditionalLockBufferForCleanup().
! They first get an exclusive lock and then check to see if the shared pin
! count is currently 1.  If not, ConditionalLockBufferForCleanup() releases
! the exclusive lock and then returns false, while LockBufferForCleanup()
! releases the exclusive lock (but not the caller's pin) and waits until
! signaled by another backend, whereupon it tries again.  The signal will
! occur when UnpinBuffer decrements the shared pin count to 1.  As

I don't think that's true. If 2 other backends have a pin then AFAIK you'd wake up twice. There's also this comment in LockBufferForCleanup:

/*
 * Remove flag marking us as waiter. Normally this will not be set
 * anymore, but ProcWaitForSignal() can return for other signals as
 * well.  We take care to only reset the flag if we're the waiter, as
 * theoretically another backend could have started waiting. That's
 * impossible with the current usages due to table level locking, but
 * better be safe.
 */

! indicated above, this operation might have to wait a good while before
! it acquires lock, but that shouldn't matter much for concurrent VACUUM.
! The current implementation only supports a single waiter for pin-count-1
! on any particular shared buffer.  This is enough for VACUUM's use, since
! we don't allow multiple VACUUMs concurrently on a single relation anyway.
! Anyone wishing to obtain a clean up lock outside of a VACUUM must use
! the conditional variant of the function.

That last statement should possibly be worded more strongly or just removed. If someone thinks they want to use this mechanism for something other than VACUUM there's probably a lot of other things to consider beyond just buffer locking.