From: | Jeff Janes <jeff(dot)janes(at)gmail(dot)com> |
---|---|
To: | Pavan Deolasee <pavan(dot)deolasee(at)gmail(dot)com> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Setting visibility map in VACUUM's second phase |
Date: | 2013-02-02 21:01:36 |
Message-ID: | CAMkU=1xHLeCO-QXnSnDyFYRDWEr_G+x5oWYu4S=k_FRfh6fsiw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sat, Jan 26, 2013 at 11:25 PM, Pavan Deolasee
<pavan(dot)deolasee(at)gmail(dot)com> wrote:
> On Thu, Jan 24, 2013 at 9:31 PM, Jeff Janes <jeff(dot)janes(at)gmail(dot)com> wrote:
>> On Thu, Jan 24, 2013 at 1:28 AM, Pavan Deolasee
>> <pavan(dot)deolasee(at)gmail(dot)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
Attachment | Content-Type | Size |
---|---|---|
vacuum-secondphase-setvm-v4.patch | application/octet-stream | 7.4 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2013-02-02 21:03:54 | Re: autovacuum not prioritising for-wraparound tables |
Previous Message | Steve Singer | 2013-02-02 20:43:24 | Re: PL/Python result object str handler |