On Sun, Jan 29, 2012 at 12:18 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
> 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 for looking at the patch.
>
> The patch works fine. RecentXminPageno does move forwards as it is
> supposed to and there are no uints anywhere in that calculation.
Maybe it is system dependent. Or, are you running this patch on top
of some other uncommitted patch (other than the pgbench one)?
RecentXmin is a TransactionID, which is a uint32.
I think the TransactionIdToPage macro preserves that.
If I cast to a int, then I see advancement:
if (ClogCtl->shared->RecentXminPageno < (int) TransactionIdToPage(RecentXmin))
...
> I've specifically designed the pgbench changes required to simulate
> conditions of clog contention to help in the evaluation of this patch.
Yep, I've used that one for the testing.
Cheers,
Jeff