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