Re: Hot Standby tuning for btree_xlog_vacuum()

Just wanted to say thanks for the review, since I haven't yet managed to
fix and commit this. I expect to later this month.

On Mon, 2010-09-27 at 23:06 -0400, Robert Haas wrote:
> On Thu, Apr 29, 2010 at 4:12 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
> > Simple tuning of btree_xlog_vacuum() using an idea I had a while back,
> > just never implemented. XXX comments removed.
> >
> > Allows us to avoid reading in blocks during VACUUM replay that are only
> > required for correctness of index scans.
> 
> Review:
> 
> 1. The block comment in XLogConfirmBufferIsUnpinned appears to be
> copied from somewhere else, and doesn't really seem appropriate for a
> new function since it refers to "the original coding of this routine".
>  I think you could just delete the parenthesized portion of the
> comment.
> 
> 2. This bit from ConfirmBufferIsUnpinned looks odd to me.
> 
> +         /*
> +          * Found it.  Now, pin/unpin the buffer to prove it's unpinned.
> +          */
> +         if (PinBuffer(buf, NULL))
> +             UnpinBuffer(buf, false);
> 
> I don't think pinning and unpinning the buffer is sufficient to
> provide that it isn't otherwise pinned.  If the buffer isn't in shared
> buffers at all, it seems clear that no one can have it pinned.  But if
> it's present in shared buffers, it seems like you still need
> LockBufferForCleanup().

-- Simon Riggs           http://www.2ndQuadrant.com/books/PostgreSQL Development, 24x7 Support, Training and Services