On Mon, Aug 21, 2017 at 07:41:52AM +0100, Simon Riggs wrote:
> On 19 August 2017 at 20:54, Noah Misch <noah(at)leadboat(dot)com> wrote:
> > On Tue, Dec 06, 2016 at 01:59:18PM -0500, Robert Haas wrote:
> >> On Tue, Dec 6, 2016 at 1:36 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >> > Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> >> >> On Tue, Nov 15, 2016 at 3:55 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >> >>> Account for catalog snapshot in PGXACT->xmin updates.
> >> >
> >> >> It seems to me that this makes it possible for
> >> >> ThereAreNoPriorRegisteredSnapshots() to fail spuriously. The only
> >> >> core code that calls that function is in copy.c, and apparently we
> >> >> never reach that point with the catalog snapshot set. But that seems
> >> >> fragile.
> >
> > I recently noticed this by way of the copy2 regression test failing, in 9.4
> > and later, under REPEATABLE READ and SERIALIZABLE. That failure started with
> > $SUBJECT.
> > Fair summary. ThereAreNoPriorRegisteredSnapshots() has functioned that way
> > since an early submission[1], and I don't see that we ever discussed it
> > explicitly. Adding Simon for his recollection.
>
> The intention, IIRC, was to allow the current snapshot (i.e. 1) but no
> earlier (i.e. prior) snapshots (so not >=2)
>
> The name was chosen so it was (hopefully) clear what it did --
> ThereAreNoPriorRegisteredSnapshots
I now understand the function name, so I added a comment but didn't rename it.
I plan to use the attached patch after the minor release tags land. If
there's significant support, I could instead push before the wrap.