On Fri, 2009-04-03 at 15:46 -0400, Tom Lane wrote:
> Simon Riggs <simon@2ndQuadrant.com> writes:
> > No need to wait for idle-in-transaction sessions during index builds.
> > GetCurrentVirtualXIDs() specifically *includes* backends that have
> > proc->xmin == InvalidTransactionId (0), but I'm not sure why.
>
> On further consideration, this patch is simply *wrong*, and would still
> be wrong even if we changed GetCurrentVirtualXIDs to take ProcArrayLock
> exclusive instead of shared.
>
> If a backend currently has no snapshot, then if it takes a snapshot
> immediately after we finish running GetCurrentVirtualXIDs, it will
> set its proc->xmin (and that of the snapshot) to the oldest currently
> running XID. There is no reason to assume that that value is >=
> limitXmin, which is what you propose we do.
>
> A safe modification of the patch would be to determine the oldest
> running XID and exclude xmin-less VXIDs when that number is greater
> than limitXmin. However, I think that that would be pretty useless:
> for the one current use of GetCurrentVirtualXIDs, limitXmin is the
> xmax of the snapshot we used for the index build, and we can assume
> that our *own* XID is less than that, never mind anyone else's.
>
> So I don't think this works...
I see your logic and agree with it.
However, the basic premise is that idle-in-transaction sessions do not
need to block index builds. I think that's still true in many but not
all cases, so we just need to modify it slightly to include the cases
you mention. I propose:
1. GetCurrentVirtualXIDs() ignoring procs with proc->xmin == 0
2. Wait for all of those to disappear
3. GetCurrentVirtualXIDs() again with same limitXmin, again ignoring
xmin==0 procs.
4. Wait again.
This then catches any idle-in-transactions that took a snapshot after
the first step and before the second. It doesn't guarantee that there
are no backends that have an xmin less than limitXmin, but it does mean,
I think, that they can't see any non-indexed row versions.
Not sure if that helps my original thought for Hot Standby, but that's a
different issue.
-- Simon Riggs www.2ndQuadrant.comPostgreSQL Training, Services and Support