Re: [COMMITTERS] pgsql: Add the "snapshot too old" feature

On Thu, Aug 25, 2016 at 4:51 PM, Alvaro Herrera
<alvherre(at)2ndquadrant(dot)com> wrote:
> Kevin Grittner wrote:
>> On Thu, Aug 25, 2016 at 2:56 PM, Alvaro Herrera
>> <alvherre(at)2ndquadrant(dot)com> 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.

That would cover things as long as data is always moved from the
pending list to a data page before it is vacuumed away. If that is
true, there is no need to check the metapage *or* the pending list
-- but I'm pretty skeptical that this is the case. The real
question is whether pages are ever removed from the pending list.

> 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.

The implementation does allow false positives and slightly less
aggressive early cleanup than could be achieved -- in order to
avoid the extra locking that would be needed to achieve higher
precision. Since I expect that the threshold will usually be set
to at least a couple hours, these effects should have minimal
impact.

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company