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

On Tue, May 24, 2016 at 3:48 PM, Kevin Grittner <kgrittn@gmail.com> wrote:
> On Tue, May 24, 2016 at 12:00 PM, Andres Freund <andres@anarazel.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@gmail.com> wrote:
>>>> On Fri, May 6, 2016 at 7:48 PM, Andres Freund <andres@anarazel.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