On Sat, Jan 26, 2013 at 11:25 PM, Pavan Deolasee <pavan.deola...@gmail.com> wrote: > On Thu, Jan 24, 2013 at 9:31 PM, Jeff Janes <jeff.ja...@gmail.com> wrote: >> On Thu, Jan 24, 2013 at 1:28 AM, Pavan Deolasee >> <pavan.deola...@gmail.com> wrote: > >>> >>> Good idea. Even though the cost of pinning/unpinning may not be high >>> with respect to the vacuum cost itself, but it seems to be a good idea >>> because we already do that at other places. Do you have any other >>> review comments on the patch or I'll fix this and send an updated >>> patch soon. >> >> That was the only thing that stood out to me. > > The attached patch gets that improvement. Also rebased on the latest head.
Hi Pavan,
I get this warning:
vacuumlazy.c:890: warning: passing argument 6 of 'lazy_vacuum_page'
makes pointer from integer without a cast
and make check then fails.
I've added '&' to that line, and it now passes make check with --enable-cassert.
At line 1096, when you release the vmbuffer, you don't set it to
InvalidBuffer like the other places in the code do. It seems like
this does would lead to a crash or assertion failure, but it does not
seem to do so.
other places:
if (BufferIsValid(vmbuffer))
{
ReleaseBuffer(vmbuffer);
vmbuffer = InvalidBuffer;
}
Also, the "Note: If you change anything below, also look at" should
probably say "Note: If you change anything in the for loop below, also
look at". Otherwise I'd be wondering how far below the caveat
applies.
I've attached a patch with these changes made. Does this look OK?
Thanks,
Jeff
vacuum-secondphase-setvm-v4.patch
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
