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

Andres Freund wrote:
> On 2016-07-20 11:26:11 -0400, Robert Haas wrote:
> > On Wed, Jul 20, 2016 at 3:39 AM, Andres Freund <andres@anarazel.de> wrote:
> > >>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.
> > 
> > And how do you obtain that?  The functions that reference
> > SnapshotToast are toast_delete_datum, toastrel_value_exists, and
> > toast_fetch_datum, toast_fetch_datum_slice, but none of those take a
> > snapshot as an argument, nor is there any reasonable way to make them
> > do so.  Those are indirectly called by things like bttextcmp, which
> > don't know what snapshot was used to fetch the datum that they are
> > detoasting and can't reasonably be made to know.
> > 
> > I mean, you could do something *approximately* correct by calling
> > GetActiveSnapshot() but that doesn't seem likely to be correct in
> > detail.
> 
> GetActiveSnapshot() seems like it should work well enough in this case,
> or we could use pairingheap_first() to get the actual oldest registered
> one.

Hmm.  Why is the active snapshot not sufficient?  Perhaps we need some
kind of redesign or minor tweak to snapmgr to keep track of the oldest
snapshot of a resowner or something like that?

-- 
Álvaro Herrera                http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services