Re: GetCurrentVirtualXIDs() - Mailing list pgsql-hackers

Simon Riggs <simon@2ndQuadrant.com> writes:
> However, the basic premise is that idle-in-transaction sessions do not
> need to block index builds.

[ thinks for awhile... ]  Actually, I believe that your premise is
correct; the problem is with your proof ;-).  Considering only the
xmins is insufficient to prove that this is safe; but as we know,
xmin comparisons are an oversimplification of snapshot relationships.

What the indexcmds.c comments say are
  * Now take the "reference snapshot" that will be used by validate_index()  * to filter candidate tuples.  Beware!
Theremight still be snapshots in  * use that treat some transaction as in-progress that our reference  * snapshot
treatsas committed.  If such a recently-committed transaction  * deleted tuples in the table, we will not include them
inthe index; yet  * those transactions which see the deleting one as still-in-progress will  * expect them to be there
oncewe mark the index as valid.
 

and then (after validate_index)
  * The index is now valid in the sense that it contains all currently  * interesting tuples.  But since it might not
containtuples deleted just  * before the reference snap was taken, we have to wait out any  * transactions that might
haveolder snapshots.  Obtain a list of VXIDs  * of such transactions, and wait for them individually.
 

The important point here is that we only have to wait for transactions
that might have snapshots older than our reference snapshot.  A
transaction with no snapshots (evidenced by its xmin = 0), a fortiori,
has no older snapshots.  And even if it's in process of taking one when
GetCurrentVirtualXIDs examines it, it cannot conclude that any
transaction that our reference snap saw as committed is still running.
(This is true even if the other guy started his GetSnapshotData before
we did and has somehow managed to not finish yet.  In that case, he's
been holding ProcArrayLock shared the whole time, and no transaction
could have exited "running" state at all.)

So on third thought I think the patch logic is sound; but I think that
as documentation we had better add another bool parameter to
GetCurrentVirtualXIDs indicating whether it's okay to ignore procs
with xmin = 0.  It seems at least possible that some future usage of
GetCurrentVirtualXIDs might need that done differently.

(This logic also shows that we need not actually wait for a transaction
to *complete*; as soon as it's gone idle and has no snapshots, we could
disregard it.  But that would take a much more invasive patch to
implement, so it's definitely too late for 8.4.)

Comments?
        regards, tom lane