Kevin Grittner wrote:
> On Thu, Aug 25, 2016 at 2:56 PM, Alvaro Herrera
> <> wrote:
> > I'm wondering about the TestForOldSnapshot call in scanPendingInsert().
> > Why do we apply it to the metapage buffer (line 1717 in master)?
> If there is any chance that GinPageGetMeta(page)->head could have
> changed from a valid block number to InvalidBlockNumber or a
> different pending-list page due to a vacuum freeing pages from the
> pending-list, the metapage must be checked -- there is no other way
> to detect that data might have disappeared.

Hmm ... but the disappearing data is moved to regular GIN pages, right?
It doesn't just go away.  I suppose that the error would be raised as
soon as we scan a GIN data page that, because of receiving data from the
pending list, has a newer LSN.  I don't know GIN in detail but perhaps
it's possible that the data is inserted into data pages in lsn A, then
removed from the pending list in lsn B (where A < B).  If the snapshot's
LSN lies in between, a spurious error would be raised.

> > FWIW I like the "revert" commit, because it easily shows me in what
> > places you considered a snapshot-too-old test and decided not to add
> > one.  Bare BufferGetPage calls (the current situation) don't indicate that.
> I'm glad there is some value from having done that little dance.  :-)

I'm sure that was an annoying thing to do, so I agree.

> Thanks for looking this over so closely!

You're welcome.  I'm not actually reviewing this patch specifically,
just trying to figure out a different new feature and reading a few AMs
code while at it.

Álvaro Herrera      
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Sent via pgsql-hackers mailing list (
To make changes to your subscription:

Reply via email to