On Tue, May 24, 2016 at 3:48 PM, Kevin Grittner <kgri...@gmail.com> wrote: > On Tue, May 24, 2016 at 12:00 PM, Andres Freund <and...@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 <kgri...@gmail.com> wrote: >>>> On Fri, May 6, 2016 at 7:48 PM, Andres Freund <and...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers