Craig Ringer wrote: > A SELECT ... FOR SHARE will incorrectly block on another open > transaction that ran SELECT ... FOR SHARE and still holds the locks if > it has to follow a ctid chain from the current snapshot through a > committed update to a share-locked tuple. > > This also affects uniqueness checks in btrees, where it can cause > unnecessary waiting. It's also an issue with FOR KEY UPDATE, in that it > can cause an update to block when it doesn't have to.
Interesting bug. Thanks for the patch. I have applied it all the way back to 8.4 (with adjustments for 9.2 and beyond). > The attached test case runs under isolationtester to exersise the > problem. I've tested it against 9.2, 9.3, and HEAD, but Andres looked > over the code and says the underlying bug goes back to the commit of > MVCC, it's just become easier to trigger. To reliably test this with > isolationtester I had to horribly abuse pg_advisory_lock(...) so that I > could force the first SELECT ... FOR UPDATE to acquire its snapshot > before the UPDATE runs. I didn't apply the test case. I think if we want to test tqual.c behavior we will need to introduce a large battery of tests. I would like to see more opinions on whether that's something we want. > A backtrace of the point where it's incorrectly blocked is attached. > What's happening is that the test for TransactionIdIsInProgress > unconditionally sets snapshot->xmax, even if xmax was only set for > locking purposes. This will cause the caller to wait for the xid in xmax > when it doesn't need to. Yeah, after actually going over the code I think the bug is clear. (I was initially unsure about SatisfiesDirty returning true not false for this case; but the return value was correct, only snapshot->xmax was being set incorrectly. If you examine the callers they would misbehave if the return value were changed; for example EvalPlanQualFetch would completely skip the tuple if SatisfiesDirty returned false, which is not what we want; we want the tuple to be processed.) I think the comments on what exactly SatisfiesDirty does about in-progress transactions ought to be more specific. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers