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 <si...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
