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

On Tue, May 24, 2016 at 3:48 PM, Kevin Grittner <kgrittn(at)gmail(dot)com> wrote:
> On Tue, May 24, 2016 at 12:00 PM, Andres Freund <andres(at)anarazel(dot)de> wrote:
>> On 2016-05-24 11:24:44 -0500, Kevin Grittner wrote:
>>> On Fri, May 6, 2016 at 8:28 PM, Kevin Grittner <kgrittn(at)gmail(dot)com> wrote:
>>>> On Fri, May 6, 2016 at 7:48 PM, Andres Freund <andres(at)anarazel(dot)de> wrote:
>>>
>>>>> That comment reminds me of a question I had: Did you consider the effect
>>>>> of this patch on analyze? It uses a snapshot, and by memory you've not
>>>>> built in a defense against analyze being cancelled.
>
> The primary defense is not considering a cancellation except in 30
> of the 500 places a page is used. None of those 30 are, as far as
> I can see (upon review in response to your question), used in the
> analyze process.

It's not obvious to me how this is supposed to work. If ANALYZE's
snapshot is subject to being ignored for xmin purposes because of
snapshot_too_old, then I would think it would need to consider
cancelling itself if it reads a page with possibly-removed data, just
like in any other case. It seems that we might not actually need a
snapshot set for ANALYZE in all cases, because the comments say:

/* functions in indexes may want a snapshot set */
PushActiveSnapshot(GetTransactionSnapshot());

If we can get away with it, it would be a rather large win to only set
a snapshot when the table has an expression index. For purposes of
"snapshot too old", though, it will be important that a function in an
index which tries to read data from some other table which has been
pruned cancels itself when necessary.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company