Re: CLOG contention, part 2

On Sat, Jan 21, 2012 at 7:31 AM, Simon Riggs  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);


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.

Cheers,

Jeff