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

On July 19, 2016 7:43:05 PM PDT, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>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().

That's why I suggested copying them from the current mvcc snapshot.

Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.