On 01/14/2014 11:22 PM, Peter Geoghegan wrote:
> The problem was that
> inserted-then-deleted-in-same-xact tuples (both regular and promise)
> were invisible to all xacts' dirty snapshots, when they should have
> only been invisible to the deleting xact's dirty snapshot.
Right.
> So it isn't
> obvious to me how you interlock things such that another xact doesn't
> incorrectly decide that it has to wait on what is really a promise
> tuple's xact for the full duration of that xact, having found no
> speculative insertion token to ShareLock (which implies unprincipled
> deadlocking), while simultaneously having other sessions not fail to
> see as dirty-visible a same-xact-inserted-deleted non-promise tuple
> (thereby ensuring those other sessions correctly conclude that it is
> necessary to wait for the end of the xmin/xmax xact). If you set the
> xmin to invalid too late, it doesn't help any existing waiters.
If a backend finds no speculative insertion token to ShareLock, then it
really isn't a speculative insertion, and the process should sleep on
the xid as usual.
Once we remove the modification in HeapTupleSatisfiesDirty() that made
it return false when xmin == xmax, the problem that arises is that
another backend that sees the killed tuple incorrectly determines that
it has to wait for it that transaction to finish, even though it was a
speculatively inserted tuple that was killed, and hence can be ignored.
We can avoid that problem by setting xmin to invalid, or otherwise
marking the tuple as dead.
Attached is a patch doing that, to again demonstrate what I mean. I'm
not sure if setting xmin to invalid is really the best way to mark the
tuple dead; I don't think a tuple's xmin can currently be
InvalidTransaction under any other circumstances, so there might be some
code out there that's not prepared for it. So using an infomask bit
might indeed be better. Or something else entirely.
- Heikki