On Fri, Jan 27, 2012 at 10:05 PM, Jeff Janes <jeff.janes@gmail.com> wrote:
> On Sat, Jan 21, 2012 at 7:31 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
>>
>> Yes, it was. Sorry about that. New version attached, retesting while
>> you read this.
>
> In my hands I could never get this patch to do anything. The new
> cache was never used.
>
> I think that that was because RecentXminPageno never budged from -1.
>
> I think that that, in turn, is because the comparison below can never
> return true, because the comparison is casting both sides to uint, and
> -1 cast to uint is very large
>
> /* When we commit advance ClogCtl's shared RecentXminPageno if needed */
> if (ClogCtl->shared->RecentXminPageno < TransactionIdToPage(RecentXmin))
> ClogCtl->shared->RecentXminPageno =
> TransactionIdToPage(RecentXmin);
Thanks, will look again.
> Also, I think the general approach is wrong. The only reason to have
> these pages in shared memory is that we can control access to them to
> prevent write/write and read/write corruption. Since these pages are
> never written, they don't need to be in shared memory. Just read
> each page into backend-local memory as it is needed, either
> palloc/pfree each time or using a single reserved block for the
> lifetime of the session. Let the kernel worry about caching them so
> that the above mentioned reads are cheap.
Will think on that.
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
