Re: Re: [COMMITTERS] pgsql: Avoid extra locks in GetSnapshotData if old_snapshot_threshold <

On Wed, Jul 20, 2016 at 7:57 AM, Andres Freund <andres(at)anarazel(dot)de> wrote:
>
>
> On July 19, 2016 7:14:42 PM PDT, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>>On Wed, Jul 20, 2016 at 5:02 AM, Andres Freund <andres(at)anarazel(dot)de>
>>wrote:
>>> On 2016-07-19 18:09:59 -0500, Kevin Grittner wrote:
>>>> As far as I can see, to do this the way that Andres and Amit
>>>> suggest involves tying in to indexam.c and other code in incredibly
>>>> ugly ways.
>>>
>>> Could you explain the problem you're seing?
>>>
>>> Isn't pretty much all all that we need to do:
>>> 1) add a InitSnapshotToast(Snapshot originMVCCSnap), which sets
>>SnapshotData->lsn
>>> to the the origin snapshot's lsn
>>> 2) adapt TestForOldSnapshot() to accept both HeapTupleSatisfiesMVCC
>>and
>>> HeapTupleSatisfiesToast?
>>>
>>
>>I also think so. However, it is not clear what is the best place to
>>initialize toast snapshot. One idea could be to do it in
>>GetSnapshotData() after capturing the required information for the
>>valid value of old_snapshot_threshold. Do you have something else in
>>mind?
>
> There's very few callsites using toast snapshots. I'd just do it there. Don't think we ever use GetSnapshotData for them.
>

I think Snapshot's members whenTaken and lsn are updated/initialized
only in GetSnapshotData(). So if GetSnapshotData() is not used, how
will you expect those fields to be updated. We need those fields to
be updated for TestForOldSnapshot().

--
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com