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

On Wed, Jun 8, 2016 at 9:40 AM, Kevin Grittner <kgrittn@gmail.com> wrote:
>>> Of course, ii_BrokenHotChain should be renamed to something like
>>> ii_UnsafeForOldSnapshots, and some comments need to be updated; but
>>> the above is the substance of it.
>>
>> I don't know why we'd want to rename it like that...
>
> If we made the above change, the old name would be misleading, but
> I've thought better of that and attach a slightly different
> approach (tested but not yet with comment adjustments); attached.

Kevin asked me to look at this patch, and maybe update it, but after
some further study, I am not at all convinced that there's any actual
bug here.  Here's why: in order for the HeapTupleSatisfiesVacuum() in
IndexBuildHeapRangeScan() to return HEAPTUPLE_DEAD instead of
HEAPTUPLE_RECENTLY_DEAD, it would have to be using an OldestXmin value
that doesn't include all of the snapshots in the system.  But that
will never happen, because that xmin comes directly from
GetOldestXmin(heapRelation, true), which knows nothing about
snapshot_too_old and will therefore never exclude any snapshots.  If
we were to pass the output of HeapTupleSatisfiesVacuum() through
TransactionIdLimitedForOldSnapshots() before using it here, we would
have a bug.  But we don't do that.

Do you have a test case that demonstrates a problem, or an explanation
of why you think there is one?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company