On 2013-11-25 17:10:39 -0300, Alvaro Herrera wrote: > > I am not sure whether that's the origin of the problem but at the very > > least it seems to me that heap_lock_updated_tuple_rec() is missing > > several important pieces: > > a) do the priorXmax==xmin dance to check we're still following the same > > ctid chain. Currently we could easily stumble across completely > > unrelated tuples if a chain element aborted and got vacuumed. > > This seems simple to handle by adding the check you propose to the loop. > Basically if the xmax doesn't match the xmin, we reached the end, > there's nothing more to lock and we can return success without any > further work:
Right, that's what we do in other places (notably EvalPlanQualFetch()). > > b) Check whether a chain element actually aborted - currently we're > > only doing that in the HEAP_KEYS_UPDATED updated case, but that seems > > wrong (we can't check for committed tho!). > > Let me point out that this is exactly the same code that would be > affected by my proposed fix for #8434, which would have this check the > updateXid in all cases, not only in KEYS_UPDATED as currently. Hm. I don't see a patch in any of the mails about #8434 although I seem to remember talking with you about one. Maybe that was on IM? > I don't understand your comment about "can't check for committed". I > mean, if the updating transaction committed, then we need to return > failure to caller, HeapTupleUpdated. I mean that in the !KEYS_UPDATED case we don't need to abort if we're only acquiring a key share... Greetings, Andres Freund -- Andres Freund 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
