On Fri, 2009-04-03 at 15:46 -0400, Tom Lane wrote: > Simon Riggs <si...@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.com PostgreSQL Training, Services and Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers