On 2016-07-28 15:40:48 -0400, Robert Haas wrote: > On Wed, Jul 27, 2016 at 7:26 PM, Andres Freund <and...@anarazel.de> wrote: > >> Also, I wonder why it's right to use > >> pairingheap_first() instead of looking at the oldest snapshot on the > >> active snapshot stack, or conversely why that is right and this is > >> not. Or maybe we need to check both. > > > > Well, generally, the older the snapshot we use is, the more we'll error > > out spuriously. The reason to use the oldest from the pairing heap is > > that that'll be the most conservative value, right? If there's neither > > an active, nor a registered snapshot, we'll not prevent pruning in the > > first place (and xmin ought to be invalid). As registered snapshots are > > usually "older" than active ones, we definitely have to check those. But > > you're right, it seems safer to also check active ones - which squares > > with SnapshotResetXmin(). > > OK. That's a bit inconvenient because we don't have an O(1) way to > access the bottom of the active snapshot stack; I've attempted to add > such a mechanism here.
I think just iterating through the active snapshots would have been fine. Afaics there's no guarantee that the first active snapshot pushed is the relevant one - in contrast to registered one, which are ordered by virtue of the heap. > >> But there's a bit of spooky action at a > >> distance: if we don't see any snapshots, then we have to assume that > >> the scan is being performed with a non-MVCC snapshot and therefore we > >> don't need to worry. But there's no way to verify that via an > >> assertion, because the connection between the relevant MVCC snapshot > >> and the corresponding TOAST snapshot is pretty indirect, mediated only > >> by snapmgr.c. > > > > Hm. Could we perhaps assert that the session has a valid xmin? > > I don't think so. CLUSTER? That should have one during any toast lookups afaics - the relevant code is /* Start a new transaction for each relation. */ StartTransactionCommand(); /* functions in indexes may want a snapshot set */ PushActiveSnapshot(GetTransactionSnapshot()); /* Do the job. */ cluster_rel(rvtc->tableOid, rvtc->indexOid, true, stmt->verbose); PopActiveSnapshot(); CommitTransactionCommand(); right? And Snapshot GetSnapshotData(Snapshot snapshot) { ... if (!TransactionIdIsValid(MyPgXact->xmin)) MyPgXact->xmin = TransactionXmin = xmin; sets xmin. Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers